Rename ChannelClosed to ChannelFailure

A NetworkUpdate indicating ChannelClosed actually corresponds to a
channel failure as described in BOLT 4:

0x2000 (NODE): node failure (otherwise channel)

Rename the enum variant to ChannelFailure and close_channel_from_update
to fail_channel in NetworkGraph.

Author: jkczyz

fuzz/src/router.rs

@@ -195,7 +195,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 			},
 			4 => {
 				let short_channel_id = slice_to_be64(get_slice!(8));
-				net_graph.close_channel_from_update(short_channel_id, false);
+				net_graph.fail_channel(short_channel_id, false);
 			},
 			_ if node_pks.is_empty() => {},
 			_ => {

lightning/src/ln/functional_test_utils.rs

@@ -1134,7 +1134,7 @@ macro_rules! expect_payment_failed_with_update {
 						assert_eq!(msg.contents.short_channel_id, $scid);
 						assert_eq!(msg.contents.flags & 2, 0);
 					},
-					&Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent }) if $chan_closed => {
+					&Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) if $chan_closed => {
 						assert_eq!(short_channel_id, $scid);
 						assert!(is_permanent);
 					},

lightning/src/ln/onion_route_tests.rs

@@ -174,8 +174,8 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
 							panic!("channel_update not found!");
 						}
 					},
-					&NetworkUpdate::ChannelClosed { ref short_channel_id, ref is_permanent } => {
-						if let NetworkUpdate::ChannelClosed { short_channel_id: ref expected_short_channel_id, is_permanent: ref expected_is_permanent } = expected_channel_update.unwrap() {
+					&NetworkUpdate::ChannelFailure { ref short_channel_id, ref is_permanent } => {
+						if let NetworkUpdate::ChannelFailure { short_channel_id: ref expected_short_channel_id, is_permanent: ref expected_is_permanent } = expected_channel_update.unwrap() {
 							assert!(*short_channel_id == *expected_short_channel_id);
 							assert!(*is_permanent == *expected_is_permanent);
 						} else {
@@ -280,7 +280,7 @@ fn test_fee_failures() {
 	let short_channel_id = channels[0].0.contents.short_channel_id;
 	run_onion_failure_test("fee_insufficient", 0, &nodes, &route, &payment_hash, &payment_secret, |msg| {
 		msg.amount_msat -= 1;
-	}, || {}, true, Some(UPDATE|12), Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent: true}), Some(short_channel_id));
+	}, || {}, true, Some(UPDATE|12), Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent: true}), Some(short_channel_id));
 
 	// In an earlier version, we spuriously failed to forward payments if the expected feerate
 	// changed between the channel open and the payment.
@@ -340,7 +340,7 @@ fn test_onion_failure() {
 		// describing a length-1 TLV payload, which is obviously bogus.
 		new_payloads[0].data[0] = 1;
 		msg.onion_routing_packet = onion_utils::construct_onion_packet_bogus_hopdata(new_payloads, onion_keys, [0; 32], &payment_hash);
-	}, ||{}, true, Some(PERM|22), Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent: true}), Some(short_channel_id));
+	}, ||{}, true, Some(PERM|22), Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent: true}), Some(short_channel_id));
 
 	// final node failure
 	let short_channel_id = channels[1].0.contents.short_channel_id;
@@ -357,7 +357,7 @@ fn test_onion_failure() {
 		// length-1 TLV payload, which is obviously bogus.
 		new_payloads[1].data[0] = 1;
 		msg.onion_routing_packet = onion_utils::construct_onion_packet_bogus_hopdata(new_payloads, onion_keys, [0; 32], &payment_hash);
-	}, ||{}, false, Some(PERM|22), Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent: true}), Some(short_channel_id));
+	}, ||{}, false, Some(PERM|22), Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent: true}), Some(short_channel_id));
 
 	// the following three with run_onion_failure_test_with_fail_intercept() test only the origin node
 	// receiving simulated fail messages
@@ -452,7 +452,7 @@ fn test_onion_failure() {
 		let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap();
 		msg.reason = onion_utils::build_first_hop_failure_packet(&onion_keys[0].shared_secret[..], PERM|8, &[0;0]);
 		// short_channel_id from the processing node
-	}, ||{}, true, Some(PERM|8), Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent: true}), Some(short_channel_id));
+	}, ||{}, true, Some(PERM|8), Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent: true}), Some(short_channel_id));
 
 	let short_channel_id = channels[1].0.contents.short_channel_id;
 	run_onion_failure_test_with_fail_intercept("required_channel_feature_missing", 100, &nodes, &route, &payment_hash, &payment_secret, |msg| {
@@ -462,13 +462,13 @@ fn test_onion_failure() {
 		let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap();
 		msg.reason = onion_utils::build_first_hop_failure_packet(&onion_keys[0].shared_secret[..], PERM|9, &[0;0]);
 		// short_channel_id from the processing node
-	}, ||{}, true, Some(PERM|9), Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent: true}), Some(short_channel_id));
+	}, ||{}, true, Some(PERM|9), Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent: true}), Some(short_channel_id));
 
 	let mut bogus_route = route.clone();
 	bogus_route.paths[0][1].short_channel_id -= 1;
 	let short_channel_id = bogus_route.paths[0][1].short_channel_id;
 	run_onion_failure_test("unknown_next_peer", 0, &nodes, &bogus_route, &payment_hash, &payment_secret, |_| {}, ||{}, true, Some(PERM|10),
-	  Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent:true}), Some(short_channel_id));
+	  Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent:true}), Some(short_channel_id));
 
 	let short_channel_id = channels[1].0.contents.short_channel_id;
 	let amt_to_forward = nodes[1].node.channel_state.lock().unwrap().by_id.get(&channels[1].2).unwrap().get_counterparty_htlc_minimum_msat() - 1;
