From 5af299e7d8e1d67302d0eb967b94938f5eec5003 Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Wed, 21 Oct 2020 16:31:22 -0400 Subject: [PATCH 01/12] Add gossip_queries feature flag Support for the gossip_queries feature flag (bits 6/7) is added to the Features struct. This feature is available in the Init and Node contexts. The gossip_queries feature is not fully implemented so this feature is disabled when sent to peers in the Init message. --- lightning/src/ln/features.rs | 31 ++++++++++++++++++++++++++++--- lightning/src/ln/peer_handler.rs | 7 ++++--- lightning/src/ln/wire.rs | 11 ++++++----- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index f573ac4c43a..0c3c360189f 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -104,7 +104,7 @@ mod sealed { ], optional_features: [ // Byte 0 - DataLossProtect | InitialRoutingSync | UpfrontShutdownScript, + DataLossProtect | InitialRoutingSync | UpfrontShutdownScript | GossipQueries, // Byte 1 VariableLengthOnion | PaymentSecret, // Byte 2 @@ -122,7 +122,7 @@ mod sealed { ], optional_features: [ // Byte 0 - DataLossProtect | UpfrontShutdownScript, + DataLossProtect | UpfrontShutdownScript | GossipQueries, // Byte 1 VariableLengthOnion | PaymentSecret, // Byte 2 @@ -243,6 +243,8 @@ mod sealed { "Feature flags for `initial_routing_sync`."); define_feature!(5, UpfrontShutdownScript, [InitContext, NodeContext], "Feature flags for `option_upfront_shutdown_script`."); + define_feature!(7, GossipQueries, [InitContext, NodeContext], + "Feature flags for `gossip_queries`."); define_feature!(9, VariableLengthOnion, [InitContext, NodeContext], "Feature flags for `var_onion_optin`."); define_feature!(13, StaticRemoteKey, [InitContext, NodeContext], @@ -473,6 +475,21 @@ impl Features { } } + +impl Features { + #[cfg(test)] + pub(crate) fn requires_gossip_queries(&self) -> bool { + ::requires_feature(&self.flags) + } + pub(crate) fn supports_gossip_queries(&self) -> bool { + ::supports_feature(&self.flags) + } + pub(crate) fn clear_gossip_queries(mut self) -> Self { + ::clear_bits(&mut self.flags); + self + } +} + impl Features { #[cfg(test)] pub(crate) fn requires_variable_length_onion(&self) -> bool { @@ -568,6 +585,11 @@ mod tests { assert!(!InitFeatures::known().requires_upfront_shutdown_script()); assert!(!NodeFeatures::known().requires_upfront_shutdown_script()); + assert!(InitFeatures::known().supports_gossip_queries()); + assert!(NodeFeatures::known().supports_gossip_queries()); + assert!(!InitFeatures::known().requires_gossip_queries()); + assert!(!NodeFeatures::known().requires_gossip_queries()); + assert!(InitFeatures::known().supports_data_loss_protect()); assert!(NodeFeatures::known().supports_data_loss_protect()); assert!(!InitFeatures::known().requires_data_loss_protect()); @@ -620,9 +642,10 @@ mod tests { #[test] fn convert_to_context_with_relevant_flags() { - let init_features = InitFeatures::known().clear_upfront_shutdown_script(); + let init_features = InitFeatures::known().clear_upfront_shutdown_script().clear_gossip_queries(); assert!(init_features.initial_routing_sync()); assert!(!init_features.supports_upfront_shutdown_script()); + assert!(!init_features.supports_gossip_queries()); let node_features: NodeFeatures = init_features.to_context(); { @@ -639,8 +662,10 @@ mod tests { // Check that cleared flags are kept blank when converting back: // - initial_routing_sync was not applicable to NodeContext // - upfront_shutdown_script was cleared before converting + // - gossip_queries was cleared before converting let features: InitFeatures = node_features.to_context_internal(); assert!(!features.initial_routing_sync()); assert!(!features.supports_upfront_shutdown_script()); + assert!(!init_features.supports_gossip_queries()); } } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 6e997e53b15..1e57177a0d6 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -584,7 +584,7 @@ impl PeerManager PeerManager PeerManager) { + fn check_init_msg(buffer: Vec, expect_unknown: bool) { let mut reader = ::std::io::Cursor::new(buffer); let decoded_msg = read(&mut reader).unwrap(); match decoded_msg { Message::Init(msgs::Init { features }) => { assert!(features.supports_variable_length_onion()); assert!(features.supports_upfront_shutdown_script()); - assert!(features.supports_unknown_bits()); + assert!(features.supports_gossip_queries()); + assert_eq!(expect_unknown, features.supports_unknown_bits()); assert!(!features.requires_unknown_bits()); assert!(!features.initial_routing_sync()); }, @@ -450,7 +451,7 @@ mod tests { Message::NodeAnnouncement(msgs::NodeAnnouncement { contents: msgs::UnsignedNodeAnnouncement { features, ..}, ..}) => { assert!(features.supports_variable_length_onion()); assert!(features.supports_upfront_shutdown_script()); - assert!(features.supports_unknown_bits()); + assert!(features.supports_gossip_queries()); assert!(!features.requires_unknown_bits()); }, _ => panic!("Expected node announcement, found message type: {}", decoded_msg.type_id()) From 3220f3b18233518688d2c689270d0adba25ed79e Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Wed, 21 Oct 2020 17:20:26 -0400 Subject: [PATCH 02/12] Add gossip_queries messages to wire decoding To enable gossip_queries message decoding, this commit implements the wire module's Encoding trait for each message type. It also adds these messages to the wire module's Message enum and the read function to enable decoding of a buffer. --- lightning/src/ln/peer_handler.rs | 15 +++++++++++ lightning/src/ln/wire.rs | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 1e57177a0d6..19585616101 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -841,6 +841,21 @@ impl PeerManager { + // TODO: handle message + }, + wire::Message::ReplyShortChannelIdsEnd(_msg) => { + // TODO: handle message + }, + wire::Message::QueryChannelRange(_msg) => { + // TODO: handle message + }, + wire::Message::ReplyChannelRange(_msg) => { + // TODO: handle message + }, + wire::Message::GossipTimestampFilter(_msg) => { + // TODO: handle message + }, // Unknown messages: wire::Message::Unknown(msg_type) if msg_type.is_even() => { diff --git a/lightning/src/ln/wire.rs b/lightning/src/ln/wire.rs index 6de11ea06d8..8197ce151ef 100644 --- a/lightning/src/ln/wire.rs +++ b/lightning/src/ln/wire.rs @@ -55,6 +55,11 @@ pub enum Message { ChannelAnnouncement(msgs::ChannelAnnouncement), NodeAnnouncement(msgs::NodeAnnouncement), ChannelUpdate(msgs::ChannelUpdate), + QueryShortChannelIds(msgs::QueryShortChannelIds), + ReplyShortChannelIdsEnd(msgs::ReplyShortChannelIdsEnd), + QueryChannelRange(msgs::QueryChannelRange), + ReplyChannelRange(msgs::ReplyChannelRange), + GossipTimestampFilter(msgs::GossipTimestampFilter), /// A message that could not be decoded because its type is unknown. Unknown(MessageType), } @@ -90,6 +95,11 @@ impl Message { &Message::ChannelAnnouncement(ref msg) => msg.type_id(), &Message::NodeAnnouncement(ref msg) => msg.type_id(), &Message::ChannelUpdate(ref msg) => msg.type_id(), + &Message::QueryShortChannelIds(ref msg) => msg.type_id(), + &Message::ReplyShortChannelIdsEnd(ref msg) => msg.type_id(), + &Message::QueryChannelRange(ref msg) => msg.type_id(), + &Message::ReplyChannelRange(ref msg) => msg.type_id(), + &Message::GossipTimestampFilter(ref msg) => msg.type_id(), &Message::Unknown(type_id) => type_id, } } @@ -186,6 +196,21 @@ pub fn read(buffer: &mut R) -> Result { Ok(Message::ChannelUpdate(Readable::read(buffer)?)) }, + msgs::QueryShortChannelIds::TYPE => { + Ok(Message::QueryShortChannelIds(Readable::read(buffer)?)) + }, + msgs::ReplyShortChannelIdsEnd::TYPE => { + Ok(Message::ReplyShortChannelIdsEnd(Readable::read(buffer)?)) + }, + msgs::QueryChannelRange::TYPE => { + Ok(Message::QueryChannelRange(Readable::read(buffer)?)) + }, + msgs::ReplyChannelRange::TYPE => { + Ok(Message::ReplyChannelRange(Readable::read(buffer)?)) + } + msgs::GossipTimestampFilter::TYPE => { + Ok(Message::GossipTimestampFilter(Readable::read(buffer)?)) + }, _ => { Ok(Message::Unknown(MessageType(message_type))) }, @@ -312,6 +337,26 @@ impl Encode for msgs::ChannelUpdate { const TYPE: u16 = 258; } +impl Encode for msgs::QueryShortChannelIds { + const TYPE: u16 = 261; +} + +impl Encode for msgs::ReplyShortChannelIdsEnd { + const TYPE: u16 = 262; +} + +impl Encode for msgs::QueryChannelRange { + const TYPE: u16 = 263; +} + +impl Encode for msgs::ReplyChannelRange { + const TYPE: u16 = 264; +} + +impl Encode for msgs::GossipTimestampFilter { + const TYPE: u16 = 265; +} + #[cfg(test)] mod tests { use super::*; From 34271fb75037372ff6063594d7a537c07670c863 Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Thu, 22 Oct 2020 08:47:24 -0400 Subject: [PATCH 03/12] Add send message events for gossip_queries This change enables initiating gossip queries with a peer using the SendMessageEvent enum. Specifically we add an event for sending query_channel_range to discover the existance of channels and an event for sending query_short_channel_ids to request routing gossip messages for a set of channels. These events are handled inside the process_events method of PeerManager which sends the serialized message to the peer. --- lightning/src/ln/channelmanager.rs | 2 ++ lightning/src/ln/peer_handler.rs | 10 ++++++++++ lightning/src/util/events.rs | 17 ++++++++++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0f5e7f8ad63..c39eef3ea47 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3404,6 +3404,8 @@ impl true, &events::MessageSendEvent::HandleError { ref node_id, .. } => node_id != counterparty_node_id, &events::MessageSendEvent::PaymentFailureNetworkUpdate { .. } => true, + &events::MessageSendEvent::SendChannelRangeQuery { .. } => false, + &events::MessageSendEvent::SendShortIdsQuery { .. } => false, } }); } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 19585616101..b318e503182 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -1131,6 +1131,16 @@ impl PeerManager { + let (mut descriptor, peer) = get_peer_for_forwarding!(node_id, {}); + peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg))); + self.do_attempt_write_data(&mut descriptor, peer); + }, + MessageSendEvent::SendShortIdsQuery { ref node_id, ref msg } => { + let (mut descriptor, peer) = get_peer_for_forwarding!(node_id, {}); + peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg))); + self.do_attempt_write_data(&mut descriptor, peer); } } } diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 3eeacdc732c..6f6f32daeab 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -346,7 +346,22 @@ pub enum MessageSendEvent { PaymentFailureNetworkUpdate { /// The channel/node update which should be sent to NetGraphMsgHandler update: msgs::HTLCFailChannelUpdate, - } + }, + /// Query a peer for channels with funding transaction UTXOs in a block range. + SendChannelRangeQuery { + /// The node_id of this message recipient + node_id: PublicKey, + /// The query_channel_range which should be sent. + msg: msgs::QueryChannelRange, + }, + /// Request routing gossip messages from a peer for a list of channels identified by + /// their short_channel_ids. + SendShortIdsQuery { + /// The node_id of this message recipient + node_id: PublicKey, + /// The query_short_channel_ids which should be sent. + msg: msgs::QueryShortChannelIds, + }, } /// A trait indicating an object may generate message send events From 55e5aafcfe3b4d55df1fa500846e3a0f093f85c8 Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Thu, 22 Oct 2020 10:51:54 -0400 Subject: [PATCH 04/12] Add gossip_queries methods to RoutingMessageHandler trait Defines message handlers for gossip_queries messages in the RoutingMessageHandler trait. The MessageSendEventsProvider supertrait is added to RoutingMessageHandler so that the implementor can use SendMessageEvents to send messages to a peer at the appropriate time. The trait methods are stubbed in NetGraphMsgHandler which implements RoutingMessageHandler and return a "not implemented" error. --- lightning-net-tokio/src/lib.rs | 6 +++ lightning/src/ln/msgs.rs | 23 ++++++++- lightning/src/ln/peer_handler.rs | 17 ++++--- lightning/src/routing/network_graph.rs | 68 ++++++++++++++++++++++++++ lightning/src/util/test_utils.rs | 30 ++++++++++++ 5 files changed, 135 insertions(+), 9 deletions(-) diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index 36384380fb1..6f80ef2e396 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -536,6 +536,12 @@ mod tests { 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 should_request_full_sync(&self, _node_id: &PublicKey) -> bool { false } + fn query_channel_range(&self, _their_node_id: &PublicKey, _chain_hash: bitcoin::BlockHash, _first_blocknum: u32, _block_range: u32) -> Result<(), LightningError> { Ok(()) } + fn query_short_channel_ids(&self, _their_node_id: &PublicKey, _chain_hash: bitcoin::BlockHash, _short_channel_ids: Vec) -> Result<(), LightningError> { Ok(()) } + 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(()) } + fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: &QueryShortChannelIds) -> Result<(), LightningError> { Ok(()) } } impl ChannelMessageHandler for MsgHandler { fn handle_open_channel(&self, _their_node_id: &PublicKey, _their_features: InitFeatures, _msg: &OpenChannel) {} diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index f57cf0fd1fb..9111a1ed657 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -804,7 +804,7 @@ pub trait ChannelMessageHandler : events::MessageSendEventsProvider + Send + Syn } /// A trait to describe an object which can receive routing messages. -pub trait RoutingMessageHandler : Send + Sync { +pub trait RoutingMessageHandler : Send + Sync + events::MessageSendEventsProvider { /// Handle an incoming node_announcement message, returning true if it should be forwarded on, /// false or returning an Err otherwise. fn handle_node_announcement(&self, msg: &NodeAnnouncement) -> Result; @@ -827,6 +827,27 @@ pub trait RoutingMessageHandler : Send + Sync { fn get_next_node_announcements(&self, starting_point: Option<&PublicKey>, batch_amount: u8) -> Vec; /// Returns whether a full sync should be requested from a peer. fn should_request_full_sync(&self, node_id: &PublicKey) -> bool; + /// Queries a peer for a list of channels with a funding UTXO in the requested + /// chain and range of blocks. + fn query_channel_range(&self, their_node_id: &PublicKey, chain_hash: BlockHash, first_blocknum: u32, number_of_blocks: u32) -> Result<(), LightningError>; + /// Queries a peer for routing gossip messages for a set of channels identified + /// by their short_channel_ids. + fn query_short_channel_ids(&self, their_node_id: &PublicKey, chain_hash: BlockHash, short_channel_ids: Vec) -> Result<(), LightningError>; + /// 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. + fn handle_reply_channel_range(&self, their_node_id: &PublicKey, msg: &ReplyChannelRange) -> Result<(), LightningError>; + /// Handles the reply of a query we initiated asking for routing gossip + /// messages for a list of channels. We should receive this message when + /// a node has completed its best effort to send us the pertaining routing + /// gossip messages. + fn handle_reply_short_channel_ids_end(&self, their_node_id: &PublicKey, msg: &ReplyShortChannelIdsEnd) -> Result<(), LightningError>; + /// Handles when a peer asks us to send a list of short_channel_ids + /// for the requested range of blocks. + fn handle_query_channel_range(&self, their_node_id: &PublicKey, msg: &QueryChannelRange) -> Result<(), LightningError>; + /// Handles when a peer asks us to send routing gossip messages for a + /// list of short_channel_ids. + fn handle_query_short_channel_ids(&self, their_node_id: &PublicKey, msg: &QueryShortChannelIds) -> Result<(), LightningError>; } mod fuzzy_internal_msgs { diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index b318e503182..beac542abdd 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -841,17 +841,17 @@ impl PeerManager { - // TODO: handle message + wire::Message::QueryShortChannelIds(msg) => { + self.message_handler.route_handler.handle_query_short_channel_ids(&peer.their_node_id.unwrap(), &msg)?; }, - wire::Message::ReplyShortChannelIdsEnd(_msg) => { - // TODO: handle message + wire::Message::ReplyShortChannelIdsEnd(msg) => { + self.message_handler.route_handler.handle_reply_short_channel_ids_end(&peer.their_node_id.unwrap(), &msg)?; }, - wire::Message::QueryChannelRange(_msg) => { - // TODO: handle message + wire::Message::QueryChannelRange(msg) => { + self.message_handler.route_handler.handle_query_channel_range(&peer.their_node_id.unwrap(), &msg)?; }, - wire::Message::ReplyChannelRange(_msg) => { - // TODO: handle message + wire::Message::ReplyChannelRange(msg) => { + self.message_handler.route_handler.handle_reply_channel_range(&peer.their_node_id.unwrap(), &msg)?; }, wire::Message::GossipTimestampFilter(_msg) => { // TODO: handle message @@ -880,6 +880,7 @@ impl PeerManager where C::Target: chain::Access pub network_graph: RwLock, chain_access: Option, full_syncs_requested: AtomicUsize, + pending_events: Mutex>, logger: L, } @@ -77,6 +82,7 @@ impl NetGraphMsgHandler where C::Target: chain::Access }), full_syncs_requested: AtomicUsize::new(0), chain_access, + pending_events: Mutex::new(vec![]), logger, } } @@ -89,6 +95,7 @@ impl NetGraphMsgHandler where C::Target: chain::Access network_graph: RwLock::new(network_graph), full_syncs_requested: AtomicUsize::new(0), chain_access, + pending_events: Mutex::new(vec![]), logger, } } @@ -212,6 +219,67 @@ impl RoutingMessageHandler for N false } } + + fn query_channel_range(&self, _their_node_id: &PublicKey, _chain_hash: BlockHash, _first_blocknum: u32, _number_of_blocks: u32) -> Result<(), LightningError> { + // TODO + Err(LightningError { + err: String::from("Not implemented"), + action: ErrorAction::IgnoreError, + }) + } + + fn query_short_channel_ids(&self, _their_node_id: &PublicKey, _chain_hash: BlockHash, _short_channel_ids: Vec) -> Result<(), LightningError> { + // TODO + Err(LightningError { + err: String::from("Not implemented"), + action: ErrorAction::IgnoreError, + }) + } + + fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: &ReplyChannelRange) -> Result<(), LightningError> { + // TODO + Err(LightningError { + err: String::from("Not implemented"), + action: ErrorAction::IgnoreError, + }) + } + + fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: &ReplyShortChannelIdsEnd) -> Result<(), LightningError> { + // TODO + Err(LightningError { + err: String::from("Not implemented"), + action: ErrorAction::IgnoreError, + }) + } + + fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: &QueryChannelRange) -> Result<(), LightningError> { + // TODO + Err(LightningError { + err: String::from("Not implemented"), + action: ErrorAction::IgnoreError, + }) + } + + fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: &QueryShortChannelIds) -> Result<(), LightningError> { + // TODO + Err(LightningError { + err: String::from("Not implemented"), + action: ErrorAction::IgnoreError, + }) + } +} + +impl events::MessageSendEventsProvider for NetGraphMsgHandler +where + C::Target: chain::Access, + L::Target: Logger, +{ + fn get_and_clear_pending_msg_events(&self) -> Vec { + let mut ret = Vec::new(); + let mut pending_events = self.pending_events.lock().unwrap(); + std::mem::swap(&mut ret, &mut pending_events); + ret + } } #[derive(PartialEq, Debug)] diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 6c3552d5df0..55353c0f26b 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -319,6 +319,36 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler { fn should_request_full_sync(&self, _node_id: &PublicKey) -> bool { self.request_full_sync.load(Ordering::Acquire) } + + fn query_channel_range(&self, _their_node_id: &PublicKey, _chain_hash: BlockHash, _first_blocknum: u32, _number_of_blocks: u32) -> Result<(), msgs::LightningError> { + Ok(()) + } + + fn query_short_channel_ids(&self, _their_node_id: &PublicKey, _chain_hash: BlockHash, _short_channel_ids: Vec) -> Result<(), msgs::LightningError> { + Ok(()) + } + + fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: &msgs::ReplyChannelRange) -> Result<(), msgs::LightningError> { + Ok(()) + } + + fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: &msgs::ReplyShortChannelIdsEnd) -> Result<(), msgs::LightningError> { + Ok(()) + } + + fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: &msgs::QueryChannelRange) -> Result<(), msgs::LightningError> { + Ok(()) + } + + fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: &msgs::QueryShortChannelIds) -> Result<(), msgs::LightningError> { + Ok(()) + } +} + +impl events::MessageSendEventsProvider for TestRoutingMessageHandler { + fn get_and_clear_pending_msg_events(&self) -> Vec { + vec![] + } } pub struct TestLogger { From 69da2daeae6cad74cbd087e5a6ea3a5376005ee0 Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Thu, 22 Oct 2020 12:44:53 -0400 Subject: [PATCH 05/12] Implement gossip_queries sync methods in NetGraphMsgHandler To perform a sync of routing gossip messages with a peer requires a two step process where we first initiate a channel range query to discover channels in a block range. Next we request the routing gossip messages for discovered channels. This code implements logic in NetGraphMsgHandler for performing these two tasks while taking into account the specification and variance in implementation. --- lightning/src/routing/network_graph.rs | 1176 +++++++++++++++++++++++- 1 file changed, 1151 insertions(+), 25 deletions(-) diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index 9f20219eba5..b0cfb982e08 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -37,9 +37,21 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Mutex; use std::collections::BTreeMap; use std::collections::btree_map::Entry as BtreeEntry; +use std::collections::HashMap; use std::ops::Deref; use bitcoin::hashes::hex::ToHex; +/// Maximum number of short_channel_id values that can be encoded in a +/// single reply_channel_range or query_short_channel_ids messages when +/// using raw encoding. The maximum value ensures that the 8-byte SCIDs +/// fit inside the maximum size of the Lightning message, 65535-bytes. +const MAX_SHORT_CHANNEL_ID_BATCH_SIZE: usize = 8000; + +/// Maximum number of reply_channel_range messages we will allow in +/// reply to a query_channel_range. This value creates an upper-limit +/// on the number of SCIDs we process in reply to a single query. +const MAX_REPLY_CHANNEL_RANGE_PER_QUERY: usize = 250; + /// Represents the network as nodes and channels between them #[derive(PartialEq)] pub struct NetworkGraph { @@ -64,6 +76,8 @@ pub struct NetGraphMsgHandler where C::Target: chain::Access chain_access: Option, full_syncs_requested: AtomicUsize, pending_events: Mutex>, + chan_range_query_tasks: Mutex>, + scid_query_tasks: Mutex>, logger: L, } @@ -83,6 +97,8 @@ impl NetGraphMsgHandler where C::Target: chain::Access full_syncs_requested: AtomicUsize::new(0), chain_access, pending_events: Mutex::new(vec![]), + chan_range_query_tasks: Mutex::new(HashMap::new()), + scid_query_tasks: Mutex::new(HashMap::new()), logger, } } @@ -96,6 +112,8 @@ impl NetGraphMsgHandler where C::Target: chain::Access full_syncs_requested: AtomicUsize::new(0), chain_access, pending_events: Mutex::new(vec![]), + chan_range_query_tasks: Mutex::new(HashMap::new()), + scid_query_tasks: Mutex::new(HashMap::new()), logger, } } @@ -107,6 +125,28 @@ impl NetGraphMsgHandler where C::Target: chain::Access pub fn read_locked_graph<'a>(&'a self) -> LockedNetworkGraph<'a> { LockedNetworkGraph(self.network_graph.read().unwrap()) } + + /// Enqueues a message send event for a batch of short_channel_ids + /// in a task. + fn finalize_query_short_ids(&self, task: &mut ScidQueryTask) { + let scid_size = std::cmp::min(task.short_channel_ids.len(), MAX_SHORT_CHANNEL_ID_BATCH_SIZE); + let mut short_channel_ids: Vec = Vec::with_capacity(scid_size); + for scid in task.short_channel_ids.drain(..scid_size) { + short_channel_ids.push(scid); + } + + log_debug!(self.logger, "Sending query_short_channel_ids peer={}, batch_size={}", log_pubkey!(task.node_id), scid_size); + + // enqueue the message to the peer + let mut pending_events = self.pending_events.lock().unwrap(); + pending_events.push(events::MessageSendEvent::SendShortIdsQuery { + node_id: task.node_id.clone(), + msg: QueryShortChannelIds { + chain_hash: task.chain_hash.clone(), + short_channel_ids, + } + }); + } } impl<'a> LockedNetworkGraph<'a> { @@ -220,38 +260,270 @@ impl RoutingMessageHandler for N } } - fn query_channel_range(&self, _their_node_id: &PublicKey, _chain_hash: BlockHash, _first_blocknum: u32, _number_of_blocks: u32) -> Result<(), LightningError> { - // TODO - Err(LightningError { - err: String::from("Not implemented"), - action: ErrorAction::IgnoreError, - }) + fn query_channel_range(&self, their_node_id: &PublicKey, chain_hash: BlockHash, first_blocknum: u32, number_of_blocks: u32) -> Result<(), LightningError> { + // We must ensure that we only have a single in-flight query + // to the remote peer. If we already have a query, then we fail + let mut query_range_tasks_lock = self.chan_range_query_tasks.lock().unwrap(); + let query_range_tasks = &mut *query_range_tasks_lock; + if query_range_tasks.contains_key(their_node_id) { + return Err(LightningError { + err: String::from("query_channel_range already in-flight"), + action: ErrorAction::IgnoreError, + }); + } + + // Construct a new task to keep track of the query until the full + // range query has been completed + let task = ChanRangeQueryTask::new(their_node_id, chain_hash, first_blocknum, number_of_blocks); + query_range_tasks.insert(their_node_id.clone(), task); + + // Enqueue the message send event + 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(events::MessageSendEvent::SendChannelRangeQuery { + node_id: their_node_id.clone(), + msg: QueryChannelRange { + chain_hash, + first_blocknum, + number_of_blocks, + }, + }); + Ok(()) } - fn query_short_channel_ids(&self, _their_node_id: &PublicKey, _chain_hash: BlockHash, _short_channel_ids: Vec) -> Result<(), LightningError> { - // TODO - Err(LightningError { - err: String::from("Not implemented"), - action: ErrorAction::IgnoreError, - }) + /// A query should only request channels referring to unspent outputs. + /// This method does not validate this requirement and expects the + /// caller to ensure SCIDs are unspent. + fn query_short_channel_ids(&self, their_node_id: &PublicKey, chain_hash: BlockHash, short_channel_ids: Vec) -> Result<(), LightningError> { + // Create a new task or add to the existing task + let mut query_scids_tasks_lock = self.scid_query_tasks.lock().unwrap(); + let query_scids_tasks = &mut *query_scids_tasks_lock; + + // For an existing task we append the short_channel_ids which will be sent when the + // current in-flight batch completes. + if let Some(task) = query_scids_tasks.get_mut(their_node_id) { + task.add(short_channel_ids); + return Ok(()); + } + + // For a new task we create the task with short_channel_ids and send the first + // batch immediately. + query_scids_tasks.insert(their_node_id.clone(), ScidQueryTask::new( + their_node_id, + chain_hash.clone(), + short_channel_ids, + )); + let task = query_scids_tasks.get_mut(their_node_id).unwrap(); + self.finalize_query_short_ids(task); + return Ok(()); } - fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: &ReplyChannelRange) -> Result<(), LightningError> { - // TODO - Err(LightningError { - err: String::from("Not implemented"), - action: ErrorAction::IgnoreError, - }) + 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(),); + + // First we obtain a lock on the task hashmap. In order to avoid borrowing issues + // we will access the task as needed. + let mut query_range_tasks = self.chan_range_query_tasks.lock().unwrap(); + + // If there is no currently executing task then we have received + // an invalid message and will return an error + if query_range_tasks.get(their_node_id).is_none() { + return Err(LightningError { + err: String::from("Received unknown reply_channel_range message"), + action: ErrorAction::IgnoreError, + }); + } + + // Now that we know we have a task, we can extract a few values for use + // in validations without having to access the task repeatedly + let (task_chain_hash, task_first_blocknum, task_number_of_blocks, task_received_first_block, task_received_last_block, task_number_of_replies) = { + let task = query_range_tasks.get(their_node_id).unwrap(); + (task.chain_hash, task.first_blocknum, task.number_of_blocks, task.received_first_block, task.received_last_block, task.number_of_replies) + }; + + // Validate the chain_hash matches the chain_hash we used in the query. + // If it does not, then the message is malformed and we return an error + if msg.chain_hash != task_chain_hash { + query_range_tasks.remove(their_node_id); + return Err(LightningError { + err: String::from("Received reply_channel_range with invalid chain_hash"), + action: ErrorAction::IgnoreError, + }); + } + + // Validate that the remote node maintains up-to-date channel + // information for chain_hash. Some nodes use the full_information + // flag to indicate multi-part messages so we must check whether + // we received information as well. + if !msg.full_information && msg.short_channel_ids.len() == 0 { + query_range_tasks.remove(their_node_id); + return Err(LightningError { + err: String::from("Received reply_channel_range with no information available"), + action: ErrorAction::IgnoreError, + }); + } + + // Calculate the last block for the message and the task + let msg_last_block = last_blocknum(msg.first_blocknum, msg.number_of_blocks); + let task_last_block = last_blocknum(task_first_blocknum, task_number_of_blocks); + + // On the first message... + if task_received_first_block.is_none() { + // The replies can be a superset of the queried block range, but the + // replies must include our requested query range. We check if the + // start of the replies is greater than the start of our query. If + // so, the start of our query is excluded and the message is malformed. + if msg.first_blocknum > task_first_blocknum { + query_range_tasks.remove(their_node_id); + return Err(LightningError { + err: String::from("Failing reply_channel_range with invalid first_blocknum"), + action: ErrorAction::IgnoreError, + }); + } + + // Next, we ensure the reply has at least some information matching + // our query. If the received last_blocknum is less than our query's + // first_blocknum then the reply does not encompass the query range + // and the message is malformed. + if msg_last_block < task_first_blocknum { + query_range_tasks.remove(their_node_id); + return Err(LightningError { + err: String::from("Failing reply_channel_range with non-overlapping first reply"), + action: ErrorAction::IgnoreError, + }); + } + + // Capture the first block and last block so that subsequent messages + // can be validated. + let task = query_range_tasks.get_mut(their_node_id).unwrap(); + task.received_first_block = Some(msg.first_blocknum); + task.received_last_block = Some(msg_last_block); + } + // On subsequent message(s)... + else { + // We need to validate the sequence of the reply message is expected. + // Subsequent messages must set the first_blocknum to the previous + // message's first_blocknum plus number_of_blocks. There is discrepancy + // in implementation where some resume on the last sent block. We will + // loosen the restriction and accept either, and otherwise consider the + // message malformed and return an error. + let task_received_last_block = task_received_last_block.unwrap(); + if msg.first_blocknum != task_received_last_block && msg.first_blocknum != task_received_last_block + 1 { + query_range_tasks.remove(their_node_id); + return Err(LightningError { + err: String::from("Failing reply_channel_range with invalid sequence"), + action: ErrorAction::IgnoreError, + }); + } + + // Next we check to see that we have received a realistic number of + // reply messages for a query. This caps the allocation exposure + // for short_channel_ids that will be batched and sent in query channels. + if task_number_of_replies + 1 > MAX_REPLY_CHANNEL_RANGE_PER_QUERY { + query_range_tasks.remove(their_node_id); + return Err(LightningError { + err: String::from("Failing reply_channel_range due to excessive messages"), + action: ErrorAction::IgnoreError, + }); + } + + // Capture the last_block in our task so that subsequent messages + // can be validated. + let task = query_range_tasks.get_mut(their_node_id).unwrap(); + task.number_of_replies += 1; + task.received_last_block = Some(msg_last_block); + } + + // We filter the short_channel_ids to those inside the query range. + // The most significant 3-bytes of the short_channel_id are the block. + { + let mut filtered_short_channel_ids: Vec = msg.short_channel_ids.clone().into_iter().filter(|short_channel_id| { + let block = short_channel_id >> 40; + return block >= query_range_tasks.get(their_node_id).unwrap().first_blocknum as u64 && block <= task_last_block as u64; + }).collect(); + let task = query_range_tasks.get_mut(their_node_id).unwrap(); + task.short_channel_ids.append(&mut filtered_short_channel_ids); + } + + // The final message is indicated by a last_blocknum that is equal to + // or greater than the query's last_blocknum. + if msg_last_block >= task_last_block { + log_debug!(self.logger, "Completed query_channel_range: peer={}, first_blocknum={}, number_of_blocks={}", log_pubkey!(their_node_id), task_first_blocknum, task_number_of_blocks); + + // We can now fire off a query to obtain routing messages for the + // accumulated short_channel_ids. + { + let task = query_range_tasks.get_mut(their_node_id).unwrap(); + let mut short_channel_ids = Vec::new(); + std::mem::swap(&mut short_channel_ids, &mut task.short_channel_ids); + self.query_short_channel_ids(their_node_id, task.chain_hash, short_channel_ids)?; + } + + // We can remove the query range task now that the query is complete. + query_range_tasks.remove(their_node_id); + } + Ok(()) } - fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: &ReplyShortChannelIdsEnd) -> Result<(), LightningError> { - // TODO - Err(LightningError { - err: String::from("Not implemented"), - action: ErrorAction::IgnoreError, - }) + /// When a query is initiated the remote peer will begin streaming + /// gossip messages. In the event of a failure, we may have received + /// some channel information. Before trying with another peer, the + /// caller should update its set of SCIDs that need to be queried. + fn handle_reply_short_channel_ids_end(&self, their_node_id: &PublicKey, msg: &ReplyShortChannelIdsEnd) -> Result<(), LightningError> { + log_debug!(self.logger, "Handling reply_short_channel_ids_end peer={}, full_information={}", log_pubkey!(their_node_id), msg.full_information); + + // First we obtain a lock on the task hashmap. In order to avoid borrowing issues + // we will access the task as needed. + let mut query_short_channel_ids_tasks = self.scid_query_tasks.lock().unwrap(); + + // If there is no existing task then we have received an unknown + // message and should return an error. + if query_short_channel_ids_tasks.get(their_node_id).is_none() { + return Err(LightningError { + err: String::from("Unknown reply_short_channel_ids_end message"), + action: ErrorAction::IgnoreError, + }); + } + + // If the reply's chain_hash does not match the task's chain_hash then + // the reply is malformed and we should return an error. + if msg.chain_hash != query_short_channel_ids_tasks.get(their_node_id).unwrap().chain_hash { + query_short_channel_ids_tasks.remove(their_node_id); + return Err(LightningError { + err: String::from("Received reply_short_channel_ids_end with incorrect chain_hash"), + action: ErrorAction::IgnoreError + }); + } + + // If the remote node does not have up-to-date information for the + // chain_hash they will set full_information=false. We can fail + // the result and try again with a different peer. + if !msg.full_information { + query_short_channel_ids_tasks.remove(their_node_id); + return Err(LightningError { + err: String::from("Received reply_short_channel_ids_end with no information"), + action: ErrorAction::IgnoreError + }); + } + + // If we have more scids to process we send the next batch in the task + { + let task = query_short_channel_ids_tasks.get_mut(their_node_id).unwrap(); + if task.short_channel_ids.len() > 0 { + self.finalize_query_short_ids(task); + return Ok(()); + } + } + + // Otherwise the task is complete and we can remove it + log_debug!(self.logger, "Completed query_short_channel_ids peer={}", log_pubkey!(their_node_id)); + query_short_channel_ids_tasks.remove(their_node_id); + Ok(()) } + /// There are potential DoS vectors when handling inbound queries. + /// Handling requests with first_blocknum very far away may trigger repeated + /// disk I/O if the NetworkGraph is not fully in-memory. fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: &QueryChannelRange) -> Result<(), LightningError> { // TODO Err(LightningError { @@ -260,6 +532,9 @@ impl RoutingMessageHandler for N }) } + /// There are potential DoS vectors when handling inbound queries. + /// Handling requests with first_blocknum very far away may trigger repeated + /// disk I/O if the NetworkGraph is not fully in-memory. fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: &QueryShortChannelIds) -> Result<(), LightningError> { // TODO Err(LightningError { @@ -282,6 +557,118 @@ where } } +/// Safely calculates the last_blocknum given a first_blocknum and +/// number_of_blocks by returning the u32::MAX-1 if there is an overflow +fn last_blocknum(first_blocknum: u32, number_of_blocks: u32) -> u32 { + match first_blocknum.checked_add(number_of_blocks) { + Some(val) => val - 1, + None => 0xffff_ffff - 1, + } +} + +/// Maintains state for a channel range query that we initiated. +/// The query may result in one or more reply_channel_range messages +/// being received. This struct helps determine the status of the query +/// when there are multiple replies. It also collects results for initiating +/// SCID queries. +/// +/// The task is complete and can be cleaned up when a reply meets or +/// exceeds the last block in the query. The collected SCIDs in the task +/// can be used to generate an ScidQueryTask. +/// +/// A query may fail if the recipient does not maintain up-to-date +/// information for the chain or if the recipient fails to reply within +/// a reasonable amount of time. In either event, the query can be +/// re-initiated with a different peer. +pub struct ChanRangeQueryTask { + /// The public key of the node we will be sending queries to + pub node_id: PublicKey, + /// The genesis hash of the blockchain being queried + pub chain_hash: BlockHash, + /// The height of the first block for the channel UTXOs being queried + pub first_blocknum: u32, + /// The number of blocks to include in the query results + pub number_of_blocks: u32, + /// Tracks the number of reply messages we have received + pub number_of_replies: usize, + /// The height of the first block received in a reply. This value + /// should be less than or equal to the first_blocknum requested in + /// the query_channel_range. This allows the range of the replies to + /// contain, but not necessarily strictly, the queried range. + pub received_first_block: Option, + /// The height of the last block received in a reply. This value + /// will get incrementally closer to the target of + /// first_blocknum plus number_of_blocks from the query_channel_range. + pub received_last_block: Option, + /// Contains short_channel_ids received in one or more reply messages. + /// These will be sent in one ore more query_short_channel_ids messages + /// when the task is complete. + pub short_channel_ids: Vec, +} + +impl ChanRangeQueryTask { + /// Constructs a new GossipQueryRangeTask + pub fn new(their_node_id: &PublicKey, chain_hash: BlockHash, first_blocknum: u32, number_of_blocks: u32) -> Self { + ChanRangeQueryTask { + node_id: their_node_id.clone(), + chain_hash, + first_blocknum, + number_of_blocks, + number_of_replies: 0, + received_first_block: None, + received_last_block: None, + short_channel_ids: vec![], + } + } +} + +/// Maintains state when sending one or more short_channel_ids messages +/// to a peer. Only a single SCID query can be in-flight with a peer. The +/// number of SCIDs per query is limited by the size of a Lightning message +/// payload. When querying a large number of SCIDs (results of a large +/// channel range query for instance), multiple query_short_channel_ids +/// messages need to be sent. This task maintains the list of awaiting +/// SCIDs to be queried. +/// +/// When a successful reply_short_channel_ids_end message is received, the +/// next batch of SCIDs can be sent. When no remaining SCIDs exist in the +/// task, the task is complete and can be cleaned up. +/// +/// The recipient may reply indicating that up-to-date information for the +/// chain is not maintained. A query may also fail to complete within a +/// reasonable amount of time. In either event, the short_channel_ids +/// can be queried from a different peer after validating the set of +/// SCIDs that still need to be queried. +pub struct ScidQueryTask { + /// The public key of the node we will be sending queries to + pub node_id: PublicKey, + /// The genesis hash of the blockchain being queried + pub chain_hash: BlockHash, + /// A vector of short_channel_ids that we would like routing gossip + /// information for. This list will be chunked and sent to the peer + /// in one or more query_short_channel_ids messages. + pub short_channel_ids: Vec, +} + +impl ScidQueryTask { + /// Constructs a new GossipQueryShortChannelIdsTask + pub fn new(their_node_id: &PublicKey, chain_hash: BlockHash, short_channel_ids: Vec) -> Self { + ScidQueryTask { + node_id: their_node_id.clone(), + chain_hash, + short_channel_ids, + } + } + + /// Adds short_channel_ids to the pending list of short_channel_ids + /// to be sent in the next request. You can add additional values + /// while a query is in-flight. These new values will be sent once + /// the active query has completed. + pub fn add(&mut self, mut short_channel_ids: Vec) { + self.short_channel_ids.append(&mut short_channel_ids); + } +} + #[derive(PartialEq, Debug)] /// Details about one direction of a channel. Received /// within a channel update. @@ -954,10 +1341,11 @@ mod tests { use routing::network_graph::{NetGraphMsgHandler, NetworkGraph}; use ln::msgs::{OptionalField, RoutingMessageHandler, UnsignedNodeAnnouncement, NodeAnnouncement, UnsignedChannelAnnouncement, ChannelAnnouncement, UnsignedChannelUpdate, ChannelUpdate, HTLCFailChannelUpdate, - MAX_VALUE_MSAT}; + ReplyChannelRange, ReplyShortChannelIdsEnd, QueryChannelRange, QueryShortChannelIds, MAX_VALUE_MSAT}; use util::test_utils; use util::logger::Logger; use util::ser::{Readable, Writeable}; + use util::events::{MessageSendEvent, MessageSendEventsProvider}; use bitcoin::hashes::sha256d::Hash as Sha256dHash; use bitcoin::hashes::Hash; @@ -1881,4 +2269,742 @@ mod tests { network.write(&mut w).unwrap(); assert!(::read(&mut ::std::io::Cursor::new(&w.0)).unwrap() == *network); } + + #[test] + fn sending_query_channel_range() { + let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(); + let node_privkey_1 = &SecretKey::from_slice(&[42; 32]).unwrap(); + let node_privkey_2 = &SecretKey::from_slice(&[41; 32]).unwrap(); + let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_privkey_1); + let node_id_2 = PublicKey::from_secret_key(&secp_ctx, node_privkey_2); + + let chain_hash = genesis_block(Network::Testnet).header.block_hash(); + let first_blocknum = 0; + let number_of_blocks = 0xffff_ffff; + + // When no active query exists for the node, it should send a query message and generate a task + { + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, first_blocknum, number_of_blocks); + assert!(result.is_ok()); + + // It should create a task for the query + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().contains_key(&node_id_1)); + + // It should send a query_channel_range message with the correct information + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendChannelRangeQuery{ node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.first_blocknum, first_blocknum); + assert_eq!(msg.number_of_blocks, number_of_blocks); + }, + _ => panic!("Expected MessageSendEvent::SendChannelRangeQuery") + }; + } + + // When an active query exists for the node, when there is a subsequent query request, it + // should fail to initiate a new query + { + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, first_blocknum, number_of_blocks); + assert_eq!(result.is_err(), true); + } + + // When no active query exists for a different node, it should send a query message + { + let result = net_graph_msg_handler.query_channel_range(&node_id_2, chain_hash, first_blocknum, number_of_blocks); + assert_eq!(result.is_ok(), true); + + // It should create a task for the query + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().contains_key(&node_id_2)); + + // It should send a query_channel_message with the correct information + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendChannelRangeQuery{ node_id, msg } => { + assert_eq!(node_id, &node_id_2); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.first_blocknum, first_blocknum); + assert_eq!(msg.number_of_blocks, number_of_blocks); + }, + _ => panic!("Expected MessageSendEvent::SendChannelRangeQuery") + }; + } + } + + #[test] + fn sending_query_short_channel_ids() { + let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(); + let node_privkey_1 = &SecretKey::from_slice(&[42; 32]).unwrap(); + let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_privkey_1); + + let chain_hash = genesis_block(Network::Testnet).header.block_hash(); + + // The first query should send the batch of scids to the peer + { + let short_channel_ids: Vec = vec![0, 1, 2]; + let result = net_graph_msg_handler.query_short_channel_ids(&node_id_1, chain_hash, short_channel_ids.clone()); + assert!(result.is_ok()); + + // Validate that we have enqueued a send message event and that it contains the correct information + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendShortIdsQuery{ node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.short_channel_ids, short_channel_ids); + }, + _ => panic!("Expected MessageSendEvent::SendShortIdsQuery") + }; + } + + // Subsequent queries for scids should enqueue them to be sent in the next batch which will + // be sent when a reply_short_channel_ids_end message is handled. + { + let short_channel_ids: Vec = vec![3, 4, 5]; + let result = net_graph_msg_handler.query_short_channel_ids(&node_id_1, chain_hash, short_channel_ids.clone()); + assert!(result.is_ok()); + + // Validate that we have not enqueued another send message event yet + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 0); + + // Validate the task has the queued scids + assert_eq!( + net_graph_msg_handler.scid_query_tasks.lock().unwrap().get(&node_id_1).unwrap().short_channel_ids, + short_channel_ids + ); + } + } + + #[test] + fn handling_reply_channel_range() { + let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(); + let node_privkey_1 = &SecretKey::from_slice(&[42; 32]).unwrap(); + let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_privkey_1); + + let chain_hash = genesis_block(Network::Testnet).header.block_hash(); + + // Test receipt of an unknown reply message. We expect an error + { + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1000, + number_of_blocks: 1050, + short_channel_ids: vec![ + 0x0003e8_000000_0000, // 1000x0x0 + 0x0003e9_000000_0000, // 1001x0x0 + 0x0003f0_000000_0000 // 1008x0x0 + ], + }); + assert!(result.is_err()); + } + + // Test receipt of a single reply_channel_range that exactly matches the queried range. + // It sends a query_short_channel_ids with the returned scids and removes the pending task + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle a single successful reply that matches the queried channel range + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1000, + number_of_blocks: 100, + short_channel_ids: vec![ + 0x0003e8_000000_0000, // 1000x0x0 + 0x0003e9_000000_0000, // 1001x0x0 + 0x0003f0_000000_0000 // 1008x0x0 + ], + }); + assert!(result.is_ok()); + + // The query is now complete, so we expect the task to be removed + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + + // We expect to emit a query_short_channel_ids message with scids in our query range + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendShortIdsQuery { node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.short_channel_ids, vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000]); + }, + _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), + } + + // Clean up scid_task + net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); + } + + // Test receipt of a single reply_channel_range for a query that has a u32 overflow. We expect + // it sends a query_short_channel_ids with the returned scids and removes the pending task. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 0xffff_ffff); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle a single successful reply that matches the queried channel range + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1000, + number_of_blocks: 0xffff_ffff, + short_channel_ids: vec![ + 0x0003e8_000000_0000, // 1000x0x0 + 0x0003e9_000000_0000, // 1001x0x0 + 0x0003f0_000000_0000 // 1008x0x0 + ], + }); + assert!(result.is_ok()); + + // The query is now complete, so we expect the task to be removed + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + + // We expect to emit a query_short_channel_ids message with scids in our query range + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendShortIdsQuery { node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.short_channel_ids, vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000]); + }, + _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), + } + + // Clean up scid_task + net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); + } + + // Test receipt of a single reply that encompasses the queried channel range. This is allowed + // since a reply must contain at least part of the query range. Receipt of the reply should + // send a query_short_channel_ids message with scids filtered to the query range and remove + // the pending task. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // 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, + first_blocknum: 0, + number_of_blocks: 2000, + short_channel_ids: vec![ + 0x0003e0_000000_0000, // 992x0x0 + 0x0003e8_000000_0000, // 1000x0x0 + 0x0003e9_000000_0000, // 1001x0x0 + 0x0003f0_000000_0000, // 1008x0x0 + 0x00044c_000000_0000, // 1100x0x0 + 0x0006e0_000000_0000, // 1760x0x0 + ], + }); + assert!(result.is_ok()); + + // The query is now complete, so we expect the task to be removed + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + + // We expect to emit a query_short_channel_ids message with scids filtered to those + // within the original query range. + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendShortIdsQuery { node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.short_channel_ids, vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000]); + }, + _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), + } + + // Clean up scid_task + net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); + } + + // Test receipt of multiple reply messages for a single query. This happens when the number + // of scids in the query range exceeds the size limits of a single reply message. We expect + // to initiate a query_short_channel_ids for the first batch of scids and we enqueue the + // remaining scids for later processing. We remove the range query task after receipt of all + // reply messages. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle the first reply message + let reply_1_scids = vec![ + 0x0003e8_000000_0000, // 1000x0x0 + 0x0003e9_000000_0000, // 1001x0x0 + 0x000419_000000_0000, // 1049x0x0 + ]; + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1000, + number_of_blocks: 50, + short_channel_ids: reply_1_scids.clone(), + }); + assert!(result.is_ok()); + + // Handle the next reply in the sequence, which must start at the previous message's + // first_blocknum plus number_of_blocks. The scids in this reply will be queued. + let reply_2_scids = vec![ + 0x00041a_000000_0000, // 1050x0x0 + 0x000432_000000_0000, // 1074x0x0 + ]; + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1050, + number_of_blocks: 25, + short_channel_ids: reply_2_scids.clone(), + }); + assert!(result.is_ok()); + + // Handle the final reply in the sequence, which must meet or exceed the initial query's + // first_blocknum plus number_of_blocks. The scids in this reply will be queued. + let reply_3_scids = vec![ + 0x000433_000000_0000, // 1075x0x0 + 0x00044b_000000_0000, // 1099x0x0 + ]; + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1075, + number_of_blocks: 25, + short_channel_ids: reply_3_scids.clone(), + }); + assert!(result.is_ok()); + + // After the final reply we expect the query task to be removed + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + + // We expect to emit a query_short_channel_ids message with the accumulated scids that + // match the queried channel range. + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendShortIdsQuery { node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.short_channel_ids, [reply_1_scids, reply_2_scids, reply_3_scids].concat()); + }, + _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), + } + + // Clean up scid_task + net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); + } + + // Test receipt of a sequence of replies with a valid first reply and a second reply that + // resumes on the same block as the first reply. The spec requires a subsequent + // first_blocknum to equal the prior first_blocknum plus number_of_blocks, however + // due to discrepancies in implementation we must loosen this restriction. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle the first reply message + let reply_1_scids = vec![ + 0x0003e8_000000_0000, // 1000x0x0 + 0x0003e9_000000_0000, // 1001x0x0 + 0x000419_000000_0000, // 1049x0x0 + ]; + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1000, + number_of_blocks: 50, + short_channel_ids: reply_1_scids.clone(), + }); + assert!(result.is_ok()); + + // Handle the next reply in the sequence, which is non-spec but resumes on the last block + // of the first message. + let reply_2_scids = vec![ + 0x000419_000001_0000, // 1049x1x0 + 0x00041a_000000_0000, // 1050x0x0 + 0x000432_000000_0000, // 1074x0x0 + ]; + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1049, + number_of_blocks: 51, + short_channel_ids: reply_2_scids.clone(), + }); + assert!(result.is_ok()); + + // After the final reply we expect the query task to be removed + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + + // We expect to emit a query_short_channel_ids message with the accumulated scids that + // match the queried channel range + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendShortIdsQuery { node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.short_channel_ids, [reply_1_scids, reply_2_scids].concat()); + }, + _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), + } + + // Clean up scid_task + net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); + } + + // Test receipt of reply with a chain_hash that does not match the query. We expect to return + // an error and to remove the query task. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle the reply with a mismatched chain_hash. We expect IgnoreError result and the + // task should be removed. + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash: genesis_block(Network::Bitcoin).header.block_hash(), + full_information: true, + first_blocknum: 1000, + number_of_blocks: 1050, + short_channel_ids: vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000], + }); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Received reply_channel_range with invalid chain_hash"); + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + } + + // Test receipt of a reply that indicates the remote node does not maintain up-to-date + // information for the chain_hash. Because of discrepancies in implementation we use + // full_information=false and short_channel_ids=[] as the signal. We should expect an error + // and the task should be removed. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle the reply indicating the peer was unable to fulfill our request. + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: false, + first_blocknum: 1000, + number_of_blocks: 100, + short_channel_ids: vec![], + }); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Received reply_channel_range with no information available"); + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + } + + // Test receipt of a reply that has a first_blocknum that is above the first_blocknum + // requested in our query. The reply must contain the queried block range. We expect an + // error result and the task should be removed. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle the reply that has a first_blocknum above the query's first_blocknum + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1001, + number_of_blocks: 100, + short_channel_ids: vec![], + }); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Failing reply_channel_range with invalid first_blocknum"); + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + } + + // Test receipt of a first reply that does not overlap the query range at all. The first message + // must have some overlap with the query. We expect an error result and the task should + // be removed. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle a reply that contains a block range that precedes the queried block range + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 0, + number_of_blocks: 1000, + short_channel_ids: vec![], + }); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Failing reply_channel_range with non-overlapping first reply"); + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + } + + // Test receipt of a sequence of replies with a valid first reply and a second reply that is + // non-sequential. The spec requires a subsequent first_blocknum to equal the prior + // first_blocknum plus number_of_blocks. We expect an IgnoreError result and the task should + // be removed. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle the first reply + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1000, + number_of_blocks: 50, + short_channel_ids: vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000], + }); + assert!(result.is_ok()); + + // Handle the second reply which does not start at the proper first_blocknum. We expect + // to return an error and remove the task. + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: 1051, + number_of_blocks: 50, + short_channel_ids: vec![0x0003f1_000000_0000,0x0003f2_000000_0000], + }); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Failing reply_channel_range with invalid sequence"); + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + } + + // Test receipt of too many reply messages. We expect an IgnoreError result and the task should + // be removed. + { + // Initiate a channel range query to create a query task + let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 0xffff_ffff); + assert!(result.is_ok()); + + // Clear the SendRangeQuery event + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle a sequence of replies that will fail once the max number of reply has been exceeded. + for block in 1000..=1000 + super::MAX_REPLY_CHANNEL_RANGE_PER_QUERY + 10 { + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + chain_hash, + full_information: true, + first_blocknum: block as u32, + number_of_blocks: 1, + short_channel_ids: vec![(block as u64) << 40], + }); + if block <= 1000 + super::MAX_REPLY_CHANNEL_RANGE_PER_QUERY { + assert!(result.is_ok()); + } else if block == 1001 + super::MAX_REPLY_CHANNEL_RANGE_PER_QUERY { + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Failing reply_channel_range due to excessive messages"); + } else { + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Received unknown reply_channel_range message"); + } + } + + // Expect the task to be removed + assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); + } + } + + #[test] + fn handling_reply_short_channel_ids() { + let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(); + let node_privkey = &SecretKey::from_slice(&[41; 32]).unwrap(); + let node_id = PublicKey::from_secret_key(&secp_ctx, node_privkey); + + let chain_hash = genesis_block(Network::Testnet).header.block_hash(); + + // Test receipt of a reply when no query exists. We expect an error to be returned + { + let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { + chain_hash, + full_information: true, + }); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Unknown reply_short_channel_ids_end message"); + } + + // Test receipt of a reply that is for a different chain_hash. We expect an error and the task + // should be removed. + { + // Initiate a query to create a pending query task + let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e8_000000_0000]); + assert!(result.is_ok()); + + // Process reply with incorrect chain_hash + let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { + chain_hash: genesis_block(Network::Bitcoin).header.block_hash(), + full_information: true, + }); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Received reply_short_channel_ids_end with incorrect chain_hash"); + + // Expect the task to be removed + assert!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().is_empty()); + } + + // Test receipt of a reply that indicates the peer does not maintain up-to-date information + // for the chain_hash requested in the query. We expect an error and task should be removed. + { + // Initiate a query to create a pending query task + let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e8_000000_0000]); + assert!(result.is_ok()); + + // Process failed reply + let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { + chain_hash, + full_information: false, + }); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().err, "Received reply_short_channel_ids_end with no information"); + + // Expect the task to be removed + assert!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().is_empty()); + } + + // Test receipt of a successful reply when there are no additional scids to query. We expect + // the task to be removed. + { + // Initiate a query to create a pending query task + let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e8_000000_0000]); + assert!(result.is_ok()); + + // Process success reply + let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { + chain_hash, + full_information: true, + }); + assert!(result.is_ok()); + + // Expect the task to be removed + assert!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().is_empty()); + } + + // Test receipt of a successful reply when there are additional scids to query. We expect + // additional queries to be sent until the task can be removed. + { + // Initiate a query to create a pending query task + let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e8_000000_0000]); + assert!(result.is_ok()); + + // Initiate a second query to add pending scids to the task + let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e9_000000_0000]); + assert!(result.is_ok()); + assert_eq!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().get(&node_id).unwrap().short_channel_ids, vec![0x0003e9_000000_0000]); + + // Initiate a third query to add pending scids to the task + let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003f0_000000_0000]); + assert!(result.is_ok()); + assert_eq!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().get(&node_id).unwrap().short_channel_ids, vec![0x0003e9_000000_0000, 0x0003f0_000000_0000]); + + // Clear all of the pending send events + net_graph_msg_handler.get_and_clear_pending_msg_events(); + + // Handle the first successful reply, which will send the next batch of scids in a new query + let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { + chain_hash, + full_information: true, + }); + assert!(result.is_ok()); + + // We expect the second batch to be sent in an event + let expected_node_id = &node_id; + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendShortIdsQuery { node_id, msg } => { + assert_eq!(node_id, expected_node_id); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.short_channel_ids, vec![0x0003e9_000000_0000, 0x0003f0_000000_0000]); + }, + _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), + } + + // We expect the scids to be cleared from the task + assert_eq!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().get(&node_id).unwrap().short_channel_ids.len(), 0); + + // Handle the second successful reply + let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { + chain_hash, + full_information: true, + }); + assert!(result.is_ok()); + + // We expect the task should be removed + assert!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().is_empty()); + } + } + + #[test] + fn handling_query_channel_range() { + let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(); + let node_privkey = &SecretKey::from_slice(&[41; 32]).unwrap(); + let node_id = PublicKey::from_secret_key(&secp_ctx, node_privkey); + + let chain_hash = genesis_block(Network::Testnet).header.block_hash(); + + let result = net_graph_msg_handler.handle_query_channel_range(&node_id, &QueryChannelRange { + chain_hash, + first_blocknum: 0, + number_of_blocks: 0xffff_ffff, + }); + assert!(result.is_err()); + } + + #[test] + fn handling_query_short_channel_ids() { + let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(); + let node_privkey = &SecretKey::from_slice(&[41; 32]).unwrap(); + let node_id = PublicKey::from_secret_key(&secp_ctx, node_privkey); + + let chain_hash = genesis_block(Network::Testnet).header.block_hash(); + + let result = net_graph_msg_handler.handle_query_short_channel_ids(&node_id, &QueryShortChannelIds { + chain_hash, + short_channel_ids: vec![0x0003e8_000000_0000], + }); + assert!(result.is_err()); + } } From d183b975da73c40c513feca92e09f8295b70b13f Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Sun, 29 Nov 2020 15:20:35 -0500 Subject: [PATCH 06/12] Add genesis block hash to NetworkGraph This changes adds the genesis block hash as a BlockHash to the NetworkGraph struct. Making the NetworkGraph aware allows the message handler to validate the chain_hash for received messages. This change also adds the hash value to the Writeable and Readable methods. --- fuzz/src/full_stack.rs | 5 +++-- fuzz/src/router.rs | 4 +++- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/routing/network_graph.rs | 23 +++++++++++++---------- lightning/src/routing/router.rs | 4 ++-- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 3aeb3d233b0..6375d0765d7 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -19,6 +19,7 @@ use bitcoin::blockdata::script::{Builder, Script}; use bitcoin::blockdata::opcodes; use bitcoin::consensus::encode::deserialize; use bitcoin::network::constants::Network; +use bitcoin::blockdata::constants::genesis_block; use bitcoin::hashes::Hash as TraitImport; use bitcoin::hashes::HashEngine as TraitImportEngine; @@ -343,7 +344,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { config.peer_channel_config_limits.min_dust_limit_satoshis = 0; let channelmanager = Arc::new(ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0)); let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret()); - let net_graph_msg_handler = Arc::new(NetGraphMsgHandler::new(None, Arc::clone(&logger))); + let net_graph_msg_handler = Arc::new(NetGraphMsgHandler::new(genesis_block(Network::Bitcoin).header.block_hash(), None, Arc::clone(&logger))); let peers = RefCell::new([false; 256]); let mut loss_detector = MoneyLossDetector::new(&peers, channelmanager.clone(), monitor.clone(), PeerManager::new(MessageHandler { @@ -609,7 +610,7 @@ mod tests { // What each byte represents is broken down below, and then everything is concatenated into // one large test at the end (you want %s/ -.*//g %s/\n\| \|\t\|\///g). - // Following BOLT 8, lightning message on the wire are: 2-byte encrypted message length + + // Following BOLT 8, lightning message on the wire are: 2-byte encrypted message length + // 16-byte MAC of the encrypted message length + encrypted Lightning message + 16-byte MAC // of the Lightning message // I.e 2nd inbound read, len 18 : 0006 (encrypted message length) + 03000000000000000000000000000000 (MAC of the encrypted message length) diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 671adcfda51..4eb85714f3e 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -21,6 +21,8 @@ use lightning::util::ser::Readable; use lightning::routing::network_graph::{NetworkGraph, RoutingFees}; use bitcoin::secp256k1::key::PublicKey; +use bitcoin::network::constants::Network; +use bitcoin::blockdata::constants::genesis_block; use utils::test_logger; @@ -155,7 +157,7 @@ pub fn do_test(data: &[u8], out: Out) { let logger: Arc = Arc::new(test_logger::TestLogger::new("".to_owned(), out)); let our_pubkey = get_pubkey!(); - let mut net_graph = NetworkGraph::new(); + let mut net_graph = NetworkGraph::new(genesis_block(Network::Bitcoin).header.block_hash()); let mut node_pks = HashSet::new(); let mut scid = 42; diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 8ff3d4eacf8..d54045f8e61 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1171,7 +1171,7 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec, nodes: BTreeMap, } @@ -87,13 +88,10 @@ impl NetGraphMsgHandler where C::Target: chain::Access /// Chain monitor is used to make sure announced channels exist on-chain, /// channel data is correct, and that the announcement is signed with /// channel owners' keys. - pub fn new(chain_access: Option, logger: L) -> Self { + pub fn new(genesis_hash: BlockHash, chain_access: Option, logger: L) -> Self { NetGraphMsgHandler { secp_ctx: Secp256k1::verification_only(), - network_graph: RwLock::new(NetworkGraph { - channels: BTreeMap::new(), - nodes: BTreeMap::new(), - }), + network_graph: RwLock::new(NetworkGraph::new(genesis_hash)), full_syncs_requested: AtomicUsize::new(0), chain_access, pending_events: Mutex::new(vec![]), @@ -903,6 +901,7 @@ impl Readable for NodeInfo { impl Writeable for NetworkGraph { fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { + self.genesis_hash.write(writer)?; (self.channels.len() as u64).write(writer)?; for (ref chan_id, ref chan_info) in self.channels.iter() { (*chan_id).write(writer)?; @@ -919,6 +918,7 @@ impl Writeable for NetworkGraph { impl Readable for NetworkGraph { fn read(reader: &mut R) -> Result { + let genesis_hash: BlockHash = Readable::read(reader)?; let channels_count: u64 = Readable::read(reader)?; let mut channels = BTreeMap::new(); for _ in 0..channels_count { @@ -934,6 +934,7 @@ impl Readable for NetworkGraph { nodes.insert(node_id, node_info); } Ok(NetworkGraph { + genesis_hash, channels, nodes, }) @@ -979,8 +980,9 @@ impl NetworkGraph { } /// Creates a new, empty, network graph. - pub fn new() -> NetworkGraph { + pub fn new(genesis_hash: BlockHash) -> NetworkGraph { Self { + genesis_hash, channels: BTreeMap::new(), nodes: BTreeMap::new(), } @@ -1365,7 +1367,8 @@ mod tests { fn create_net_graph_msg_handler() -> (Secp256k1, NetGraphMsgHandler, Arc>) { let secp_ctx = Secp256k1::new(); let logger = Arc::new(test_utils::TestLogger::new()); - let net_graph_msg_handler = NetGraphMsgHandler::new(None, Arc::clone(&logger)); + let genesis_hash = genesis_block(Network::Testnet).header.block_hash(); + let net_graph_msg_handler = NetGraphMsgHandler::new(genesis_hash, None, Arc::clone(&logger)); (secp_ctx, net_graph_msg_handler) } @@ -1526,7 +1529,7 @@ mod tests { }; // Test if the UTXO lookups were not supported - let mut net_graph_msg_handler = NetGraphMsgHandler::new(None, Arc::clone(&logger)); + let mut net_graph_msg_handler = NetGraphMsgHandler::new(genesis_block(Network::Testnet).header.block_hash(), None, Arc::clone(&logger)); match net_graph_msg_handler.handle_channel_announcement(&valid_announcement) { Ok(res) => assert!(res), _ => panic!() @@ -1550,7 +1553,7 @@ mod tests { // Test if an associated transaction were not on-chain (or not confirmed). let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet)); *chain_source.utxo_ret.lock().unwrap() = Err(chain::AccessError::UnknownTx); - net_graph_msg_handler = NetGraphMsgHandler::new(Some(chain_source.clone()), Arc::clone(&logger)); + net_graph_msg_handler = NetGraphMsgHandler::new(chain_source.clone().genesis_hash, Some(chain_source.clone()), Arc::clone(&logger)); unsigned_announcement.short_channel_id += 1; msghash = hash_to_message!(&Sha256dHash::hash(&unsigned_announcement.encode()[..])[..]); @@ -1674,7 +1677,7 @@ mod tests { let secp_ctx = Secp256k1::new(); let logger: Arc = Arc::new(test_utils::TestLogger::new()); let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet)); - let net_graph_msg_handler = NetGraphMsgHandler::new(Some(chain_source.clone()), Arc::clone(&logger)); + let net_graph_msg_handler = NetGraphMsgHandler::new(genesis_block(Network::Testnet).header.block_hash(), Some(chain_source.clone()), Arc::clone(&logger)); let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap(); let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap(); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 490b6b4048c..f73a9a501bf 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -538,7 +538,7 @@ mod tests { fn build_graph() -> (Secp256k1, NetGraphMsgHandler, std::sync::Arc>, std::sync::Arc) { let secp_ctx = Secp256k1::new(); let logger = Arc::new(test_utils::TestLogger::new()); - let net_graph_msg_handler = NetGraphMsgHandler::new(None, Arc::clone(&logger)); + let net_graph_msg_handler = NetGraphMsgHandler::new(genesis_block(Network::Testnet).header.block_hash(), None, Arc::clone(&logger)); // Build network from our_id to node7: // // -1(1)2- node0 -1(3)2- @@ -1258,7 +1258,7 @@ mod tests { inbound_capacity_msat: 100000, is_live: true, }]; - let route = get_route(&source_node_id, &NetworkGraph::new(), &target_node_id, Some(&our_chans.iter().collect::>()), &last_hops.iter().collect::>(), 100, 42, Arc::new(test_utils::TestLogger::new())).unwrap(); + let route = get_route(&source_node_id, &NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()), &target_node_id, Some(&our_chans.iter().collect::>()), &last_hops.iter().collect::>(), 100, 42, Arc::new(test_utils::TestLogger::new())).unwrap(); assert_eq!(route.paths[0].len(), 2); From 14d4492b683977078ddc1ed5e5c75e31d00f21f0 Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Wed, 18 Nov 2020 13:32:55 -0500 Subject: [PATCH 07/12] Refactor gossip_queries sync to be stateless This commit simplifies the sync process for routing gossip messages. When a sync is initiated, the process is handled statelessly by immediately issuing SCID queries as channel range replies are received. This greatly simplifies the state machine at the cost of fully validating and conforming to the current spec. --- lightning-net-tokio/src/lib.rs | 3 +- lightning/src/ln/msgs.rs | 10 +- lightning/src/routing/network_graph.rs | 1044 ++---------------------- lightning/src/util/test_utils.rs | 8 +- 4 files changed, 70 insertions(+), 995 deletions(-) diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index 6f80ef2e396..df0d6cb1f0b 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -536,8 +536,7 @@ mod tests { 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 should_request_full_sync(&self, _node_id: &PublicKey) -> bool { false } - fn query_channel_range(&self, _their_node_id: &PublicKey, _chain_hash: bitcoin::BlockHash, _first_blocknum: u32, _block_range: u32) -> Result<(), LightningError> { Ok(()) } - fn query_short_channel_ids(&self, _their_node_id: &PublicKey, _chain_hash: bitcoin::BlockHash, _short_channel_ids: Vec) -> Result<(), LightningError> { Ok(()) } + fn sync_routing_table(&self, _their_node_id: &PublicKey) { } 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 9111a1ed657..671f4211ccc 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -827,12 +827,10 @@ pub trait RoutingMessageHandler : Send + Sync + events::MessageSendEventsProvide fn get_next_node_announcements(&self, starting_point: Option<&PublicKey>, batch_amount: u8) -> Vec; /// Returns whether a full sync should be requested from a peer. fn should_request_full_sync(&self, node_id: &PublicKey) -> bool; - /// Queries a peer for a list of channels with a funding UTXO in the requested - /// chain and range of blocks. - fn query_channel_range(&self, their_node_id: &PublicKey, chain_hash: BlockHash, first_blocknum: u32, number_of_blocks: u32) -> Result<(), LightningError>; - /// Queries a peer for routing gossip messages for a set of channels identified - /// by their short_channel_ids. - fn query_short_channel_ids(&self, their_node_id: &PublicKey, chain_hash: BlockHash, short_channel_ids: Vec) -> Result<(), LightningError>; + /// Initiates routing gossip sync by querying a peer to discover channels + /// and their associated routing gossip messages. This method will use a + /// sync strategy defined by the implementor. + fn sync_routing_table(&self, their_node_id: &PublicKey); /// 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/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index bce9633012c..c1258ce37a2 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -37,21 +37,9 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Mutex; use std::collections::BTreeMap; use std::collections::btree_map::Entry as BtreeEntry; -use std::collections::HashMap; use std::ops::Deref; use bitcoin::hashes::hex::ToHex; -/// Maximum number of short_channel_id values that can be encoded in a -/// single reply_channel_range or query_short_channel_ids messages when -/// using raw encoding. The maximum value ensures that the 8-byte SCIDs -/// fit inside the maximum size of the Lightning message, 65535-bytes. -const MAX_SHORT_CHANNEL_ID_BATCH_SIZE: usize = 8000; - -/// Maximum number of reply_channel_range messages we will allow in -/// reply to a query_channel_range. This value creates an upper-limit -/// on the number of SCIDs we process in reply to a single query. -const MAX_REPLY_CHANNEL_RANGE_PER_QUERY: usize = 250; - /// Represents the network as nodes and channels between them #[derive(PartialEq)] pub struct NetworkGraph { @@ -77,8 +65,6 @@ pub struct NetGraphMsgHandler where C::Target: chain::Access chain_access: Option, full_syncs_requested: AtomicUsize, pending_events: Mutex>, - chan_range_query_tasks: Mutex>, - scid_query_tasks: Mutex>, logger: L, } @@ -95,8 +81,6 @@ impl NetGraphMsgHandler where C::Target: chain::Access full_syncs_requested: AtomicUsize::new(0), chain_access, pending_events: Mutex::new(vec![]), - chan_range_query_tasks: Mutex::new(HashMap::new()), - scid_query_tasks: Mutex::new(HashMap::new()), logger, } } @@ -110,8 +94,6 @@ impl NetGraphMsgHandler where C::Target: chain::Access full_syncs_requested: AtomicUsize::new(0), chain_access, pending_events: Mutex::new(vec![]), - chan_range_query_tasks: Mutex::new(HashMap::new()), - scid_query_tasks: Mutex::new(HashMap::new()), logger, } } @@ -123,28 +105,6 @@ impl NetGraphMsgHandler where C::Target: chain::Access pub fn read_locked_graph<'a>(&'a self) -> LockedNetworkGraph<'a> { LockedNetworkGraph(self.network_graph.read().unwrap()) } - - /// Enqueues a message send event for a batch of short_channel_ids - /// in a task. - fn finalize_query_short_ids(&self, task: &mut ScidQueryTask) { - let scid_size = std::cmp::min(task.short_channel_ids.len(), MAX_SHORT_CHANNEL_ID_BATCH_SIZE); - let mut short_channel_ids: Vec = Vec::with_capacity(scid_size); - for scid in task.short_channel_ids.drain(..scid_size) { - short_channel_ids.push(scid); - } - - log_debug!(self.logger, "Sending query_short_channel_ids peer={}, batch_size={}", log_pubkey!(task.node_id), scid_size); - - // enqueue the message to the peer - let mut pending_events = self.pending_events.lock().unwrap(); - pending_events.push(events::MessageSendEvent::SendShortIdsQuery { - node_id: task.node_id.clone(), - msg: QueryShortChannelIds { - chain_hash: task.chain_hash.clone(), - short_channel_ids, - } - }); - } } impl<'a> LockedNetworkGraph<'a> { @@ -258,264 +218,88 @@ impl RoutingMessageHandler for N } } - fn query_channel_range(&self, their_node_id: &PublicKey, chain_hash: BlockHash, first_blocknum: u32, number_of_blocks: u32) -> Result<(), LightningError> { - // We must ensure that we only have a single in-flight query - // to the remote peer. If we already have a query, then we fail - let mut query_range_tasks_lock = self.chan_range_query_tasks.lock().unwrap(); - let query_range_tasks = &mut *query_range_tasks_lock; - if query_range_tasks.contains_key(their_node_id) { - return Err(LightningError { - err: String::from("query_channel_range already in-flight"), - action: ErrorAction::IgnoreError, - }); - } - - // Construct a new task to keep track of the query until the full - // range query has been completed - let task = ChanRangeQueryTask::new(their_node_id, chain_hash, first_blocknum, number_of_blocks); - query_range_tasks.insert(their_node_id.clone(), task); - - // Enqueue the message send event + /// 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. + /// 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 + /// tracking this directly. + fn sync_routing_table(&self, their_node_id: &PublicKey) { + 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(events::MessageSendEvent::SendChannelRangeQuery { node_id: their_node_id.clone(), msg: QueryChannelRange { - chain_hash, + chain_hash: self.network_graph.read().unwrap().genesis_hash, first_blocknum, number_of_blocks, }, }); - Ok(()) - } - - /// A query should only request channels referring to unspent outputs. - /// This method does not validate this requirement and expects the - /// caller to ensure SCIDs are unspent. - fn query_short_channel_ids(&self, their_node_id: &PublicKey, chain_hash: BlockHash, short_channel_ids: Vec) -> Result<(), LightningError> { - // Create a new task or add to the existing task - let mut query_scids_tasks_lock = self.scid_query_tasks.lock().unwrap(); - let query_scids_tasks = &mut *query_scids_tasks_lock; - - // For an existing task we append the short_channel_ids which will be sent when the - // current in-flight batch completes. - if let Some(task) = query_scids_tasks.get_mut(their_node_id) { - task.add(short_channel_ids); - return Ok(()); - } - - // For a new task we create the task with short_channel_ids and send the first - // batch immediately. - query_scids_tasks.insert(their_node_id.clone(), ScidQueryTask::new( - their_node_id, - chain_hash.clone(), - short_channel_ids, - )); - let task = query_scids_tasks.get_mut(their_node_id).unwrap(); - self.finalize_query_short_ids(task); - return Ok(()); } + /// Statelessly processes a reply to a channel range query by immediately + /// sending an SCID query with SCIDs in the reply. To keep this handler + /// 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. In the event of a failure, we may have received + /// some channel information. Before trying with another peer, the + /// caller should update its set of SCIDs that need to be queried. 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(),); - // First we obtain a lock on the task hashmap. In order to avoid borrowing issues - // we will access the task as needed. - let mut query_range_tasks = self.chan_range_query_tasks.lock().unwrap(); - - // If there is no currently executing task then we have received - // an invalid message and will return an error - if query_range_tasks.get(their_node_id).is_none() { - return Err(LightningError { - err: String::from("Received unknown reply_channel_range message"), - action: ErrorAction::IgnoreError, - }); - } - - // Now that we know we have a task, we can extract a few values for use - // in validations without having to access the task repeatedly - let (task_chain_hash, task_first_blocknum, task_number_of_blocks, task_received_first_block, task_received_last_block, task_number_of_replies) = { - let task = query_range_tasks.get(their_node_id).unwrap(); - (task.chain_hash, task.first_blocknum, task.number_of_blocks, task.received_first_block, task.received_last_block, task.number_of_replies) - }; - - // Validate the chain_hash matches the chain_hash we used in the query. - // If it does not, then the message is malformed and we return an error - if msg.chain_hash != task_chain_hash { - query_range_tasks.remove(their_node_id); - return Err(LightningError { - err: String::from("Received reply_channel_range with invalid chain_hash"), - action: ErrorAction::IgnoreError, - }); - } - // Validate that the remote node maintains up-to-date channel // information for chain_hash. Some nodes use the full_information // flag to indicate multi-part messages so we must check whether - // we received information as well. + // we received SCIDs as well. if !msg.full_information && msg.short_channel_ids.len() == 0 { - query_range_tasks.remove(their_node_id); return Err(LightningError { err: String::from("Received reply_channel_range with no information available"), action: ErrorAction::IgnoreError, }); } - // Calculate the last block for the message and the task - let msg_last_block = last_blocknum(msg.first_blocknum, msg.number_of_blocks); - let task_last_block = last_blocknum(task_first_blocknum, task_number_of_blocks); - - // On the first message... - if task_received_first_block.is_none() { - // The replies can be a superset of the queried block range, but the - // replies must include our requested query range. We check if the - // start of the replies is greater than the start of our query. If - // so, the start of our query is excluded and the message is malformed. - if msg.first_blocknum > task_first_blocknum { - query_range_tasks.remove(their_node_id); - return Err(LightningError { - err: String::from("Failing reply_channel_range with invalid first_blocknum"), - action: ErrorAction::IgnoreError, - }); - } - - // Next, we ensure the reply has at least some information matching - // our query. If the received last_blocknum is less than our query's - // first_blocknum then the reply does not encompass the query range - // and the message is malformed. - if msg_last_block < task_first_blocknum { - query_range_tasks.remove(their_node_id); - return Err(LightningError { - err: String::from("Failing reply_channel_range with non-overlapping first reply"), - action: ErrorAction::IgnoreError, - }); - } - - // Capture the first block and last block so that subsequent messages - // can be validated. - let task = query_range_tasks.get_mut(their_node_id).unwrap(); - task.received_first_block = Some(msg.first_blocknum); - task.received_last_block = Some(msg_last_block); - } - // On subsequent message(s)... - else { - // We need to validate the sequence of the reply message is expected. - // Subsequent messages must set the first_blocknum to the previous - // message's first_blocknum plus number_of_blocks. There is discrepancy - // in implementation where some resume on the last sent block. We will - // loosen the restriction and accept either, and otherwise consider the - // message malformed and return an error. - let task_received_last_block = task_received_last_block.unwrap(); - if msg.first_blocknum != task_received_last_block && msg.first_blocknum != task_received_last_block + 1 { - query_range_tasks.remove(their_node_id); - return Err(LightningError { - err: String::from("Failing reply_channel_range with invalid sequence"), - action: ErrorAction::IgnoreError, - }); - } - - // Next we check to see that we have received a realistic number of - // reply messages for a query. This caps the allocation exposure - // for short_channel_ids that will be batched and sent in query channels. - if task_number_of_replies + 1 > MAX_REPLY_CHANNEL_RANGE_PER_QUERY { - query_range_tasks.remove(their_node_id); - return Err(LightningError { - err: String::from("Failing reply_channel_range due to excessive messages"), - action: ErrorAction::IgnoreError, - }); - } - - // Capture the last_block in our task so that subsequent messages - // can be validated. - let task = query_range_tasks.get_mut(their_node_id).unwrap(); - task.number_of_replies += 1; - task.received_last_block = Some(msg_last_block); - } - - // We filter the short_channel_ids to those inside the query range. - // The most significant 3-bytes of the short_channel_id are the block. - { - let mut filtered_short_channel_ids: Vec = msg.short_channel_ids.clone().into_iter().filter(|short_channel_id| { - let block = short_channel_id >> 40; - return block >= query_range_tasks.get(their_node_id).unwrap().first_blocknum as u64 && block <= task_last_block as u64; - }).collect(); - let task = query_range_tasks.get_mut(their_node_id).unwrap(); - task.short_channel_ids.append(&mut filtered_short_channel_ids); + // Copy the SCIDs into a new vector to be sent in the SCID query + let scid_size = msg.short_channel_ids.len(); + let mut short_channel_ids: Vec = Vec::with_capacity(scid_size); + for scid in msg.short_channel_ids.iter() { + short_channel_ids.push(scid.clone()); } - // The final message is indicated by a last_blocknum that is equal to - // or greater than the query's last_blocknum. - if msg_last_block >= task_last_block { - log_debug!(self.logger, "Completed query_channel_range: peer={}, first_blocknum={}, number_of_blocks={}", log_pubkey!(their_node_id), task_first_blocknum, task_number_of_blocks); - - // We can now fire off a query to obtain routing messages for the - // accumulated short_channel_ids. - { - let task = query_range_tasks.get_mut(their_node_id).unwrap(); - let mut short_channel_ids = Vec::new(); - std::mem::swap(&mut short_channel_ids, &mut task.short_channel_ids); - self.query_short_channel_ids(their_node_id, task.chain_hash, short_channel_ids)?; + log_debug!(self.logger, "Sending query_short_channel_ids peer={}, batch_size={}", log_pubkey!(their_node_id), scid_size); + let mut pending_events = self.pending_events.lock().unwrap(); + pending_events.push(events::MessageSendEvent::SendShortIdsQuery { + node_id: their_node_id.clone(), + msg: QueryShortChannelIds { + chain_hash: msg.chain_hash.clone(), + short_channel_ids, } + }); - // We can remove the query range task now that the query is complete. - query_range_tasks.remove(their_node_id); - } Ok(()) } - /// When a query is initiated the remote peer will begin streaming + /// When an SCID query is initiated the remote peer will begin streaming /// gossip messages. In the event of a failure, we may have received /// some channel information. Before trying with another peer, the /// caller should update its set of SCIDs that need to be queried. fn handle_reply_short_channel_ids_end(&self, their_node_id: &PublicKey, msg: &ReplyShortChannelIdsEnd) -> Result<(), LightningError> { log_debug!(self.logger, "Handling reply_short_channel_ids_end peer={}, full_information={}", log_pubkey!(their_node_id), msg.full_information); - // First we obtain a lock on the task hashmap. In order to avoid borrowing issues - // we will access the task as needed. - let mut query_short_channel_ids_tasks = self.scid_query_tasks.lock().unwrap(); - - // If there is no existing task then we have received an unknown - // message and should return an error. - if query_short_channel_ids_tasks.get(their_node_id).is_none() { - return Err(LightningError { - err: String::from("Unknown reply_short_channel_ids_end message"), - action: ErrorAction::IgnoreError, - }); - } - - // If the reply's chain_hash does not match the task's chain_hash then - // the reply is malformed and we should return an error. - if msg.chain_hash != query_short_channel_ids_tasks.get(their_node_id).unwrap().chain_hash { - query_short_channel_ids_tasks.remove(their_node_id); - return Err(LightningError { - err: String::from("Received reply_short_channel_ids_end with incorrect chain_hash"), - action: ErrorAction::IgnoreError - }); - } - // If the remote node does not have up-to-date information for the // chain_hash they will set full_information=false. We can fail // the result and try again with a different peer. if !msg.full_information { - query_short_channel_ids_tasks.remove(their_node_id); return Err(LightningError { err: String::from("Received reply_short_channel_ids_end with no information"), action: ErrorAction::IgnoreError }); } - // If we have more scids to process we send the next batch in the task - { - let task = query_short_channel_ids_tasks.get_mut(their_node_id).unwrap(); - if task.short_channel_ids.len() > 0 { - self.finalize_query_short_ids(task); - return Ok(()); - } - } - - // Otherwise the task is complete and we can remove it - log_debug!(self.logger, "Completed query_short_channel_ids peer={}", log_pubkey!(their_node_id)); - query_short_channel_ids_tasks.remove(their_node_id); Ok(()) } @@ -555,118 +339,6 @@ where } } -/// Safely calculates the last_blocknum given a first_blocknum and -/// number_of_blocks by returning the u32::MAX-1 if there is an overflow -fn last_blocknum(first_blocknum: u32, number_of_blocks: u32) -> u32 { - match first_blocknum.checked_add(number_of_blocks) { - Some(val) => val - 1, - None => 0xffff_ffff - 1, - } -} - -/// Maintains state for a channel range query that we initiated. -/// The query may result in one or more reply_channel_range messages -/// being received. This struct helps determine the status of the query -/// when there are multiple replies. It also collects results for initiating -/// SCID queries. -/// -/// The task is complete and can be cleaned up when a reply meets or -/// exceeds the last block in the query. The collected SCIDs in the task -/// can be used to generate an ScidQueryTask. -/// -/// A query may fail if the recipient does not maintain up-to-date -/// information for the chain or if the recipient fails to reply within -/// a reasonable amount of time. In either event, the query can be -/// re-initiated with a different peer. -pub struct ChanRangeQueryTask { - /// The public key of the node we will be sending queries to - pub node_id: PublicKey, - /// The genesis hash of the blockchain being queried - pub chain_hash: BlockHash, - /// The height of the first block for the channel UTXOs being queried - pub first_blocknum: u32, - /// The number of blocks to include in the query results - pub number_of_blocks: u32, - /// Tracks the number of reply messages we have received - pub number_of_replies: usize, - /// The height of the first block received in a reply. This value - /// should be less than or equal to the first_blocknum requested in - /// the query_channel_range. This allows the range of the replies to - /// contain, but not necessarily strictly, the queried range. - pub received_first_block: Option, - /// The height of the last block received in a reply. This value - /// will get incrementally closer to the target of - /// first_blocknum plus number_of_blocks from the query_channel_range. - pub received_last_block: Option, - /// Contains short_channel_ids received in one or more reply messages. - /// These will be sent in one ore more query_short_channel_ids messages - /// when the task is complete. - pub short_channel_ids: Vec, -} - -impl ChanRangeQueryTask { - /// Constructs a new GossipQueryRangeTask - pub fn new(their_node_id: &PublicKey, chain_hash: BlockHash, first_blocknum: u32, number_of_blocks: u32) -> Self { - ChanRangeQueryTask { - node_id: their_node_id.clone(), - chain_hash, - first_blocknum, - number_of_blocks, - number_of_replies: 0, - received_first_block: None, - received_last_block: None, - short_channel_ids: vec![], - } - } -} - -/// Maintains state when sending one or more short_channel_ids messages -/// to a peer. Only a single SCID query can be in-flight with a peer. The -/// number of SCIDs per query is limited by the size of a Lightning message -/// payload. When querying a large number of SCIDs (results of a large -/// channel range query for instance), multiple query_short_channel_ids -/// messages need to be sent. This task maintains the list of awaiting -/// SCIDs to be queried. -/// -/// When a successful reply_short_channel_ids_end message is received, the -/// next batch of SCIDs can be sent. When no remaining SCIDs exist in the -/// task, the task is complete and can be cleaned up. -/// -/// The recipient may reply indicating that up-to-date information for the -/// chain is not maintained. A query may also fail to complete within a -/// reasonable amount of time. In either event, the short_channel_ids -/// can be queried from a different peer after validating the set of -/// SCIDs that still need to be queried. -pub struct ScidQueryTask { - /// The public key of the node we will be sending queries to - pub node_id: PublicKey, - /// The genesis hash of the blockchain being queried - pub chain_hash: BlockHash, - /// A vector of short_channel_ids that we would like routing gossip - /// information for. This list will be chunked and sent to the peer - /// in one or more query_short_channel_ids messages. - pub short_channel_ids: Vec, -} - -impl ScidQueryTask { - /// Constructs a new GossipQueryShortChannelIdsTask - pub fn new(their_node_id: &PublicKey, chain_hash: BlockHash, short_channel_ids: Vec) -> Self { - ScidQueryTask { - node_id: their_node_id.clone(), - chain_hash, - short_channel_ids, - } - } - - /// Adds short_channel_ids to the pending list of short_channel_ids - /// to be sent in the next request. You can add additional values - /// while a query is in-flight. These new values will be sent once - /// the active query has completed. - pub fn add(&mut self, mut short_channel_ids: Vec) { - self.short_channel_ids.append(&mut short_channel_ids); - } -} - #[derive(PartialEq, Debug)] /// Details about one direction of a channel. Received /// within a channel update. @@ -2274,113 +1946,28 @@ mod tests { } #[test] - fn sending_query_channel_range() { + fn calling_sync_routing_table() { let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(); let node_privkey_1 = &SecretKey::from_slice(&[42; 32]).unwrap(); - let node_privkey_2 = &SecretKey::from_slice(&[41; 32]).unwrap(); let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_privkey_1); - let node_id_2 = PublicKey::from_secret_key(&secp_ctx, node_privkey_2); let chain_hash = genesis_block(Network::Testnet).header.block_hash(); let first_blocknum = 0; let number_of_blocks = 0xffff_ffff; - - // When no active query exists for the node, it should send a query message and generate a task - { - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, first_blocknum, number_of_blocks); - assert!(result.is_ok()); - - // It should create a task for the query - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().contains_key(&node_id_1)); - - // It should send a query_channel_range message with the correct information - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - match &events[0] { - MessageSendEvent::SendChannelRangeQuery{ node_id, msg } => { - assert_eq!(node_id, &node_id_1); - assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.first_blocknum, first_blocknum); - assert_eq!(msg.number_of_blocks, number_of_blocks); - }, - _ => panic!("Expected MessageSendEvent::SendChannelRangeQuery") - }; - } - - // When an active query exists for the node, when there is a subsequent query request, it - // should fail to initiate a new query - { - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, first_blocknum, number_of_blocks); - assert_eq!(result.is_err(), true); - } - - // When no active query exists for a different node, it should send a query message - { - let result = net_graph_msg_handler.query_channel_range(&node_id_2, chain_hash, first_blocknum, number_of_blocks); - assert_eq!(result.is_ok(), true); - - // It should create a task for the query - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().contains_key(&node_id_2)); - - // It should send a query_channel_message with the correct information - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - match &events[0] { - MessageSendEvent::SendChannelRangeQuery{ node_id, msg } => { - assert_eq!(node_id, &node_id_2); - assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.first_blocknum, first_blocknum); - assert_eq!(msg.number_of_blocks, number_of_blocks); - }, - _ => panic!("Expected MessageSendEvent::SendChannelRangeQuery") - }; - } - } - - #[test] - fn sending_query_short_channel_ids() { - let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(); - let node_privkey_1 = &SecretKey::from_slice(&[42; 32]).unwrap(); - let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_privkey_1); - - let chain_hash = genesis_block(Network::Testnet).header.block_hash(); - - // The first query should send the batch of scids to the peer - { - let short_channel_ids: Vec = vec![0, 1, 2]; - let result = net_graph_msg_handler.query_short_channel_ids(&node_id_1, chain_hash, short_channel_ids.clone()); - assert!(result.is_ok()); - - // Validate that we have enqueued a send message event and that it contains the correct information - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - match &events[0] { - MessageSendEvent::SendShortIdsQuery{ node_id, msg } => { - assert_eq!(node_id, &node_id_1); - assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.short_channel_ids, short_channel_ids); - }, - _ => panic!("Expected MessageSendEvent::SendShortIdsQuery") - }; - } - - // Subsequent queries for scids should enqueue them to be sent in the next batch which will - // be sent when a reply_short_channel_ids_end message is handled. - { - let short_channel_ids: Vec = vec![3, 4, 5]; - let result = net_graph_msg_handler.query_short_channel_ids(&node_id_1, chain_hash, short_channel_ids.clone()); - assert!(result.is_ok()); - - // Validate that we have not enqueued another send message event yet - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 0); - - // Validate the task has the queued scids - assert_eq!( - net_graph_msg_handler.scid_query_tasks.lock().unwrap().get(&node_id_1).unwrap().short_channel_ids, - short_channel_ids - ); - } + net_graph_msg_handler.sync_routing_table(&node_id_1); + + // It should send a query_channel_message with the correct information + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::SendChannelRangeQuery{ node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.first_blocknum, first_blocknum); + assert_eq!(msg.number_of_blocks, number_of_blocks); + }, + _ => panic!("Expected MessageSendEvent::SendChannelRangeQuery") + }; } #[test] @@ -2391,120 +1978,9 @@ mod tests { let chain_hash = genesis_block(Network::Testnet).header.block_hash(); - // Test receipt of an unknown reply message. We expect an error - { - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1000, - number_of_blocks: 1050, - short_channel_ids: vec![ - 0x0003e8_000000_0000, // 1000x0x0 - 0x0003e9_000000_0000, // 1001x0x0 - 0x0003f0_000000_0000 // 1008x0x0 - ], - }); - assert!(result.is_err()); - } - - // Test receipt of a single reply_channel_range that exactly matches the queried range. - // It sends a query_short_channel_ids with the returned scids and removes the pending task + // Test receipt of a single reply that should enqueue an SCID query + // matching the SCIDs in the reply { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle a single successful reply that matches the queried channel range - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1000, - number_of_blocks: 100, - short_channel_ids: vec![ - 0x0003e8_000000_0000, // 1000x0x0 - 0x0003e9_000000_0000, // 1001x0x0 - 0x0003f0_000000_0000 // 1008x0x0 - ], - }); - assert!(result.is_ok()); - - // The query is now complete, so we expect the task to be removed - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); - - // We expect to emit a query_short_channel_ids message with scids in our query range - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - match &events[0] { - MessageSendEvent::SendShortIdsQuery { node_id, msg } => { - assert_eq!(node_id, &node_id_1); - assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.short_channel_ids, vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000]); - }, - _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), - } - - // Clean up scid_task - net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); - } - - // Test receipt of a single reply_channel_range for a query that has a u32 overflow. We expect - // it sends a query_short_channel_ids with the returned scids and removes the pending task. - { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 0xffff_ffff); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle a single successful reply that matches the queried channel range - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1000, - number_of_blocks: 0xffff_ffff, - short_channel_ids: vec![ - 0x0003e8_000000_0000, // 1000x0x0 - 0x0003e9_000000_0000, // 1001x0x0 - 0x0003f0_000000_0000 // 1008x0x0 - ], - }); - assert!(result.is_ok()); - - // The query is now complete, so we expect the task to be removed - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); - - // We expect to emit a query_short_channel_ids message with scids in our query range - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - match &events[0] { - MessageSendEvent::SendShortIdsQuery { node_id, msg } => { - assert_eq!(node_id, &node_id_1); - assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.short_channel_ids, vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000]); - }, - _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), - } - - // Clean up scid_task - net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); - } - - // Test receipt of a single reply that encompasses the queried channel range. This is allowed - // since a reply must contain at least part of the query range. Receipt of the reply should - // send a query_short_channel_ids message with scids filtered to the query range and remove - // the pending task. - { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - // 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, @@ -2522,203 +1998,30 @@ mod tests { }); assert!(result.is_ok()); - // The query is now complete, so we expect the task to be removed - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); - - // We expect to emit a query_short_channel_ids message with scids filtered to those - // within the original query range. - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - match &events[0] { - MessageSendEvent::SendShortIdsQuery { node_id, msg } => { - assert_eq!(node_id, &node_id_1); - assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.short_channel_ids, vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000]); - }, - _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), - } - - // Clean up scid_task - net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); - } - - // Test receipt of multiple reply messages for a single query. This happens when the number - // of scids in the query range exceeds the size limits of a single reply message. We expect - // to initiate a query_short_channel_ids for the first batch of scids and we enqueue the - // remaining scids for later processing. We remove the range query task after receipt of all - // reply messages. - { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle the first reply message - let reply_1_scids = vec![ - 0x0003e8_000000_0000, // 1000x0x0 - 0x0003e9_000000_0000, // 1001x0x0 - 0x000419_000000_0000, // 1049x0x0 - ]; - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1000, - number_of_blocks: 50, - short_channel_ids: reply_1_scids.clone(), - }); - assert!(result.is_ok()); - - // Handle the next reply in the sequence, which must start at the previous message's - // first_blocknum plus number_of_blocks. The scids in this reply will be queued. - let reply_2_scids = vec![ - 0x00041a_000000_0000, // 1050x0x0 - 0x000432_000000_0000, // 1074x0x0 - ]; - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1050, - number_of_blocks: 25, - short_channel_ids: reply_2_scids.clone(), - }); - assert!(result.is_ok()); - - // Handle the final reply in the sequence, which must meet or exceed the initial query's - // first_blocknum plus number_of_blocks. The scids in this reply will be queued. - let reply_3_scids = vec![ - 0x000433_000000_0000, // 1075x0x0 - 0x00044b_000000_0000, // 1099x0x0 - ]; - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1075, - number_of_blocks: 25, - short_channel_ids: reply_3_scids.clone(), - }); - assert!(result.is_ok()); - - // After the final reply we expect the query task to be removed - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); - - // We expect to emit a query_short_channel_ids message with the accumulated scids that - // match the queried channel range. + // We expect to emit a query_short_channel_ids message with the received scids let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); match &events[0] { MessageSendEvent::SendShortIdsQuery { node_id, msg } => { assert_eq!(node_id, &node_id_1); assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.short_channel_ids, [reply_1_scids, reply_2_scids, reply_3_scids].concat()); + assert_eq!(msg.short_channel_ids, vec![ + 0x0003e0_000000_0000, // 992x0x0 + 0x0003e8_000000_0000, // 1000x0x0 + 0x0003e9_000000_0000, // 1001x0x0 + 0x0003f0_000000_0000, // 1008x0x0 + 0x00044c_000000_0000, // 1100x0x0 + 0x0006e0_000000_0000, // 1760x0x0 + ]); }, _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), } - - // Clean up scid_task - net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); - } - - // Test receipt of a sequence of replies with a valid first reply and a second reply that - // resumes on the same block as the first reply. The spec requires a subsequent - // first_blocknum to equal the prior first_blocknum plus number_of_blocks, however - // due to discrepancies in implementation we must loosen this restriction. - { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle the first reply message - let reply_1_scids = vec![ - 0x0003e8_000000_0000, // 1000x0x0 - 0x0003e9_000000_0000, // 1001x0x0 - 0x000419_000000_0000, // 1049x0x0 - ]; - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1000, - number_of_blocks: 50, - short_channel_ids: reply_1_scids.clone(), - }); - assert!(result.is_ok()); - - // Handle the next reply in the sequence, which is non-spec but resumes on the last block - // of the first message. - let reply_2_scids = vec![ - 0x000419_000001_0000, // 1049x1x0 - 0x00041a_000000_0000, // 1050x0x0 - 0x000432_000000_0000, // 1074x0x0 - ]; - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1049, - number_of_blocks: 51, - short_channel_ids: reply_2_scids.clone(), - }); - assert!(result.is_ok()); - - // After the final reply we expect the query task to be removed - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); - - // We expect to emit a query_short_channel_ids message with the accumulated scids that - // match the queried channel range - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - match &events[0] { - MessageSendEvent::SendShortIdsQuery { node_id, msg } => { - assert_eq!(node_id, &node_id_1); - assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.short_channel_ids, [reply_1_scids, reply_2_scids].concat()); - }, - _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), - } - - // Clean up scid_task - net_graph_msg_handler.scid_query_tasks.lock().unwrap().clear(); - } - - // Test receipt of reply with a chain_hash that does not match the query. We expect to return - // an error and to remove the query task. - { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle the reply with a mismatched chain_hash. We expect IgnoreError result and the - // task should be removed. - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash: genesis_block(Network::Bitcoin).header.block_hash(), - full_information: true, - first_blocknum: 1000, - number_of_blocks: 1050, - short_channel_ids: vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000], - }); - assert!(result.is_err()); - assert_eq!(result.err().unwrap().err, "Received reply_channel_range with invalid chain_hash"); - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); } // Test receipt of a reply that indicates the remote node does not maintain up-to-date // information for the chain_hash. Because of discrepancies in implementation we use - // full_information=false and short_channel_ids=[] as the signal. We should expect an error - // and the task should be removed. + // full_information=false and short_channel_ids=[] as the signal. { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - // Handle the reply indicating the peer was unable to fulfill our request. let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { chain_hash, @@ -2729,125 +2032,6 @@ mod tests { }); assert!(result.is_err()); assert_eq!(result.err().unwrap().err, "Received reply_channel_range with no information available"); - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); - } - - // Test receipt of a reply that has a first_blocknum that is above the first_blocknum - // requested in our query. The reply must contain the queried block range. We expect an - // error result and the task should be removed. - { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle the reply that has a first_blocknum above the query's first_blocknum - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1001, - number_of_blocks: 100, - short_channel_ids: vec![], - }); - assert!(result.is_err()); - assert_eq!(result.err().unwrap().err, "Failing reply_channel_range with invalid first_blocknum"); - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); - } - - // Test receipt of a first reply that does not overlap the query range at all. The first message - // must have some overlap with the query. We expect an error result and the task should - // be removed. - { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle a reply that contains a block range that precedes the queried block range - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 0, - number_of_blocks: 1000, - short_channel_ids: vec![], - }); - assert!(result.is_err()); - assert_eq!(result.err().unwrap().err, "Failing reply_channel_range with non-overlapping first reply"); - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); - } - - // Test receipt of a sequence of replies with a valid first reply and a second reply that is - // non-sequential. The spec requires a subsequent first_blocknum to equal the prior - // first_blocknum plus number_of_blocks. We expect an IgnoreError result and the task should - // be removed. - { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 100); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle the first reply - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1000, - number_of_blocks: 50, - short_channel_ids: vec![0x0003e8_000000_0000,0x0003e9_000000_0000,0x0003f0_000000_0000], - }); - assert!(result.is_ok()); - - // Handle the second reply which does not start at the proper first_blocknum. We expect - // to return an error and remove the task. - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: 1051, - number_of_blocks: 50, - short_channel_ids: vec![0x0003f1_000000_0000,0x0003f2_000000_0000], - }); - assert!(result.is_err()); - assert_eq!(result.err().unwrap().err, "Failing reply_channel_range with invalid sequence"); - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); - } - - // Test receipt of too many reply messages. We expect an IgnoreError result and the task should - // be removed. - { - // Initiate a channel range query to create a query task - let result = net_graph_msg_handler.query_channel_range(&node_id_1, chain_hash, 1000, 0xffff_ffff); - assert!(result.is_ok()); - - // Clear the SendRangeQuery event - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle a sequence of replies that will fail once the max number of reply has been exceeded. - for block in 1000..=1000 + super::MAX_REPLY_CHANNEL_RANGE_PER_QUERY + 10 { - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { - chain_hash, - full_information: true, - first_blocknum: block as u32, - number_of_blocks: 1, - short_channel_ids: vec![(block as u64) << 40], - }); - if block <= 1000 + super::MAX_REPLY_CHANNEL_RANGE_PER_QUERY { - assert!(result.is_ok()); - } else if block == 1001 + super::MAX_REPLY_CHANNEL_RANGE_PER_QUERY { - assert!(result.is_err()); - assert_eq!(result.err().unwrap().err, "Failing reply_channel_range due to excessive messages"); - } else { - assert!(result.is_err()); - assert_eq!(result.err().unwrap().err, "Received unknown reply_channel_range message"); - } - } - - // Expect the task to be removed - assert!(net_graph_msg_handler.chan_range_query_tasks.lock().unwrap().is_empty()); } } @@ -2859,124 +2043,24 @@ mod tests { let chain_hash = genesis_block(Network::Testnet).header.block_hash(); - // Test receipt of a reply when no query exists. We expect an error to be returned + // Test receipt of a successful reply { let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { chain_hash, full_information: true, }); - assert!(result.is_err()); - assert_eq!(result.err().unwrap().err, "Unknown reply_short_channel_ids_end message"); - } - - // Test receipt of a reply that is for a different chain_hash. We expect an error and the task - // should be removed. - { - // Initiate a query to create a pending query task - let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e8_000000_0000]); assert!(result.is_ok()); - - // Process reply with incorrect chain_hash - let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { - chain_hash: genesis_block(Network::Bitcoin).header.block_hash(), - full_information: true, - }); - assert!(result.is_err()); - assert_eq!(result.err().unwrap().err, "Received reply_short_channel_ids_end with incorrect chain_hash"); - - // Expect the task to be removed - assert!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().is_empty()); } // Test receipt of a reply that indicates the peer does not maintain up-to-date information - // for the chain_hash requested in the query. We expect an error and task should be removed. + // for the chain_hash requested in the query. { - // Initiate a query to create a pending query task - let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e8_000000_0000]); - assert!(result.is_ok()); - - // Process failed reply let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { chain_hash, full_information: false, }); assert!(result.is_err()); assert_eq!(result.err().unwrap().err, "Received reply_short_channel_ids_end with no information"); - - // Expect the task to be removed - assert!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().is_empty()); - } - - // Test receipt of a successful reply when there are no additional scids to query. We expect - // the task to be removed. - { - // Initiate a query to create a pending query task - let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e8_000000_0000]); - assert!(result.is_ok()); - - // Process success reply - let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { - chain_hash, - full_information: true, - }); - assert!(result.is_ok()); - - // Expect the task to be removed - assert!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().is_empty()); - } - - // Test receipt of a successful reply when there are additional scids to query. We expect - // additional queries to be sent until the task can be removed. - { - // Initiate a query to create a pending query task - let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e8_000000_0000]); - assert!(result.is_ok()); - - // Initiate a second query to add pending scids to the task - let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003e9_000000_0000]); - assert!(result.is_ok()); - assert_eq!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().get(&node_id).unwrap().short_channel_ids, vec![0x0003e9_000000_0000]); - - // Initiate a third query to add pending scids to the task - let result = net_graph_msg_handler.query_short_channel_ids(&node_id, chain_hash, vec![0x0003f0_000000_0000]); - assert!(result.is_ok()); - assert_eq!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().get(&node_id).unwrap().short_channel_ids, vec![0x0003e9_000000_0000, 0x0003f0_000000_0000]); - - // Clear all of the pending send events - net_graph_msg_handler.get_and_clear_pending_msg_events(); - - // Handle the first successful reply, which will send the next batch of scids in a new query - let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { - chain_hash, - full_information: true, - }); - assert!(result.is_ok()); - - // We expect the second batch to be sent in an event - let expected_node_id = &node_id; - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - match &events[0] { - MessageSendEvent::SendShortIdsQuery { node_id, msg } => { - assert_eq!(node_id, expected_node_id); - assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.short_channel_ids, vec![0x0003e9_000000_0000, 0x0003f0_000000_0000]); - }, - _ => panic!("expected MessageSendEvent::SendShortIdsQuery"), - } - - // We expect the scids to be cleared from the task - assert_eq!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().get(&node_id).unwrap().short_channel_ids.len(), 0); - - // Handle the second successful reply - let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { - chain_hash, - full_information: true, - }); - assert!(result.is_ok()); - - // We expect the task should be removed - assert!(net_graph_msg_handler.scid_query_tasks.lock().unwrap().is_empty()); } } diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 55353c0f26b..dfbb283b1a4 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -320,13 +320,7 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler { self.request_full_sync.load(Ordering::Acquire) } - fn query_channel_range(&self, _their_node_id: &PublicKey, _chain_hash: BlockHash, _first_blocknum: u32, _number_of_blocks: u32) -> Result<(), msgs::LightningError> { - Ok(()) - } - - fn query_short_channel_ids(&self, _their_node_id: &PublicKey, _chain_hash: BlockHash, _short_channel_ids: Vec) -> Result<(), msgs::LightningError> { - Ok(()) - } + fn sync_routing_table(&self, _their_node_id: &PublicKey) {} fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: &msgs::ReplyChannelRange) -> Result<(), msgs::LightningError> { Ok(()) From 7e1e0ac97cc2f96a72dfb97fd5edcd039267e681 Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Thu, 3 Dec 2020 11:52:54 -0500 Subject: [PATCH 08/12] Pass gossip_queries messages to handler via ownership This change modifies gossip_queries methods in RoutingMessageHandler to move the message instead of passing a reference. This allows the message handler to be more efficient by not requiring a full copy of SCIDs passed in messages. --- lightning-net-tokio/src/lib.rs | 8 ++--- lightning/src/ln/msgs.rs | 16 ++++++---- lightning/src/ln/peer_handler.rs | 8 ++--- lightning/src/routing/network_graph.rs | 43 +++++++++----------------- lightning/src/util/test_utils.rs | 8 ++--- 5 files changed, 36 insertions(+), 47 deletions(-) diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index df0d6cb1f0b..a90d673b6fd 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -537,10 +537,10 @@ mod tests { fn get_next_node_announcements(&self, _starting_point: Option<&PublicKey>, _batch_amount: u8) -> Vec { Vec::new() } fn should_request_full_sync(&self, _node_id: &PublicKey) -> bool { false } fn sync_routing_table(&self, _their_node_id: &PublicKey) { } - 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(()) } - fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: &QueryShortChannelIds) -> Result<(), LightningError> { Ok(()) } + 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(()) } + fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: QueryShortChannelIds) -> Result<(), LightningError> { Ok(()) } } impl ChannelMessageHandler for MsgHandler { fn handle_open_channel(&self, _their_node_id: &PublicKey, _their_features: InitFeatures, _msg: &OpenChannel) {} diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 671f4211ccc..8cd3f8acc7c 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -834,18 +834,22 @@ pub trait RoutingMessageHandler : Send + Sync + events::MessageSendEventsProvide /// 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. - fn handle_reply_channel_range(&self, their_node_id: &PublicKey, msg: &ReplyChannelRange) -> Result<(), LightningError>; + fn handle_reply_channel_range(&self, their_node_id: &PublicKey, msg: ReplyChannelRange) -> Result<(), LightningError>; /// Handles the reply of a query we initiated asking for routing gossip /// messages for a list of channels. We should receive this message when /// a node has completed its best effort to send us the pertaining routing /// gossip messages. - fn handle_reply_short_channel_ids_end(&self, their_node_id: &PublicKey, msg: &ReplyShortChannelIdsEnd) -> Result<(), LightningError>; + fn handle_reply_short_channel_ids_end(&self, their_node_id: &PublicKey, msg: ReplyShortChannelIdsEnd) -> Result<(), LightningError>; /// Handles when a peer asks us to send a list of short_channel_ids - /// for the requested range of blocks. - fn handle_query_channel_range(&self, their_node_id: &PublicKey, msg: &QueryChannelRange) -> Result<(), LightningError>; + /// for the requested range of blocks. There are potential DoS vectors when + /// handling inbound queries. Handling requests with first_blocknum very far + /// away may trigger repeated disk I/O if the NetworkGraph is not fully in-memory. + fn handle_query_channel_range(&self, their_node_id: &PublicKey, msg: QueryChannelRange) -> Result<(), LightningError>; /// Handles when a peer asks us to send routing gossip messages for a - /// list of short_channel_ids. - fn handle_query_short_channel_ids(&self, their_node_id: &PublicKey, msg: &QueryShortChannelIds) -> Result<(), LightningError>; + /// list of short_channel_ids. There are potential DoS vectors when handling + /// inbound queries. Handling requests with first_blocknum very far away may + /// trigger repeated disk I/O if the NetworkGraph is not fully in-memory. + fn handle_query_short_channel_ids(&self, their_node_id: &PublicKey, msg: QueryShortChannelIds) -> Result<(), LightningError>; } mod fuzzy_internal_msgs { diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index beac542abdd..daa74ad5b00 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -842,16 +842,16 @@ impl PeerManager { - self.message_handler.route_handler.handle_query_short_channel_ids(&peer.their_node_id.unwrap(), &msg)?; + self.message_handler.route_handler.handle_query_short_channel_ids(&peer.their_node_id.unwrap(), msg)?; }, wire::Message::ReplyShortChannelIdsEnd(msg) => { - self.message_handler.route_handler.handle_reply_short_channel_ids_end(&peer.their_node_id.unwrap(), &msg)?; + self.message_handler.route_handler.handle_reply_short_channel_ids_end(&peer.their_node_id.unwrap(), msg)?; }, wire::Message::QueryChannelRange(msg) => { - self.message_handler.route_handler.handle_query_channel_range(&peer.their_node_id.unwrap(), &msg)?; + self.message_handler.route_handler.handle_query_channel_range(&peer.their_node_id.unwrap(), msg)?; }, wire::Message::ReplyChannelRange(msg) => { - self.message_handler.route_handler.handle_reply_channel_range(&peer.their_node_id.unwrap(), &msg)?; + self.message_handler.route_handler.handle_reply_channel_range(&peer.their_node_id.unwrap(), msg)?; }, wire::Message::GossipTimestampFilter(_msg) => { // TODO: handle message diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index c1258ce37a2..48900ecc446 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -246,10 +246,8 @@ impl 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. In the event of a failure, we may have received - /// some channel information. Before trying with another peer, the - /// caller should update its set of SCIDs that need to be queried. - fn handle_reply_channel_range(&self, their_node_id: &PublicKey, msg: &ReplyChannelRange) -> Result<(), LightningError> { + /// original query range. + 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(),); // Validate that the remote node maintains up-to-date channel @@ -263,20 +261,13 @@ impl RoutingMessageHandler for N }); } - // Copy the SCIDs into a new vector to be sent in the SCID query - let scid_size = msg.short_channel_ids.len(); - let mut short_channel_ids: Vec = Vec::with_capacity(scid_size); - for scid in msg.short_channel_ids.iter() { - short_channel_ids.push(scid.clone()); - } - - log_debug!(self.logger, "Sending query_short_channel_ids peer={}, batch_size={}", log_pubkey!(their_node_id), scid_size); + log_debug!(self.logger, "Sending query_short_channel_ids peer={}, batch_size={}", log_pubkey!(their_node_id), msg.short_channel_ids.len()); let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push(events::MessageSendEvent::SendShortIdsQuery { node_id: their_node_id.clone(), msg: QueryShortChannelIds { - chain_hash: msg.chain_hash.clone(), - short_channel_ids, + chain_hash: msg.chain_hash, + short_channel_ids: msg.short_channel_ids, } }); @@ -287,7 +278,7 @@ impl RoutingMessageHandler for N /// gossip messages. In the event of a failure, we may have received /// some channel information. Before trying with another peer, the /// caller should update its set of SCIDs that need to be queried. - fn handle_reply_short_channel_ids_end(&self, their_node_id: &PublicKey, msg: &ReplyShortChannelIdsEnd) -> Result<(), LightningError> { + fn handle_reply_short_channel_ids_end(&self, their_node_id: &PublicKey, msg: ReplyShortChannelIdsEnd) -> Result<(), LightningError> { log_debug!(self.logger, "Handling reply_short_channel_ids_end peer={}, full_information={}", log_pubkey!(their_node_id), msg.full_information); // If the remote node does not have up-to-date information for the @@ -303,10 +294,7 @@ impl RoutingMessageHandler for N Ok(()) } - /// There are potential DoS vectors when handling inbound queries. - /// Handling requests with first_blocknum very far away may trigger repeated - /// disk I/O if the NetworkGraph is not fully in-memory. - fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: &QueryChannelRange) -> Result<(), LightningError> { + fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: QueryChannelRange) -> Result<(), LightningError> { // TODO Err(LightningError { err: String::from("Not implemented"), @@ -314,10 +302,7 @@ impl RoutingMessageHandler for N }) } - /// There are potential DoS vectors when handling inbound queries. - /// Handling requests with first_blocknum very far away may trigger repeated - /// disk I/O if the NetworkGraph is not fully in-memory. - fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: &QueryShortChannelIds) -> Result<(), LightningError> { + fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: QueryShortChannelIds) -> Result<(), LightningError> { // TODO Err(LightningError { err: String::from("Not implemented"), @@ -1982,7 +1967,7 @@ mod tests { // 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 { + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, ReplyChannelRange { chain_hash, full_information: true, first_blocknum: 0, @@ -2023,7 +2008,7 @@ mod tests { // full_information=false and short_channel_ids=[] as the signal. { // Handle the reply indicating the peer was unable to fulfill our request. - let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, &ReplyChannelRange { + let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, ReplyChannelRange { chain_hash, full_information: false, first_blocknum: 1000, @@ -2045,7 +2030,7 @@ mod tests { // Test receipt of a successful reply { - let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { + let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, ReplyShortChannelIdsEnd { chain_hash, full_information: true, }); @@ -2055,7 +2040,7 @@ mod tests { // Test receipt of a reply that indicates the peer does not maintain up-to-date information // for the chain_hash requested in the query. { - let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, &ReplyShortChannelIdsEnd { + let result = net_graph_msg_handler.handle_reply_short_channel_ids_end(&node_id, ReplyShortChannelIdsEnd { chain_hash, full_information: false, }); @@ -2072,7 +2057,7 @@ mod tests { let chain_hash = genesis_block(Network::Testnet).header.block_hash(); - let result = net_graph_msg_handler.handle_query_channel_range(&node_id, &QueryChannelRange { + let result = net_graph_msg_handler.handle_query_channel_range(&node_id, QueryChannelRange { chain_hash, first_blocknum: 0, number_of_blocks: 0xffff_ffff, @@ -2088,7 +2073,7 @@ mod tests { let chain_hash = genesis_block(Network::Testnet).header.block_hash(); - let result = net_graph_msg_handler.handle_query_short_channel_ids(&node_id, &QueryShortChannelIds { + let result = net_graph_msg_handler.handle_query_short_channel_ids(&node_id, QueryShortChannelIds { chain_hash, short_channel_ids: vec![0x0003e8_000000_0000], }); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index dfbb283b1a4..62bfda09487 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -322,19 +322,19 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler { fn sync_routing_table(&self, _their_node_id: &PublicKey) {} - fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: &msgs::ReplyChannelRange) -> Result<(), msgs::LightningError> { + fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyChannelRange) -> Result<(), msgs::LightningError> { Ok(()) } - fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: &msgs::ReplyShortChannelIdsEnd) -> Result<(), msgs::LightningError> { + fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyShortChannelIdsEnd) -> Result<(), msgs::LightningError> { Ok(()) } - fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: &msgs::QueryChannelRange) -> Result<(), msgs::LightningError> { + fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::QueryChannelRange) -> Result<(), msgs::LightningError> { Ok(()) } - fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: &msgs::QueryShortChannelIds) -> Result<(), msgs::LightningError> { + fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: msgs::QueryShortChannelIds) -> Result<(), msgs::LightningError> { Ok(()) } } From 7a4a29ffe0dbfdd2fe82376024ae680401c74700 Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Thu, 3 Dec 2020 12:00:36 -0500 Subject: [PATCH 09/12] Pass Init message to sync_routing_table method This commit modifies sync_routing_table in RoutingMessageHandler to accept a reference to the Init message received by the peer. This allows the method to use the Peer's features to drive the operations of the gossip_queries routing table sync. --- lightning-net-tokio/src/lib.rs | 2 +- lightning/src/ln/msgs.rs | 2 +- lightning/src/routing/network_graph.rs | 46 +++++++++++++++++--------- lightning/src/util/test_utils.rs | 2 +- 4 files changed, 33 insertions(+), 19 deletions(-) diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index a90d673b6fd..eb4d347e6a9 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -536,7 +536,7 @@ mod tests { 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 should_request_full_sync(&self, _node_id: &PublicKey) -> bool { false } - fn sync_routing_table(&self, _their_node_id: &PublicKey) { } + fn sync_routing_table(&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 8cd3f8acc7c..073637413a8 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -830,7 +830,7 @@ pub trait RoutingMessageHandler : Send + Sync + events::MessageSendEventsProvide /// Initiates routing gossip sync by querying a peer to discover channels /// and their associated routing gossip messages. This method will use a /// sync strategy defined by the implementor. - fn sync_routing_table(&self, their_node_id: &PublicKey); + fn sync_routing_table(&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/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index 48900ecc446..3a7cd4eb985 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -23,7 +23,7 @@ use bitcoin::hash_types::BlockHash; use chain; use chain::Access; use ln::features::{ChannelFeatures, NodeFeatures}; -use ln::msgs::{DecodeError, ErrorAction, LightningError, RoutingMessageHandler, NetAddress, MAX_VALUE_MSAT}; +use ln::msgs::{DecodeError, ErrorAction, Init, LightningError, RoutingMessageHandler, NetAddress, MAX_VALUE_MSAT}; use ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, OptionalField}; use ln::msgs::{QueryChannelRange, ReplyChannelRange, QueryShortChannelIds, ReplyShortChannelIdsEnd}; use ln::msgs; @@ -226,7 +226,10 @@ impl RoutingMessageHandler for N /// 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) { + fn sync_routing_table(&self, their_node_id: &PublicKey, init_msg: &Init) { + if !init_msg.features.supports_gossip_queries() { + 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); @@ -996,9 +999,9 @@ impl NetworkGraph { #[cfg(test)] mod tests { use chain; - use ln::features::{ChannelFeatures, NodeFeatures}; + use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures}; use routing::network_graph::{NetGraphMsgHandler, NetworkGraph}; - use ln::msgs::{OptionalField, RoutingMessageHandler, UnsignedNodeAnnouncement, NodeAnnouncement, + use ln::msgs::{Init, OptionalField, RoutingMessageHandler, UnsignedNodeAnnouncement, NodeAnnouncement, UnsignedChannelAnnouncement, ChannelAnnouncement, UnsignedChannelUpdate, ChannelUpdate, HTLCFailChannelUpdate, ReplyChannelRange, ReplyShortChannelIdsEnd, QueryChannelRange, QueryShortChannelIds, MAX_VALUE_MSAT}; use util::test_utils; @@ -1939,20 +1942,31 @@ mod tests { let chain_hash = genesis_block(Network::Testnet).header.block_hash(); let first_blocknum = 0; let number_of_blocks = 0xffff_ffff; - net_graph_msg_handler.sync_routing_table(&node_id_1); + + // 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); + let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 0); + } // It should send a query_channel_message with the correct information - let events = net_graph_msg_handler.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - match &events[0] { - MessageSendEvent::SendChannelRangeQuery{ node_id, msg } => { - assert_eq!(node_id, &node_id_1); - assert_eq!(msg.chain_hash, chain_hash); - assert_eq!(msg.first_blocknum, first_blocknum); - assert_eq!(msg.number_of_blocks, number_of_blocks); - }, - _ => panic!("Expected MessageSendEvent::SendChannelRangeQuery") - }; + { + let init_msg = Init { features: InitFeatures::known() }; + net_graph_msg_handler.sync_routing_table(&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] { + MessageSendEvent::SendChannelRangeQuery{ node_id, msg } => { + assert_eq!(node_id, &node_id_1); + assert_eq!(msg.chain_hash, chain_hash); + assert_eq!(msg.first_blocknum, first_blocknum); + assert_eq!(msg.number_of_blocks, number_of_blocks); + }, + _ => panic!("Expected MessageSendEvent::SendChannelRangeQuery") + }; + } } #[test] diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 62bfda09487..d1daf6bcddd 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -320,7 +320,7 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler { self.request_full_sync.load(Ordering::Acquire) } - fn sync_routing_table(&self, _their_node_id: &PublicKey) {} + fn sync_routing_table(&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 e742894492c55802b241eebc585bbd28aa16481b Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Thu, 3 Dec 2020 12:48:40 -0500 Subject: [PATCH 10/12] Change routing table sync to use gossip_queries This commit changes outbound routing table sync to use gossip_queries instead of the effectively deprecated initial_routing_sync feature. This change removes setting of initial_routing_sync in our outbound Init message. Instead we now call sync_routing_table after receiving an Init message from a peer. If the peer supports gossip_queries and should_request_full_sync returns true, we initiate a full gossip_queries sync. --- lightning/src/ln/features.rs | 5 +++ lightning/src/ln/msgs.rs | 14 ++++--- lightning/src/ln/peer_handler.rs | 58 ++------------------------ lightning/src/routing/network_graph.rs | 45 ++++++++++++++++---- 4 files changed, 55 insertions(+), 67 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 0c3c360189f..3ce3d4fa7e6 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -484,6 +484,7 @@ impl Features { pub(crate) fn supports_gossip_queries(&self) -> bool { ::supports_feature(&self.flags) } + #[cfg(test)] pub(crate) fn clear_gossip_queries(mut self) -> Self { ::clear_bits(&mut self.flags); self @@ -514,6 +515,10 @@ impl Features { pub(crate) fn initial_routing_sync(&self) -> bool { ::supports_feature(&self.flags) } + // We are no longer setting initial_routing_sync now that gossip_queries + // is enabled. This feature is ignored by a peer when gossip_queries has + // been negotiated. + #[cfg(test)] pub(crate) fn clear_initial_routing_sync(&mut self) { ::clear_bits(&mut self.flags) } diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 073637413a8..55d3acdfffe 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -804,6 +804,12 @@ pub trait ChannelMessageHandler : events::MessageSendEventsProvider + Send + Syn } /// A trait to describe an object which can receive routing messages. +/// +/// # Implementor DoS Warnings +/// +/// For `gossip_queries` messages there are potential DoS vectors when handling +/// inbound queries. Implementors using an on-disk network graph should be aware of +/// repeated disk I/O for queries accessing different parts of the network graph. pub trait RoutingMessageHandler : Send + Sync + events::MessageSendEventsProvider { /// Handle an incoming node_announcement message, returning true if it should be forwarded on, /// false or returning an Err otherwise. @@ -841,14 +847,10 @@ pub trait RoutingMessageHandler : Send + Sync + events::MessageSendEventsProvide /// gossip messages. fn handle_reply_short_channel_ids_end(&self, their_node_id: &PublicKey, msg: ReplyShortChannelIdsEnd) -> Result<(), LightningError>; /// Handles when a peer asks us to send a list of short_channel_ids - /// for the requested range of blocks. There are potential DoS vectors when - /// handling inbound queries. Handling requests with first_blocknum very far - /// away may trigger repeated disk I/O if the NetworkGraph is not fully in-memory. + /// for the requested range of blocks. fn handle_query_channel_range(&self, their_node_id: &PublicKey, msg: QueryChannelRange) -> Result<(), LightningError>; /// Handles when a peer asks us to send routing gossip messages for a - /// list of short_channel_ids. There are potential DoS vectors when handling - /// inbound queries. Handling requests with first_blocknum very far away may - /// trigger repeated disk I/O if the NetworkGraph is not fully in-memory. + /// list of short_channel_ids. fn handle_query_short_channel_ids(&self, their_node_id: &PublicKey, msg: QueryShortChannelIds) -> Result<(), LightningError>; } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index daa74ad5b00..890df356cc0 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -584,11 +584,7 @@ impl PeerManager PeerManager(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 @@ -1404,41 +1391,4 @@ mod tests { assert_eq!(cfgs[1].routing_handler.chan_upds_recvd.load(Ordering::Acquire), 100); assert_eq!(cfgs[1].routing_handler.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 cfgs = create_peermgr_cfgs(2); - cfgs[0].routing_handler.request_full_sync.store(true, Ordering::Release); - let peers = create_network(2, &cfgs); - 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 cfgs = create_peermgr_cfgs(2); - cfgs[1].routing_handler.request_full_sync.store(true, Ordering::Release); - let peers = create_network(2, &cfgs); - 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/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index 3a7cd4eb985..a16e67529e4 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -219,17 +219,26 @@ impl RoutingMessageHandler for N } /// 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 +258,10 @@ impl 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 +1979,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 +2012,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, From e0bb63bc600b9f5ec53948a1160033fec699b3b4 Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Wed, 9 Dec 2020 15:06:54 -0500 Subject: [PATCH 11/12] Remove should_request_full_sync from RoutingMessageHandler This method was used to set the initial_routing_sync flag when sending an outbound Init message to a peer. Since we are now relying on gossip_queries instead of initial_routing_sync, synchronization can be fully encapsulate into RoutingMessageHandler via sync_routing_table. This commit removes should_request_full_sync from the trait RoutingMessageHandler. The implementation is still used in NetGraphMsgHandler and has been converted into a private method instead of a trait function. --- lightning-net-tokio/src/lib.rs | 1 - lightning/src/ln/msgs.rs | 2 -- lightning/src/routing/network_graph.rs | 23 ++++++++++++----------- lightning/src/util/test_utils.rs | 4 ---- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index eb4d347e6a9..8e5885ca9bf 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -535,7 +535,6 @@ mod tests { fn handle_htlc_fail_channel_update(&self, _update: &HTLCFailChannelUpdate) { } 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 should_request_full_sync(&self, _node_id: &PublicKey) -> bool { false } fn sync_routing_table(&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(()) } diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 55d3acdfffe..b392a22ed19 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -831,8 +831,6 @@ pub trait RoutingMessageHandler : Send + Sync + events::MessageSendEventsProvide /// immediately higher (as defined by ::cmp) than starting_point. /// If None is provided for starting_point, we start at the first node. fn get_next_node_announcements(&self, starting_point: Option<&PublicKey>, batch_amount: u8) -> Vec; - /// Returns whether a full sync should be requested from a peer. - fn should_request_full_sync(&self, node_id: &PublicKey) -> bool; /// Initiates routing gossip sync by querying a peer to discover channels /// and their associated routing gossip messages. This method will use a /// sync strategy defined by the implementor. diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index a16e67529e4..8075462c938 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -105,6 +105,18 @@ impl NetGraphMsgHandler 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,17 +219,6 @@ impl 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 /// using gossip_queries. The default strategy used by this implementation /// is to sync the full block range with several peers. diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index d1daf6bcddd..c944e572cd4 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -316,10 +316,6 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler { Vec::new() } - fn should_request_full_sync(&self, _node_id: &PublicKey) -> bool { - self.request_full_sync.load(Ordering::Acquire) - } - fn sync_routing_table(&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> { From c026764e8d818a719d40aafd3d6580cac1e47c45 Mon Sep 17 00:00:00 2001 From: bmancini55 Date: Tue, 15 Dec 2020 13:48:14 -0500 Subject: [PATCH 12/12] Fix comment for sync_routing_table Corrects the comment for sync_routing_table in RoutingMessageHandler to be less prescriptive about the implementor's actions. --- lightning/src/ln/msgs.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index b392a22ed19..25553c7080f 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -831,9 +831,9 @@ pub trait RoutingMessageHandler : Send + Sync + events::MessageSendEventsProvide /// immediately higher (as defined by ::cmp) than starting_point. /// If None is provided for starting_point, we start at the first node. fn get_next_node_announcements(&self, starting_point: Option<&PublicKey>, batch_amount: u8) -> Vec; - /// Initiates routing gossip sync by querying a peer to discover channels - /// and their associated routing gossip messages. This method will use a - /// sync strategy defined by the implementor. + /// 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); /// 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