From 1b70e6ddde389586214e41af9f3c9ad5e7543963 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Fri, 13 Jun 2025 14:23:25 -0400 Subject: [PATCH 01/16] ln/refactor: add utility to get commitment feerate by channel type --- lightning/src/ln/chan_utils.rs | 20 ++++++++++++++++- lightning/src/ln/channel.rs | 35 ++++++------------------------ lightning/src/ln/channelmanager.rs | 19 +++------------- 3 files changed, 29 insertions(+), 45 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 3a63582c423..6aba9453c9d 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -25,7 +25,9 @@ use bitcoin::hashes::ripemd160::Hash as Ripemd160; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::{Hash, HashEngine}; -use crate::chain::chaininterface::fee_for_weight; +use crate::chain::chaininterface::{ + fee_for_weight, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator, +}; use crate::chain::package::WEIGHT_REVOKED_OUTPUT; use crate::ln::msgs::DecodeError; use crate::sign::EntropySource; @@ -244,6 +246,22 @@ pub(crate) fn htlc_tx_fees_sat(feerate_per_kw: u32, num_accepted_htlcs: usize, n htlc_tx_fees_sat } +/// Returns a fee estimate for the commitment transaction depending on channel type. +pub(super) fn commitment_sat_per_1000_weight_for_type<'a, F: Deref>( + fee_estimator: &'a LowerBoundedFeeEstimator, channel_type: &ChannelTypeFeatures, +) -> u32 +where + F::Target: FeeEstimator, +{ + if channel_type.supports_anchor_zero_fee_commitments() { + 0 + } else if channel_type.supports_anchors_zero_fee_htlc_tx() { + fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee) + } else { + fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee) + } +} + // Various functions for key derivation and transaction creation for use within channels. Primarily // used in Channel and ChannelMonitor. diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 09c9f5804fc..a4f9378423d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -41,10 +41,11 @@ use crate::ln::chan_utils; #[cfg(splicing)] use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT; use crate::ln::chan_utils::{ - get_commitment_transaction_number_obscure_factor, htlc_success_tx_weight, - htlc_timeout_tx_weight, max_htlcs, ChannelPublicKeys, ChannelTransactionParameters, - ClosingTransaction, CommitmentTransaction, CounterpartyChannelTransactionParameters, - CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, + commitment_sat_per_1000_weight_for_type, get_commitment_transaction_number_obscure_factor, + htlc_success_tx_weight, htlc_timeout_tx_weight, max_htlcs, ChannelPublicKeys, + ChannelTransactionParameters, ClosingTransaction, CommitmentTransaction, + CounterpartyChannelTransactionParameters, CounterpartyCommitmentSecrets, + HTLCOutputInCommitment, HolderCommitmentTransaction, }; use crate::ln::channel_state::{ ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, @@ -3416,16 +3417,7 @@ where debug_assert!(!channel_type.supports_any_optional_bits()); debug_assert!(!channel_type.requires_unknown_bits_from(&channelmanager::provided_channel_type_features(&config))); - let commitment_feerate = if channel_type.supports_anchor_zero_fee_commitments() { - 0 - } else { - let commitment_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() { - ConfirmationTarget::AnchorChannelFee - } else { - ConfirmationTarget::NonAnchorChannelFee - }; - fee_estimator.bounded_sat_per_1000_weight(commitment_conf_target) - }; + let commitment_feerate = commitment_sat_per_1000_weight_for_type(&fee_estimator, &channel_type); let value_to_self_msat = channel_value_satoshis * 1000 - push_msat; let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type); @@ -5462,20 +5454,7 @@ where let next_channel_type = get_initial_channel_type(user_config, &eligible_features); - // Note that we can't get `anchor_zero_fee_commitments` type here, which requires zero - // fees, because we downgrade from this channel type first. If there were a superior - // channel type that downgrades to `anchor_zero_fee_commitments`, we'd need to handle - // fee setting differently here. If we proceeded to open a `anchor_zero_fee_commitments` - // channel with non-zero fees, we could produce a non-standard commitment transaction that - // puts us at risk of losing funds. We would expect our peer to reject such a channel - // open, but we don't want to rely on their validation. - assert!(!next_channel_type.supports_anchor_zero_fee_commitments()); - let conf_target = if next_channel_type.supports_anchors_zero_fee_htlc_tx() { - ConfirmationTarget::AnchorChannelFee - } else { - ConfirmationTarget::NonAnchorChannelFee - }; - self.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(conf_target); + self.feerate_per_kw = commitment_sat_per_1000_weight_for_type(&fee_estimator, &next_channel_type); funding.channel_transaction_parameters.channel_type_features = next_channel_type; Ok(()) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 94bc9972a30..b410494ecce 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -56,6 +56,7 @@ use crate::events::{ InboundChannelFunds, PaymentFailureReason, ReplayEvent, }; use crate::events::{FundingInfo, PaidBolt12Invoice}; +use crate::ln::chan_utils::commitment_sat_per_1000_weight_for_type; // Since this struct is returned in `list_channels` methods, expose it here in case users want to // construct one themselves. use crate::ln::channel::PendingV2Channel; @@ -6988,9 +6989,6 @@ where PersistenceNotifierGuard::optionally_notify(self, || { let mut should_persist = NotifyOption::SkipPersistNoEvents; - let non_anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); - let anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee); - let per_peer_state = self.per_peer_state.read().unwrap(); for (_cp_id, peer_state_mutex) in per_peer_state.iter() { let mut peer_state_lock = peer_state_mutex.lock().unwrap(); @@ -6998,11 +6996,7 @@ where for (chan_id, chan) in peer_state.channel_by_id.iter_mut() .filter_map(|(chan_id, chan)| chan.as_funded_mut().map(|chan| (chan_id, chan))) { - let new_feerate = if chan.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - anchor_feerate - } else { - non_anchor_feerate - }; + let new_feerate = commitment_sat_per_1000_weight_for_type(&self.fee_estimator, chan.funding.get_channel_type()); let chan_needs_persist = self.update_channel_fee(chan_id, chan, new_feerate); if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; } } @@ -7038,9 +7032,6 @@ where PersistenceNotifierGuard::optionally_notify(self, || { let mut should_persist = NotifyOption::SkipPersistNoEvents; - let non_anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); - let anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee); - let mut handle_errors: Vec<(Result<(), _>, _)> = Vec::new(); let mut timed_out_mpp_htlcs = Vec::new(); let mut pending_peers_awaiting_removal = Vec::new(); @@ -7056,11 +7047,7 @@ where peer_state.channel_by_id.retain(|chan_id, chan| { match chan.as_funded_mut() { Some(funded_chan) => { - let new_feerate = if funded_chan.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - anchor_feerate - } else { - non_anchor_feerate - }; + let new_feerate = commitment_sat_per_1000_weight_for_type(&self.fee_estimator, funded_chan.funding.get_channel_type()); let chan_needs_persist = self.update_channel_fee(chan_id, funded_chan, new_feerate); if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; } From c98e32321314b54736e7874a7ff141124e71445c Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 14 Jul 2025 15:31:58 -0400 Subject: [PATCH 02/16] f Remove unnecessary lifetime --- lightning/src/ln/chan_utils.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 6aba9453c9d..e4c5b60f2f2 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -247,8 +247,8 @@ pub(crate) fn htlc_tx_fees_sat(feerate_per_kw: u32, num_accepted_htlcs: usize, n } /// Returns a fee estimate for the commitment transaction depending on channel type. -pub(super) fn commitment_sat_per_1000_weight_for_type<'a, F: Deref>( - fee_estimator: &'a LowerBoundedFeeEstimator, channel_type: &ChannelTypeFeatures, +pub(super) fn commitment_sat_per_1000_weight_for_type( + fee_estimator: &LowerBoundedFeeEstimator, channel_type: &ChannelTypeFeatures, ) -> u32 where F::Target: FeeEstimator, From 6b5f26a7dfa5577b96ba65bffedec671dbefc12d Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 11 Jun 2025 13:05:02 -0400 Subject: [PATCH 03/16] ln: return Option for dust_exposure_limiting_feerate This fee rate is currently used in two scenarios: - To count any fees above what we consider to be a sane estimate towards our dust exposure. - To get a maximum dust exposure (when using MaxDustHTLCExposure::FeeEstimator strategy). When we have zero fee commitments: - Commitments are zero fee, so we don't need to count fees towards dust exposure. - The amount of dust we have is not dependent on fees, as everything is zero fee. - We still want to limit our total dust exposure. This commit updates get_dust_exposure_limiting_feerate to allow a None value to prepare for support for zero fee commitments. This clearly allows us to indicate when we don't care about fee rates for dust considerations. In get_max_dust_htlc_exposure_msat, we simply hardcode a value of 1 sat/vbyte if a feerate dependent strategy is being used. --- lightning/src/ln/channel.rs | 19 ++++++++++++------- lightning/src/ln/functional_tests.rs | 2 +- lightning/src/util/config.rs | 8 ++++++++ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a4f9378423d..679d282e520 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3999,17 +3999,21 @@ where fn get_dust_exposure_limiting_feerate( &self, fee_estimator: &LowerBoundedFeeEstimator, - ) -> u32 + ) -> Option where F::Target: FeeEstimator, { - fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MaximumFeeEstimate) + Some(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MaximumFeeEstimate)) } - pub fn get_max_dust_htlc_exposure_msat(&self, limiting_feerate_sat_per_kw: u32) -> u64 { + /// Returns the maximum configured dust exposure. + /// + /// Uses a default of 1 sat/vbyte if `limiting_feerate_sat_per_kw` is `None` and the dust + /// exposure policy depends on fee rate. + pub fn get_max_dust_htlc_exposure_msat(&self, limiting_feerate_sat_per_kw: Option) -> u64 { match self.config.options.max_dust_htlc_exposure { MaxDustHTLCExposure::FeeRateMultiplier(multiplier) => { - (limiting_feerate_sat_per_kw as u64).saturating_mul(multiplier) + (limiting_feerate_sat_per_kw.unwrap_or(250) as u64).saturating_mul(multiplier) }, MaxDustHTLCExposure::FixedLimitMsat(limit) => limit, } @@ -4356,7 +4360,7 @@ where #[rustfmt::skip] fn can_accept_incoming_htlc( &self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC, - dust_exposure_limiting_feerate: u32, logger: &L, + dust_exposure_limiting_feerate: Option, logger: &L, ) -> Result<(), LocalHTLCFailureReason> where L::Target: Logger, @@ -4696,7 +4700,7 @@ where #[rustfmt::skip] fn get_pending_htlc_stats( &self, funding: &FundingScope, outbound_feerate_update: Option, - dust_exposure_limiting_feerate: u32, + dust_exposure_limiting_feerate: Option, ) -> HTLCStats { let context = self; let uses_0_htlc_fee_anchors = funding.get_channel_type().supports_anchors_zero_fee_htlc_tx(); @@ -4775,7 +4779,8 @@ where let excess_feerate_opt = outbound_feerate_update .or(self.pending_update_fee.map(|(fee, _)| fee)) .unwrap_or(self.feerate_per_kw) - .checked_sub(dust_exposure_limiting_feerate); + .checked_sub(dust_exposure_limiting_feerate.unwrap_or(0)); + let extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat = excess_feerate_opt.map(|excess_feerate| { let extra_htlc_commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1 + on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()); let extra_htlc_htlc_tx_fees_sat = chan_utils::htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1, on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 8ca290ef165..0087376e50f 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -9953,7 +9953,7 @@ fn do_test_max_dust_htlc_exposure( let chan = chan_lock.channel_by_id.get(&channel_id).unwrap(); ( chan.context().get_dust_buffer_feerate(None) as u64, - chan.context().get_max_dust_htlc_exposure_msat(253), + chan.context().get_max_dust_htlc_exposure_msat(Some(253)), ) }; assert_eq!(dust_buffer_feerate, expected_dust_buffer_feerate as u64); diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 361fe24bc93..a0c74673799 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -442,12 +442,20 @@ pub enum MaxDustHTLCExposure { /// on HTLC outputs means your channel may be subject to more dust exposure in the event of /// increases in fee rate. /// + /// Note that because zero-commitment-fee anchor channels do not allow for feerate updates (and + /// thus never experience dust exposure changes due to feerate shifts, resulting in no + /// force-closes due to dust exposure limits), such channels will calculate their maximum + /// dust exposure using a constant feerate of 250 sat/KW when using this variant. + /// /// # Backwards Compatibility /// This variant only became available in LDK 0.0.116, so if you downgrade to a prior version /// by default this will be set to a [`Self::FixedLimitMsat`] of 5,000,000 msat. /// /// [`FeeEstimator`]: crate::chain::chaininterface::FeeEstimator /// [`ConfirmationTarget::MaximumFeeEstimate`]: crate::chain::chaininterface::ConfirmationTarget::MaximumFeeEstimate + // + // TODO: link ChannelHandshakeConfig::negotiate_anchor_zero_fee_commitment in zero fee + // commitment doc once field is no longer behind cfg[test] flag. FeeRateMultiplier(u64), } From dace534b7aeb772a37bb14e14a9d313e80d9339e Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 14 Jul 2025 15:38:00 -0400 Subject: [PATCH 04/16] f debug assert that zero fee commitments have no excess --- lightning/src/ln/channel.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 679d282e520..d7bfdfe0f15 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4781,6 +4781,11 @@ where .unwrap_or(self.feerate_per_kw) .checked_sub(dust_exposure_limiting_feerate.unwrap_or(0)); + let is_zero_fee_comm = funding.get_channel_type().supports_anchor_zero_fee_commitments(); + if is_zero_fee_comm { + debug_assert_eq!(excess_feerate_opt, Some(0)); + } + let extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat = excess_feerate_opt.map(|excess_feerate| { let extra_htlc_commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1 + on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()); let extra_htlc_htlc_tx_fees_sat = chan_utils::htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1, on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()); From bd782d36566d3d2d36d31ec4e14ffaaab099ca1b Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 14 Jul 2025 15:41:41 -0400 Subject: [PATCH 05/16] ln: add channel type awareness to get_dust_exposure_limiting_feerate Co-authored-by: Matt Corallo --- lightning/src/ln/channel.rs | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index d7bfdfe0f15..54b3356dae3 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3997,13 +3997,20 @@ where cmp::max(self.config.options.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA) } + /// Returns a maximum "sane" fee rate used to reason about our dust exposure. + /// Will be Some if the `channel_type`'s dust exposure depends on its commitment fee rate, and + /// None otherwise. fn get_dust_exposure_limiting_feerate( - &self, fee_estimator: &LowerBoundedFeeEstimator, + &self, fee_estimator: &LowerBoundedFeeEstimator, channel_type: &ChannelTypeFeatures, ) -> Option where F::Target: FeeEstimator, { - Some(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MaximumFeeEstimate)) + if channel_type.supports_anchor_zero_fee_commitments() { + None + } else { + Some(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MaximumFeeEstimate)) + } } /// Returns the maximum configured dust exposure. @@ -4139,7 +4146,9 @@ where return Err(ChannelError::close("Remote side tried to send more than the total value of the channel".to_owned())); } - let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator); + let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate( + &fee_estimator, funding.get_channel_type(), + ); let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); if htlc_stats.pending_inbound_htlcs + 1 > self.holder_max_accepted_htlcs as usize { return Err(ChannelError::close(format!("Remote tried to push more than our max accepted HTLCs ({})", self.holder_max_accepted_htlcs))); @@ -4215,7 +4224,9 @@ where F::Target: FeeEstimator, { // Check that we won't be pushed over our dust exposure limit by the feerate increase. - let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator); + let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate( + &fee_estimator, funding.get_channel_type(), + ); let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { @@ -4332,7 +4343,9 @@ where L::Target: Logger, { // Before proposing a feerate update, check that we can actually afford the new fee. - let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator); + let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate( + &fee_estimator, funding.get_channel_type(), + ); let htlc_stats = self.get_pending_htlc_stats(funding, Some(feerate_per_kw), dust_exposure_limiting_feerate); let stats = self.build_commitment_stats(funding, true, true, Some(feerate_per_kw), Some(htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize)); let holder_balance_msat = stats.local_balance_before_fee_msat - htlc_stats.outbound_holding_cell_msat; @@ -4781,7 +4794,9 @@ where .unwrap_or(self.feerate_per_kw) .checked_sub(dust_exposure_limiting_feerate.unwrap_or(0)); + // Dust exposure is only decoupled from feerate for zero fee commitment channels. let is_zero_fee_comm = funding.get_channel_type().supports_anchor_zero_fee_commitments(); + debug_assert_eq!(is_zero_fee_comm, dust_exposure_limiting_feerate.is_none()); if is_zero_fee_comm { debug_assert_eq!(excess_feerate_opt, Some(0)); } @@ -4917,7 +4932,9 @@ where // Note that we have to handle overflow due to the case mentioned in the docs in general // here. - let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator); + let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate( + &fee_estimator, funding.get_channel_type(), + ); let htlc_stats = context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); // Subtract any non-HTLC outputs from the local and remote balances @@ -9210,7 +9227,9 @@ where return Err(LocalHTLCFailureReason::ChannelClosed) } - let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); + let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate( + &fee_estimator, self.funding.get_channel_type(), + ); core::iter::once(&self.funding) .chain(self.pending_funding.iter()) From 44b3474cc763fc1649a37bf6878a043483beee27 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 23 Jun 2025 10:32:37 -0400 Subject: [PATCH 06/16] ln: remove unnecessary nonzero anchor channel check in build_htlc_output LDK does not support a channel type that supports both zero fee and nonzero fee anchors (as this is impossible), so we can drop the unnecessary check in build_htlc_output. --- lightning/src/ln/chan_utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index e4c5b60f2f2..1d6aada1d6a 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -829,7 +829,7 @@ pub(crate) fn build_htlc_output( } else { htlc_success_tx_weight(channel_type_features) }; - let output_value = if channel_type_features.supports_anchors_zero_fee_htlc_tx() && !channel_type_features.supports_anchors_nonzero_fee_htlc_tx() { + let output_value = if channel_type_features.supports_anchors_zero_fee_htlc_tx() { htlc.to_bitcoin_amount() } else { let total_fee = Amount::from_sat(feerate_per_kw as u64 * weight / 1000); From 4afcf60daa8f5d442a6eadae310e617ae4acdce9 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 7 Jul 2025 13:36:37 -0400 Subject: [PATCH 07/16] ln/refactor: move success/timeout dust limits into helper This commit pulls calculation of second stage fees into a helper function. A side effect of this refactor is that it fixes a rounding issue in commit_and_htlc_tx_fees_sat. Previously, rounding down of fees would happen after multiplying by the number of HTLCs. Now the fees will be rounded down before multiplying by the number of HTLCs. This wasn't a serious issue - it would just cause us very slightly over estimate our dust exposure at certain fee amounts that needed rounding. A hard-coded value in test_nondust_htlc_excess_fees_are_dust is updated to account for this rounding change. --- lightning/src/ln/chan_utils.rs | 47 ++++-- lightning/src/ln/channel.rs | 178 ++++++++------------ lightning/src/ln/functional_tests.rs | 77 +++++---- lightning/src/ln/htlc_reserve_unit_tests.rs | 6 +- 4 files changed, 151 insertions(+), 157 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 1d6aada1d6a..9c693a0e13c 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -235,15 +235,27 @@ pub(crate) fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_t / 1000 } +/// Returns the fees for success and timeout second stage HTLC transactions. +pub(super) fn second_stage_tx_fees_sat( + channel_type: &ChannelTypeFeatures, feerate_sat_per_1000_weight: u32, +) -> (u64, u64) { + if channel_type.supports_anchors_zero_fee_htlc_tx() { + (0, 0) + } else { + ( + feerate_sat_per_1000_weight as u64 * htlc_success_tx_weight(channel_type) / 1000, + feerate_sat_per_1000_weight as u64 * htlc_timeout_tx_weight(channel_type) / 1000, + ) + } +} + #[rustfmt::skip] pub(crate) fn htlc_tx_fees_sat(feerate_per_kw: u32, num_accepted_htlcs: usize, num_offered_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 { - let htlc_tx_fees_sat = if !channel_type_features.supports_anchors_zero_fee_htlc_tx() { - num_accepted_htlcs as u64 * htlc_success_tx_weight(channel_type_features) * feerate_per_kw as u64 / 1000 - + num_offered_htlcs as u64 * htlc_timeout_tx_weight(channel_type_features) * feerate_per_kw as u64 / 1000 - } else { - 0 - }; - htlc_tx_fees_sat + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( + channel_type_features, feerate_per_kw, + ); + + num_accepted_htlcs as u64 * htlc_success_tx_fee_sat + num_offered_htlcs as u64 * htlc_timeout_tx_fee_sat } /// Returns a fee estimate for the commitment transaction depending on channel type. @@ -824,16 +836,17 @@ pub(crate) fn build_htlc_input(commitment_txid: &Txid, htlc: &HTLCOutputInCommit pub(crate) fn build_htlc_output( feerate_per_kw: u32, contest_delay: u16, htlc: &HTLCOutputInCommitment, channel_type_features: &ChannelTypeFeatures, broadcaster_delayed_payment_key: &DelayedPaymentKey, revocation_key: &RevocationKey ) -> TxOut { - let weight = if htlc.offered { - htlc_timeout_tx_weight(channel_type_features) - } else { - htlc_success_tx_weight(channel_type_features) - }; - let output_value = if channel_type_features.supports_anchors_zero_fee_htlc_tx() { - htlc.to_bitcoin_amount() - } else { - let total_fee = Amount::from_sat(feerate_per_kw as u64 * weight / 1000); - htlc.to_bitcoin_amount() - total_fee + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( + channel_type_features, feerate_per_kw, + ); + + let output_value = { + let total_fee = if htlc.offered { + htlc_timeout_tx_fee_sat + } else { + htlc_success_tx_fee_sat + }; + htlc.to_bitcoin_amount() - Amount::from_sat(total_fee) }; TxOut { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 54b3356dae3..5b432a7fce0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -42,10 +42,9 @@ use crate::ln::chan_utils; use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT; use crate::ln::chan_utils::{ commitment_sat_per_1000_weight_for_type, get_commitment_transaction_number_obscure_factor, - htlc_success_tx_weight, htlc_timeout_tx_weight, max_htlcs, ChannelPublicKeys, - ChannelTransactionParameters, ClosingTransaction, CommitmentTransaction, - CounterpartyChannelTransactionParameters, CounterpartyCommitmentSecrets, - HTLCOutputInCommitment, HolderCommitmentTransaction, + max_htlcs, second_stage_tx_fees_sat, ChannelPublicKeys, ChannelTransactionParameters, + ClosingTransaction, CommitmentTransaction, CounterpartyChannelTransactionParameters, + CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, }; use crate::ln::channel_state::{ ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, @@ -290,18 +289,11 @@ impl InboundHTLCOutput { &self, local: bool, feerate_per_kw: u32, broadcaster_dust_limit_sat: u64, features: &ChannelTypeFeatures, ) -> bool { - let htlc_tx_fee_sat = if features.supports_anchors_zero_fee_htlc_tx() { - 0 - } else { - let htlc_tx_weight = if !local { - // this is an offered htlc - htlc_timeout_tx_weight(features) - } else { - htlc_success_tx_weight(features) - }; - // As required by the spec, round down - feerate_per_kw as u64 * htlc_tx_weight / 1000 - }; + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = + second_stage_tx_fees_sat(features, feerate_per_kw); + + let htlc_tx_fee_sat = + if !local { htlc_timeout_tx_fee_sat } else { htlc_success_tx_fee_sat }; self.amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee_sat } } @@ -436,17 +428,14 @@ impl OutboundHTLCOutput { &self, local: bool, feerate_per_kw: u32, broadcaster_dust_limit_sat: u64, features: &ChannelTypeFeatures, ) -> bool { - let htlc_tx_fee_sat = if features.supports_anchors_zero_fee_htlc_tx() { - 0 + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = + second_stage_tx_fees_sat(features, feerate_per_kw); + + let htlc_tx_fee_sat = if local { + // This is an offered HTLC. + htlc_timeout_tx_fee_sat } else { - let htlc_tx_weight = if local { - // this is an offered htlc - htlc_timeout_tx_weight(features) - } else { - htlc_success_tx_weight(features) - }; - // As required by the spec, round down - feerate_per_kw as u64 * htlc_tx_weight / 1000 + htlc_success_tx_fee_sat }; self.amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee_sat } @@ -4387,13 +4376,11 @@ where on_counterparty_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat); return Err(LocalHTLCFailureReason::DustLimitCounterparty) } - let htlc_success_dust_limit = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - 0 - } else { - let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; - dust_buffer_feerate * htlc_success_tx_weight(funding.get_channel_type()) / 1000 - }; - let exposure_dust_limit_success_sats = htlc_success_dust_limit + self.holder_dust_limit_satoshis; + let dust_buffer_feerate = self.get_dust_buffer_feerate(None); + let (htlc_success_tx_fee_sat, _) = second_stage_tx_fees_sat( + &funding.get_channel_type(), dust_buffer_feerate, + ); + let exposure_dust_limit_success_sats = htlc_success_tx_fee_sat + self.holder_dust_limit_satoshis; if msg.amount_msat / 1000 < exposure_dust_limit_success_sats { let on_holder_tx_dust_htlc_exposure_msat = htlc_stats.on_holder_tx_dust_exposure_msat; if on_holder_tx_dust_htlc_exposure_msat > max_dust_htlc_exposure_msat { @@ -4716,15 +4703,11 @@ where dust_exposure_limiting_feerate: Option, ) -> HTLCStats { let context = self; - let uses_0_htlc_fee_anchors = funding.get_channel_type().supports_anchors_zero_fee_htlc_tx(); - let dust_buffer_feerate = context.get_dust_buffer_feerate(outbound_feerate_update); - let (htlc_timeout_dust_limit, htlc_success_dust_limit) = if uses_0_htlc_fee_anchors { - (0, 0) - } else { - (dust_buffer_feerate as u64 * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000, - dust_buffer_feerate as u64 * htlc_success_tx_weight(funding.get_channel_type()) / 1000) - }; + let dust_buffer_feerate = self.get_dust_buffer_feerate(outbound_feerate_update); + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( + funding.get_channel_type(), dust_buffer_feerate, + ); let mut on_holder_tx_dust_exposure_msat = 0; let mut on_counterparty_tx_dust_exposure_msat = 0; @@ -4735,8 +4718,8 @@ where let mut pending_inbound_htlcs_value_msat = 0; { - let counterparty_dust_limit_timeout_sat = htlc_timeout_dust_limit + context.counterparty_dust_limit_satoshis; - let holder_dust_limit_success_sat = htlc_success_dust_limit + context.holder_dust_limit_satoshis; + let counterparty_dust_limit_timeout_sat = htlc_timeout_tx_fee_sat + context.counterparty_dust_limit_satoshis; + let holder_dust_limit_success_sat = htlc_success_tx_fee_sat + context.holder_dust_limit_satoshis; for htlc in context.pending_inbound_htlcs.iter() { pending_inbound_htlcs_value_msat += htlc.amount_msat; if htlc.amount_msat / 1000 < counterparty_dust_limit_timeout_sat { @@ -4755,8 +4738,8 @@ where let mut on_holder_tx_outbound_holding_cell_htlcs_count = 0; let mut pending_outbound_htlcs = self.pending_outbound_htlcs.len(); { - let counterparty_dust_limit_success_sat = htlc_success_dust_limit + context.counterparty_dust_limit_satoshis; - let holder_dust_limit_timeout_sat = htlc_timeout_dust_limit + context.holder_dust_limit_satoshis; + let counterparty_dust_limit_success_sat = htlc_success_tx_fee_sat + context.counterparty_dust_limit_satoshis; + let holder_dust_limit_timeout_sat = htlc_timeout_tx_fee_sat + context.holder_dust_limit_satoshis; for htlc in context.pending_outbound_htlcs.iter() { pending_outbound_htlcs_value_msat += htlc.amount_msat; if htlc.amount_msat / 1000 < counterparty_dust_limit_success_sat { @@ -4855,13 +4838,12 @@ where } } let mut inbound_details = Vec::new(); - let htlc_success_dust_limit = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - 0 - } else { - let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; - dust_buffer_feerate * htlc_success_tx_weight(funding.get_channel_type()) / 1000 - }; - let holder_dust_limit_success_sat = htlc_success_dust_limit + self.holder_dust_limit_satoshis; + + let dust_buffer_feerate = self.get_dust_buffer_feerate(None); + let (htlc_success_tx_fee_sat, _) = second_stage_tx_fees_sat( + funding.get_channel_type(), dust_buffer_feerate, + ); + let holder_dust_limit_success_sat = htlc_success_tx_fee_sat + self.holder_dust_limit_satoshis; for htlc in self.pending_inbound_htlcs.iter() { if let Some(state_details) = (&htlc.state).into() { inbound_details.push(InboundHTLCDetails{ @@ -4881,13 +4863,12 @@ where #[rustfmt::skip] pub fn get_pending_outbound_htlc_details(&self, funding: &FundingScope) -> Vec { let mut outbound_details = Vec::new(); - let htlc_timeout_dust_limit = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - 0 - } else { - let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; - dust_buffer_feerate * htlc_success_tx_weight(funding.get_channel_type()) / 1000 - }; - let holder_dust_limit_timeout_sat = htlc_timeout_dust_limit + self.holder_dust_limit_satoshis; + + let dust_buffer_feerate = self.get_dust_buffer_feerate(None); + let (_, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( + funding.get_channel_type(), dust_buffer_feerate, + ); + let holder_dust_limit_timeout_sat = htlc_timeout_tx_fee_sat + self.holder_dust_limit_satoshis; for htlc in self.pending_outbound_htlcs.iter() { outbound_details.push(OutboundHTLCDetails{ htlc_id: Some(htlc.htlc_id), @@ -4950,6 +4931,9 @@ where funding.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) * 1000); let mut available_capacity_msat = outbound_capacity_msat; + let (real_htlc_success_tx_fee_sat, real_htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( + funding.get_channel_type(), context.feerate_per_kw, + ); if funding.is_outbound() { // We should mind channel commit tx fee when computing how much of the available capacity @@ -4959,11 +4943,7 @@ where // and the answer will in turn change the amount itself — making it a circular // dependency. // This complicates the computation around dust-values, up to the one-htlc-value. - let mut real_dust_limit_timeout_sat = context.holder_dust_limit_satoshis; - if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - real_dust_limit_timeout_sat += context.feerate_per_kw as u64 * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000; - } - + let real_dust_limit_timeout_sat = real_htlc_timeout_tx_fee_sat + context.holder_dust_limit_satoshis; let htlc_above_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000, HTLCInitiator::LocalOffered); let mut max_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(funding, htlc_above_dust, Some(())); let htlc_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000 - 1, HTLCInitiator::LocalOffered); @@ -4990,11 +4970,7 @@ where } else { // If the channel is inbound (i.e. counterparty pays the fee), we need to make sure // sending a new HTLC won't reduce their balance below our reserve threshold. - let mut real_dust_limit_success_sat = context.counterparty_dust_limit_satoshis; - if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - real_dust_limit_success_sat += context.feerate_per_kw as u64 * htlc_success_tx_weight(funding.get_channel_type()) / 1000; - } - + let real_dust_limit_success_sat = real_htlc_success_tx_fee_sat + context.counterparty_dust_limit_satoshis; let htlc_above_dust = HTLCCandidate::new(real_dust_limit_success_sat * 1000, HTLCInitiator::LocalOffered); let max_reserved_commit_tx_fee_msat = context.next_remote_commit_tx_fee_msat(funding, Some(htlc_above_dust), None); @@ -5016,35 +4992,34 @@ where let mut dust_exposure_dust_limit_msat = 0; let max_dust_htlc_exposure_msat = context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - (context.counterparty_dust_limit_satoshis, context.holder_dust_limit_satoshis) - } else { - let dust_buffer_feerate = context.get_dust_buffer_feerate(None) as u64; - (context.counterparty_dust_limit_satoshis + dust_buffer_feerate * htlc_success_tx_weight(funding.get_channel_type()) / 1000, - context.holder_dust_limit_satoshis + dust_buffer_feerate * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000) - }; + let dust_buffer_feerate = self.get_dust_buffer_feerate(None); + let (buffer_htlc_success_tx_fee_sat, buffer_htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( + funding.get_channel_type(), dust_buffer_feerate, + ); + let buffer_dust_limit_success_sat = buffer_htlc_success_tx_fee_sat + context.counterparty_dust_limit_satoshis; + let buffer_dust_limit_timeout_sat = buffer_htlc_timeout_tx_fee_sat + context.holder_dust_limit_satoshis; if let Some(extra_htlc_dust_exposure) = htlc_stats.extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat { if extra_htlc_dust_exposure > max_dust_htlc_exposure_msat { // If adding an extra HTLC would put us over the dust limit in total fees, we cannot // send any non-dust HTLCs. - available_capacity_msat = cmp::min(available_capacity_msat, htlc_success_dust_limit * 1000); + available_capacity_msat = cmp::min(available_capacity_msat, buffer_dust_limit_success_sat * 1000); } } - if htlc_stats.on_counterparty_tx_dust_exposure_msat.saturating_add(htlc_success_dust_limit * 1000) > max_dust_htlc_exposure_msat.saturating_add(1) { + if htlc_stats.on_counterparty_tx_dust_exposure_msat.saturating_add(buffer_dust_limit_success_sat * 1000) > max_dust_htlc_exposure_msat.saturating_add(1) { // Note that we don't use the `counterparty_tx_dust_exposure` (with // `htlc_dust_exposure_msat`) here as it only applies to non-dust HTLCs. remaining_msat_below_dust_exposure_limit = Some(max_dust_htlc_exposure_msat.saturating_sub(htlc_stats.on_counterparty_tx_dust_exposure_msat)); - dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, htlc_success_dust_limit * 1000); + dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, buffer_dust_limit_success_sat * 1000); } - if htlc_stats.on_holder_tx_dust_exposure_msat as i64 + htlc_timeout_dust_limit as i64 * 1000 - 1 > max_dust_htlc_exposure_msat.try_into().unwrap_or(i64::max_value()) { + if htlc_stats.on_holder_tx_dust_exposure_msat as i64 + buffer_dust_limit_timeout_sat as i64 * 1000 - 1 > max_dust_htlc_exposure_msat.try_into().unwrap_or(i64::max_value()) { remaining_msat_below_dust_exposure_limit = Some(cmp::min( remaining_msat_below_dust_exposure_limit.unwrap_or(u64::max_value()), max_dust_htlc_exposure_msat.saturating_sub(htlc_stats.on_holder_tx_dust_exposure_msat))); - dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, htlc_timeout_dust_limit * 1000); + dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, buffer_dust_limit_timeout_sat * 1000); } if let Some(remaining_limit_msat) = remaining_msat_below_dust_exposure_limit { @@ -5088,14 +5063,11 @@ where let context = self; assert!(funding.is_outbound()); - let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - (0, 0) - } else { - (context.feerate_per_kw as u64 * htlc_success_tx_weight(funding.get_channel_type()) / 1000, - context.feerate_per_kw as u64 * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000) - }; - let real_dust_limit_success_sat = htlc_success_dust_limit + context.holder_dust_limit_satoshis; - let real_dust_limit_timeout_sat = htlc_timeout_dust_limit + context.holder_dust_limit_satoshis; + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( + funding.get_channel_type(), context.feerate_per_kw, + ); + let real_dust_limit_success_sat = htlc_success_tx_fee_sat + context.holder_dust_limit_satoshis; + let real_dust_limit_timeout_sat = htlc_timeout_tx_fee_sat + context.holder_dust_limit_satoshis; let mut addl_htlcs = 0; if fee_spike_buffer_htlc.is_some() { addl_htlcs += 1; } @@ -5197,14 +5169,11 @@ where let context = self; assert!(!funding.is_outbound()); - let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - (0, 0) - } else { - (context.feerate_per_kw as u64 * htlc_success_tx_weight(funding.get_channel_type()) / 1000, - context.feerate_per_kw as u64 * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000) - }; - let real_dust_limit_success_sat = htlc_success_dust_limit + context.counterparty_dust_limit_satoshis; - let real_dust_limit_timeout_sat = htlc_timeout_dust_limit + context.counterparty_dust_limit_satoshis; + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( + funding.get_channel_type(), context.feerate_per_kw, + ); + let real_dust_limit_success_sat = htlc_success_tx_fee_sat + context.counterparty_dust_limit_satoshis; + let real_dust_limit_timeout_sat = htlc_timeout_tx_fee_sat + context.counterparty_dust_limit_satoshis; let mut addl_htlcs = 0; if fee_spike_buffer_htlc.is_some() { addl_htlcs += 1; } @@ -13308,9 +13277,7 @@ mod tests { use crate::chain::chaininterface::LowerBoundedFeeEstimator; use crate::chain::transaction::OutPoint; use crate::chain::BestBlock; - use crate::ln::chan_utils::{ - self, commit_tx_fee_sat, htlc_success_tx_weight, htlc_timeout_tx_weight, - }; + use crate::ln::chan_utils::{self, commit_tx_fee_sat}; use crate::ln::channel::{ AwaitingChannelReadyFlags, ChannelState, FundedChannel, HTLCCandidate, HTLCInitiator, HTLCUpdateAwaitingACK, InboundHTLCOutput, InboundHTLCState, InboundV1Channel, @@ -13623,16 +13590,19 @@ mod tests { let commitment_tx_fee_0_htlcs = commit_tx_fee_sat(chan.context.feerate_per_kw, 0, chan.funding.get_channel_type()) * 1000; let commitment_tx_fee_1_htlc = commit_tx_fee_sat(chan.context.feerate_per_kw, 1, chan.funding.get_channel_type()) * 1000; + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = chan_utils::second_stage_tx_fees_sat( + &chan.funding.get_channel_type(), 253 + ); // If HTLC_SUCCESS_TX_WEIGHT and HTLC_TIMEOUT_TX_WEIGHT were swapped: then this HTLC would be // counted as dust when it shouldn't be. - let htlc_amt_above_timeout = ((253 * htlc_timeout_tx_weight(chan.funding.get_channel_type()) / 1000) + chan.context.holder_dust_limit_satoshis + 1) * 1000; + let htlc_amt_above_timeout = (htlc_timeout_tx_fee_sat + chan.context.holder_dust_limit_satoshis + 1) * 1000; let htlc_candidate = HTLCCandidate::new(htlc_amt_above_timeout, HTLCInitiator::LocalOffered); let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(&chan.funding, htlc_candidate, None); assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc); // If swapped: this HTLC would be counted as non-dust when it shouldn't be. - let dust_htlc_amt_below_success = ((253 * htlc_success_tx_weight(chan.funding.get_channel_type()) / 1000) + chan.context.holder_dust_limit_satoshis - 1) * 1000; + let dust_htlc_amt_below_success = (htlc_success_tx_fee_sat + chan.context.holder_dust_limit_satoshis - 1) * 1000; let htlc_candidate = HTLCCandidate::new(dust_htlc_amt_below_success, HTLCInitiator::RemoteOffered); let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(&chan.funding, htlc_candidate, None); assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs); @@ -13640,13 +13610,13 @@ mod tests { chan.funding.channel_transaction_parameters.is_outbound_from_holder = false; // If swapped: this HTLC would be counted as non-dust when it shouldn't be. - let dust_htlc_amt_above_timeout = ((253 * htlc_timeout_tx_weight(chan.funding.get_channel_type()) / 1000) + chan.context.counterparty_dust_limit_satoshis + 1) * 1000; + let dust_htlc_amt_above_timeout = (htlc_timeout_tx_fee_sat + chan.context.counterparty_dust_limit_satoshis + 1) * 1000; let htlc_candidate = HTLCCandidate::new(dust_htlc_amt_above_timeout, HTLCInitiator::LocalOffered); let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(&chan.funding, Some(htlc_candidate), None); assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs); // If swapped: this HTLC would be counted as dust when it shouldn't be. - let htlc_amt_below_success = ((253 * htlc_success_tx_weight(chan.funding.get_channel_type()) / 1000) + chan.context.counterparty_dust_limit_satoshis - 1) * 1000; + let htlc_amt_below_success = (htlc_success_tx_fee_sat + chan.context.counterparty_dust_limit_satoshis - 1) * 1000; let htlc_candidate = HTLCCandidate::new(htlc_amt_below_success, HTLCInitiator::RemoteOffered); let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(&chan.funding, Some(htlc_candidate), None); assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 0087376e50f..315d75247d2 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -25,8 +25,8 @@ use crate::events::{ PaymentPurpose, }; use crate::ln::chan_utils::{ - commitment_tx_base_weight, htlc_success_tx_weight, htlc_timeout_tx_weight, - COMMITMENT_TX_WEIGHT_PER_HTLC, OFFERED_HTLC_SCRIPT_WEIGHT, + commitment_tx_base_weight, second_stage_tx_fees_sat, COMMITMENT_TX_WEIGHT_PER_HTLC, + OFFERED_HTLC_SCRIPT_WEIGHT, }; use crate::ln::channel::{ get_holder_selected_channel_reserve_satoshis, Channel, ChannelError, InboundV1Channel, @@ -9865,12 +9865,12 @@ fn do_test_max_dust_htlc_exposure( nondust_htlc_count_in_limit, &ChannelTypeFeatures::empty(), ); - commitment_tx_cost_msat += if on_holder_tx { - htlc_success_tx_weight(&ChannelTypeFeatures::empty()) - } else { - htlc_timeout_tx_weight(&ChannelTypeFeatures::empty()) - } * (initial_feerate as u64 - 253) - * nondust_htlc_count_in_limit; + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = + second_stage_tx_fees_sat(&ChannelTypeFeatures::empty(), initial_feerate as u32 - 253); + let per_htlc_cost = + if on_holder_tx { htlc_success_tx_fee_sat } else { htlc_timeout_tx_fee_sat }; + + commitment_tx_cost_msat += per_htlc_cost * nondust_htlc_count_in_limit; { let mut feerate_lock = chanmon_cfgs[0].fee_estimator.sat_per_kw.lock().unwrap(); *feerate_lock = initial_feerate; @@ -9902,8 +9902,6 @@ fn do_test_max_dust_htlc_exposure( get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, node_a_id); nodes[0].node.handle_accept_channel(node_b_id, &accept_channel); - let channel_type_features = ChannelTypeFeatures::only_static_remote_key(); - let (chan_id, tx, _) = create_funding_transaction(&nodes[0], &node_b_id, 1_000_000, 42); if on_holder_tx { @@ -9947,31 +9945,40 @@ fn do_test_max_dust_htlc_exposure( let (mut route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000); - let (dust_buffer_feerate, max_dust_htlc_exposure_msat) = { + let ( + dust_buffer_feerate, + max_dust_htlc_exposure_msat, + htlc_success_tx_fee_sat, + htlc_timeout_tx_fee_sat, + ) = { let per_peer_state = nodes[0].node.per_peer_state.read().unwrap(); let chan_lock = per_peer_state.get(&node_b_id).unwrap().lock().unwrap(); let chan = chan_lock.channel_by_id.get(&channel_id).unwrap(); + let dust_buffer_feerate = chan.context().get_dust_buffer_feerate(None) as u64; + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( + &chan.as_funded().unwrap().funding.get_channel_type(), + dust_buffer_feerate as u32, + ); ( - chan.context().get_dust_buffer_feerate(None) as u64, + dust_buffer_feerate, chan.context().get_max_dust_htlc_exposure_msat(Some(253)), + htlc_success_tx_fee_sat, + htlc_timeout_tx_fee_sat, ) }; assert_eq!(dust_buffer_feerate, expected_dust_buffer_feerate as u64); let dust_outbound_htlc_on_holder_tx_msat: u64 = - (dust_buffer_feerate * htlc_timeout_tx_weight(&channel_type_features) / 1000 - + open_channel.common_fields.dust_limit_satoshis - - 1) * 1000; + (htlc_timeout_tx_fee_sat + open_channel.common_fields.dust_limit_satoshis - 1) * 1000; let dust_outbound_htlc_on_holder_tx: u64 = max_dust_htlc_exposure_msat / dust_outbound_htlc_on_holder_tx_msat; // Substract 3 sats for multiplier and 2 sats for fixed limit to make sure we are 50% below the dust limit. // This is to make sure we fully use the dust limit. If we don't, we could end up with `dust_ibd_htlc_on_holder_tx` being 1 // while `max_dust_htlc_exposure_msat` is not equal to `dust_outbound_htlc_on_holder_tx_msat`. - let dust_inbound_htlc_on_holder_tx_msat: u64 = - (dust_buffer_feerate * htlc_success_tx_weight(&channel_type_features) / 1000 - + open_channel.common_fields.dust_limit_satoshis - - if multiplier_dust_limit { 3 } else { 2 }) - * 1000; + let dust_inbound_htlc_on_holder_tx_msat: u64 = (htlc_success_tx_fee_sat + + open_channel.common_fields.dust_limit_satoshis + - if multiplier_dust_limit { 3 } else { 2 }) + * 1000; let dust_inbound_htlc_on_holder_tx: u64 = max_dust_htlc_exposure_msat / dust_inbound_htlc_on_holder_tx_msat; @@ -9979,11 +9986,10 @@ fn do_test_max_dust_htlc_exposure( // indeed, dust on both transactions. let dust_htlc_on_counterparty_tx: u64 = 4; let dust_htlc_on_counterparty_tx_msat: u64 = 1_250_000; - let calcd_dust_htlc_on_counterparty_tx_msat: u64 = - (dust_buffer_feerate * htlc_timeout_tx_weight(&channel_type_features) / 1000 - + open_channel.common_fields.dust_limit_satoshis - - if multiplier_dust_limit { 3 } else { 2 }) - * 1000; + let calcd_dust_htlc_on_counterparty_tx_msat: u64 = (htlc_timeout_tx_fee_sat + + open_channel.common_fields.dust_limit_satoshis + - if multiplier_dust_limit { 3 } else { 2 }) + * 1000; assert!(dust_htlc_on_counterparty_tx_msat < dust_inbound_htlc_on_holder_tx_msat); assert!(dust_htlc_on_counterparty_tx_msat < calcd_dust_htlc_on_counterparty_tx_msat); @@ -10288,9 +10294,9 @@ pub fn test_nondust_htlc_excess_fees_are_dust() { // At this point we have somewhere between dust_limit and dust_limit * 2 left in our dust // exposure limit, and we want to max that out using non-dust HTLCs. - 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; + let (htlc_success_tx_fee_sat, _) = + second_stage_tx_fees_sat(&ChannelTypeFeatures::empty(), EXCESS_FEERATE); + let max_htlcs_remaining = dust_limit * 2 / (htlc_success_tx_fee_sat * 1000); assert!( max_htlcs_remaining < chan_utils::max_htlcs(&chan_ty).into(), "We should be able to fill our dust limit without too many HTLCs" @@ -10330,7 +10336,7 @@ pub fn test_nondust_htlc_excess_fees_are_dust() { ); nodes[0].logger.assert_log("lightning::ln::channel", format!("Cannot accept value that would put our total dust exposure at {} over the limit {} on counterparty commitment tx", - 2535000, 2530000), 1); + 2531000, 2530000), 1); check_added_monitors(&nodes[0], 1); // Clear the failed htlc @@ -10432,9 +10438,10 @@ fn do_test_nondust_htlc_fees_dust_exposure_delta(features: ChannelTypeFeatures) let mut expected_dust_exposure_msat = BASE_DUST_EXPOSURE_MSAT + EXCESS_FEERATE * (commitment_tx_base_weight(&features) + COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000 * 1000; + + let (_, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat(&features, EXCESS_FEERATE as u32); if features == ChannelTypeFeatures::only_static_remote_key() { - expected_dust_exposure_msat += - EXCESS_FEERATE * htlc_timeout_tx_weight(&features) / 1000 * 1000; + expected_dust_exposure_msat += htlc_timeout_tx_fee_sat * 1000; assert_eq!(expected_dust_exposure_msat, 533_492); } else { assert_eq!(expected_dust_exposure_msat, 528_492); @@ -10549,12 +10556,13 @@ fn do_test_nondust_htlc_fees_dust_exposure_delta(features: ChannelTypeFeatures) // The `expected_dust_exposure_msat` for the outbound htlc changes in the non-anchor case, as the htlc success and timeout transactions have different weights // only_static_remote_key: 500_492 + 22 * (724 + 172) / 1000 * 1000 + 22 * 703 / 1000 * 1000 = 534_492 + let (htlc_success_tx_fee_sat, _) = second_stage_tx_fees_sat(&features, EXCESS_FEERATE as u32); if features == ChannelTypeFeatures::only_static_remote_key() { expected_dust_exposure_msat = BASE_DUST_EXPOSURE_MSAT + EXCESS_FEERATE * (commitment_tx_base_weight(&features) + COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000 * 1000 - + EXCESS_FEERATE * htlc_success_tx_weight(&features) / 1000 * 1000; + + htlc_success_tx_fee_sat * 1000; assert_eq!(expected_dust_exposure_msat, 534_492); } else { assert_eq!(expected_dust_exposure_msat, 528_492); @@ -10575,9 +10583,10 @@ fn do_test_nondust_htlc_fees_dust_exposure_delta(features: ChannelTypeFeatures) let res = nodes[1].node.send_payment_with_route(route_1_0, payment_hash_1_0, onion, id); unwrap_send_err!(nodes[1], res, true, APIError::ChannelUnavailable { .. }, {}); + let (htlc_success_tx_fee_sat, _) = + second_stage_tx_fees_sat(&features, node_1_dust_buffer_feerate as u32); let dust_limit = if features == ChannelTypeFeatures::only_static_remote_key() { - MIN_CHAN_DUST_LIMIT_SATOSHIS * 1000 - + htlc_success_tx_weight(&features) * node_1_dust_buffer_feerate / 1000 * 1000 + MIN_CHAN_DUST_LIMIT_SATOSHIS * 1000 + htlc_success_tx_fee_sat * 1000 } else { MIN_CHAN_DUST_LIMIT_SATOSHIS * 1000 }; diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs index 4f1f5c581df..66a22efa75c 100644 --- a/lightning/src/ln/htlc_reserve_unit_tests.rs +++ b/lightning/src/ln/htlc_reserve_unit_tests.rs @@ -2,7 +2,7 @@ use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose}; use crate::ln::chan_utils::{ - self, commitment_tx_base_weight, htlc_success_tx_weight, CommitmentTransaction, + self, commitment_tx_base_weight, second_stage_tx_fees_sat, CommitmentTransaction, COMMITMENT_TX_WEIGHT_PER_HTLC, }; use crate::ln::channel::{ @@ -1084,8 +1084,10 @@ pub fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() { push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, &default_config) * 1000; create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, push_amt); + let (htlc_success_tx_fee_sat, _) = + second_stage_tx_fees_sat(&channel_type_features, feerate_per_kw); let dust_amt = crate::ln::channel::MIN_CHAN_DUST_LIMIT_SATOSHIS * 1000 - + feerate_per_kw as u64 * htlc_success_tx_weight(&channel_type_features) / 1000 * 1000 + + htlc_success_tx_fee_sat * 1000 - 1; // In the previous code, routing this dust payment would cause nodes[0] to perceive a channel // reserve violation even though it's a dust HTLC and therefore shouldn't count towards the From af158d3a38a3b76b701adb925cd42867244539a4 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 14 Jul 2025 14:24:36 -0400 Subject: [PATCH 08/16] f Standardize formatting of local branch for htlc fees --- lightning/src/ln/channel.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 5b432a7fce0..99e926b39e0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -292,8 +292,12 @@ impl InboundHTLCOutput { let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat(features, feerate_per_kw); - let htlc_tx_fee_sat = - if !local { htlc_timeout_tx_fee_sat } else { htlc_success_tx_fee_sat }; + let htlc_tx_fee_sat = if !local { + // This is an offered HTLC. + htlc_timeout_tx_fee_sat + } else { + htlc_success_tx_fee_sat + }; self.amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee_sat } } From f9c64f68f16b16af17d157e22b5dbc66f882dae8 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Thu, 12 Jun 2025 10:02:38 -0400 Subject: [PATCH 09/16] ln: set zero second stage fees for zero fee commitments Update second_stage_tx_fees_sat to return zero for zero fee commitments. As is the case for anchors_zero_fee_commitments, this changes ensures that we won't trim the second stage transaction fee off of the HTLC amount. --- lightning/src/ln/chan_utils.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 9c693a0e13c..821e923b5be 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -239,7 +239,9 @@ pub(crate) fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_t pub(super) fn second_stage_tx_fees_sat( channel_type: &ChannelTypeFeatures, feerate_sat_per_1000_weight: u32, ) -> (u64, u64) { - if channel_type.supports_anchors_zero_fee_htlc_tx() { + if channel_type.supports_anchors_zero_fee_htlc_tx() + || channel_type.supports_anchor_zero_fee_commitments() + { (0, 0) } else { ( From e5aadb7f7e1d70e720e0ed3dcbd315b530aa325b Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Thu, 12 Jun 2025 10:05:51 -0400 Subject: [PATCH 10/16] ln: return early and assert in fee calculation and checks When we have zero fee commitments, we don't need to calculate our fee rate or check that it isn't stale because it is always zero. Co-authored-by: Matt Corallo --- lightning/src/ln/channel.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 99e926b39e0..f0426e6b6df 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5067,6 +5067,11 @@ where let context = self; assert!(funding.is_outbound()); + if funding.get_channel_type().supports_anchor_zero_fee_commitments() { + debug_assert_eq!(context.feerate_per_kw, 0); + return 0; + } + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( funding.get_channel_type(), context.feerate_per_kw, ); @@ -5168,11 +5173,16 @@ where fn next_remote_commit_tx_fee_msat( &self, funding: &FundingScope, htlc: Option, fee_spike_buffer_htlc: Option<()>, ) -> u64 { - debug_assert!(htlc.is_some() || fee_spike_buffer_htlc.is_some(), "At least one of the options must be set"); - let context = self; assert!(!funding.is_outbound()); + if funding.get_channel_type().supports_anchor_zero_fee_commitments() { + debug_assert_eq!(context.feerate_per_kw, 0); + return 0 + } + + debug_assert!(htlc.is_some() || fee_spike_buffer_htlc.is_some(), "At least one of the options must be set"); + let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = second_stage_tx_fees_sat( funding.get_channel_type(), context.feerate_per_kw, ); @@ -7974,6 +7984,12 @@ where // unable to increase the fee, we don't try to force-close directly here. return Ok(()); } + + if self.funding.get_channel_type().supports_anchor_zero_fee_commitments() { + debug_assert_eq!(self.context.feerate_per_kw, 0); + return Ok(()); + } + if self.context.feerate_per_kw < min_feerate { log_info!(logger, "Closing channel as feerate of {} is below required {} (the minimum required rate over the past day)", From fb6970ec11af796a17aad9add8f51b762d3b4c79 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 7 Jul 2025 13:38:22 -0400 Subject: [PATCH 11/16] ln: do not send and reject receive of update_fee for zero fee commits Co-authored-by: Matt Corallo --- lightning/src/chain/package.rs | 3 ++ lightning/src/ln/channel.rs | 7 ++++ lightning/src/ln/update_fee_tests.rs | 56 ++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 467e2179b43..1f991b3e464 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -99,6 +99,9 @@ pub(crate) fn verify_channel_type_features(channel_type_features: &Option match action { + ErrorAction::DisconnectPeerWithWarning { ref msg, .. } => { + assert_eq!(msg.channel_id, channel_id); + assert!(msg + .data + .contains("Update fee message received for zero fee commitment channel")); + }, + _ => panic!("Expected DisconnectPeerWithWarning, got {:?}", action), + }, + _ => panic!("Expected HandleError event, got {:?}", events_1[0]), + } + assert_zero_fee(); +} From 0c709f82ed367c6acbba84ca9da8f8cd0c06085d Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 15 Jul 2025 10:47:51 -0400 Subject: [PATCH 12/16] f move check for fee update into channelmanager The logic of "no feerate change has happened" belongs in channel manager (previously it just didn't work for zero fees). Change channel to just assert that we're not making these calls (but don't panic, since this isn't going to put us at risk of lost funds). --- lightning/src/ln/channel.rs | 8 ++++---- lightning/src/ln/channelmanager.rs | 13 ++++++++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8bc19cd857e..1a820be54cf 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4335,10 +4335,6 @@ where F::Target: FeeEstimator, L::Target: Logger, { - if funding.get_channel_type().supports_anchor_zero_fee_commitments() { - return false; - } - // Before proposing a feerate update, check that we can actually afford the new fee. let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate( &fee_estimator, funding.get_channel_type(), @@ -7727,6 +7723,10 @@ where panic!("Cannot update fee while peer is disconnected/we're awaiting a monitor update (ChannelManager should have caught this)"); } + // Sending a fee update for zero fee commitments will trigger a warning and disconnect + // from our peer, but does not result in a loss of funds so we do not panic here. + debug_assert!(!self.funding.get_channel_type().supports_anchor_zero_fee_commitments()); + let can_send_update_fee = core::iter::once(&self.funding) .chain(self.pending_funding.iter()) .all(|funding| self.context.can_send_update_fee(funding, feerate_per_kw, fee_estimator, logger)); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b410494ecce..8e359741826 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6963,10 +6963,17 @@ where let logger = WithChannelContext::from(&self.logger, &chan.context, None); - // If the feerate has decreased by less than half, don't bother - if new_feerate <= chan.context.get_feerate_sat_per_1000_weight() && new_feerate * 2 > chan.context.get_feerate_sat_per_1000_weight() { - return NotifyOption::SkipPersistNoEvents; + let current_feerate = chan.context.get_feerate_sat_per_1000_weight(); + let update_fee_required = match new_feerate.cmp(¤t_feerate) { + cmp::Ordering::Greater => true, + cmp::Ordering::Equal => false, + // Only bother with a fee update if feerate has decreased at least half. + cmp::Ordering::Less => new_feerate * 2 <= current_feerate, + }; + if !update_fee_required { + return NotifyOption::SkipPersistNoEvents } + if !chan.context.is_live() { log_trace!(logger, "Channel {} does not qualify for a feerate change from {} to {} as it cannot currently be updated (probably the peer is disconnected).", chan_id, chan.context.get_feerate_sat_per_1000_weight(), new_feerate); From 6ab70e1a383541297be0648a0067a670f5c3a015 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 14 Jul 2025 14:51:49 -0400 Subject: [PATCH 13/16] f Fix typo in update fee test --- lightning/src/ln/update_fee_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/ln/update_fee_tests.rs b/lightning/src/ln/update_fee_tests.rs index 1d985459eff..95a2dad9169 100644 --- a/lightning/src/ln/update_fee_tests.rs +++ b/lightning/src/ln/update_fee_tests.rs @@ -1377,7 +1377,7 @@ pub fn do_can_afford_given_trimmed_htlcs(inequality_regions: core::cmp::Ordering } #[test] -pub fn test_zero_fee_commiments_no_update_fee() { +pub fn test_zero_fee_commitments_no_update_fee() { // Tests that option_zero_fee_commitment channels do not sent update_fee messages, and that // they'll disconnect and warn if they receive them. let mut cfg = test_default_channel_config(); From e057a1937a22f9287b1254481e5e886638d7eb83 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 9 Jul 2025 16:41:45 -0400 Subject: [PATCH 14/16] ln: add test helper for fee spike buffer test and use TxBuilder Update test helper in preparation to test zero fee commitments, where the local balance will differ from the previously hardcoded value. --- lightning/src/ln/htlc_reserve_unit_tests.rs | 85 ++++++++++++--------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs index 66a22efa75c..0d983d7c49b 100644 --- a/lightning/src/ln/htlc_reserve_unit_tests.rs +++ b/lightning/src/ln/htlc_reserve_unit_tests.rs @@ -2,12 +2,11 @@ use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose}; use crate::ln::chan_utils::{ - self, commitment_tx_base_weight, second_stage_tx_fees_sat, CommitmentTransaction, - COMMITMENT_TX_WEIGHT_PER_HTLC, + self, commitment_tx_base_weight, second_stage_tx_fees_sat, COMMITMENT_TX_WEIGHT_PER_HTLC, }; use crate::ln::channel::{ get_holder_selected_channel_reserve_satoshis, Channel, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, - MIN_AFFORDABLE_HTLC_COUNT, + MIN_AFFORDABLE_HTLC_COUNT, MIN_CHAN_DUST_LIMIT_SATOSHIS, }; use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder}; use crate::ln::functional_test_utils::*; @@ -16,6 +15,7 @@ use crate::ln::onion_utils::{self, AttributionData}; use crate::ln::outbound_payment::RecipientOnionFields; use crate::routing::router::PaymentParameters; use crate::sign::ecdsa::EcdsaChannelSigner; +use crate::sign::tx_builder::{SpecTxBuilder, TxBuilder}; use crate::types::features::ChannelTypeFeatures; use crate::types::payment::PaymentPreimage; use crate::util::config::UserConfig; @@ -772,16 +772,22 @@ pub fn test_basic_channel_reserve() { } #[xtest(feature = "_externalize_tests")] -pub fn test_fee_spike_violation_fails_htlc() { +fn test_fee_spike_violation_fails_htlc() { + do_test_fee_spike_buffer(None, true) +} +pub fn do_test_fee_spike_buffer(cfg: Option, htlc_fails: bool) { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[cfg.clone(), cfg]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let node_a_id = nodes[0].node.get_our_node_id(); let node_b_id = nodes[1].node.get_our_node_id(); - let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000); + let chan_amt_sat = 100000; + let push_amt_msat = 95000000; + let chan = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, chan_amt_sat, push_amt_msat); let (mut route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 3460000); @@ -792,11 +798,12 @@ pub fn test_fee_spike_violation_fails_htlc() { let cur_height = nodes[1].node.best_block.read().unwrap().height + 1; + let payment_amt_msat = 3460001; let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv); let recipient_onion_fields = RecipientOnionFields::secret_only(payment_secret); let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads( &route.paths[0], - 3460001, + payment_amt_msat, &recipient_onion_fields, cur_height, &None, @@ -858,16 +865,15 @@ pub fn test_fee_spike_violation_fails_htlc() { // Build the remote commitment transaction so we can sign it, and then later use the // signature for the commitment_signed message. - let local_chan_balance = 1313; - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { offered: false, - amount_msat: 3460001, + amount_msat: payment_amt_msat, cltv_expiry: htlc_cltv, payment_hash, transaction_output_index: Some(1), }; + let local_chan_balance_msat = chan_amt_sat * 1000 - push_amt_msat; let commitment_number = INITIAL_COMMITMENT_NUMBER - 1; let res = { @@ -877,15 +883,17 @@ pub fn test_fee_spike_violation_fails_htlc() { let channel = get_channel_ref!(nodes[0], nodes[1], per_peer_lock, peer_state_lock, chan.2); let chan_signer = channel.as_funded().unwrap().get_signer(); - let commitment_tx = CommitmentTransaction::new( + let (commitment_tx, _stats) = SpecTxBuilder {}.build_commitment_transaction( + false, commitment_number, &remote_point, - 95000, - local_chan_balance, - feerate_per_kw, - vec![accepted_htlc_info], - &channel.funding().channel_transaction_parameters.as_counterparty_broadcastable(), + &channel.funding().channel_transaction_parameters, &secp_ctx, + local_chan_balance_msat, + vec![accepted_htlc_info], + feerate_per_kw, + MIN_CHAN_DUST_LIMIT_SATOSHIS, + &nodes[0].logger, ); let params = &channel.funding().channel_transaction_parameters; chan_signer @@ -918,28 +926,35 @@ pub fn test_fee_spike_violation_fails_htlc() { }; nodes[1].node.handle_revoke_and_ack(node_a_id, &raa_msg); expect_pending_htlcs_forwardable!(nodes[1]); - expect_htlc_handling_failed_destinations!( - nodes[1].node.get_and_clear_pending_events(), - &[HTLCHandlingFailureType::Receive { payment_hash }] - ); - let events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - // Make sure the HTLC failed in the way we expect. - match events[0] { - MessageSendEvent::UpdateHTLCs { - updates: msgs::CommitmentUpdate { ref update_fail_htlcs, .. }, - .. - } => { - assert_eq!(update_fail_htlcs.len(), 1); - update_fail_htlcs[0].clone() - }, - _ => panic!("Unexpected event"), - }; - nodes[1].logger.assert_log("lightning::ln::channel", + if htlc_fails { + expect_htlc_handling_failed_destinations!( + nodes[1].node.get_and_clear_pending_events(), + &[HTLCHandlingFailureType::Receive { payment_hash }] + ); + + let events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + + // Make sure the HTLC failed in the way we expect. + match events[0] { + MessageSendEvent::UpdateHTLCs { + updates: msgs::CommitmentUpdate { ref update_fail_htlcs, .. }, + .. + } => { + assert_eq!(update_fail_htlcs.len(), 1); + update_fail_htlcs[0].clone() + }, + _ => panic!("Unexpected event"), + }; + nodes[1].logger.assert_log("lightning::ln::channel", format!("Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", raa_msg.channel_id), 1); - check_added_monitors(&nodes[1], 3); + check_added_monitors(&nodes[1], 3); + } else { + expect_payment_claimable!(nodes[1], payment_hash, payment_secret, payment_amt_msat); + check_added_monitors(&nodes[1], 2); + } } #[xtest(feature = "_externalize_tests")] From f83f683a75b0e89c6ac003bc79e0a3dc54fd6761 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Thu, 3 Jul 2025 11:50:54 -0400 Subject: [PATCH 15/16] ln: do not maintain fee spike buffer for zero fee commitment channels --- lightning/src/ln/channel.rs | 31 ++++++++++++++++----- lightning/src/ln/htlc_reserve_unit_tests.rs | 10 +++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1a820be54cf..a75e6983179 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4414,14 +4414,22 @@ where funding.get_channel_type() ); - // `Some(())` is for the fee spike buffer we keep for the remote. This deviates from - // the spec because the fee spike buffer requirement doesn't exist on the receiver's - // side, only on the sender's. Note that with anchor outputs we are no longer as - // sensitive to fee spikes, so we need to account for them. + // `Some(())` is for the fee spike buffer we keep for the remote if the channel is + // not zero fee.. This deviates from the spec because the fee spike buffer requirement + // doesn't exist on the receiver's side, only on the sender's. Note that with anchor + // outputs we are no longer as sensitive to fee spikes, so we need to account for them. // // A `None` `HTLCCandidate` is used as in this case because we're already accounting for // the incoming HTLC as it has been fully committed by both sides. - let mut remote_fee_cost_incl_stuck_buffer_msat = self.next_remote_commit_tx_fee_msat(funding, None, Some(())); + let fee_spike_buffer_htlc = if funding.get_channel_type().supports_anchor_zero_fee_commitments() { + None + } else { + Some(()) + }; + + let mut remote_fee_cost_incl_stuck_buffer_msat = self.next_remote_commit_tx_fee_msat( + funding, None, fee_spike_buffer_htlc, + ); if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; } @@ -4947,11 +4955,18 @@ where // and the answer will in turn change the amount itself — making it a circular // dependency. // This complicates the computation around dust-values, up to the one-htlc-value. + let fee_spike_buffer_htlc = if funding.get_channel_type().supports_anchor_zero_fee_commitments() { + None + } else { + Some(()) + }; + let real_dust_limit_timeout_sat = real_htlc_timeout_tx_fee_sat + context.holder_dust_limit_satoshis; let htlc_above_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000, HTLCInitiator::LocalOffered); - let mut max_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(funding, htlc_above_dust, Some(())); + let mut max_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(&funding, htlc_above_dust, fee_spike_buffer_htlc); let htlc_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000 - 1, HTLCInitiator::LocalOffered); - let mut min_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(funding, htlc_dust, Some(())); + let mut min_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(&funding, htlc_dust, fee_spike_buffer_htlc); + if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { max_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; min_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; @@ -5069,6 +5084,7 @@ where if funding.get_channel_type().supports_anchor_zero_fee_commitments() { debug_assert_eq!(context.feerate_per_kw, 0); + debug_assert!(fee_spike_buffer_htlc.is_none()); return 0; } @@ -5178,6 +5194,7 @@ where if funding.get_channel_type().supports_anchor_zero_fee_commitments() { debug_assert_eq!(context.feerate_per_kw, 0); + debug_assert!(fee_spike_buffer_htlc.is_none()); return 0 } diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs index 0d983d7c49b..90d1d3d424b 100644 --- a/lightning/src/ln/htlc_reserve_unit_tests.rs +++ b/lightning/src/ln/htlc_reserve_unit_tests.rs @@ -775,6 +775,16 @@ pub fn test_basic_channel_reserve() { fn test_fee_spike_violation_fails_htlc() { do_test_fee_spike_buffer(None, true) } + +#[test] +fn test_zero_fee_commitments_no_fee_spike_buffer() { + let mut cfg = test_default_channel_config(); + cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + cfg.manually_accept_inbound_channels = true; + + do_test_fee_spike_buffer(Some(cfg), false) +} + pub fn do_test_fee_spike_buffer(cfg: Option, htlc_fails: bool) { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); From f76a270ac6d2532c288d5313594e8acbefece3a8 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 14 Jul 2025 14:52:48 -0400 Subject: [PATCH 16/16] f Remove unnecessary second . --- lightning/src/ln/channel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a75e6983179..32fd4694536 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4415,7 +4415,7 @@ where ); // `Some(())` is for the fee spike buffer we keep for the remote if the channel is - // not zero fee.. This deviates from the spec because the fee spike buffer requirement + // not zero fee. This deviates from the spec because the fee spike buffer requirement // doesn't exist on the receiver's side, only on the sender's. Note that with anchor // outputs we are no longer as sensitive to fee spikes, so we need to account for them. //