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.

diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs
index bb6ba2c6e51cb0a2ac65d86023279c08954532b9..9f4fd512ca25a9e4112d20588edfa149ad235cc7 100644 (file)
--- a/fuzz/src/router.rs
+++ b/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() => {},
                        _ => {

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 8929a9774381e9342f20790e094e51c64b36d959..7b653c89d9d08c634b78922702ffb36bd3eb385f 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/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);
                                                        }

diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs
index 802cd3acab45bd4592a7f80cdb982cfadeb7ed52..1c3203edad8b470cc3f6010379d89cd18bb7cce6 100644 (file)
--- a/lightning/src/ln/onion_route_tests.rs
+++ b/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| {

diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs
index 9e4ae058f8d517bdfb93abb021d02a7eb588e8a5..02676b73bd81f8cfb5edb48281726997ac0b2f55 100644 (file)
--- a/lightning/src/ln/onion_utils.rs
+++ b/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,
                                                                                        });

diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs
index 76af50bd785dd45eb5f131a9276eaeb2cd1cfc81..7cbe373f5013f884f83c11d193fce009835ae705 100644 (file)
--- a/lightning/src/routing/network_graph.rs
+++ b/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,
                        }),