f enable tracking for no-std too, revert to system clock

Author: dunxen

lightning/src/routing/gossip.rs

@@ -45,10 +45,6 @@ use bitcoin::hashes::hex::ToHex;
 
 #[cfg(feature = "std")]
 use std::time::{SystemTime, UNIX_EPOCH};
-#[cfg(all(not(test), feature = "std"))]
-use std::time::Instant;
-#[cfg(all(test, feature = "std"))]
-use util::time::{Time, tests::SinceEpoch as Instant};
 
 /// We remove stale channel directional info two weeks after the last update, per BOLT 7's
 /// suggestion.
@@ -138,18 +134,27 @@ pub struct NetworkGraph<L: Deref> where L::Target: Logger {
 	channels: RwLock<BTreeMap<u64, ChannelInfo>>,
 	nodes: RwLock<BTreeMap<NodeId, NodeInfo>>,
 	// Lock order: removed_channels -> removed_nodes
-	#[cfg(feature = "std")]
+	//
+	// NOTE: In the following `removed_*` maps, we use seconds since UNIX epoch to track time instead
+	// of `std::time::Instant`s for a few reasons:
+	//   * We want it to be possible to do tracking in no-std environments where we can compare
+	//     a provided current UNIX timestamp with the time at which we started tracking.
+	//   * In the future, if we decide to persist these maps, they will already be serializable.
+	//   * Although we lose out on the platform's monotonic clock, the system clock in a std
+	//     environment should be practical over the time period we are considering (on the order of a
+	//     week).
+	//
 	/// Keeps track of short channel IDs for channels we have explicitly removed due to permanent
-	/// failure so that we don't resync them from gossip. Each SCID is mapped to the instant when we
-	/// removed it from the network graph so that we can forget we removed it after some time (by
-	/// removing it from this map) and it can be resynced from gossip if it appears again.
-	removed_channels: Mutex<HashMap<u64, Instant>>,
-	#[cfg(feature = "std")]
+	/// failure so that we don't resync them from gossip. Each SCID is mapped to the time in seconds
+	/// since the UNIX epoch when we removed it from the network graph so that we can forget we
+	/// removed it after some time (by removing it from this map) and it can be resynced from gossip
+	/// if it appears again.
+	removed_channels: Mutex<HashMap<u64, u64>>,
 	/// Keeps track of [`NodeId`]s we have explicitly removed due to permanent failure so that we
-	/// don't resync them from gossip. Each [`NodeId`] is mapped to the instant when we removed it
-	/// from the network graph so that we can forget we removed it after some time (by removing it
-	/// from this map) and it can be resynced from gossip if it appears again.
-	removed_nodes: Mutex<HashMap<NodeId, Instant>>,
+	/// don't resync them from gossip. Each [`NodeId`] is mapped to the time in seconds since the UNIX
+	/// epoch when we removed it from the network graph so that we can forget we removed it after some
+	/// time (by removing it from this map) and it can be resynced from gossip if it appears again.
+	removed_nodes: Mutex<HashMap<NodeId, u64>>,
 }
 
 /// A read-only view of [`NetworkGraph`].
@@ -1225,9 +1230,7 @@ impl<L: Deref> ReadableArgs<L> for NetworkGraph<L> where L::Target: Logger {
 			channels: RwLock::new(channels),
 			nodes: RwLock::new(nodes),
 			last_rapid_gossip_sync_timestamp: Mutex::new(last_rapid_gossip_sync_timestamp),
-			#[cfg(feature = "std")]
 			removed_nodes: Mutex::new(HashMap::new()),
-			#[cfg(feature = "std")]
 			removed_channels: Mutex::new(HashMap::new()),
 		})
 	}
@@ -1265,10 +1268,8 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 			channels: RwLock::new(BTreeMap::new()),
 			nodes: RwLock::new(BTreeMap::new()),
 			last_rapid_gossip_sync_timestamp: Mutex::new(None),
-			#[cfg(feature = "std")]
-			removed_nodes: Mutex::new(HashMap::new()),
-			#[cfg(feature = "std")]
 			removed_channels: Mutex::new(HashMap::new()),
+			removed_nodes: Mutex::new(HashMap::new()),
 		}
 	}
 
@@ -1516,7 +1517,6 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 			}
 		}
 
