From: Matt Corallo <git@bluematt.me>
Date: Sat, 13 Nov 2021 01:06:09 +0000 (+0000)
Subject: Update `ChannelUpdate::timestamp` when channels are dis-/en-abled
X-Git-Tag: v0.0.104~8^2~3
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=8f89371bae42d127f28b4362322682fe22718175;p=rust-lightning

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 39ecc316a..4e98ba9ea 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 d76840236..0cf7995bc 100644
--- 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 f144dbd9a..af54f9f8d 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<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]