From b1fb7fdb9bacfa486320f2fbdc594d96b355c85a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 17 Mar 2022 22:04:48 +0000 Subject: [PATCH 1/2] Rename `RoutingMessageHandler::sync_routing_table` `peer_connected` Its somewhat strange to have a trait method which is named after the intended action, rather than the action that occurred, leaving it up to the implementor what action they want to take. --- lightning-net-tokio/src/lib.rs | 2 +- lightning/src/ln/msgs.rs | 2 +- lightning/src/ln/peer_handler.rs | 4 ++-- lightning/src/routing/network_graph.rs | 9 ++++----- lightning/src/util/test_utils.rs | 2 +- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index 2582cc597f2..c5c18b5df52 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -496,7 +496,7 @@ mod tests { fn handle_channel_update(&self, _msg: &ChannelUpdate) -> Result { Ok(false) } fn get_next_channel_announcements(&self, _starting_point: u64, _batch_amount: u8) -> Vec<(ChannelAnnouncement, Option, Option)> { Vec::new() } fn get_next_node_announcements(&self, _starting_point: Option<&PublicKey>, _batch_amount: u8) -> Vec { Vec::new() } - fn sync_routing_table(&self, _their_node_id: &PublicKey, _init_msg: &Init) { } + fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &Init) { } fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: ReplyChannelRange) -> Result<(), LightningError> { Ok(()) } fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) } fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: QueryChannelRange) -> Result<(), LightningError> { Ok(()) } diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index fab396e3c71..769d648fcb6 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -886,7 +886,7 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider { /// Called when a connection is established with a peer. This can be used to /// perform routing table synchronization using a strategy defined by the /// implementor. - fn sync_routing_table(&self, their_node_id: &PublicKey, init: &Init); + fn peer_connected(&self, their_node_id: &PublicKey, init: &Init); /// Handles the reply of a query we initiated to learn about channels /// for a given range of blocks. We can expect to receive one or more /// replies to a single query. diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index d3ac2ebe9a3..9d79c6385f4 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -69,7 +69,7 @@ impl RoutingMessageHandler for IgnoringMessageHandler { fn get_next_channel_announcements(&self, _starting_point: u64, _batch_amount: u8) -> Vec<(msgs::ChannelAnnouncement, Option, Option)> { Vec::new() } fn get_next_node_announcements(&self, _starting_point: Option<&PublicKey>, _batch_amount: u8) -> Vec { Vec::new() } - fn sync_routing_table(&self, _their_node_id: &PublicKey, _init: &msgs::Init) {} + fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) {} fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyChannelRange) -> Result<(), LightningError> { Ok(()) } fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) } fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::QueryChannelRange) -> Result<(), LightningError> { Ok(()) } @@ -1018,7 +1018,7 @@ impl P return Err(PeerHandleError{ no_connection_possible: true }.into()); } - self.message_handler.route_handler.sync_routing_table(&peer.their_node_id.unwrap(), &msg); + self.message_handler.route_handler.peer_connected(&peer.their_node_id.unwrap(), &msg); self.message_handler.chan_handler.peer_connected(&peer.their_node_id.unwrap(), &msg); peer.their_features = Some(msg.features); diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index a5e497bb1cf..8f3511d6dd6 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -395,8 +395,7 @@ where C::Target: chain::Access, L::Target: Logger /// 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) { - + fn peer_connected(&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 (); @@ -2271,7 +2270,7 @@ mod tests { // It should ignore if gossip_queries feature is not enabled { let init_msg = Init { features: InitFeatures::known().clear_gossip_queries() }; - net_graph_msg_handler.sync_routing_table(&node_id_1, &init_msg); + net_graph_msg_handler.peer_connected(&node_id_1, &init_msg); let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 0); } @@ -2279,7 +2278,7 @@ mod tests { // It should send a query_channel_message with the correct information { let init_msg = Init { features: InitFeatures::known() }; - net_graph_msg_handler.sync_routing_table(&node_id_1, &init_msg); + net_graph_msg_handler.peer_connected(&node_id_1, &init_msg); let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); match &events[0] { @@ -2303,7 +2302,7 @@ mod tests { 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); + net_graph_msg_handler.peer_connected(&node_id, &init_msg); let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); if n <= 5 { assert_eq!(events.len(), 1); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 27c2d9de874..3c36cdf066a 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -384,7 +384,7 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler { Vec::new() } - fn sync_routing_table(&self, _their_node_id: &PublicKey, _init_msg: &msgs::Init) {} + fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &msgs::Init) {} fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyChannelRange) -> Result<(), msgs::LightningError> { Ok(()) From 3cca221f8b41e9a42df748f972d14d851c826626 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 17 Mar 2022 22:14:43 +0000 Subject: [PATCH 2/2] Send a `gossip_timestamp_filter` on connect to enable gossip sync On connection, if our peer supports gossip queries, and we never send a `gossip_timestamp_filter`, our peer is supposed to never send us gossip outside of explicit queries. Thus, we'll end up always having stale gossip information after the first few connections we make to peers. The solution is to send a dummy `gossip_timestamp_filter` immediately after connecting to peers. --- lightning/src/ln/channelmanager.rs | 1 + lightning/src/ln/peer_handler.rs | 3 +++ lightning/src/routing/network_graph.rs | 36 ++++++++++++++++++++++---- lightning/src/util/events.rs | 10 ++++++- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f3913c5167c..3214bd4deb9 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5893,6 +5893,7 @@ impl &events::MessageSendEvent::SendChannelRangeQuery { .. } => false, &events::MessageSendEvent::SendShortIdsQuery { .. } => false, &events::MessageSendEvent::SendReplyChannelRange { .. } => false, + &events::MessageSendEvent::SendGossipTimestampFilter { .. } => false, } }); } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 9d79c6385f4..4c4e22c0c2b 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -1477,6 +1477,9 @@ impl P msg.sync_complete); self.enqueue_message(get_peer_for_forwarding!(node_id), msg); } + MessageSendEvent::SendGossipTimestampFilter { ref node_id, ref msg } => { + self.enqueue_message(get_peer_for_forwarding!(node_id), msg); + } } } diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index 8f3511d6dd6..e7c8271bf35 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -25,7 +25,7 @@ use chain; use chain::Access; use ln::features::{ChannelFeatures, NodeFeatures}; use ln::msgs::{DecodeError, ErrorAction, Init, LightningError, RoutingMessageHandler, NetAddress, MAX_VALUE_MSAT}; -use ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, OptionalField}; +use ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, OptionalField, GossipTimestampFilter}; use ln::msgs::{QueryChannelRange, ReplyChannelRange, QueryShortChannelIds, ReplyShortChannelIdsEnd}; use ln::msgs; use util::ser::{Writeable, Readable, Writer}; @@ -401,6 +401,22 @@ where C::Target: chain::Access, L::Target: Logger return (); } + // Send a gossip_timestamp_filter to enable gossip message receipt. Note that we have to + // use a "all timestamps" filter as sending the current timestamp would result in missing + // gossip messages that are simply sent late. We could calculate the intended filter time + // by looking at the current time and subtracting two weeks (before which we'll reject + // messages), but there's not a lot of reason to bother - our peers should be discarding + // the same messages. + let mut pending_events = self.pending_events.lock().unwrap(); + pending_events.push(MessageSendEvent::SendGossipTimestampFilter { + node_id: their_node_id.clone(), + msg: GossipTimestampFilter { + chain_hash: self.network_graph.genesis_hash, + first_timestamp: 0, + timestamp_range: u32::max_value(), + }, + }); + // Check if we need to perform a full synchronization with this peer if !self.should_request_full_sync(&their_node_id) { return (); @@ -409,7 +425,6 @@ where C::Target: chain::Access, L::Target: Logger 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); - let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push(MessageSendEvent::SendChannelRangeQuery { node_id: their_node_id.clone(), msg: QueryChannelRange { @@ -2280,8 +2295,17 @@ mod tests { let init_msg = Init { features: InitFeatures::known() }; net_graph_msg_handler.peer_connected(&node_id_1, &init_msg); let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); + assert_eq!(events.len(), 2); match &events[0] { + MessageSendEvent::SendGossipTimestampFilter{ node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.first_timestamp, 0); + assert_eq!(msg.timestamp_range, u32::max_value()); + }, + _ => panic!("Expected MessageSendEvent::SendChannelRangeQuery") + }; + match &events[1] { MessageSendEvent::SendChannelRangeQuery{ node_id, msg } => { assert_eq!(node_id, &node_id_1); assert_eq!(msg.chain_hash, chain_hash); @@ -2305,9 +2329,11 @@ mod tests { net_graph_msg_handler.peer_connected(&node_id, &init_msg); let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); if n <= 5 { - assert_eq!(events.len(), 1); + assert_eq!(events.len(), 2); } else { - assert_eq!(events.len(), 0); + // Even after the we stop sending the explicit query, we should still send a + // gossip_timestamp_filter on each new connection. + assert_eq!(events.len(), 1); } } diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 3b90015cab9..623b2ea01ac 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -909,7 +909,15 @@ pub enum MessageSendEvent { node_id: PublicKey, /// The reply_channel_range which should be sent. msg: msgs::ReplyChannelRange, - } + }, + /// Sends a timestamp filter for inbound gossip. This should be sent on each new connection to + /// enable receiving gossip messages from the peer. + SendGossipTimestampFilter { + /// The node_id of this message recipient + node_id: PublicKey, + /// The gossip_timestamp_filter which should be sent. + msg: msgs::GossipTimestampFilter, + }, } /// A trait indicating an object may generate message send events