f spelling and dont immediately drop chan_announcement pre-chan_update

Author: TheBlueMatt

lightning/src/routing/network_graph.rs

@@ -43,6 +43,11 @@ use sync::Mutex;
 use core::ops::Deref;
 use bitcoin::hashes::hex::ToHex;
 
+#[cfg(feature = "std")]
+use std::time::{SystemTime, UNIX_EPOCH};
+
+const STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS: u64 = 60 * 60 * 24 * 14;
+
 /// The maximum number of extra bytes which we do not understand in a gossip message before we will
 /// refuse to relay the message.
 const MAX_EXCESS_BYTES_FOR_RELAY: usize = 1024;
@@ -628,6 +633,10 @@ pub struct ChannelInfo {
 	/// Everything else is useful only for sending out for initial routing sync.
 	/// Not stored if contains excess data to prevent DoS.
 	pub announcement_message: Option<ChannelAnnouncement>,
+	/// The timestamp when we received the announcement, if we are running with feature = "std"
+	/// (which we can probably assume we are - no-std environments probably won't have a full
+	/// network graph in memory!).
+	announcement_received_time: u64,
 }
 
 impl fmt::Display for ChannelInfo {
@@ -640,6 +649,7 @@ impl fmt::Display for ChannelInfo {
 
 impl_writeable_tlv_based!(ChannelInfo, {
 	(0, features, required),
+	(1, announcement_received_time, (default_value, 0)),
 	(2, node_one, required),
 	(4, one_to_two, required),
 	(6, node_two, required),
@@ -952,6 +962,13 @@ impl NetworkGraph {
 			},
 		};
 
+		#[allow(unused_mut, unused_assignments)]
+		let mut announcement_received_time = 0;
+		#[cfg(feature = "std")]
+		{
+			announcement_received_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs();
+		}
+
 		let chan_info = ChannelInfo {
 				features: msg.features.clone(),
 				node_one: NodeId::from_pubkey(&msg.node_id_1),
@@ -961,6 +978,7 @@ impl NetworkGraph {
 				capacity_sats: utxo_value,
 				announcement_message: if msg.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY
 					{ full_msg.cloned() } else { None },
+				announcement_received_time,
 			};
 
 		let mut channels = self.channels.write().unwrap();
@@ -1052,15 +1070,14 @@ impl NetworkGraph {
 	///
 	/// While there is no formal requirement that nodes regularly re-broadcast their channel
 	/// updates every two weeks, the non-normative section of BOLT 7 currently suggests that
-	/// pruning occurrs for updates which are at least two weeks old, which we implement here.
+	/// pruning occurs for updates which are at least two weeks old, which we implement here.
 	///
 	/// This method is automatically called immediately before writing the network graph via
 	/// [`Writeable::write`].
 	///
 	/// This method is only available with the `std` feature. See
 	/// [`NetworkGraph::remove_stale_channels_with_time`] for `no-std` use.
 	pub fn remove_stale_channels(&self) {
-		use std::time::{SystemTime, UNIX_EPOCH};
 		let time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs();
 		self.remove_stale_channels_with_time(time);
 	}
@@ -1074,11 +1091,11 @@ impl NetworkGraph {
 	/// pruning occurrs for updates which are at least two weeks old, which we implement here.
 	///
 	/// This function takes the current unix time as an argument. For users with the `std` feature
-	/// enabled, [`NetworkGraph::remove_stale_channels`] may be preferrable.
+	/// enabled, [`NetworkGraph::remove_stale_channels`] may be preferable.
 	pub fn remove_stale_channels_with_time(&self, current_time_unix: u64) {
 		let mut channels = self.channels.write().unwrap();
 		// Time out if we haven't received an update in at least 14 days.
-		let min_time_unix: u32 = (current_time_unix - 60 * 60 * 14) as u32;
+		let min_time_unix: u32 = (current_time_unix - STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS) as u32;
 		// Sadly BTreeMap::retain was only stabilized in 1.53 so we can't switch to it for some
 		// time.
 		let mut scids_to_remove = Vec::new();
@@ -1090,7 +1107,12 @@ impl NetworkGraph {
 				info.two_to_one = None;
 			}
 			if info.one_to_two.is_none() && info.two_to_one.is_none() {
-				scids_to_remove.push(*scid);
+				// We check the announcement_received_time here to ensure we don't drop
+				// announcements that we just received and are just waiting for our peer to send a
+				// channel_update for.
+				if info.announcement_received_time < min_time_unix as u64 {
+					scids_to_remove.push(*scid);
+				}
 			}
 		}
 		if !scids_to_remove.is_empty() {
@@ -1919,10 +1941,23 @@ mod tests {
 		}
 
 		if remove_by_timeout {
-			network_graph.remove_stale_channels_with_time(100 + 60 * 60 * 14);
+			network_graph.remove_stale_channels_with_time(100 + 60 * 60 * 24 * 14);
 			assert_eq!(network_graph.read_only().channels().len(), 1);
 			assert_eq!(network_graph.read_only().nodes().len(), 2);
-			network_graph.remove_stale_channels_with_time(101 + 60 * 60 * 14);
+
+			network_graph.remove_stale_channels_with_time(101 + 60 * 60 * 24 * 14);
+			#[cfg(feature = "std")]
+			{
+				// In std mode, a further check is performed before fully removing the channel -
+				// the channel_announcement must have been received at least two weeks ago. We
+				// fudge that here by indicating the time has jumped two weeks.
+				assert_eq!(network_graph.read_only().channels().len(), 1);
+				assert_eq!(network_graph.read_only().nodes().len(), 2);
+
+				use std::time::{SystemTime, UNIX_EPOCH};
+				let announcement_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs();
+				network_graph.remove_stale_channels_with_time(announcement_time + 1 + 60 * 60 * 24 * 14);
+			}
 		} else {
 			// Permanent closing deletes a channel
 			net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {