mirrors/rust-lightning
1
0
Fork 0
mirror of https://github.com/lightningdevkit/rust-lightning.git synced 2025-02-24 23:08:36 +01:00
Code Issues Projects Releases Packages Wiki Activity

Move checking of specific require peer feature bits to handlers

Browse source 
As we remove the concept of a global "known/supported" feature set
in LDK, we should also remove the concept of a global "required"
feature set. This does so by moving the checks for specific
required features into handlers.

Specifically, it allows the handler `peer_connected` method to
return an `Err` if the peer should be disconnected. Only one such
required feature bit is currently set - `static_remote_key`, which
is required in `ChannelManager`.
This commit is contained in:
Matt Corallo 2022-09-12 19:06:17 +00:00
parent 29484d8e2c
commit bbb590b551
8 changed files with 54 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
lightning-net-tokio/src/lib.rs
View file

@ -577,7 +577,7 @@ mod tests {
fn handle_channel_update(&self, _msg: &ChannelUpdate) -> Result<bool, LightningError> { Ok(false) }
fn get_next_channel_announcement(&self, _starting_point: u64) -> Option<(ChannelAnnouncement, Option<ChannelUpdate>, Option<ChannelUpdate>)> { None }
fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option<NodeAnnouncement> { None }
fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &Init) { }
fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &Init) -> Result<(), ()> { 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(()) }
@ -608,10 +608,11 @@ mod tests {
self.pubkey_disconnected.clone().try_send(()).unwrap();
}
}
fn peer_connected(&self, their_node_id: &PublicKey, _msg: &Init) {
fn peer_connected(&self, their_node_id: &PublicKey, _init_msg: &Init) -> Result<(), ()> {
if *their_node_id == self.expected_pubkey {
self.pubkey_connected.clone().try_send(()).unwrap();
}
Ok(())
}
fn handle_channel_reestablish(&self, _their_node_id: &PublicKey, _msg: &ChannelReestablish) {}
fn handle_error(&self, _their_node_id: &PublicKey, _msg: &ErrorMessage) {}

8
lightning/src/ln/channelmanager.rs
View file

@ -6034,7 +6034,12 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
}
}
fn peer_connected(&self, counterparty_node_id: &PublicKey, init_msg: &msgs::Init) {
fn peer_connected(&self, counterparty_node_id: &PublicKey, init_msg: &msgs::Init) -> Result<(), ()> {
if !init_msg.features.supports_static_remote_key() {
log_debug!(self.logger, "Peer {} does not support static remote key, disconnecting with no_connection_possible", log_pubkey!(counterparty_node_id));
return Err(());
}
log_debug!(self.logger, "Generating channel_reestablish events for {}", log_pubkey!(counterparty_node_id));
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
@ -6085,6 +6090,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
retain
});
//TODO: Also re-broadcast announcement_signatures
Ok(())
}
fn handle_error(&self, counterparty_node_id: &PublicKey, msg: &msgs::ErrorMessage) {

18
lightning/src/ln/msgs.rs
View file

@ -889,7 +889,11 @@ pub trait ChannelMessageHandler : MessageSendEventsProvider {
fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool);
/// Handle a peer reconnecting, possibly generating channel_reestablish message(s).
fn peer_connected(&self, their_node_id: &PublicKey, msg: &Init);
///
/// May return an `Err(())` if the features the peer supports are not sufficient to communicate
/// with us. Implementors should be somewhat conservative about doing so, however, as other
/// message handlers may still wish to communicate with this peer.
fn peer_connected(&self, their_node_id: &PublicKey, msg: &Init) -> Result<(), ()>;
/// Handle an incoming channel_reestablish message from the given peer.
fn handle_channel_reestablish(&self, their_node_id: &PublicKey, msg: &ChannelReestablish);
@ -943,7 +947,11 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider {
/// Called when a connection is established with a peer. This can be used to
/// perform routing table synchronization using a strategy defined by the
/// implementor.
fn peer_connected(&self, their_node_id: &PublicKey, init: &Init);
///
/// May return an `Err(())` if the features the peer supports are not sufficient to communicate
/// with us. Implementors should be somewhat conservative about doing so, however, as other
/// message handlers may still wish to communicate with this peer.
fn peer_connected(&self, their_node_id: &PublicKey, init: &Init) -> Result<(), ()>;
/// 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.
@ -979,7 +987,11 @@ pub trait OnionMessageHandler : OnionMessageProvider {
fn handle_onion_message(&self, peer_node_id: &PublicKey, msg: &OnionMessage);
/// Called when a connection is established with a peer. Can be used to track which peers
/// advertise onion message support and are online.
fn peer_connected(&self, their_node_id: &PublicKey, init: &Init);
///
/// May return an `Err(())` if the features the peer supports are not sufficient to communicate
/// with us. Implementors should be somewhat conservative about doing so, however, as other
/// message handlers may still wish to communicate with this peer.
fn peer_connected(&self, their_node_id: &PublicKey, init: &Init) -> Result<(), ()>;
/// Indicates a connection to the peer failed/an existing connection was lost. Allows handlers to
/// drop and refuse to forward onion messages to this peer.
///

6
lightning/src/ln/onion_route_tests.rs
View file

@ -887,10 +887,12 @@ fn test_do_not_default_to_onion_payload_tlv_format_when_unsupported() {
let chanmon_cfgs = create_chanmon_cfgs(4);
let mut node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
// Set `node[1]` config to `InitFeatures::empty()` which return `false` for
// `supports_variable_length_onion()`
// Set `node[1]` config to `InitFeatures::empty()` + `static_remote_key` which implies
// `!supports_variable_length_onion()` but still supports the required static-remote-key
// feature.
let mut node_1_cfg = &mut node_cfgs[1];
node_1_cfg.features = InitFeatures::empty();
node_1_cfg.features.set_static_remote_key_required();
let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs);

22
lightning/src/ln/peer_handler.rs
View file

@ -72,7 +72,7 @@ impl RoutingMessageHandler for IgnoringMessageHandler {
fn get_next_channel_announcement(&self, _starting_point: u64) ->
Option<(msgs::ChannelAnnouncement, Option<msgs::ChannelUpdate>, Option<msgs::ChannelUpdate>)> { None }
fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option<msgs::NodeAnnouncement> { None }
fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) {}
fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) -> Result<(), ()> { Ok(()) }
fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyChannelRange) -> Result<(), LightningError> { Ok(()) }
fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) }
fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::QueryChannelRange) -> Result<(), LightningError> { Ok(()) }
@ -87,7 +87,7 @@ impl OnionMessageProvider for IgnoringMessageHandler {
}
impl OnionMessageHandler for IgnoringMessageHandler {
fn handle_onion_message(&self, _their_node_id: &PublicKey, _msg: &msgs::OnionMessage) {}
fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) {}
fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) -> Result<(), ()> { Ok(()) }
fn peer_disconnected(&self, _their_node_id: &PublicKey, _no_connection_possible: bool) {}
fn provided_node_features(&self) -> NodeFeatures { NodeFeatures::empty() }
fn provided_init_features(&self, _their_node_id: &PublicKey) -> InitFeatures {
@ -208,7 +208,7 @@ impl ChannelMessageHandler for ErroringMessageHandler {
// msgs::ChannelUpdate does not contain the channel_id field, so we just drop them.
fn handle_channel_update(&self, _their_node_id: &PublicKey, _msg: &msgs::ChannelUpdate) {}
fn peer_disconnected(&self, _their_node_id: &PublicKey, _no_connection_possible: bool) {}
fn peer_connected(&self, _their_node_id: &PublicKey, _msg: &msgs::Init) {}
fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) -> Result<(), ()> { Ok(()) }
fn handle_error(&self, _their_node_id: &PublicKey, _msg: &msgs::ErrorMessage) {}
fn provided_node_features(&self) -> NodeFeatures { NodeFeatures::empty() }
fn provided_init_features(&self, _their_node_id: &PublicKey) -> InitFeatures {
@ -1210,14 +1210,18 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
peer_lock.sync_status = InitSyncTracker::ChannelsSyncing(0);
}
if !msg.features.supports_static_remote_key() {
log_debug!(self.logger, "Peer {} does not support static remote key, disconnecting with no_connection_possible", log_pubkey!(their_node_id));
if let Err(()) = self.message_handler.route_handler.peer_connected(&their_node_id, &msg) {
log_debug!(self.logger, "Route Handler decided we couldn't communicate with peer {}", log_pubkey!(their_node_id));
return Err(PeerHandleError{ no_connection_possible: true }.into());
}
if let Err(()) = self.message_handler.chan_handler.peer_connected(&their_node_id, &msg) {
log_debug!(self.logger, "Channel Handler decided we couldn't communicate with peer {}", log_pubkey!(their_node_id));
return Err(PeerHandleError{ no_connection_possible: true }.into());
}
if let Err(()) = self.message_handler.onion_message_handler.peer_connected(&their_node_id, &msg) {
log_debug!(self.logger, "Onion Message Handler decided we couldn't communicate with peer {}", log_pubkey!(their_node_id));
return Err(PeerHandleError{ no_connection_possible: true }.into());
}
self.message_handler.route_handler.peer_connected(&their_node_id, &msg);
self.message_handler.chan_handler.peer_connected(&their_node_id, &msg);
self.message_handler.onion_message_handler.peer_connected(&their_node_id, &msg);
peer_lock.their_features = Some(msg.features);
return Ok(None);

3
lightning/src/onion_message/messenger.rs
View file

@ -335,11 +335,12 @@ impl<Signer: Sign, K: Deref, L: Deref> OnionMessageHandler for OnionMessenger<Si
};
}
fn peer_connected(&self, their_node_id: &PublicKey, init: &msgs::Init) {
fn peer_connected(&self, their_node_id: &PublicKey, init: &msgs::Init) -> Result<(), ()> {
if init.features.supports_onion_messages() {
let mut peers = self.pending_messages.lock().unwrap();
peers.insert(their_node_id.clone(), VecDeque::new());
}
Ok(())
}
fn peer_disconnected(&self, their_node_id: &PublicKey, _no_connection_possible: bool) {

7
lightning/src/routing/gossip.rs
View file

@ -368,10 +368,12 @@ where C::Target: chain::Access, L::Target: Logger
/// to request gossip messages for each channel. The sync is considered complete
/// when the final reply_scids_end message is received, though we are not
/// tracking this directly.
fn peer_connected(&self, their_node_id: &PublicKey, init_msg: &Init) {
fn peer_connected(&self, their_node_id: &PublicKey, init_msg: &Init) -> Result<(), ()> {
// We will only perform a sync with peers that support gossip_queries.
if !init_msg.features.supports_gossip_queries() {
return ();
// Don't disconnect peers for not supporting gossip queries. We may wish to have
// channels with peers even without being able to exchange gossip.
return Ok(());
}
// The lightning network's gossip sync system is completely broken in numerous ways.
@ -445,6 +447,7 @@ where C::Target: chain::Access, L::Target: Logger
timestamp_range: u32::max_value(),
},
});
Ok(())
}
fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: ReplyChannelRange) -> Result<(), LightningError> {

8
lightning/src/util/test_utils.rs
View file

@ -350,9 +350,10 @@ impl msgs::ChannelMessageHandler for TestChannelMessageHandler {
self.received_msg(wire::Message::ChannelReestablish(msg.clone()));
}
fn peer_disconnected(&self, _their_node_id: &PublicKey, _no_connection_possible: bool) {}
fn peer_connected(&self, _their_node_id: &PublicKey, _msg: &msgs::Init) {
fn peer_connected(&self, _their_node_id: &PublicKey, _msg: &msgs::Init) -> Result<(), ()> {
// Don't bother with `received_msg` for Init as its auto-generated and we don't want to
// bother re-generating the expected Init message in all tests.
Ok(())
}
fn handle_error(&self, _their_node_id: &PublicKey, msg: &msgs::ErrorMessage) {
self.received_msg(wire::Message::Error(msg.clone()));
@ -465,9 +466,9 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler {
None
}
fn peer_connected(&self, their_node_id: &PublicKey, init_msg: &msgs::Init) {
fn peer_connected(&self, their_node_id: &PublicKey, init_msg: &msgs::Init) -> Result<(), ()> {
if !init_msg.features.supports_gossip_queries() {
return ();
return Ok(());
}
#[allow(unused_mut, unused_assignments)]
@ -491,6 +492,7 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler {
timestamp_range: u32::max_value(),
},
});
Ok(())
}
fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyChannelRange) -> Result<(), msgs::LightningError> {