X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Frouting%2Fnetwork_graph.rs;h=8075462c938b797d0f8d923d786be9239533317c;hb=74cd96f972d539b9e19276bef0e9307c6b3299f1;hp=3a7cd4eb985ad3713dd468277cedb59131f9e738;hpb=7a4a29ffe0dbfdd2fe82376024ae680401c74700;p=rust-lightning

diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs
index 3a7cd4eb..8075462c 100644
--- a/lightning/src/routing/network_graph.rs
+++ b/lightning/src/routing/network_graph.rs
@@ -105,6 +105,18 @@ impl<C: Deref, L: Deref> NetGraphMsgHandler<C, L> where C::Target: chain::Access
 	pub fn read_locked_graph<'a>(&'a self) -> LockedNetworkGraph<'a> {
 		LockedNetworkGraph(self.network_graph.read().unwrap())
 	}
+
+	/// Returns true when a full routing table sync should be performed with a peer.
+	fn should_request_full_sync(&self, _node_id: &PublicKey) -> bool {
+		//TODO: Determine whether to request a full sync based on the network map.
+		const FULL_SYNCS_TO_REQUEST: usize = 5;
+		if self.full_syncs_requested.load(Ordering::Acquire) < FULL_SYNCS_TO_REQUEST {
+			self.full_syncs_requested.fetch_add(1, Ordering::AcqRel);
+			true
+		} else {
+			false
+		}
+	}
 }
 
 impl<'a> LockedNetworkGraph<'a> {
@@ -207,29 +219,27 @@ impl<C: Deref + Sync + Send, L: Deref + Sync + Send> RoutingMessageHandler for N
 		result
 	}
 
-	fn should_request_full_sync(&self, _node_id: &PublicKey) -> bool {
-		//TODO: Determine whether to request a full sync based on the network map.
-		const FULL_SYNCS_TO_REQUEST: usize = 5;
-		if self.full_syncs_requested.load(Ordering::Acquire) < FULL_SYNCS_TO_REQUEST {
-			self.full_syncs_requested.fetch_add(1, Ordering::AcqRel);
-			true
-		} else {
-			false
-		}
-	}
-
 	/// Initiates a stateless sync of routing gossip information with a peer
-	/// by calling query_channel_range. The default strategy used by this
-	/// implementation is to sync for the full block range with several peers.
+	/// using gossip_queries. The default strategy used by this implementation
+	/// is to sync the full block range with several peers.
+	///
 	/// We should expect one or more reply_channel_range messages in response
-	/// to our query. Each reply will enqueue a query_scid message to request
-	/// gossip messages for each channel. The sync is considered complete when
-	/// the final reply_scids_end message is received, though we are not
+	/// to our query_channel_range. Each reply will enqueue a query_scid message
+	/// to request gossip messages for each channel. The sync is considered complete
+	/// when the final reply_scids_end message is received, though we are not
 	/// tracking this directly.
 	fn sync_routing_table(&self, their_node_id: &PublicKey, init_msg: &Init) {
+
+		// We will only perform a sync with peers that support gossip_queries.
 		if !init_msg.features.supports_gossip_queries() {
 			return ();
 		}
+
+		// Check if we need to perform a full synchronization with this peer
+		if !self.should_request_full_sync(their_node_id) {
+			return ();
+		}
+
 		let first_blocknum = 0;
 		let number_of_blocks = 0xffffffff;
 		log_debug!(self.logger, "Sending query_channel_range peer={}, first_blocknum={}, number_of_blocks={}", log_pubkey!(their_node_id), first_blocknum, number_of_blocks);
@@ -249,7 +259,10 @@ impl<C: Deref + Sync + Send, L: Deref + Sync + Send> RoutingMessageHandler for N
 	/// stateless, it does not validate the sequencing of replies for multi-
 	/// reply ranges. It does not validate whether the reply(ies) cover the
 	/// queried range. It also does not filter SCIDs to only those in the
-	/// original query range.
+	/// original query range. We also do not validate that the chain_hash
+	/// matches the chain_hash of the NetworkGraph. Any chan_ann message that
+	/// does not match our chain_hash will be rejected when the announcement is
+	/// processed.
 	fn handle_reply_channel_range(&self, their_node_id: &PublicKey, msg: ReplyChannelRange) -> Result<(), LightningError> {
 		log_debug!(self.logger, "Handling reply_channel_range peer={}, first_blocknum={}, number_of_blocks={}, full_information={}, scids={}", log_pubkey!(their_node_id), msg.first_blocknum, msg.number_of_blocks, msg.full_information, msg.short_channel_ids.len(),);
 
@@ -1967,6 +1980,26 @@ mod tests {
 				_ => panic!("Expected MessageSendEvent::SendChannelRangeQuery")
 			};
 		}
+
+		// It should not enqueue a query when should_request_full_sync return false.
+		// The initial implementation allows syncing with the first 5 peers after
+		// which should_request_full_sync will return false
+		{
+			let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+			let init_msg = Init { features: InitFeatures::known() };
+			for n in 1..7 {
+				let node_privkey = &SecretKey::from_slice(&[n; 32]).unwrap();
+				let node_id = PublicKey::from_secret_key(&secp_ctx, node_privkey);
+				net_graph_msg_handler.sync_routing_table(&node_id, &init_msg);
+				let events = net_graph_msg_handler.get_and_clear_pending_msg_events();
+				if n <= 5 {
+					assert_eq!(events.len(), 1);
+				} else {
+					assert_eq!(events.len(), 0);
+				}
+
+			}
+		}
 	}
 
 	#[test]
@@ -1980,7 +2013,6 @@ mod tests {
 		// Test receipt of a single reply that should enqueue an SCID query
 		// matching the SCIDs in the reply
 		{
-			// Handle a single successful reply that encompasses the queried channel range
 			let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, ReplyChannelRange {
 				chain_hash,
 				full_information: true,