@@ -487,13 +487,13 @@ fn test_onion_failure() {
 	let short_channel_id = channels[0].0.contents.short_channel_id;
 	run_onion_failure_test("fee_insufficient", 0, &nodes, &route, &payment_hash, &payment_secret, |msg| {
 		msg.amount_msat -= 1;
-	}, || {}, true, Some(UPDATE|12), Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent: true}), Some(short_channel_id));
+	}, || {}, true, Some(UPDATE|12), Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent: true}), Some(short_channel_id));
 
 	let short_channel_id = channels[0].0.contents.short_channel_id;
 	run_onion_failure_test("incorrect_cltv_expiry", 0, &nodes, &route, &payment_hash, &payment_secret, |msg| {
 		// need to violate: cltv_expiry - cltv_expiry_delta >= outgoing_cltv_value
 		msg.cltv_expiry -= 1;
-	}, || {}, true, Some(UPDATE|13), Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent: true}), Some(short_channel_id));
+	}, || {}, true, Some(UPDATE|13), Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent: true}), Some(short_channel_id));
 
 	let short_channel_id = channels[1].0.contents.short_channel_id;
 	run_onion_failure_test("expiry_too_soon", 0, &nodes, &route, &payment_hash, &payment_secret, |msg| {

lightning/src/ln/onion_utils.rs

@@ -394,7 +394,7 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 						}
 						else if error_code & PERM == PERM {
 							if !payment_failed {
-								network_update = Some(NetworkUpdate::ChannelClosed {
+								network_update = Some(NetworkUpdate::ChannelFailure {
 									short_channel_id: failing_route_hop.short_channel_id,
 									is_permanent: true,
 								});
@@ -425,7 +425,7 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 										if is_chan_update_invalid {
 											// This probably indicates the node which forwarded
 											// to the node in question corrupted something.
-											network_update = Some(NetworkUpdate::ChannelClosed {
+											network_update = Some(NetworkUpdate::ChannelFailure {
 												short_channel_id: route_hop.short_channel_id,
 												is_permanent: true,
 											});

lightning/src/routing/network_graph.rs

@@ -152,9 +152,9 @@ pub enum NetworkUpdate {
 		/// The update to apply via [`NetworkGraph::update_channel`].
 		msg: ChannelUpdate,
 	},
-	/// An error indicating only that a channel has been closed, which should be applied via
-	/// [`NetworkGraph::close_channel_from_update`].
-	ChannelClosed {
+	/// An error indicating only that a channel has failed, which should be applied via
+	/// [`NetworkGraph::fail_channel`].
+	ChannelFailure {
 		/// The short channel id of the closed channel.
 		short_channel_id: u64,
 		/// Whether the channel should be permanently removed or temporarily disabled until a new
@@ -176,7 +176,7 @@ impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate,
 	(0, ChannelUpdateMessage) => {
 		(0, msg, required),
 	},
-	(2, ChannelClosed) => {
+	(2, ChannelFailure) => {
 		(0, short_channel_id, required),
 		(2, is_permanent, required),
 	},
@@ -264,10 +264,10 @@ where C::Target: chain::Access, L::Target: Logger
 				log_debug!(self.logger, "Updating channel with channel_update from a payment failure. Channel {} is {}.", short_channel_id, status);
 				let _ = self.network_graph.update_channel(msg, &self.secp_ctx);
 			},
-			NetworkUpdate::ChannelClosed { short_channel_id, is_permanent } => {
+			NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 				let action = if is_permanent { "Removing" } else { "Disabling" };
 				log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
-				self.network_graph.close_channel_from_update(short_channel_id, is_permanent);
+				self.network_graph.fail_channel(short_channel_id, is_permanent);
 			},
 			NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 				let action = if is_permanent { "Removing" } else { "Disabling" };
@@ -1008,11 +1008,11 @@ impl NetworkGraph {
 		Ok(())
 	}
 
-	/// Close a channel if a corresponding HTLC fail was sent.
+	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
 	/// If permanent, removes a channel from the local storage.
 	/// May cause the removal of nodes too, if this was their last channel.
 	/// If not permanent, makes channels unavailable for routing.
-	pub fn close_channel_from_update(&self, short_channel_id: u64, is_permanent: bool) {
+	pub fn fail_channel(&self, short_channel_id: u64, is_permanent: bool) {
 		let mut channels = self.channels.write().unwrap();
 		if is_permanent {
 			if let Some(chan) = channels.remove(&short_channel_id) {
@@ -1839,7 +1839,7 @@ mod tests {
 				rejected_by_dest: false,
 				all_paths_failed: true,
 				path: vec![],
-				network_update: Some(NetworkUpdate::ChannelClosed {
+				network_update: Some(NetworkUpdate::ChannelFailure {
 					short_channel_id,
 					is_permanent: false,
 				}),
@@ -1865,7 +1865,7 @@ mod tests {
 				rejected_by_dest: false,
 				all_paths_failed: true,
 				path: vec![],
-				network_update: Some(NetworkUpdate::ChannelClosed {
+				network_update: Some(NetworkUpdate::ChannelFailure {
 					short_channel_id,
 					is_permanent: true,
 				}),