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.

diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs
index 39ecc316a5a88c858144ed6d3b65a8ee3889997a..4e98ba9ea0fc81b032f75b29a31f7612e8722bde 100644 (file)
--- 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 d76840236a1b04945de7703eae62906a6ddd4469..0cf7995bce38a03907c5a4147433085a9ae49054 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -4167,6 +4167,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
 
        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 f144dbd9ad61faab79d73e82def57ef91cfd7106..af54f9f8d29d7fbfa8eab19164c45d5886473949 100644 (file)
--- 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<u64> = [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]