From 6b1f867eaaaf9b5fd4317aefa52ab183b7e5f980 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 12 Sep 2022 17:47:16 +0000 Subject: [PATCH] List supported/required feature bits explicitly in ChannelManager Historically, LDK has considered the "set of known/supported feature bits" to be an LDK-level thing. Increasingly this doesn't make sense - different message handlers may provide or require different feature sets. In a previous PR, we began the process of transitioning with feature bits sent to peers being sourced from the attached message handler. This commit makes further progress by moving the concept of which feature bits are supported by our ChannelManager into channelmanager.rs itself, via the new `provided_*_features` methods, rather than in features.rs via the `known_channel_features` and `known` methods. --- lightning/src/ln/channel.rs | 6 ++-- lightning/src/ln/channelmanager.rs | 51 ++++++++++++++++++++++++++++-- lightning/src/ln/features.rs | 30 ++---------------- lightning/src/ln/peer_handler.rs | 19 +++++++++-- lightning/src/util/test_utils.rs | 5 +-- 5 files changed, 72 insertions(+), 39 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 72f017fcf..ffc218e96 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -23,11 +23,11 @@ use bitcoin::secp256k1::{Secp256k1,ecdsa::Signature}; use bitcoin::secp256k1; use ln::{PaymentPreimage, PaymentHash}; -use ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures}; +use ln::features::{ChannelTypeFeatures, InitFeatures}; use ln::msgs; use ln::msgs::{DecodeError, OptionalField, DataLossProtect}; use ln::script::{self, ShutdownScript}; -use ln::channelmanager::{CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; +use ln::channelmanager::{self, CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction}; use ln::chan_utils; use chain::BestBlock; @@ -5254,7 +5254,7 @@ impl Channel { let were_node_one = node_id.serialize()[..] < self.counterparty_node_id.serialize()[..]; let msg = msgs::UnsignedChannelAnnouncement { - features: ChannelFeatures::known(), + features: channelmanager::provided_channel_features(), chain_hash, short_channel_id: self.get_short_channel_id().unwrap(), node_id_1: if were_node_one { node_id } else { self.get_counterparty_node_id() }, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d6a161026..74d8abaf6 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -43,7 +43,9 @@ use chain::transaction::{OutPoint, TransactionData}; // construct one themselves. use ln::{inbound_payment, PaymentHash, PaymentPreimage, PaymentSecret}; use ln::channel::{Channel, ChannelError, ChannelUpdateStatus, UpdateFulfillCommitFetch}; -use ln::features::{ChannelTypeFeatures, InitFeatures, NodeFeatures}; +use ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures}; +#[cfg(any(feature = "_test_utils", test))] +use ln::features::InvoiceFeatures; use routing::router::{PaymentParameters, Route, RouteHop, RoutePath, RouteParameters}; use ln::msgs; use ln::onion_utils; @@ -6127,14 +6129,57 @@ impl } fn provided_node_features(&self) -> NodeFeatures { - NodeFeatures::known_channel_features() + provided_node_features() } fn provided_init_features(&self, _their_init_features: &PublicKey) -> InitFeatures { - InitFeatures::known_channel_features() + provided_init_features() } } +/// Fetches the set of [`NodeFeatures`] flags which are provided by or required by +/// [`ChannelManager`]. +pub fn provided_node_features() -> NodeFeatures { + provided_init_features().to_context() +} + +/// Fetches the set of [`InvoiceFeatures`] flags which are provided by or required by +/// [`ChannelManager`]. +/// +/// Note that the invoice feature flags can vary depending on if the invoice is a "phantom invoice" +/// or not. Thus, this method is not public. +#[cfg(any(feature = "_test_utils", test))] +pub fn provided_invoice_features() -> InvoiceFeatures { + provided_init_features().to_context() +} + +/// Fetches the set of [`ChannelFeatures`] flags which are provided by or required by +/// [`ChannelManager`]. +pub fn provided_channel_features() -> ChannelFeatures { + provided_init_features().to_context() +} + +/// Fetches the set of [`InitFeatures`] flags which are provided by or required by +/// [`ChannelManager`]. +pub fn provided_init_features() -> InitFeatures { + // Note that if new features are added here which other peers may (eventually) require, we + // should also add the corresponding (optional) bit to the ChannelMessageHandler impl for + // ErroringMessageHandler. + let mut features = InitFeatures::empty(); + features.set_data_loss_protect_optional(); + features.set_upfront_shutdown_script_optional(); + features.set_variable_length_onion_required(); + features.set_static_remote_key_required(); + features.set_payment_secret_required(); + features.set_basic_mpp_optional(); + features.set_wumbo_optional(); + features.set_shutdown_any_segwit_optional(); + features.set_channel_type_optional(); + features.set_scid_privacy_optional(); + features.set_zero_conf_optional(); + features +} + const SERIALIZATION_VERSION: u8 = 1; const MIN_SERIALIZATION_VERSION: u8 = 1; diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 642298f56..9509f0f23 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -168,9 +168,6 @@ mod sealed { , ], optional_features: [ - // Note that if new "non-channel-related" flags are added here they should be - // explicitly cleared in InitFeatures::known_channel_features and - // NodeFeatures::known_channel_features. // Byte 0 DataLossProtect | InitialRoutingSync | UpfrontShutdownScript | GossipQueries, // Byte 1 @@ -552,24 +549,6 @@ impl InitFeatures { pub(crate) fn to_context(&self) -> Features { self.to_context_internal() } - - /// Returns the set of known init features that are related to channels. At least some of - /// these features are likely required for peers to talk to us. - pub fn known_channel_features() -> InitFeatures { - Self::known() - .clear_initial_routing_sync() - .clear_gossip_queries() - .clear_onion_messages() - } -} - -impl NodeFeatures { - /// Returns the set of known node features that are related to channels. - pub fn known_channel_features() -> NodeFeatures { - Self::known() - .clear_gossip_queries() - .clear_onion_messages() - } } impl InvoiceFeatures { @@ -788,6 +767,7 @@ impl Features { impl Features { + #[cfg(test)] pub(crate) fn clear_gossip_queries(mut self) -> Self { ::clear_bits(&mut self.flags); self @@ -796,19 +776,13 @@ impl Features { impl Features { // Note that initial_routing_sync is ignored if gossip_queries is set. + #[cfg(test)] pub(crate) fn clear_initial_routing_sync(mut self) -> Self { ::clear_bits(&mut self.flags); self } } -impl Features { - pub(crate) fn clear_onion_messages(mut self) -> Self { - ::clear_bits(&mut self.flags); - self - } -} - impl Features { #[cfg(test)] pub(crate) fn clear_shutdown_anysegwit(mut self) -> Self { diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 0cdea600b..d38afcbac 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -213,9 +213,22 @@ impl ChannelMessageHandler for ErroringMessageHandler { 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 { - // Use our known channel feature set as peers may otherwise not be willing to talk to us at - // all. - InitFeatures::known_channel_features() + // Set a number of features which various nodes may require to talk to us. It's totally + // reasonable to indicate we "support" all kinds of channel features...we just reject all + // channels. + let mut features = InitFeatures::empty(); + features.set_data_loss_protect_optional(); + features.set_upfront_shutdown_script_optional(); + features.set_variable_length_onion_optional(); + features.set_static_remote_key_optional(); + features.set_payment_secret_optional(); + features.set_basic_mpp_optional(); + features.set_wumbo_optional(); + features.set_shutdown_any_segwit_optional(); + features.set_channel_type_optional(); + features.set_scid_privacy_optional(); + features.set_zero_conf_optional(); + features } } impl Deref for ErroringMessageHandler { diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 2f772443d..ed15a2008 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -17,6 +17,7 @@ use chain::channelmonitor; use chain::channelmonitor::MonitorEvent; use chain::transaction::OutPoint; use chain::keysinterface; +use ln::channelmanager; use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures}; use ln::{msgs, wire}; use ln::script::ShutdownScript; @@ -359,10 +360,10 @@ impl msgs::ChannelMessageHandler for TestChannelMessageHandler { self.received_msg(wire::Message::Error(msg.clone())); } fn provided_node_features(&self) -> NodeFeatures { - NodeFeatures::known_channel_features() + channelmanager::provided_node_features() } fn provided_init_features(&self, _their_init_features: &PublicKey) -> InitFeatures { - InitFeatures::known_channel_features() + channelmanager::provided_init_features() } } -- 2.39.5