Set initial_routing_sync in InitFeatures

The initial_routing_sync feature is set by peer_handler whenever a full
sync of the network graph is desired. It is not explicitly set when
creating features with InitFeatures::supported().

An upcoming refactor will change supported() to known(), which will
return all features known by the implementation. Thus, the
initial_routing_sync flag will need to be set by default. This commit
makes the behavior change ahead of the refactor.

diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs
index 1e52268ac351dffb67290ff073ef97d7c3953980..f6912662497fb33f712838f40c907b8cf65e4235 100644 (file)
--- a/lightning/src/ln/features.rs
+++ b/lightning/src/ln/features.rs
@@ -81,7 +81,7 @@ impl InitFeatures {
        /// Create a Features with the features we support
        pub fn supported() -> InitFeatures {
                InitFeatures {
-                       flags: vec![2 | 1 << 5, 1 << (9-8) | 1 << (15 - 8), 1 << (17 - 8*2)],
+                       flags: vec![2 | 1 << 3 | 1 << 5, 1 << (9-8) | 1 << (15 - 8), 1 << (17 - 8*2)],
                        mark: PhantomData,
                }
        }
@@ -286,11 +286,9 @@ impl<T: sealed::InitialRoutingSync> Features<T> {
        pub(crate) fn initial_routing_sync(&self) -> bool {
                self.flags.len() > 0 && (self.flags[0] & (1 << 3)) != 0
        }
-       pub(crate) fn set_initial_routing_sync(&mut self) {
-               if self.flags.len() == 0 {
-                       self.flags.resize(1, 1 << 3);
-               } else {
-                       self.flags[0] |= 1 << 3;
+       pub(crate) fn clear_initial_routing_sync(&mut self) {
+               if self.flags.len() > 0 {
+                       self.flags[0] &= !(1 << 3);
                }
        }
 }
@@ -364,9 +362,9 @@ mod tests {
                assert!(NodeFeatures::supported().supports_basic_mpp());
 
                let mut init_features = InitFeatures::supported();
-               init_features.set_initial_routing_sync();
-               assert!(!init_features.requires_unknown_bits());
-               assert!(!init_features.supports_unknown_bits());
+               assert!(init_features.initial_routing_sync());
+               init_features.clear_initial_routing_sync();
+               assert!(!init_features.initial_routing_sync());
        }
 
        #[test]
@@ -381,8 +379,8 @@ mod tests {
        #[test]
        fn test_node_with_known_relevant_init_flags() {
                // Create an InitFeatures with initial_routing_sync supported.
-               let mut init_features = InitFeatures::supported();
-               init_features.set_initial_routing_sync();
+               let init_features = InitFeatures::supported();
+               assert!(init_features.initial_routing_sync());
 
                // Attempt to pull out non-node-context feature flags from these InitFeatures.
                let res = NodeFeatures::with_known_relevant_init_flags(&init_features);

diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index 15fa105d0c0465e5d3dd27ac421bca59e8004f91..9399cfecf69f890cbcf5b96c033c66b7b04945d5 100644 (file)
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -549,8 +549,8 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
                                                                        peer.their_node_id = Some(their_node_id);
                                                                        insert_node_id!();
                                                                        let mut features = InitFeatures::supported();
-                                                                       if self.message_handler.route_handler.should_request_full_sync(&peer.their_node_id.unwrap()) {
-                                                                               features.set_initial_routing_sync();
+                                                                       if !self.message_handler.route_handler.should_request_full_sync(&peer.their_node_id.unwrap()) {
+                                                                               features.clear_initial_routing_sync();
                                                                        }
 
                                                                        let resp = msgs::Init { features };
@@ -643,8 +643,8 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
 
                                                                                                if !peer.outbound {
                                                                                                        let mut features = InitFeatures::supported();
-                                                                                                       if self.message_handler.route_handler.should_request_full_sync(&peer.their_node_id.unwrap()) {
-                                                                                                               features.set_initial_routing_sync();
+                                                                                                       if !self.message_handler.route_handler.should_request_full_sync(&peer.their_node_id.unwrap()) {
+                                                                                                               features.clear_initial_routing_sync();
                                                                                                        }
 
                                                                                                        let resp = msgs::Init { features };
@@ -1247,6 +1247,13 @@ mod tests {
                (fd_a.clone(), fd_b.clone())
        }
 
+       fn establish_connection_and_read_events<'a>(peer_a: &PeerManager<FileDescriptor, &'a test_utils::TestChannelMessageHandler>, peer_b: &PeerManager<FileDescriptor, &'a test_utils::TestChannelMessageHandler>) -> (FileDescriptor, FileDescriptor) {
+               let (mut fd_a, mut fd_b) = establish_connection(peer_a, peer_b);
+               assert_eq!(peer_b.read_event(&mut fd_b, &fd_a.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
+               assert_eq!(peer_a.read_event(&mut fd_a, &fd_b.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
+               (fd_a.clone(), fd_b.clone())
+       }
+
        #[test]
        fn test_disconnect_peer() {
                // Simple test which builds a network of PeerManager, connects and brings them to NoiseState::Finished and
@@ -1421,4 +1428,47 @@ mod tests {
                assert_eq!(routing_handlers_concrete[1].clone().chan_upds_recvd.load(Ordering::Acquire), 100);
                assert_eq!(routing_handlers_concrete[1].clone().chan_anns_recvd.load(Ordering::Acquire), 50);
        }
+
+       #[test]
+       fn limit_initial_routing_sync_requests() {
+               // Inbound peer 0 requests initial_routing_sync, but outbound peer 1 does not.
+               {
+                       let chan_handlers = create_chan_handlers(2);
+                       let routing_handlers: Vec<Arc<msgs::RoutingMessageHandler>> = vec![
+                               Arc::new(test_utils::TestRoutingMessageHandler::new().set_request_full_sync()),
+                               Arc::new(test_utils::TestRoutingMessageHandler::new()),
+                       ];
+                       let peers = create_network(2, &chan_handlers, Some(&routing_handlers));
+                       let (fd_0_to_1, fd_1_to_0) = establish_connection_and_read_events(&peers[0], &peers[1]);
+
+                       let peer_0 = peers[0].peers.lock().unwrap();
+                       let peer_1 = peers[1].peers.lock().unwrap();
+
+                       let peer_0_features = peer_1.peers.get(&fd_1_to_0).unwrap().their_features.as_ref();
+                       let peer_1_features = peer_0.peers.get(&fd_0_to_1).unwrap().their_features.as_ref();
+
+                       assert!(peer_0_features.unwrap().initial_routing_sync());
+                       assert!(!peer_1_features.unwrap().initial_routing_sync());
+               }
+
+               // Outbound peer 1 requests initial_routing_sync, but inbound peer 0 does not.
+               {
+                       let chan_handlers = create_chan_handlers(2);
+                       let routing_handlers: Vec<Arc<msgs::RoutingMessageHandler>> = vec![
+                               Arc::new(test_utils::TestRoutingMessageHandler::new()),
+                               Arc::new(test_utils::TestRoutingMessageHandler::new().set_request_full_sync()),
+                       ];
+                       let peers = create_network(2, &chan_handlers, Some(&routing_handlers));
+                       let (fd_0_to_1, fd_1_to_0) = establish_connection_and_read_events(&peers[0], &peers[1]);
+
+                       let peer_0 = peers[0].peers.lock().unwrap();
+                       let peer_1 = peers[1].peers.lock().unwrap();
+
+                       let peer_0_features = peer_1.peers.get(&fd_1_to_0).unwrap().their_features.as_ref();
+                       let peer_1_features = peer_0.peers.get(&fd_0_to_1).unwrap().their_features.as_ref();
+
+                       assert!(!peer_0_features.unwrap().initial_routing_sync());
+                       assert!(peer_1_features.unwrap().initial_routing_sync());
+               }
+       }
 }

diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 905a53e6648c247022f7163c4d98819394b307e9..8385c39c1db081940bdbc373bf23d461cccc9e19 100644 (file)
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -170,11 +170,20 @@ impl events::MessageSendEventsProvider for TestChannelMessageHandler {
        }
 }
 
-pub struct TestRoutingMessageHandler {}
+pub struct TestRoutingMessageHandler {
+       request_full_sync: bool,
+}
 
 impl TestRoutingMessageHandler {
        pub fn new() -> Self {
-               TestRoutingMessageHandler {}
+               TestRoutingMessageHandler {
+                       request_full_sync: false,
+               }
+       }
+
+       pub fn set_request_full_sync(mut self) -> Self {
+               self.request_full_sync = true;
+               self
        }
 }
 impl msgs::RoutingMessageHandler for TestRoutingMessageHandler {
@@ -195,7 +204,7 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler {
                Vec::new()
        }
        fn should_request_full_sync(&self, _node_id: &PublicKey) -> bool {
-               true
+               self.request_full_sync
        }
 }