From 6836fc4cd8b3665a065b27041c4e2823439d577f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 22 Feb 2025 22:15:24 +0000 Subject: [PATCH 1/7] Stop relying on builder pattern for clearing feature bits We have a handful of methods to clear features from `*Features` objects, but have ended up with two different API semantics for them. In the next commit we'll make them all consistent, opting against the builder pattern as it turns out all of these lines utilizing it are too long for rustfmt to be happy, so best to clean them up now so that rustfmt doesn't make a mockery of our code later. --- lightning-types/src/features.rs | 9 +++------ lightning/src/ln/channel.rs | 3 ++- lightning/src/ln/functional_tests.rs | 4 +++- lightning/src/ln/shutdown_tests.rs | 18 +++++++++++++----- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs index ea1b9b5b522..372ba1e6bc7 100644 --- a/lightning-types/src/features.rs +++ b/lightning-types/src/features.rs @@ -1040,25 +1040,22 @@ impl Features { impl Features { /// Unsets the `upfront_shutdown_script` feature - pub fn clear_upfront_shutdown_script(mut self) -> Self { + pub fn clear_upfront_shutdown_script(&mut self) { ::clear_bits(&mut self.flags); - self } } impl Features { /// Unsets the `shutdown_anysegwit` feature - pub fn clear_shutdown_anysegwit(mut self) -> Self { + pub fn clear_shutdown_anysegwit(&mut self) { ::clear_bits(&mut self.flags); - self } } impl Features { /// Unsets the `wumbo` feature - pub fn clear_wumbo(mut self) -> Self { + pub fn clear_wumbo(&mut self) { ::clear_bits(&mut self.flags); - self } } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a09b3bfa77a..cf8962db75d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -11241,7 +11241,8 @@ mod tests { #[test] fn upfront_shutdown_script_incompatibility() { - let features = channelmanager::provided_init_features(&UserConfig::default()).clear_shutdown_anysegwit(); + let mut features = channelmanager::provided_init_features(&UserConfig::default()); + features.clear_shutdown_anysegwit(); let non_v0_segwit_shutdown_script = ShutdownScript::new_witness_program( &WitnessProgram::new(WitnessVersion::V16, &[0, 40]).unwrap(), ).unwrap(); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index d46fc721c13..58174d40607 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -165,7 +165,9 @@ pub fn test_funding_exceeds_no_wumbo_limit() { use crate::ln::channel::MAX_FUNDING_SATOSHIS_NO_WUMBO; let chanmon_cfgs = create_chanmon_cfgs(2); let mut node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - *node_cfgs[1].override_init_features.borrow_mut() = Some(channelmanager::provided_init_features(&test_default_channel_config()).clear_wumbo()); + let mut features = channelmanager::provided_init_features(&test_default_channel_config()); + features.clear_wumbo(); + *node_cfgs[1].override_init_features.borrow_mut() = Some(features); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index f68a9843f58..e49bc4e83be 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -772,7 +772,9 @@ fn test_upfront_shutdown_script() { } // We test that if case of peer non-signaling we don't enforce committed script at channel opening - *nodes[0].override_init_features.borrow_mut() = Some(nodes[0].node.init_features().clear_upfront_shutdown_script()); + let mut features = nodes[0].node.init_features(); + features.clear_upfront_shutdown_script(); + *nodes[0].override_init_features.borrow_mut() = Some(features); let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000); nodes[0].node.close_channel(&chan.2, &nodes[1].node.get_our_node_id()).unwrap(); let node_1_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()); @@ -824,7 +826,9 @@ fn test_unsupported_anysegwit_upfront_shutdown_script() { let chanmon_cfgs = create_chanmon_cfgs(2); let mut node_cfgs = create_node_cfgs(2, &chanmon_cfgs); // Clear shutdown_anysegwit on initiator - *node_cfgs[0].override_init_features.borrow_mut() = Some(channelmanager::provided_init_features(&test_default_channel_config()).clear_shutdown_anysegwit()); + let mut features = channelmanager::provided_init_features(&test_default_channel_config()); + features.clear_shutdown_anysegwit(); + *node_cfgs[0].override_init_features.borrow_mut() = Some(features); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); @@ -853,7 +857,9 @@ fn test_unsupported_anysegwit_upfront_shutdown_script() { let chanmon_cfgs = create_chanmon_cfgs(2); let mut node_cfgs = create_node_cfgs(2, &chanmon_cfgs); // Clear shutdown_anysegwit on responder - *node_cfgs[1].override_init_features.borrow_mut() = Some(channelmanager::provided_init_features(&test_default_channel_config()).clear_shutdown_anysegwit()); + let mut features = channelmanager::provided_init_features(&test_default_channel_config()); + features.clear_shutdown_anysegwit(); + *node_cfgs[1].override_init_features.borrow_mut() = Some(features); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); @@ -984,8 +990,10 @@ fn test_unsupported_anysegwit_shutdown_script() { let user_cfgs = [None, Some(config), None]; let chanmon_cfgs = create_chanmon_cfgs(3); let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs); - *node_cfgs[0].override_init_features.borrow_mut() = Some(channelmanager::provided_init_features(&config).clear_shutdown_anysegwit()); - *node_cfgs[1].override_init_features.borrow_mut() = Some(channelmanager::provided_init_features(&config).clear_shutdown_anysegwit()); + let mut features = channelmanager::provided_init_features(&config); + features.clear_shutdown_anysegwit(); + *node_cfgs[0].override_init_features.borrow_mut() = Some(features.clone()); + *node_cfgs[1].override_init_features.borrow_mut() = Some(features); let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &user_cfgs); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); From f4c2a011f2f56513f6792161c58ebbdb37a98aec Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 22 Feb 2025 21:14:01 +0000 Subject: [PATCH 2/7] Expose a feature-clearing method for all feature flags We've ad-hoc exposed feature-clearing methods for various feature flags over the years, but there's not a lot of reason to not just do it for all the flags, so we go ahead and do that here. --- lightning-types/src/features.rs | 88 +++++++++++++++------------------ 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs index 372ba1e6bc7..e4f7ffd60b3 100644 --- a/lightning-types/src/features.rs +++ b/lightning-types/src/features.rs @@ -251,7 +251,7 @@ mod sealed { /// useful for manipulating feature flags. macro_rules! define_feature { ($odd_bit: expr, $feature: ident, [$($context: ty),+], $doc: expr, $optional_setter: ident, - $required_setter: ident, $supported_getter: ident) => { + $required_setter: ident, $clear: ident, $supported_getter: ident) => { #[doc = $doc] /// /// See [BOLT #9] for details. @@ -354,6 +354,11 @@ mod sealed { ::set_required_bit(&mut self.flags); } + /// Unsets this feature. + pub fn $clear(&mut self) { + ::clear_bits(&mut self.flags); + } + /// Checks if this feature is supported. pub fn $supported_getter(&self) -> bool { ::supports_feature(&self.flags) @@ -377,8 +382,8 @@ mod sealed { )* }; ($odd_bit: expr, $feature: ident, [$($context: ty),+], $doc: expr, $optional_setter: ident, - $required_setter: ident, $supported_getter: ident, $required_getter: ident) => { - define_feature!($odd_bit, $feature, [$($context),+], $doc, $optional_setter, $required_setter, $supported_getter); + $required_setter: ident, $clear: ident, $supported_getter: ident, $required_getter: ident) => { + define_feature!($odd_bit, $feature, [$($context),+], $doc, $optional_setter, $required_setter, $clear, $supported_getter); impl Features { /// Checks if this feature is required. pub fn $required_getter(&self) -> bool { @@ -395,6 +400,7 @@ mod sealed { "Feature flags for `option_data_loss_protect`.", set_data_loss_protect_optional, set_data_loss_protect_required, + clear_data_loss_protect, supports_data_loss_protect, requires_data_loss_protect ); @@ -406,6 +412,7 @@ mod sealed { "Feature flags for `initial_routing_sync`.", set_initial_routing_sync_optional, set_initial_routing_sync_required, + clear_initial_routing_sync, initial_routing_sync ); define_feature!( @@ -415,6 +422,7 @@ mod sealed { "Feature flags for `option_upfront_shutdown_script`.", set_upfront_shutdown_script_optional, set_upfront_shutdown_script_required, + clear_upfront_shutdown_script, supports_upfront_shutdown_script, requires_upfront_shutdown_script ); @@ -425,6 +433,7 @@ mod sealed { "Feature flags for `gossip_queries`.", set_gossip_queries_optional, set_gossip_queries_required, + clear_gossip_queries, supports_gossip_queries, requires_gossip_queries ); @@ -435,6 +444,7 @@ mod sealed { "Feature flags for `var_onion_optin`.", set_variable_length_onion_optional, set_variable_length_onion_required, + clear_variable_length_onion, supports_variable_length_onion, requires_variable_length_onion ); @@ -445,6 +455,7 @@ mod sealed { "Feature flags for `option_static_remotekey`.", set_static_remote_key_optional, set_static_remote_key_required, + clear_static_remote_key, supports_static_remote_key, requires_static_remote_key ); @@ -455,6 +466,7 @@ mod sealed { "Feature flags for `payment_secret`.", set_payment_secret_optional, set_payment_secret_required, + clear_payment_secret, supports_payment_secret, requires_payment_secret ); @@ -465,6 +477,7 @@ mod sealed { "Feature flags for `basic_mpp`.", set_basic_mpp_optional, set_basic_mpp_required, + clear_basic_mpp, supports_basic_mpp, requires_basic_mpp ); @@ -475,6 +488,7 @@ mod sealed { "Feature flags for `option_support_large_channel` (aka wumbo channels).", set_wumbo_optional, set_wumbo_required, + clear_wumbo, supports_wumbo, requires_wumbo ); @@ -485,6 +499,7 @@ mod sealed { "Feature flags for `option_anchors_nonzero_fee_htlc_tx`.", set_anchors_nonzero_fee_htlc_tx_optional, set_anchors_nonzero_fee_htlc_tx_required, + clear_anchors_nonzero_fee_htlc_tx, supports_anchors_nonzero_fee_htlc_tx, requires_anchors_nonzero_fee_htlc_tx ); @@ -495,6 +510,7 @@ mod sealed { "Feature flags for `option_anchors_zero_fee_htlc_tx`.", set_anchors_zero_fee_htlc_tx_optional, set_anchors_zero_fee_htlc_tx_required, + clear_anchors_zero_fee_htlc_tx, supports_anchors_zero_fee_htlc_tx, requires_anchors_zero_fee_htlc_tx ); @@ -505,6 +521,7 @@ mod sealed { "Feature flags for `option_route_blinding`.", set_route_blinding_optional, set_route_blinding_required, + clear_route_blinding, supports_route_blinding, requires_route_blinding ); @@ -515,6 +532,7 @@ mod sealed { "Feature flags for `opt_shutdown_anysegwit`.", set_shutdown_any_segwit_optional, set_shutdown_any_segwit_required, + clear_shutdown_anysegwit, supports_shutdown_anysegwit, requires_shutdown_anysegwit ); @@ -525,6 +543,7 @@ mod sealed { "Feature flags for `option_dual_fund`.", set_dual_fund_optional, set_dual_fund_required, + clear_dual_fund, supports_dual_fund, requires_dual_fund ); @@ -535,6 +554,7 @@ mod sealed { "Feature flags for `option_taproot`.", set_taproot_optional, set_taproot_required, + clear_taproot, supports_taproot, requires_taproot ); @@ -545,6 +565,7 @@ mod sealed { "Feature flags for `option_quiesce`.", set_quiescence_optional, set_quiescence_required, + clear_quiescence, supports_quiescence, requires_quiescence ); @@ -555,6 +576,7 @@ mod sealed { "Feature flags for `option_onion_messages`.", set_onion_messages_optional, set_onion_messages_required, + clear_onion_messages, supports_onion_messages, requires_onion_messages ); @@ -565,6 +587,7 @@ mod sealed { "Feature flags for `option_provide_storage`.", set_provide_storage_optional, set_provide_storage_required, + clear_provide_storage, supports_provide_storage, requires_provide_storage ); @@ -575,12 +598,20 @@ mod sealed { "Feature flags for `option_channel_type`.", set_channel_type_optional, set_channel_type_required, + clear_channel_type, supports_channel_type, requires_channel_type ); - define_feature!(47, SCIDPrivacy, [InitContext, NodeContext, ChannelTypeContext], + define_feature!(47, + SCIDPrivacy, + [InitContext, NodeContext, ChannelTypeContext], "Feature flags for only forwarding with SCID aliasing. Called `option_scid_alias` in the BOLTs", - set_scid_privacy_optional, set_scid_privacy_required, supports_scid_privacy, requires_scid_privacy); + set_scid_privacy_optional, + set_scid_privacy_required, + clear_scid_privacy, + supports_scid_privacy, + requires_scid_privacy + ); define_feature!( 49, PaymentMetadata, @@ -588,6 +619,7 @@ mod sealed { "Feature flags for payment metadata in invoices.", set_payment_metadata_optional, set_payment_metadata_required, + clear_payment_metadata, supports_payment_metadata, requires_payment_metadata ); @@ -601,6 +633,7 @@ mod sealed { "Feature flags for keysend payments.", set_keysend_optional, set_keysend_required, + clear_keysend, supports_keysend, requires_keysend ); @@ -611,6 +644,7 @@ mod sealed { "Feature flags for Trampoline routing.", set_trampoline_routing_optional, set_trampoline_routing_required, + clear_trampoline_routing, supports_trampoline_routing, requires_trampoline_routing ); @@ -621,6 +655,7 @@ mod sealed { "Feature flags for DNS resolving.", set_dns_resolution_optional, set_dns_resolution_required, + clear_dns_resolution, supports_dns_resolution, requires_dns_resolution ); @@ -643,6 +678,7 @@ mod sealed { "Feature flags for an unknown feature used in testing.", set_unknown_feature_optional, set_unknown_feature_required, + clear_unknown_feature, supports_unknown_test_feature, requires_unknown_test_feature ); @@ -1038,48 +1074,6 @@ impl Features { } } -impl Features { - /// Unsets the `upfront_shutdown_script` feature - pub fn clear_upfront_shutdown_script(&mut self) { - ::clear_bits(&mut self.flags); - } -} - -impl Features { - /// Unsets the `shutdown_anysegwit` feature - pub fn clear_shutdown_anysegwit(&mut self) { - ::clear_bits(&mut self.flags); - } -} - -impl Features { - /// Unsets the `wumbo` feature - pub fn clear_wumbo(&mut self) { - ::clear_bits(&mut self.flags); - } -} - -impl Features { - /// Unsets the `scid_privacy` feature - pub fn clear_scid_privacy(&mut self) { - ::clear_bits(&mut self.flags); - } -} - -impl Features { - /// Unsets the `anchors_zero_fee_htlc_tx` feature - pub fn clear_anchors_zero_fee_htlc_tx(&mut self) { - ::clear_bits(&mut self.flags); - } -} - -impl Features { - /// Unsets the `route_blinding` feature - pub fn clear_route_blinding(&mut self) { - ::clear_bits(&mut self.flags); - } -} - #[cfg(any(test, feature = "_test_utils"))] impl Features { /// Sets an unknown feature for testing From 49dbeea662829ec24968a4ed8807247d67be938e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 22 Feb 2025 19:16:03 +0000 Subject: [PATCH 3/7] Drop useless doc reference to pre-zero-HTLC-fee anchor channels These aren't really used anywhere and were only live very briefly, so there's not really any point in informing our users that we don't support them. If anything, it will lead to confusion as the zero-HTLC-fee channel type is generally referred to simply as "anchor channels". --- lightning/src/util/config.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index ef35df1a0b1..17eca47be72 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -176,16 +176,11 @@ pub struct ChannelHandshakeConfig { /// counterparties that do not support the `anchors_zero_fee_htlc_tx` option; we will simply /// fall back to a `static_remote_key` channel. /// - /// LDK will not support the legacy `option_anchors` commitment version due to a discovered - /// vulnerability after its deployment. For more context, see the [`SIGHASH_SINGLE + update_fee - /// Considered Harmful`] mailing list post. - /// /// Default value: `false` (This value is likely to change to `true` in the future.) /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel /// [`DecodeError::InvalidValue`]: crate::ln::msgs::DecodeError::InvalidValue - /// [`SIGHASH_SINGLE + update_fee Considered Harmful`]: https://lists.linuxfoundation.org/pipermail/lightning-dev/2020-September/002796.html pub negotiate_anchors_zero_fee_htlc_tx: bool, /// The maximum number of HTLCs in-flight from our counterparty towards us at the same time. From 3e7b861b77c6865b99b964d34c12c5859cc86c52 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 11 Mar 2025 19:19:05 +0000 Subject: [PATCH 4/7] Add feature flags for `option_zero_fee_commitments` --- fuzz/src/full_stack.rs | 4 ++-- lightning-types/src/features.rs | 19 ++++++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 6594f472516..a2f4ecac227 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1141,7 +1141,7 @@ mod tests { ext_from_hex("030020", &mut test); // init message (type 16) with static_remotekey required, no channel_type/anchors/taproot, and other bits optional and mac ext_from_hex( - "0010 00021aaa 0008aaa20aaa2a0a9aaa 03000000000000000000000000000000", + "0010 00021aaa 0008aaa208aa2a0a9aaa 03000000000000000000000000000000", &mut test, ); @@ -1245,7 +1245,7 @@ mod tests { ext_from_hex("030120", &mut test); // init message (type 16) with static_remotekey required, no channel_type/anchors/taproot, and other bits optional and mac ext_from_hex( - "0010 00021aaa 0008aaa20aaa2a0a9aaa 01000000000000000000000000000000", + "0010 00021aaa 0008aaa208aa2a0a9aaa 01000000000000000000000000000000", &mut test, ); diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs index e4f7ffd60b3..79f12b645b5 100644 --- a/lightning-types/src/features.rs +++ b/lightning-types/src/features.rs @@ -76,6 +76,8 @@ //! (see [BOLT PR #1110](https://github.com/lightning/bolts/pull/1110) for more info). //! - `Quiescence` - protocol to quiesce a channel by indicating that "SomeThing Fundamental is Underway" //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#channel-quiescence) for more information). +//! - `ZeroFeeCommitments` - A channel type which always uses zero transaction fee on commitment transactions. +//! (see [BOLT PR #1228](https://github.com/lightning/bolts/pull/1228) for more info). //! //! LDK knows about the following features, but does not support them: //! - `AnchorsNonzeroFeeHtlcTx` - the initial version of anchor outputs, which was later found to be @@ -156,7 +158,7 @@ mod sealed { // Byte 4 Quiescence | OnionMessages, // Byte 5 - ProvideStorage | ChannelType | SCIDPrivacy, + ProvideStorage | ChannelType | SCIDPrivacy | AnchorZeroFeeCommitments, // Byte 6 ZeroConf, // Byte 7 @@ -177,7 +179,7 @@ mod sealed { // Byte 4 Quiescence | OnionMessages, // Byte 5 - ProvideStorage | ChannelType | SCIDPrivacy, + ProvideStorage | ChannelType | SCIDPrivacy | AnchorZeroFeeCommitments, // Byte 6 ZeroConf | Keysend, // Byte 7 @@ -242,7 +244,7 @@ mod sealed { // Byte 4 , // Byte 5 - SCIDPrivacy, + SCIDPrivacy | AnchorZeroFeeCommitments, // Byte 6 ZeroConf, ]); @@ -580,6 +582,17 @@ mod sealed { supports_onion_messages, requires_onion_messages ); + define_feature!( + 41, + AnchorZeroFeeCommitments, + [InitContext, NodeContext, ChannelTypeContext], + "Feature flags for `option_zero_fee_commitments`.", + set_anchor_zero_fee_commitments_optional, + set_anchor_zero_fee_commitments_required, + clear_anchor_zero_fee_commitments, + supports_anchor_zero_fee_commitments, + requires_anchor_zero_fee_commitments + ); define_feature!( 43, ProvideStorage, From 0faf17bbc69adbf02b49819072058da50dcdc345 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 23 Feb 2025 16:33:57 +0000 Subject: [PATCH 5/7] Drop `ChannelTransactionParameters::supports_anchors` ...as it is ambiguous now that we have multiple types of anchors. Its clearer to check the specific features at the callsites now. --- lightning/src/ln/chan_utils.rs | 5 ----- lightning/src/sign/mod.rs | 17 ++++++++++------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 1b3cb017501..abe211049b4 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -934,11 +934,6 @@ impl ChannelTransactionParameters { self.counterparty_parameters.is_some() && self.funding_outpoint.is_some() } - /// Whether the channel supports zero-fee HTLC transaction anchors. - pub(crate) fn supports_anchors(&self) -> bool { - self.channel_type_features.supports_anchors_zero_fee_htlc_tx() - } - /// Convert the holder/counterparty parameters to broadcaster/countersignatory-organized parameters, /// given that the holder is the broadcaster. /// diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 0539a696852..1f31ad1d24d 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -164,7 +164,7 @@ impl StaticPaymentOutputDescriptor { /// originated from an anchor outputs channel, as they take the form of a P2WSH script. pub fn witness_script(&self) -> Option { self.channel_transaction_parameters.as_ref().and_then(|channel_params| { - if channel_params.supports_anchors() { + if channel_params.channel_type_features.supports_anchors_zero_fee_htlc_tx() { let payment_point = channel_params.holder_pubkeys.payment_point; Some(chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_point)) } else { @@ -177,7 +177,7 @@ impl StaticPaymentOutputDescriptor { /// Note: If you have the grind_signatures feature enabled, this will be at least 1 byte /// shorter. pub fn max_witness_length(&self) -> u64 { - if self.channel_transaction_parameters.as_ref().map_or(false, |p| p.supports_anchors()) { + if self.needs_csv_1_for_spend() { let witness_script_weight = 1 /* pubkey push */ + 33 /* pubkey */ + 1 /* OP_CHECKSIGVERIFY */ + 1 /* OP_1 */ + 1 /* OP_CHECKSEQUENCEVERIFY */; 1 /* num witness items */ + 1 /* sig push */ + 73 /* sig including sighash flag */ + @@ -186,6 +186,13 @@ impl StaticPaymentOutputDescriptor { P2WPKH_WITNESS_WEIGHT } } + + /// Returns true if spending this output requires a transaction with a CheckSequenceVerify + /// value of at least 1. + pub fn needs_csv_1_for_spend(&self) -> bool { + let chan_params = self.channel_transaction_parameters.as_ref(); + chan_params.map_or(false, |p| p.channel_type_features.supports_anchors_zero_fee_htlc_tx()) + } } impl_writeable_tlv_based!(StaticPaymentOutputDescriptor, { (0, outpoint, required), @@ -440,11 +447,7 @@ impl SpendableOutputDescriptor { if !output_set.insert(descriptor.outpoint) { return Err(()); } - let sequence = if descriptor - .channel_transaction_parameters - .as_ref() - .map_or(false, |p| p.supports_anchors()) - { + let sequence = if descriptor.needs_csv_1_for_spend() { Sequence::from_consensus(1) } else { Sequence::ZERO From 85272583808cc9cda1c1acd59f472ee566b71c2c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 23 Feb 2025 23:06:48 +0000 Subject: [PATCH 6/7] Rename various anchor methods which are now ambiguous As we move towards zero-fee commitment transactions, we need to differentiate between the now-two-types of "anchor channels". We do so here by renaming a number of methods which refer to anchors as "keyed anchors" as the zero-fee commitment transaction anchors do not have a public key associated with them. We also drop `TaprootChannelSigner::sign_holder_anchor_input` as we are unlikely to support keyed anchors for taproot channels. --- lightning/src/chain/channelmonitor.rs | 2 +- lightning/src/chain/onchaintx.rs | 2 +- lightning/src/events/bump_transaction.rs | 48 +++++++++++++++-------- lightning/src/ln/chan_utils.rs | 42 ++++++++++++-------- lightning/src/sign/ecdsa.rs | 4 +- lightning/src/sign/mod.rs | 30 +++++--------- lightning/src/sign/taproot.rs | 6 --- lightning/src/util/dyn_signer.rs | 8 +--- lightning/src/util/test_channel_signer.rs | 20 ++-------- 9 files changed, 75 insertions(+), 87 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 2410ff05c12..3935d9f4266 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -5099,7 +5099,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP { let payment_point = onchain_tx_handler.channel_transaction_parameters.holder_pubkeys.payment_point; counterparty_payment_script = - chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_point).to_p2wsh(); + chan_utils::get_to_countersigner_keyed_anchor_redeemscript(&payment_point).to_p2wsh(); } let channel_id = channel_id.unwrap_or(ChannelId::v1_from_funding_outpoint(outpoint)); diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index e0c2bd1b306..35147ca51d6 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -666,7 +666,7 @@ impl OnchainTxHandler { // We'll locate an anchor output we can spend within the commitment transaction. let funding_pubkey = &self.channel_transaction_parameters.holder_pubkeys.funding_pubkey; - match chan_utils::get_anchor_output(&tx.0, funding_pubkey) { + match chan_utils::get_keyed_anchor_output(&tx.0, funding_pubkey) { // An anchor output was found, so we should yield a funding event externally. Some((idx, _)) => { // TODO: Use a lower confirmation target when both our and the diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index ab0854bfd66..a9550a67236 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -22,7 +22,7 @@ use crate::ln::types::ChannelId; use crate::ln::chan_utils; use crate::ln::chan_utils::{ ANCHOR_INPUT_WITNESS_WEIGHT, HTLC_SUCCESS_INPUT_ANCHOR_WITNESS_WEIGHT, - HTLC_TIMEOUT_INPUT_ANCHOR_WITNESS_WEIGHT, HTLCOutputInCommitment + HTLC_TIMEOUT_INPUT_ANCHOR_WITNESS_WEIGHT, HTLCOutputInCommitment, shared_anchor_script_pubkey, }; use crate::prelude::*; use crate::sign::{ @@ -62,8 +62,17 @@ impl AnchorDescriptor { /// Returns the UTXO to be spent by the anchor input, which can be obtained via /// [`Self::unsigned_tx_input`]. pub fn previous_utxo(&self) -> TxOut { + let tx_params = &self.channel_derivation_parameters.transaction_parameters; + let script_pubkey = + if tx_params.channel_type_features.supports_anchors_zero_fee_htlc_tx() { + let channel_params = tx_params.as_holder_broadcastable(); + chan_utils::get_keyed_anchor_redeemscript(&channel_params.broadcaster_pubkeys().funding_pubkey) + } else { + assert!(tx_params.channel_type_features.supports_anchor_zero_fee_commitments()); + shared_anchor_script_pubkey() + }; TxOut { - script_pubkey: self.witness_script().to_p2wsh(), + script_pubkey, value: Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI), } } @@ -79,17 +88,17 @@ impl AnchorDescriptor { } } - /// Returns the witness script of the anchor output in the commitment transaction. - pub fn witness_script(&self) -> ScriptBuf { - let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); - chan_utils::get_anchor_redeemscript(&channel_params.broadcaster_pubkeys().funding_pubkey) - } - /// Returns the fully signed witness required to spend the anchor output in the commitment /// transaction. pub fn tx_input_witness(&self, signature: &Signature) -> Witness { - let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); - chan_utils::build_anchor_input_witness(&channel_params.broadcaster_pubkeys().funding_pubkey, signature) + let tx_params = &self.channel_derivation_parameters.transaction_parameters; + if tx_params.channel_type_features.supports_anchors_zero_fee_htlc_tx() { + let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); + chan_utils::build_keyed_anchor_input_witness(&channel_params.broadcaster_pubkeys().funding_pubkey, signature) + } else { + debug_assert!(tx_params.channel_type_features.supports_anchor_zero_fee_commitments()); + Witness::from_slice(&[&[]]) + } } } @@ -111,9 +120,9 @@ pub enum BumpTransactionEvent { /// The consumer should be able to sign for any of the additional inputs included within the /// child anchor transaction. To sign its anchor input, an [`EcdsaChannelSigner`] should be /// re-derived through [`SignerProvider::derive_channel_signer`]. The anchor input signature - /// can be computed with [`EcdsaChannelSigner::sign_holder_anchor_input`], which can then be - /// provided to [`build_anchor_input_witness`] along with the `funding_pubkey` to obtain the - /// full witness required to spend. + /// can be computed with [`EcdsaChannelSigner::sign_holder_keyed_anchor_input`], which can then + /// be provided to [`build_keyed_anchor_input_witness`] along with the `funding_pubkey` to + /// obtain the full witness required to spend. /// /// It is possible to receive more than one instance of this event if a valid child anchor /// transaction is never broadcast or is but not with a sufficient fee to be mined. Care should @@ -133,8 +142,8 @@ pub enum BumpTransactionEvent { /// be not urgent. /// /// [`EcdsaChannelSigner`]: crate::sign::ecdsa::EcdsaChannelSigner - /// [`EcdsaChannelSigner::sign_holder_anchor_input`]: crate::sign::ecdsa::EcdsaChannelSigner::sign_holder_anchor_input - /// [`build_anchor_input_witness`]: crate::ln::chan_utils::build_anchor_input_witness + /// [`EcdsaChannelSigner::sign_holder_keyed_anchor_input`]: crate::sign::ecdsa::EcdsaChannelSigner::sign_holder_keyed_anchor_input + /// [`build_keyed_anchor_input_witness`]: crate::ln::chan_utils::build_keyed_anchor_input_witness ChannelClose { /// The `channel_id` of the channel which has been closed. channel_id: ChannelId, @@ -678,7 +687,7 @@ where let signer = self.signer_provider.derive_channel_signer(anchor_descriptor.channel_derivation_parameters.keys_id); let channel_parameters = &anchor_descriptor.channel_derivation_parameters.transaction_parameters; - let anchor_sig = signer.sign_holder_anchor_input(channel_parameters, &anchor_tx, 0, &self.secp)?; + let anchor_sig = signer.sign_holder_keyed_anchor_input(channel_parameters, &anchor_tx, 0, &self.secp)?; anchor_tx.input[0].witness = anchor_descriptor.tx_input_witness(&anchor_sig); #[cfg(debug_assertions)] { @@ -850,6 +859,7 @@ mod tests { use crate::util::ser::Readable; use crate::util::test_utils::{TestBroadcaster, TestLogger}; use crate::sign::KeysManager; + use crate::types::features::ChannelTypeFeatures; use bitcoin::hashes::Hash; use bitcoin::hex::FromHex; @@ -934,6 +944,10 @@ mod tests { let logger = TestLogger::new(); let handler = BumpTransactionEventHandler::new(&broadcaster, &source, &signer, &logger); + let mut transaction_parameters = ChannelTransactionParameters::test_dummy(42_000_000); + transaction_parameters.channel_type_features = + ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + handler.handle_event(&BumpTransactionEvent::ChannelClose { channel_id: ChannelId([42; 32]), counterparty_node_id: PublicKey::from_slice(&[2; 33]).unwrap(), @@ -945,7 +959,7 @@ mod tests { channel_derivation_parameters: ChannelDerivationParameters { value_satoshis: 42_000_000, keys_id: [42; 32], - transaction_parameters: ChannelTransactionParameters::test_dummy(42_000_000), + transaction_parameters, }, outpoint: OutPoint { txid: Txid::from_byte_array([42; 32]), vout: 0 }, }, diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index abe211049b4..6f192bb50ca 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -574,7 +574,7 @@ pub fn get_revokeable_redeemscript(revocation_key: &RevocationKey, contest_delay /// the channel type. pub fn get_counterparty_payment_script(channel_type_features: &ChannelTypeFeatures, payment_key: &PublicKey) -> ScriptBuf { if channel_type_features.supports_anchors_zero_fee_htlc_tx() { - get_to_countersignatory_with_anchors_redeemscript(payment_key).to_p2wsh() + get_to_countersigner_keyed_anchor_redeemscript(payment_key).to_p2wsh() } else { ScriptBuf::new_p2wpkh(&WPubkeyHash::hash(&payment_key.serialize())) } @@ -838,7 +838,7 @@ pub(crate) fn legacy_deserialization_prevention_marker_for_channel_type_features /// Gets the witnessScript for the to_remote output when anchors are enabled. #[inline] -pub fn get_to_countersignatory_with_anchors_redeemscript(payment_point: &PublicKey) -> ScriptBuf { +pub fn get_to_countersigner_keyed_anchor_redeemscript(payment_point: &PublicKey) -> ScriptBuf { Builder::new() .push_slice(payment_point.serialize()) .push_opcode(opcodes::all::OP_CHECKSIGVERIFY) @@ -847,14 +847,20 @@ pub fn get_to_countersignatory_with_anchors_redeemscript(payment_point: &PublicK .into_script() } -/// Gets the witnessScript for an anchor output from the funding public key. +/// Gets the script_pubkey for a shared anchor +pub fn shared_anchor_script_pubkey() -> ScriptBuf { + Builder::new().push_int(1).push_slice(&[0x4e, 0x73]).into_script() +} + +/// Gets the witnessScript for a keyed anchor (non-zero-fee-commitments) output from the funding +/// public key. +/// /// The witness in the spending input must be: /// /// After 16 blocks of confirmation, an alternative satisfying witness could be: /// <> /// (empty vector required to satisfy compliance with MINIMALIF-standard rule) -#[inline] -pub fn get_anchor_redeemscript(funding_pubkey: &PublicKey) -> ScriptBuf { +pub fn get_keyed_anchor_redeemscript(funding_pubkey: &PublicKey) -> ScriptBuf { Builder::new().push_slice(funding_pubkey.serialize()) .push_opcode(opcodes::all::OP_CHECKSIG) .push_opcode(opcodes::all::OP_IFDUP) @@ -865,17 +871,19 @@ pub fn get_anchor_redeemscript(funding_pubkey: &PublicKey) -> ScriptBuf { .into_script() } -/// Locates the output with an anchor script paying to `funding_pubkey` within `commitment_tx`. -pub(crate) fn get_anchor_output<'a>(commitment_tx: &'a Transaction, funding_pubkey: &PublicKey) -> Option<(u32, &'a TxOut)> { - let anchor_script = get_anchor_redeemscript(funding_pubkey).to_p2wsh(); +/// Locates the output with a keyed anchor (non-zero-fee-commitments) script paying to +/// `funding_pubkey` within `commitment_tx`. +pub(crate) fn get_keyed_anchor_output<'a>(commitment_tx: &'a Transaction, funding_pubkey: &PublicKey) -> Option<(u32, &'a TxOut)> { + let anchor_script = get_keyed_anchor_redeemscript(funding_pubkey).to_p2wsh(); commitment_tx.output.iter().enumerate() .find(|(_, txout)| txout.script_pubkey == anchor_script) .map(|(idx, txout)| (idx as u32, txout)) } -/// Returns the witness required to satisfy and spend an anchor input. -pub fn build_anchor_input_witness(funding_key: &PublicKey, funding_sig: &Signature) -> Witness { - let anchor_redeem_script = get_anchor_redeemscript(funding_key); +/// Returns the witness required to satisfy and spend a keyed anchor (non-zero-fee-commitments) +/// input. +pub fn build_keyed_anchor_input_witness(funding_key: &PublicKey, funding_sig: &Signature) -> Witness { + let anchor_redeem_script = get_keyed_anchor_redeemscript(funding_key); let mut ret = Witness::new(); ret.push_ecdsa_signature(&BitcoinSignature::sighash_all(*funding_sig)); ret.push(anchor_redeem_script.as_bytes()); @@ -1117,7 +1125,7 @@ impl<'a> DirectedChannelTransactionParameters<'a> { self.inner.funding_outpoint.unwrap().into_bitcoin_outpoint() } - /// Whether to use anchors for this channel + /// The type of channel these parameters are for pub fn channel_type_features(&self) -> &'a ChannelTypeFeatures { &self.inner.channel_type_features } @@ -1574,7 +1582,7 @@ impl CommitmentTransaction { if to_countersignatory_value_sat > Amount::ZERO { let script = if channel_parameters.channel_type_features().supports_anchors_zero_fee_htlc_tx() { - get_to_countersignatory_with_anchors_redeemscript(&countersignatory_pubkeys.payment_point).to_p2wsh() + get_to_countersigner_keyed_anchor_redeemscript(&countersignatory_pubkeys.payment_point).to_p2wsh() } else { ScriptBuf::new_p2wpkh(&Hash160::hash(&countersignatory_pubkeys.payment_point.serialize()).into()) }; @@ -1604,7 +1612,7 @@ impl CommitmentTransaction { if channel_parameters.channel_type_features().supports_anchors_zero_fee_htlc_tx() { if to_broadcaster_value_sat > Amount::ZERO || !htlcs_with_aux.is_empty() { - let anchor_script = get_anchor_redeemscript(broadcaster_funding_key); + let anchor_script = get_keyed_anchor_redeemscript(broadcaster_funding_key); txouts.push(( TxOut { script_pubkey: anchor_script.to_p2wsh(), @@ -1615,7 +1623,7 @@ impl CommitmentTransaction { } if to_countersignatory_value_sat > Amount::ZERO || !htlcs_with_aux.is_empty() { - let anchor_script = get_anchor_redeemscript(countersignatory_funding_key); + let anchor_script = get_keyed_anchor_redeemscript(countersignatory_funding_key); txouts.push(( TxOut { script_pubkey: anchor_script.to_p2wsh(), @@ -1953,7 +1961,7 @@ pub fn get_commitment_transaction_number_obscure_factor( mod tests { use super::{CounterpartyCommitmentSecrets, ChannelPublicKeys}; use crate::chain; - use crate::ln::chan_utils::{get_htlc_redeemscript, get_to_countersignatory_with_anchors_redeemscript, CommitmentTransaction, TxCreationKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, HTLCOutputInCommitment}; + use crate::ln::chan_utils::{get_htlc_redeemscript, get_to_countersigner_keyed_anchor_redeemscript, CommitmentTransaction, TxCreationKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, HTLCOutputInCommitment}; use bitcoin::secp256k1::{PublicKey, SecretKey, Secp256k1}; use crate::util::test_utils; use crate::sign::{ChannelSigner, SignerProvider}; @@ -2043,7 +2051,7 @@ mod tests { builder.channel_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); let tx = builder.build(1000, 2000); assert_eq!(tx.built.transaction.output.len(), 4); - assert_eq!(tx.built.transaction.output[3].script_pubkey, get_to_countersignatory_with_anchors_redeemscript(&builder.counterparty_pubkeys.payment_point).to_p2wsh()); + assert_eq!(tx.built.transaction.output[3].script_pubkey, get_to_countersigner_keyed_anchor_redeemscript(&builder.counterparty_pubkeys.payment_point).to_p2wsh()); // Generate broadcaster output and anchor let tx = builder.build(3000, 0); diff --git a/lightning/src/sign/ecdsa.rs b/lightning/src/sign/ecdsa.rs index 3a0eba23cec..f9c330bbc4c 100644 --- a/lightning/src/sign/ecdsa.rs +++ b/lightning/src/sign/ecdsa.rs @@ -212,7 +212,7 @@ pub trait EcdsaChannelSigner: ChannelSigner { &self, channel_parameters: &ChannelTransactionParameters, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1, ) -> Result; - /// Computes the signature for a commitment transaction's anchor output used as an + /// Computes the signature for a commitment transaction's keyed anchor output used as an /// input within `anchor_tx`, which spends the commitment transaction, at index `input`. /// /// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid @@ -222,7 +222,7 @@ pub trait EcdsaChannelSigner: ChannelSigner { /// /// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked /// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked - fn sign_holder_anchor_input( + fn sign_holder_keyed_anchor_input( &self, channel_parameters: &ChannelTransactionParameters, anchor_tx: &Transaction, input: usize, secp_ctx: &Secp256k1, ) -> Result; diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 1f31ad1d24d..8b40ba73601 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -166,7 +166,7 @@ impl StaticPaymentOutputDescriptor { self.channel_transaction_parameters.as_ref().and_then(|channel_params| { if channel_params.channel_type_features.supports_anchors_zero_fee_htlc_tx() { let payment_point = channel_params.holder_pubkeys.payment_point; - Some(chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_point)) + Some(chan_utils::get_to_countersigner_keyed_anchor_redeemscript(&payment_point)) } else { None } @@ -1178,7 +1178,7 @@ impl InMemorySigner { .unwrap_or(false); let witness_script = if supports_anchors_zero_fee_htlc_tx { - chan_utils::get_to_countersignatory_with_anchors_redeemscript(&remotepubkey.inner) + chan_utils::get_to_countersigner_keyed_anchor_redeemscript(&remotepubkey.inner) } else { ScriptBuf::new_p2pkh(&remotepubkey.pubkey_hash()) }; @@ -1640,23 +1640,19 @@ impl EcdsaChannelSigner for InMemorySigner { )) } - fn sign_holder_anchor_input( - &self, channel_parameters: &ChannelTransactionParameters, anchor_tx: &Transaction, - input: usize, secp_ctx: &Secp256k1, + fn sign_holder_keyed_anchor_input( + &self, chan_params: &ChannelTransactionParameters, anchor_tx: &Transaction, input: usize, + secp_ctx: &Secp256k1, ) -> Result { - assert!(channel_parameters.is_populated(), "Channel parameters must be fully populated"); + assert!(chan_params.is_populated(), "Channel parameters must be fully populated"); let witness_script = - chan_utils::get_anchor_redeemscript(&channel_parameters.holder_pubkeys.funding_pubkey); + chan_utils::get_keyed_anchor_redeemscript(&chan_params.holder_pubkeys.funding_pubkey); + let amt = Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI); let sighash = sighash::SighashCache::new(&*anchor_tx) - .p2wsh_signature_hash( - input, - &witness_script, - Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI), - EcdsaSighashType::All, - ) + .p2wsh_signature_hash(input, &witness_script, amt, EcdsaSighashType::All) .unwrap(); - let funding_key = self.funding_key(channel_parameters.splice_parent_funding_txid); + let funding_key = self.funding_key(chan_params.splice_parent_funding_txid); Ok(sign_with_aux_rand(secp_ctx, &hash_to_message!(&sighash[..]), &funding_key, &self)) } @@ -1751,12 +1747,6 @@ impl TaprootChannelSigner for InMemorySigner { ) -> Result { todo!() } - - fn sign_holder_anchor_input( - &self, anchor_tx: &Transaction, input: usize, secp_ctx: &Secp256k1, - ) -> Result { - todo!() - } } /// Simple implementation of [`EntropySource`], [`NodeSigner`], and [`SignerProvider`] that takes a diff --git a/lightning/src/sign/taproot.rs b/lightning/src/sign/taproot.rs index ebfaad26c85..22470f4f8b6 100644 --- a/lightning/src/sign/taproot.rs +++ b/lightning/src/sign/taproot.rs @@ -151,11 +151,5 @@ pub trait TaprootChannelSigner: ChannelSigner { &self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1, ) -> Result; - /// Computes the signature for a commitment transaction's anchor output used as an - /// input within `anchor_tx`, which spends the commitment transaction, at index `input`. - fn sign_holder_anchor_input( - &self, anchor_tx: &Transaction, input: usize, secp_ctx: &Secp256k1, - ) -> Result; - // TODO: sign channel announcement } diff --git a/lightning/src/util/dyn_signer.rs b/lightning/src/util/dyn_signer.rs index e08d7a1f0d0..4be2b0a9c18 100644 --- a/lightning/src/util/dyn_signer.rs +++ b/lightning/src/util/dyn_signer.rs @@ -117,12 +117,6 @@ impl TaprootChannelSigner for DynSigner { ) -> Result { todo!(); } - - fn sign_holder_anchor_input( - &self, anchor_tx: &Transaction, input: usize, secp_ctx: &Secp256k1, - ) -> Result { - todo!(); - } } impl Clone for DynSigner { @@ -158,7 +152,7 @@ delegate!(DynSigner, EcdsaChannelSigner, inner, channel_parameters: &ChannelTransactionParameters, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1 ) -> Result, - fn sign_holder_anchor_input(, channel_parameters: &ChannelTransactionParameters, + fn sign_holder_keyed_anchor_input(, channel_parameters: &ChannelTransactionParameters, anchor_tx: &Transaction, input: usize, secp_ctx: &Secp256k1) -> Result, fn sign_holder_htlc_transaction(, htlc_tx: &Transaction, input: usize, diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index 6fb0bc0c890..018c027a831 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -445,9 +445,9 @@ impl EcdsaChannelSigner for TestChannelSigner { Ok(self.inner.sign_closing_transaction(channel_parameters, closing_tx, secp_ctx).unwrap()) } - fn sign_holder_anchor_input( - &self, channel_parameters: &ChannelTransactionParameters, anchor_tx: &Transaction, - input: usize, secp_ctx: &Secp256k1, + fn sign_holder_keyed_anchor_input( + &self, chan_params: &ChannelTransactionParameters, anchor_tx: &Transaction, input: usize, + secp_ctx: &Secp256k1, ) -> Result { debug_assert!(MIN_CHAN_DUST_LIMIT_SATOSHIS > ANCHOR_OUTPUT_VALUE_SATOSHI); // As long as our minimum dust limit is enforced and is greater than our anchor output @@ -460,13 +460,7 @@ impl EcdsaChannelSigner for TestChannelSigner { if !self.is_signer_available(SignerOp::SignHolderAnchorInput) { return Err(()); } - EcdsaChannelSigner::sign_holder_anchor_input( - &self.inner, - channel_parameters, - anchor_tx, - input, - secp_ctx, - ) + self.inner.sign_holder_keyed_anchor_input(chan_params, anchor_tx, input, secp_ctx) } fn sign_channel_announcement_with_funding_key( @@ -547,12 +541,6 @@ impl TaprootChannelSigner for TestChannelSigner { ) -> Result { todo!() } - - fn sign_holder_anchor_input( - &self, anchor_tx: &Transaction, input: usize, secp_ctx: &Secp256k1, - ) -> Result { - todo!() - } } impl TestChannelSigner { From 68b16e24c87bae62dee16a7f344feb2c6bec22c7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 9 Mar 2025 22:23:59 +0000 Subject: [PATCH 7/7] Define a method for determining the max HTLCs rather than a const Once we support zero-fee commitment transactions, we will no longer have a constant number of maximum HTLCs in-flight per channel but rather it will depend on the channel type. Here we prepare for this by removing the `MAX_HTLCS` constant and replacing it with a function. --- lightning/src/ln/chan_utils.rs | 10 +++- lightning/src/ln/channel.rs | 47 ++++++++++--------- lightning/src/ln/functional_tests.rs | 7 +-- lightning/src/util/anchor_channel_reserves.rs | 6 ++- lightning/src/util/config.rs | 6 ++- 5 files changed, 45 insertions(+), 31 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 6f192bb50ca..af5e7d7101b 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -52,8 +52,14 @@ use super::channel_keys::{DelayedPaymentBasepoint, DelayedPaymentKey, HtlcKey, H #[allow(unused_imports)] use crate::prelude::*; -/// Maximum number of one-way in-flight HTLC (protocol-level value). -pub const MAX_HTLCS: u16 = 483; +/// Maximum number of in-flight HTLCs in each direction allowed by the lightning protocol. +/// +/// 483 for non-zero-fee-commitment channels and 114 for zero-fee-commitment channels. +/// +/// Actual maximums can be set equal to or below this value by each channel participant. +pub fn max_htlcs(_channel_type: &ChannelTypeFeatures) -> u16 { + 483 +} /// The weight of a BIP141 witnessScript for a BOLT3's "offered HTLC output" on a commitment transaction, non-anchor variant. pub const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133; /// The weight of a BIP141 witnessScript for a BOLT3's "offered HTLC output" on a commitment transaction, anchor variant. diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index cf8962db75d..b7a8a3c9193 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -43,7 +43,7 @@ use crate::ln::chan_utils::{ CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, - CounterpartyChannelTransactionParameters, MAX_HTLCS, + CounterpartyChannelTransactionParameters, max_htlcs, get_commitment_transaction_number_obscure_factor, ClosingTransaction, commit_tx_fee_sat, }; @@ -2457,8 +2457,8 @@ impl ChannelContext where SP::Target: SignerProvider { if open_channel_fields.max_accepted_htlcs < 1 { return Err(ChannelError::close("0 max_accepted_htlcs makes for a useless channel".to_owned())); } - if open_channel_fields.max_accepted_htlcs > MAX_HTLCS { - return Err(ChannelError::close(format!("max_accepted_htlcs was {}. It must not be larger than {}", open_channel_fields.max_accepted_htlcs, MAX_HTLCS))); + if open_channel_fields.max_accepted_htlcs > max_htlcs(&channel_type) { + return Err(ChannelError::close(format!("max_accepted_htlcs was {}. It must not be larger than {}", open_channel_fields.max_accepted_htlcs, max_htlcs(&channel_type)))); } // Now check against optional parameters as set by config... @@ -2686,7 +2686,7 @@ impl ChannelContext where SP::Target: SignerProvider { counterparty_htlc_minimum_msat: open_channel_fields.htlc_minimum_msat, holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat }, counterparty_max_accepted_htlcs: open_channel_fields.max_accepted_htlcs, - holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, MAX_HTLCS), + holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, max_htlcs(&channel_type)), minimum_depth, counterparty_forwarding_info: None, @@ -2923,7 +2923,7 @@ impl ChannelContext where SP::Target: SignerProvider { counterparty_htlc_minimum_msat: 0, holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat }, counterparty_max_accepted_htlcs: 0, - holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, MAX_HTLCS), + holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, max_htlcs(&channel_type)), minimum_depth: None, // Filled in in accept_channel counterparty_forwarding_info: None, @@ -3183,6 +3183,22 @@ impl ChannelContext where SP::Target: SignerProvider { if !matches!(self.channel_state, ChannelState::NegotiatingFunding(flags) if flags == NegotiatingFundingFlags::OUR_INIT_SENT) { return Err(ChannelError::close("Got an accept_channel message at a strange time".to_owned())); } + + if let Some(ty) = &common_fields.channel_type { + if *ty != self.channel_type { + return Err(ChannelError::close("Channel Type in accept_channel didn't match the one sent in open_channel.".to_owned())); + } + } else if their_features.supports_channel_type() { + // Assume they've accepted the channel type as they said they understand it. + } else { + let channel_type = ChannelTypeFeatures::from_init(&their_features); + if channel_type != ChannelTypeFeatures::only_static_remote_key() { + return Err(ChannelError::close("Only static_remote_key is supported for non-negotiated channel types".to_owned())); + } + self.channel_type = channel_type.clone(); + funding.channel_transaction_parameters.channel_type_features = channel_type; + } + if common_fields.dust_limit_satoshis > 21000000 * 100000000 { return Err(ChannelError::close(format!("Peer never wants payout outputs? dust_limit_satoshis was {}", common_fields.dust_limit_satoshis))); } @@ -3207,8 +3223,10 @@ impl ChannelContext where SP::Target: SignerProvider { if common_fields.max_accepted_htlcs < 1 { return Err(ChannelError::close("0 max_accepted_htlcs makes for a useless channel".to_owned())); } - if common_fields.max_accepted_htlcs > MAX_HTLCS { - return Err(ChannelError::close(format!("max_accepted_htlcs was {}. It must not be larger than {}", common_fields.max_accepted_htlcs, MAX_HTLCS))); + + let channel_type = &funding.channel_transaction_parameters.channel_type_features; + if common_fields.max_accepted_htlcs > max_htlcs(channel_type) { + return Err(ChannelError::close(format!("max_accepted_htlcs was {}. It must not be larger than {}", common_fields.max_accepted_htlcs, max_htlcs(channel_type)))); } // Now check against optional parameters as set by config... @@ -3234,21 +3252,6 @@ impl ChannelContext where SP::Target: SignerProvider { return Err(ChannelError::close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", peer_limits.max_minimum_depth, common_fields.minimum_depth))); } - if let Some(ty) = &common_fields.channel_type { - if *ty != self.channel_type { - return Err(ChannelError::close("Channel Type in accept_channel didn't match the one sent in open_channel.".to_owned())); - } - } else if their_features.supports_channel_type() { - // Assume they've accepted the channel type as they said they understand it. - } else { - let channel_type = ChannelTypeFeatures::from_init(&their_features); - if channel_type != ChannelTypeFeatures::only_static_remote_key() { - return Err(ChannelError::close("Only static_remote_key is supported for non-negotiated channel types".to_owned())); - } - self.channel_type = channel_type.clone(); - funding.channel_transaction_parameters.channel_type_features = channel_type; - } - let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() { match &common_fields.shutdown_scriptpubkey { &Some(ref script) => { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 58174d40607..06a900be743 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -10609,8 +10609,9 @@ pub fn test_nondust_htlc_excess_fees_are_dust() { config.channel_config.max_dust_htlc_exposure = MaxDustHTLCExposure::FeeRateMultiplier(10_000); // Make sure the HTLC limits don't get in the way - config.channel_handshake_limits.min_max_accepted_htlcs = chan_utils::MAX_HTLCS; - config.channel_handshake_config.our_max_accepted_htlcs = chan_utils::MAX_HTLCS; + let chan_ty = ChannelTypeFeatures::only_static_remote_key(); + config.channel_handshake_limits.min_max_accepted_htlcs = chan_utils::max_htlcs(&chan_ty); + config.channel_handshake_config.our_max_accepted_htlcs = chan_utils::max_htlcs(&chan_ty); config.channel_handshake_config.our_htlc_minimum_msat = 1; config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100; @@ -10651,7 +10652,7 @@ pub fn test_nondust_htlc_excess_fees_are_dust() { let commitment_tx_per_htlc_cost = htlc_success_tx_weight(&ChannelTypeFeatures::empty()) * EXCESS_FEERATE as u64; let max_htlcs_remaining = dust_limit * 2 / commitment_tx_per_htlc_cost; - assert!(max_htlcs_remaining < chan_utils::MAX_HTLCS.into(), + assert!(max_htlcs_remaining < chan_utils::max_htlcs(&chan_ty).into(), "We should be able to fill our dust limit without too many HTLCs"); for i in 0..max_htlcs_remaining + 1 { assert_ne!(i, max_htlcs_remaining); diff --git a/lightning/src/util/anchor_channel_reserves.rs b/lightning/src/util/anchor_channel_reserves.rs index 20f2b3424c1..968a60ada0b 100644 --- a/lightning/src/util/anchor_channel_reserves.rs +++ b/lightning/src/util/anchor_channel_reserves.rs @@ -25,10 +25,11 @@ use crate::chain::chainmonitor::ChainMonitor; use crate::chain::chainmonitor::Persist; use crate::chain::Filter; use crate::events::bump_transaction::Utxo; -use crate::ln::chan_utils::MAX_HTLCS; +use crate::ln::chan_utils::max_htlcs; use crate::ln::channelmanager::AChannelManager; use crate::prelude::new_hash_set; use crate::sign::ecdsa::EcdsaChannelSigner; +use crate::types::features::ChannelTypeFeatures; use crate::util::logger::Logger; use bitcoin::constants::WITNESS_SCALE_FACTOR; use bitcoin::Amount; @@ -178,7 +179,8 @@ impl Default for AnchorChannelReserveContext { fn get_reserve_per_channel_with_input( context: &AnchorChannelReserveContext, initial_input_weight: Weight, ) -> Amount { - let expected_accepted_htlcs = min(context.expected_accepted_htlcs, MAX_HTLCS) as u64; + let max_max_htlcs = max_htlcs(&ChannelTypeFeatures::only_static_remote_key()); + let expected_accepted_htlcs = min(context.expected_accepted_htlcs, max_max_htlcs) as u64; let weight = Weight::from_wu( COMMITMENT_TRANSACTION_BASE_WEIGHT + // Reserves are calculated in terms of accepted HTLCs, as their timeout defines the urgency of diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 17eca47be72..37ba74a0226 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -193,8 +193,10 @@ pub struct ChannelHandshakeConfig { /// /// Default value: `50` /// - /// Maximum value: `483` (Any values larger will be treated as `483`. This is the BOLT #2 spec - /// limit on `max_accepted_htlcs`.) + /// Maximum value: depends on channel type, see docs on [`max_htlcs`] (any values over the + /// maximum will be silently reduced to the maximum). + /// + /// [`max_htlcs`]: crate::ln::chan_utils::max_htlcs pub our_max_accepted_htlcs: u16, }