From 8f89371bae42d127f28b4362322682fe22718175 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 13 Nov 2021 01:06:09 +0000 Subject: [PATCH] Update `ChannelUpdate::timestamp` when channels are dis-/en-abled We update the `Channel::update_time_counter` field (which is copied into `ChannelUpdate::timestamp`) only when the channel is initialized or closes, and when a new block is connected. However, if a peer disconnects or reconnects, we may wish to generate `ChannelUpdate` updates in between new blocks. In such a case, we need to make sure the `timestamp` field is newer than any previous updates' `timestamp` fields, which we do here by simply incrementing it when the channel status is changed. As a side effect of this we have to update `test_background_processor` to ensure it eventually succeeds even if the serialization of the `ChannelManager` changes after the test begins. --- lightning-background-processor/src/lib.rs | 13 +++++++------ lightning/src/ln/channel.rs | 1 + lightning/src/ln/functional_tests.rs | 21 ++++++++++++--------- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 39ecc316..4e98ba9e 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -493,9 +493,10 @@ mod tests { macro_rules! check_persisted_data { ($node: expr, $filepath: expr, $expected_bytes: expr) => { - match $node.write(&mut $expected_bytes) { - Ok(()) => { - loop { + loop { + $expected_bytes.clear(); + match $node.write(&mut $expected_bytes) { + Ok(()) => { match std::fs::read($filepath) { Ok(bytes) => { if bytes == $expected_bytes { @@ -506,9 +507,9 @@ mod tests { }, Err(_) => continue } - } - }, - Err(e) => panic!("Unexpected error: {}", e) + }, + Err(e) => panic!("Unexpected error: {}", e) + } } } } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index d7684023..0cf7995b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4167,6 +4167,7 @@ impl Channel { } pub fn set_channel_update_status(&mut self, status: ChannelUpdateStatus) { + self.update_time_counter += 1; self.channel_update_status = status; } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index f144dbd9..af54f9f8 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -7313,9 +7313,9 @@ fn test_announce_disable_channels() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let short_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id; - let short_id_2 = create_announced_chan_between_nodes(&nodes, 1, 0, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id; - let short_id_3 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id; + create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); + create_announced_chan_between_nodes(&nodes, 1, 0, InitFeatures::known(), InitFeatures::known()); + create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); // Disconnect peers nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false); @@ -7325,13 +7325,13 @@ fn test_announce_disable_channels() { nodes[0].node.timer_tick_occurred(); // DisabledStaged -> Disabled let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 3); - let mut chans_disabled: HashSet = [short_id_1, short_id_2, short_id_3].iter().map(|a| *a).collect(); + let mut chans_disabled = HashMap::new(); for e in msg_events { match e { MessageSendEvent::BroadcastChannelUpdate { ref msg } => { assert_eq!(msg.contents.flags & (1<<1), 1<<1); // The "channel disabled" bit should be set // Check that each channel gets updated exactly once - if !chans_disabled.remove(&msg.contents.short_channel_id) { + if chans_disabled.insert(msg.contents.short_channel_id, msg.contents.timestamp).is_some() { panic!("Generated ChannelUpdate for wrong chan!"); } }, @@ -7367,19 +7367,22 @@ fn test_announce_disable_channels() { nodes[0].node.timer_tick_occurred(); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 3); - chans_disabled = [short_id_1, short_id_2, short_id_3].iter().map(|a| *a).collect(); for e in msg_events { match e { MessageSendEvent::BroadcastChannelUpdate { ref msg } => { assert_eq!(msg.contents.flags & (1<<1), 0); // The "channel disabled" bit should be off - // Check that each channel gets updated exactly once - if !chans_disabled.remove(&msg.contents.short_channel_id) { - panic!("Generated ChannelUpdate for wrong chan!"); + match chans_disabled.remove(&msg.contents.short_channel_id) { + // Each update should have a higher timestamp than the previous one, replacing + // the old one. + Some(prev_timestamp) => assert!(msg.contents.timestamp > prev_timestamp), + None => panic!("Generated ChannelUpdate for wrong chan!"), } }, _ => panic!("Unexpected event"), } } + // Check that each channel gets updated exactly once + assert!(chans_disabled.is_empty()); } #[test] -- 2.30.2