Improve comments and combine related timestamp edge case unit tests into one.

Author: arik-so

lightning-rapid-gossip-sync/src/processing.rs

@@ -32,7 +32,7 @@ const GOSSIP_PREFIX: [u8; 4] = [76, 68, 75, 1];
 /// avoid malicious updates being able to trigger excessive memory allocation.
 const MAX_INITIAL_NODE_ID_VECTOR_CAPACITY: u32 = 50_000;
 
-/// We remove disallow gossip data that's more than two weeks old, per BOLT 7's
+/// We remove gossip data that's more than two weeks old, per BOLT 7's
 /// suggestion.
 const STALE_RGS_UPDATE_AGE_LIMIT_SECS: u64 = 60 * 60 * 24 * 14;
 
@@ -558,42 +558,7 @@ mod tests {
 	}
 
 	#[test]
-	fn full_update_succeeds_with_edge_timestamp_age() {
-		const LATEST_SEEN_TIMESTAMP: u64 = 1642291930;
-		let valid_input = vec![
-			76, 68, 75, 1, 111, 226, 140, 10, 182, 241, 179, 114, 193, 166, 162, 70, 174, 99, 247,
-			79, 147, 30, 131, 101, 225, 90, 8, 156, 104, 214, 25, 0, 0, 0, 0, 0, 97, 227, 98, 218,
-			0, 0, 0, 4, 2, 22, 7, 207, 206, 25, 164, 197, 231, 230, 231, 56, 102, 61, 250, 251,
-			187, 172, 38, 46, 79, 247, 108, 44, 155, 48, 219, 238, 252, 53, 192, 6, 67, 2, 36, 125,
-			157, 176, 223, 175, 234, 116, 94, 248, 201, 225, 97, 235, 50, 47, 115, 172, 63, 136,
-			88, 216, 115, 11, 111, 217, 114, 84, 116, 124, 231, 107, 2, 158, 1, 242, 121, 152, 106,
-			204, 131, 186, 35, 93, 70, 216, 10, 237, 224, 183, 89, 95, 65, 3, 83, 185, 58, 138,
-			181, 64, 187, 103, 127, 68, 50, 2, 201, 19, 17, 138, 136, 149, 185, 226, 156, 137, 175,
-			110, 32, 237, 0, 217, 90, 31, 100, 228, 149, 46, 219, 175, 168, 77, 4, 143, 38, 128,
-			76, 97, 0, 0, 0, 2, 0, 0, 255, 8, 153, 192, 0, 2, 27, 0, 0, 0, 1, 0, 0, 255, 2, 68,
-			226, 0, 6, 11, 0, 1, 2, 3, 0, 0, 0, 4, 0, 40, 0, 0, 0, 0, 0, 0, 3, 232, 0, 0, 3, 232,
-			0, 0, 0, 1, 0, 0, 0, 0, 29, 129, 25, 192, 255, 8, 153, 192, 0, 2, 27, 0, 0, 60, 0, 0,
-			0, 0, 0, 0, 0, 1, 0, 0, 0, 100, 0, 0, 2, 224, 0, 0, 0, 0, 58, 85, 116, 216, 0, 29, 0,
-			0, 0, 1, 0, 0, 0, 125, 0, 0, 0, 0, 58, 85, 116, 216, 255, 2, 68, 226, 0, 6, 11, 0, 1,
-			0, 0, 1,
-		];
-
-		let block_hash = genesis_block(Network::Bitcoin).block_hash();
-		let logger = TestLogger::new();
-		let network_graph = NetworkGraph::new(block_hash, &logger);
-
-		assert_eq!(network_graph.read_only().channels().len(), 0);
-
-		let earliest_failing_time = LATEST_SEEN_TIMESTAMP + STALE_RGS_UPDATE_AGE_LIMIT_SECS;
-
-		let rapid_sync = RapidGossipSync::new(&network_graph);
-		let update_result = rapid_sync.update_network_graph_no_std(&valid_input[..], Some(earliest_failing_time));
-		assert!(update_result.is_ok());
-		assert_eq!(network_graph.read_only().channels().len(), 2);
-	}
-
-	#[test]
-	fn full_update_succeeds_with_oldest_possible_timestamp() {
+	fn full_update_succeeds_at_the_beginning_of_the_unix_era() {
 		let valid_input = vec![
 			76, 68, 75, 1, 111, 226, 140, 10, 182, 241, 179, 114, 193, 166, 162, 70, 174, 99, 247,
 			79, 147, 30, 131, 101, 225, 90, 8, 156, 104, 214, 25, 0, 0, 0, 0, 0, 97, 227, 98, 218,
@@ -619,13 +584,15 @@ mod tests {
 		assert_eq!(network_graph.read_only().channels().len(), 0);
 
 		let rapid_sync = RapidGossipSync::new(&network_graph);
+		// this is mostly for checking uint underflow issues before the fuzzer does
 		let update_result = rapid_sync.update_network_graph_no_std(&valid_input[..], Some(0));
 		assert!(update_result.is_ok());
 		assert_eq!(network_graph.read_only().channels().len(), 2);
 	}
 
 	#[test]
-	fn full_update_fails_with_old_timestamp() {
+	fn timestamp_edge_cases_are_handled_correctly() {
+		// this is the timestamp encoded in the binary data of valid_input below
 		const LATEST_SEEN_TIMESTAMP: u64 = 1642291930;
 		let valid_input = vec![
 			76, 68, 75, 1, 111, 226, 140, 10, 182, 241, 179, 114, 193, 166, 162, 70, 174, 99, 247,
@@ -647,22 +614,35 @@ mod tests {
 
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
 		let logger = TestLogger::new();
-		let network_graph = NetworkGraph::new(block_hash, &logger);
 
-		assert_eq!(network_graph.read_only().channels().len(), 0);
+		let latest_succeeding_time = LATEST_SEEN_TIMESTAMP + STALE_RGS_UPDATE_AGE_LIMIT_SECS;
+		let earliest_failing_time = latest_succeeding_time + 1;
 
-		let earliest_failing_time = LATEST_SEEN_TIMESTAMP + STALE_RGS_UPDATE_AGE_LIMIT_SECS + 1;
+		{
+			let network_graph = NetworkGraph::new(block_hash, &logger);
+			assert_eq!(network_graph.read_only().channels().len(), 0);
 
-		let rapid_sync = RapidGossipSync::new(&network_graph);
-		let update_result = rapid_sync.update_network_graph_no_std(&valid_input[..], Some(earliest_failing_time));
-		assert!(update_result.is_err());
-		if let Err(GraphSyncError::LightningError(lightning_error)) = update_result {
-			assert_eq!(
-				lightning_error.err,
-				"Rapid Gossip Sync data is more than two weeks old"
-			);
-		} else {
-			panic!("Unexpected update result: {:?}", update_result)
+			let rapid_sync = RapidGossipSync::new(&network_graph);
+			let update_result = rapid_sync.update_network_graph_no_std(&valid_input[..], Some(latest_succeeding_time));
+			assert!(update_result.is_ok());
+			assert_eq!(network_graph.read_only().channels().len(), 2);
+		}
+
+		{
+			let network_graph = NetworkGraph::new(block_hash, &logger);
+			assert_eq!(network_graph.read_only().channels().len(), 0);
+
+			let rapid_sync = RapidGossipSync::new(&network_graph);
+			let update_result = rapid_sync.update_network_graph_no_std(&valid_input[..], Some(earliest_failing_time));
+			assert!(update_result.is_err());
+			if let Err(GraphSyncError::LightningError(lightning_error)) = update_result {
+				assert_eq!(
+					lightning_error.err,
+					"Rapid Gossip Sync data is more than two weeks old"
+				);
+			} else {
+				panic!("Unexpected update result: {:?}", update_result)
+			}
 		}
 	}