From 07cea6bfeda1641c2a1440f4e4847c747507c4f0 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 23 Apr 2020 09:47:15 -0700 Subject: [PATCH] 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. --- lightning/src/ln/features.rs | 20 +++++------ lightning/src/ln/peer_handler.rs | 58 +++++++++++++++++++++++++++++--- lightning/src/util/test_utils.rs | 15 +++++++-- 3 files changed, 75 insertions(+), 18 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 1e52268a..f6912662 100644 --- 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 Features { 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 15fa105d..9399cfec 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -549,8 +549,8 @@ impl PeerManager 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 PeerManager 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, peer_b: &PeerManager) -> (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> = 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> = 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 905a53e6..8385c39c 100644 --- 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 } } -- 2.30.2