Prune stale channels from network graph after RGS sync

[benthecarman]

Closes #2155

[benthecarman]

hmm this breaks a lot of test assumptions.. could gate it around a #[cfg(not(test))] but seems kinda hacky

[TheBlueMatt]

Tests are pretty easy to update:

$ git diff
diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs
index a45dc6d6c..6f46c1f2b 100644
--- a/lightning-background-processor/src/lib.rs
+++ b/lightning-background-processor/src/lib.rs
@@ -1429,8 +1429,8 @@ mod tests {
                        ];
                        $nodes[0].rapid_gossip_sync.update_network_graph_no_std(&initialization_input[..], Some(1642291930)).unwrap();
 
-                       // this should have added two channels
-                       assert_eq!($nodes[0].network_graph.read_only().channels().len(), 3);
+                       // this should have added two channels and pruned the previous one.
+                       assert_eq!($nodes[0].network_graph.read_only().channels().len(), 2);
 
                        $receive.expect("Network graph not pruned within deadline");
 
diff --git a/lightning-rapid-gossip-sync/src/processing.rs b/lightning-rapid-gossip-sync/src/processing.rs
index 8f2d387d9..0d5b302a3 100644
--- a/lightning-rapid-gossip-sync/src/processing.rs
+++ b/lightning-rapid-gossip-sync/src/processing.rs
@@ -559,6 +559,34 @@ mod tests {
                assert_eq!(network_graph.read_only().channels().len(), 2);
        }
 
+       #[test]
+       fn prunes_after_update() {
+               // this is the timestamp encoded in the binary data of valid_input below
+               let logger = TestLogger::new();
+
+               let latest_nonpruning_time = VALID_BINARY_TIMESTAMP + 60*60*24*7;
+
+               {
+                       let network_graph = NetworkGraph::new(Network::Bitcoin, &logger);
+                       assert_eq!(network_graph.read_only().channels().len(), 0);
+
+                       let rapid_sync = RapidGossipSync::new(&network_graph, &logger);
+                       let update_result = rapid_sync.update_network_graph_no_std(&VALID_RGS_BINARY, Some(latest_nonpruning_time));
+                       assert!(update_result.is_ok());
+                       assert_eq!(network_graph.read_only().channels().len(), 2);
+               }
+
+               {
+                       let network_graph = NetworkGraph::new(Network::Bitcoin, &logger);
+                       assert_eq!(network_graph.read_only().channels().len(), 0);
+
+                       let rapid_sync = RapidGossipSync::new(&network_graph, &logger);
+                       let update_result = rapid_sync.update_network_graph_no_std(&VALID_RGS_BINARY, Some(latest_nonpruning_time + 1));
+                       assert!(update_result.is_ok());
+                       assert_eq!(network_graph.read_only().channels().len(), 0);
+               }
+       }
+
        #[test]
        fn timestamp_edge_cases_are_handled_correctly() {
                // this is the timestamp encoded in the binary data of valid_input below
@@ -574,7 +602,7 @@ mod tests {
                        let rapid_sync = RapidGossipSync::new(&network_graph, &logger);
                        let update_result = rapid_sync.update_network_graph_no_std(&VALID_RGS_BINARY, Some(latest_succeeding_time));
                        assert!(update_result.is_ok());
-                       assert_eq!(network_graph.read_only().channels().len(), 2);
+                       assert_eq!(network_graph.read_only().channels().len(), 0);
                }
 
                {

[benthecarman]

Tests are pretty easy to update:

Thanks!

[TheBlueMatt]

While you're at it can you make RapidGossipSync::update_network_graph require std and make update_network_graph_from_byte_stream also require std/remove the bounds inside the function?

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (8fcbe64) 91.30% compared to head (b336b2e) 91.30%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2161   +/-   ##
=======================================
  Coverage   91.30%   91.30%           
=======================================
  Files         102      102           
  Lines       49927    50000   +73     
  Branches    49927    50000   +73     
=======================================
+ Hits        45586    45654   +68     
- Misses       4341     4346    +5     

Impacted Files	Coverage Δ
lightning-background-processor/src/lib.rs	76.77% <ø> (ø)
lightning-rapid-gossip-sync/src/lib.rs	85.13% <ø> (ø)
lightning-rapid-gossip-sync/src/processing.rs	92.80% <100.00%> (+0.55%)	⬆️

... and 9 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[TheBlueMatt]

Looks like you just need to add some std tags to some tests.

[TheBlueMatt]

Thanks!

[arik-so]

Neat!