List supported/required feature bits explicitly in ChannelManager
authorMatt Corallo <git@bluematt.me>
Mon, 12 Sep 2022 17:47:16 +0000 (17:47 +0000)
committerMatt Corallo <git@bluematt.me>
Wed, 14 Sep 2022 20:08:54 +0000 (20:08 +0000)
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
lightning/src/ln/channelmanager.rs
lightning/src/ln/features.rs
lightning/src/ln/peer_handler.rs
lightning/src/util/test_utils.rs

index 72f017fcf81b061cd859219ea36397aa679c7013..ffc218e96210b7b8ff72b72f3b4d36f637023636 100644 (file)
@@ -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<Signer: Sign> Channel<Signer> {
                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() },
index d6a1610267d84c41b71368acfaa026a93d9aa960..74d8abaf64ddd020ef453033341a885fece8eb78 100644 (file)
@@ -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<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
        }
 
        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;
 
index 642298f56ed52265952eb2b4642163a06f5b0474..9509f0f23bf6e1c5e7a5d0c66da2a7e3c8bdc58b 100644 (file)
@@ -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<C: sealed::Context>(&self) -> Features<C> {
                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<T: sealed::UpfrontShutdownScript> Features<T> {
 
 
 impl<T: sealed::GossipQueries> Features<T> {
+       #[cfg(test)]
        pub(crate) fn clear_gossip_queries(mut self) -> Self {
                <T as sealed::GossipQueries>::clear_bits(&mut self.flags);
                self
@@ -796,19 +776,13 @@ impl<T: sealed::GossipQueries> Features<T> {
 
 impl<T: sealed::InitialRoutingSync> Features<T> {
        // Note that initial_routing_sync is ignored if gossip_queries is set.
+       #[cfg(test)]
        pub(crate) fn clear_initial_routing_sync(mut self) -> Self {
                <T as sealed::InitialRoutingSync>::clear_bits(&mut self.flags);
                self
        }
 }
 
-impl<T: sealed::OnionMessages> Features<T> {
-       pub(crate) fn clear_onion_messages(mut self) -> Self {
-               <T as sealed::OnionMessages>::clear_bits(&mut self.flags);
-               self
-       }
-}
-
 impl<T: sealed::ShutdownAnySegwit> Features<T> {
        #[cfg(test)]
        pub(crate) fn clear_shutdown_anysegwit(mut self) -> Self {
index 0cdea600b827d449923d746c306fae3582e277fb..d38afcbacb304620851d92de9eb54f4de813e5c0 100644 (file)
@@ -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 {
index 2f772443dd07872f34e56ff687c8d6a6e6fa6062..ed15a2008f7e5c31c77986ab2acc211a77fd51a5 100644 (file)
@@ -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()
        }
 }