-		#[cfg(feature = "std")]
 		{
 			let removed_channels = self.removed_channels.lock().unwrap();
 			let removed_nodes = self.removed_nodes.lock().unwrap();
@@ -1578,17 +1578,33 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 		self.add_channel_between_nodes(msg.short_channel_id, chan_info, utxo_value)
 	}
 
+	#[cfg(feature = "std")]
 	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
 	/// If permanent, removes a channel from the local storage.
 	/// May cause the removal of nodes too, if this was their last channel.
 	/// If not permanent, makes channels unavailable for routing.
+	///
+	/// This method is only available with the `std` feature. See
+	/// [`NetworkGraph::channel_failed_with_time`] for `no-std` use.
 	pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
+		let current_time_unix = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs();
+		self.channel_failed_with_time(short_channel_id, is_permanent, current_time_unix);
+	}
+
+	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
+	/// If permanent, removes a channel from the local storage.
+	/// May cause the removal of nodes too, if this was their last channel.
+	/// If not permanent, makes channels unavailable for routing.
+	///
+	/// This function takes the current unix time as an argument. For users with the `std` feature
+	/// enabled, [`NetworkGraph::channel_failed`] may be preferable.
+	pub fn channel_failed_with_time(&self, short_channel_id: u64, is_permanent: bool,
+		current_time_unix: u64) {
 		let mut channels = self.channels.write().unwrap();
 		if is_permanent {
 			if let Some(chan) = channels.remove(&short_channel_id) {
 				let mut nodes = self.nodes.write().unwrap();
-				#[cfg(feature = "std")]
-				self.removed_channels.lock().unwrap().insert(short_channel_id, Instant::now());
+				self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 				Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 			}
 		} else {
@@ -1603,40 +1619,45 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 		}
 	}
 
+	#[cfg(feature = "std")]
 	/// Marks a node in the graph as permanently failed, effectively removing it and its channels
 	/// from local storage.
 	///
