Set `channel_update` disable bit based on staged even for onions

When generating a `channel_update` either in response to a fee
configuration change or an HTLC failure, we currently poll the
channel to check if the peer's connected when setting the disabled
bit in the `channel_update`. This could cause cases where we set
the disable bit even though the peer *just* disconnected, and don't
generate a followup broadcast `channel_update` with the disabled
bit unset.

While a node generally shouldn't rebroadcast a `channel_update` it
received in an onion, there's nothing inherently stopping them from
doing so. Obviously in the fee-update case we expect the message to
propagate.

Luckily, since we already "stage" disable-changed updates, we can
check the staged state and use that to set the disabled bit in all
`channel_update` cases.

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 9f403f85b2f0aec6b841961fa47fd7e6bce424e7..5cdc07ef0b1a4718bc577a7a7239c88cc723ee46 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -2457,7 +2457,14 @@ where
                                                // hopefully an attacker trying to path-trace payments cannot make this occur
                                                // on a small/per-node/per-channel scale.
                                                if !chan.is_live() { // channel_disabled
-                                                       break Some(("Forwarding channel is not in a ready state.", 0x1000 | 20, chan_update_opt));
+                                                       // If the channel_update we're going to return is disabled (i.e. the
+                                                       // peer has been disabled for some time), return `channel_disabled`,
+                                                       // otherwise return `temporary_channel_failure`.
+                                                       if chan_update_opt.as_ref().map(|u| u.contents.flags & 2 == 2).unwrap_or(false) {
+                                                               break Some(("Forwarding channel has been disconnected for some time.", 0x1000 | 20, chan_update_opt));
+                                                       } else {
+                                                               break Some(("Forwarding channel is not in a ready state.", 0x1000 | 7, chan_update_opt));
+                                                       }
                                                }
                                                if *outgoing_amt_msat < chan.get_counterparty_htlc_minimum_msat() { // amount_below_minimum
                                                        break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, chan_update_opt));
@@ -2582,11 +2589,18 @@ where
                log_trace!(self.logger, "Generating channel update for channel {}", log_bytes!(chan.channel_id()));
                let were_node_one = self.our_network_pubkey.serialize()[..] < chan.get_counterparty_node_id().serialize()[..];
 
+               let enabled = chan.is_usable() && match chan.channel_update_status() {
+                       ChannelUpdateStatus::Enabled => true,
+                       ChannelUpdateStatus::DisabledStaged => true,
+                       ChannelUpdateStatus::Disabled => false,
+                       ChannelUpdateStatus::EnabledStaged => false,
+               };
+
                let unsigned = msgs::UnsignedChannelUpdate {
                        chain_hash: self.genesis_hash,
                        short_channel_id,
                        timestamp: chan.get_update_time_counter(),
-                       flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1),
+                       flags: (!were_node_one) as u8 | ((!enabled as u8) << 1),
                        cltv_expiry_delta: chan.get_cltv_expiry_delta(),
                        htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(),
                        htlc_maximum_msat: chan.get_announced_htlc_max_msat(),
@@ -3741,22 +3755,22 @@ where
                                                        ChannelUpdateStatus::DisabledStaged if chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Enabled),
                                                        ChannelUpdateStatus::EnabledStaged if !chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Disabled),
                                                        ChannelUpdateStatus::DisabledStaged if !chan.is_live() => {
+                                                               chan.set_channel_update_status(ChannelUpdateStatus::Disabled);
                                                                if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
                                                                        pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
                                                                                msg: update
                                                                        });
                                                                }
                                                                should_persist = NotifyOption::DoPersist;
-                                                               chan.set_channel_update_status(ChannelUpdateStatus::Disabled);
                                                        },
                                                        ChannelUpdateStatus::EnabledStaged if chan.is_live() => {
+                                                               chan.set_channel_update_status(ChannelUpdateStatus::Enabled);
                                                                if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
                                                                        pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
                                                                                msg: update
                                                                        });
                                                                }
                                                                should_persist = NotifyOption::DoPersist;
-                                                               chan.set_channel_update_status(ChannelUpdateStatus::Enabled);
                                                        },
                                                        _ => {},
                                                }

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index f8ccce05847a7d426a5b05118c25670febb4d6b0..e818bd95502ba9f9869c44e7e479204a23c2a918 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -2724,10 +2724,9 @@ macro_rules! handle_chan_reestablish_msgs {
                        }
 
                        let mut had_channel_update = false; // ChannelUpdate may be now or later, but not both
-                       if let Some(&MessageSendEvent::SendChannelUpdate { ref node_id, ref msg }) = msg_events.get(idx) {
+                       if let Some(&MessageSendEvent::SendChannelUpdate { ref node_id, .. }) = msg_events.get(idx) {
                                assert_eq!(*node_id, $dst_node.node.get_our_node_id());
                                idx += 1;
-                               assert_eq!(msg.contents.flags & 2, 0); // "disabled" flag must not be set as we just reconnected.
                                had_channel_update = true;
                        }
 
@@ -2771,10 +2770,9 @@ macro_rules! handle_chan_reestablish_msgs {
                                }
                        }
 
-                       if let Some(&MessageSendEvent::SendChannelUpdate { ref node_id, ref msg }) = msg_events.get(idx) {
+                       if let Some(&MessageSendEvent::SendChannelUpdate { ref node_id, .. }) = msg_events.get(idx) {
                                assert_eq!(*node_id, $dst_node.node.get_our_node_id());
                                idx += 1;
-                               assert_eq!(msg.contents.flags & 2, 0); // "disabled" flag must not be set as we just reconnected.
                                assert!(!had_channel_update);
                        }
 

diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs
index 743d41eea1ba731ca96ac0114a443660fe1f1738..c7fae84af53875f3c526daa5ed6a157bbe7e8c9a 100644 (file)
--- a/lightning/src/ln/onion_route_tests.rs
+++ b/lightning/src/ln/onion_route_tests.rs
@@ -587,6 +587,15 @@ fn test_onion_failure() {
                // disconnect event to the channel between nodes[1] ~ nodes[2]
                nodes[1].node.peer_disconnected(&nodes[2].node.get_our_node_id());
                nodes[2].node.peer_disconnected(&nodes[1].node.get_our_node_id());
+       }, true, Some(UPDATE|7), Some(NetworkUpdate::ChannelUpdateMessage{msg: ChannelUpdate::dummy(short_channel_id)}), Some(short_channel_id));
+       run_onion_failure_test("channel_disabled", 0, &nodes, &route, &payment_hash, &payment_secret, |_| {}, || {
+               // Tick the timer twice on each node to mark the channel as disabled.
+               nodes[1].node.timer_tick_occurred();
+               nodes[1].node.timer_tick_occurred();
+               nodes[1].node.get_and_clear_pending_msg_events();
+               nodes[2].node.timer_tick_occurred();
+               nodes[2].node.timer_tick_occurred();
+               nodes[2].node.get_and_clear_pending_msg_events();
        }, true, Some(UPDATE|20), Some(NetworkUpdate::ChannelUpdateMessage{msg: ChannelUpdate::dummy(short_channel_id)}), Some(short_channel_id));
        reconnect_nodes(&nodes[1], &nodes[2], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));