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 rename NetworkGraph
methods close_channel_from_update and fail_node to channel_failed and
node_failed, respectively.

Author: jkczyz

fuzz/src/router.rs

@@ -196,7 +196,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.channel_failed(short_channel_id, false);
 			},
 			_ if node_pks.is_empty() => {},
 			_ => {

lightning/src/ln/functional_test_utils.rs

@@ -1512,7 +1512,7 @@ macro_rules! expect_payment_failed_conditions {
 							}
 							assert_eq!(msg.contents.flags & 2, 0);
 						},
-						&Some($crate::routing::network_graph::NetworkUpdate::ChannelClosed { short_channel_id, is_permanent }) if chan_closed => {
+						&Some($crate::routing::network_graph::NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) if chan_closed => {
 							if let Some(scid) = $conditions.expected_blamed_scid {
 								assert_eq!(short_channel_id, scid);
 							}

lightning/src/ln/onion_route_tests.rs

@@ -177,8 +177,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 {
@@ -283,7 +283,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.
@@ -343,7 +343,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;
@@ -360,7 +360,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
@@ -471,7 +471,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.as_ref(), 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| {
@@ -481,13 +481,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.as_ref(), 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;
@@ -511,13 +511,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

@@ -395,7 +395,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,
 								});
@@ -440,7 +440,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

@@ -162,17 +162,17 @@ 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 that a channel failed to route a payment, which should be applied via
+	/// [`NetworkGraph::channel_failed`].
+	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
 		/// `channel_update` message is received.
 		is_permanent: bool,
 	},
-	/// An error indicating only that a node has failed, which should be applied via
-	/// [`NetworkGraph::fail_node`].
+	/// An error indicating that a node failed to route a payment, which should be applied via
+	/// [`NetworkGraph::node_failed`].
 	NodeFailure {
 		/// The node id of the failed node.
 		node_id: PublicKey,
@@ -186,7 +186,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),
 	},
@@ -282,15 +282,15 @@ 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.channel_failed(short_channel_id, is_permanent);
 			},
 			NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 				let action = if is_permanent { "Removing" } else { "Disabling" };
 				log_debug!(self.logger, "{} node graph entry for {} due to a payment failure.", action, node_id);
-				self.network_graph.fail_node(node_id, is_permanent);
+				self.network_graph.node_failed(node_id, is_permanent);
 			},
 		}
 	}
@@ -1328,11 +1328,11 @@ impl NetworkGraph {
 		self.add_channel_between_nodes(msg.short_channel_id, chan_info, utxo_value)
 	}
 
-	/// 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 channel_failed(&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) {
@@ -1352,7 +1352,7 @@ impl NetworkGraph {
 	}
 
 	/// Marks a node in the graph as failed.
-	pub fn fail_node(&self, _node_id: &PublicKey, is_permanent: bool) {
+	pub fn node_failed(&self, _node_id: &PublicKey, is_permanent: bool) {
 		if is_permanent {
 			// TODO: Wholly remove the node
 		} else {
@@ -2120,7 +2120,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,
 				}),
@@ -2145,7 +2145,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,
 			}),