-	/// If called with the "std" feature enabled, the removed node and channels will also be tracked
-	/// as removed so they're not re-added on gossip re-broadcast.
+	/// This method is only available with the `std` feature. See
+	/// [`NetworkGraph::node_failed_permanent_with_time`] for `no-std` use.
 	pub fn node_failed_permanent(&self, node_id: &PublicKey) {
+		let current_time_unix = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs();
+		self.node_failed_permanent_with_time(node_id, current_time_unix);
+	}
+
+	/// Marks a node in the graph as permanently failed, effectively removing it and its channels
+	/// from local storage.
+	///
+	/// This function takes the current unix time as an argument. For users with the `std` feature
+	/// enabled, [`NetworkGraph::node_failed_permanent`] may be preferable.
+	pub fn node_failed_permanent_with_time(&self, node_id: &PublicKey, current_time_unix: u64) {
 		let node_id = NodeId::from_pubkey(node_id);
 		let mut channels = self.channels.write().unwrap();
 		let mut nodes = self.nodes.write().unwrap();
-		#[cfg(feature = "std")]
 		let mut removed_channels = self.removed_channels.lock().unwrap();
-		#[cfg(feature = "std")]
 		let mut removed_nodes = self.removed_nodes.lock().unwrap();
-		#[cfg(feature = "std")]
-		let time = Instant::now();
 
 		if let Some(node) = nodes.remove(&node_id) {
 			for scid in node.channels.iter() {
 				if let Some(chan_info) = channels.remove(scid) {
 					let other_node_id = if node_id == chan_info.node_one { chan_info.node_two } else { chan_info.node_one };
-					if let BtreeEntry::Occupied(mut node_entry) = nodes.entry(other_node_id) {
-						node_entry.get_mut().channels.retain(|chan_id| {
+					if let BtreeEntry::Occupied(mut other_node_entry) = nodes.entry(other_node_id) {
+						other_node_entry.get_mut().channels.retain(|chan_id| {
 							*scid != *chan_id
 						});
-						if node_entry.get().channels.is_empty() {
-							node_entry.remove_entry();
+						if other_node_entry.get().channels.is_empty() {
+							other_node_entry.remove_entry();
 						}
 					}
-					#[cfg(feature = "std")]
-					removed_channels.insert(*scid, time);
+					removed_channels.insert(*scid, current_time_unix);
 				}
 			}
-			#[cfg(feature = "std")]
-			removed_nodes.insert(node_id, time);
+			removed_nodes.insert(node_id, current_time_unix);
 		}
 	}
 
@@ -1670,6 +1691,9 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 	/// updates every two weeks, the non-normative section of BOLT 7 currently suggests that
 	/// pruning occur for updates which are at least two weeks old, which we implement here.
 	///
+	/// This method will also cause us to stop tracking removed nodes and channels if they have been
+	/// in the map for a while so that these can be resynced from gossip in the future.
+	///
 	/// This function takes the current unix time as an argument. For users with the `std` feature
 	/// enabled, [`NetworkGraph::remove_stale_channels_and_tracking`] may be preferable.
 	pub fn remove_stale_channels_and_tracking_with_time(&self, current_time_unix: u64) {
@@ -1705,11 +1729,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 			}
 		}
 
-		#[cfg(feature = "std")]
-		{
-			self.removed_channels.lock().unwrap().retain(|_, time| time.elapsed().as_secs() < REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS);
-			self.removed_nodes.lock().unwrap().retain(|_, time| time.elapsed().as_secs() < REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS);
-		}
+		self.removed_channels.lock().unwrap().retain(|_, time| {
+			current_time_unix.saturating_sub(*time) < REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS });
+		self.removed_nodes.lock().unwrap().retain(|_, time| {
+			current_time_unix.saturating_sub(*time) < REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS });
 	}
 
 	/// For an already known (from announcement) channel, update info about one of the directions
@@ -2228,12 +2251,12 @@ mod tests {
 
 		#[cfg(feature = "std")]
 		{
-			use std::time::Duration;
-			use util::time::tests::SinceEpoch;
+			use std::time::{SystemTime, UNIX_EPOCH};
 
+			let tracking_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs();
 			// Mark a node as permanently failed so it's tracked as removed.
-			gossip_sync.network_graph().node_failed_permanent(
-				&PublicKey::from_secret_key(&secp_ctx, node_1_privkey));
+			gossip_sync.network_graph().node_failed_permanent_with_time(
+				&PublicKey::from_secret_key(&secp_ctx, node_1_privkey), tracking_time);
 
 			// Return error and ignore valid channel announcement if one of the nodes has been tracked as removed.
 			let valid_announcement = get_signed_channel_announcement(|unsigned_announcement| {
@@ -2244,9 +2267,7 @@ mod tests {
 				Err(e) => assert_eq!(e.err, "Channel with SCID 3 or one of its nodes was removed from our network graph recently")
 			}
 
-			// Advance mock time so node being tracked is stale, and then remove stale tracking.
-			SinceEpoch::advance(Duration::from_secs(REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS));
-			gossip_sync.network_graph().remove_stale_channels_and_tracking();
+			gossip_sync.network_graph().remove_stale_channels_and_tracking_with_time(tracking_time + REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS);
 
 			// The above channel announcement should be handled as per normal now.
 			match gossip_sync.handle_channel_announcement(&valid_announcement) {
@@ -2585,8 +2606,9 @@ mod tests {
 
 		#[cfg(feature = "std")]
 		{
-			use std::time::Duration;
-			use util::time::tests::SinceEpoch;
+			use std::time::{SystemTime, UNIX_EPOCH};
+
+			let tracking_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs();
 
 			// Clear tracked nodes and channels for clean slate
 			network_graph.removed_channels.lock().unwrap().clear();
@@ -2599,16 +2621,16 @@ mod tests {
 
 			// Mark the channel as permanently failed. This will also remove the two nodes
 			// and all of the entries will be tracked as removed.
-			network_graph.channel_failed(short_channel_id, true);
+			network_graph.channel_failed_with_time(short_channel_id, true, tracking_time);
 
 			// Should not remove from tracking if insufficient time has passed
-			SinceEpoch::advance(Duration::from_secs(REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS - 1));
-			network_graph.remove_stale_channels_and_tracking();
+			network_graph.remove_stale_channels_and_tracking_with_time(
+				tracking_time + REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS - 1);
 			assert_eq!(network_graph.removed_channels.lock().unwrap().len(), 1);
 
 			// Advance mock time so the entries are removed on next call
-			SinceEpoch::advance(Duration::from_secs(1));
-			network_graph.remove_stale_channels_and_tracking();
+			network_graph.remove_stale_channels_and_tracking_with_time(
+				tracking_time + REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS);
 			assert!(network_graph.removed_channels.lock().unwrap().is_empty());
 			assert!(network_graph.removed_nodes.lock().unwrap().is_empty());
 		}