From 098b98ef8e6711de5832397e76825d916dd65408 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Thu, 26 Jun 2025 19:00:53 +0000 Subject: [PATCH 1/8] Remove the last raw cast in `ChannelContext::build_commitment_stats` --- lightning/src/ln/channel.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1f3814598c2..b453b28c28b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4433,7 +4433,8 @@ where let mut non_dust_htlc_count = 0; let mut remote_htlc_total_msat = 0; let mut local_htlc_total_msat = 0; - let mut value_to_self_msat_offset = 0; + let mut value_to_self_claimed_msat = 0; + let mut value_to_remote_claimed_msat = 0; let mut feerate_per_kw = self.feerate_per_kw; if let Some((feerate, update_state)) = self.pending_update_fee { @@ -4456,7 +4457,7 @@ where remote_htlc_total_msat += htlc.amount_msat; } else { if htlc.state.preimage().is_some() { - value_to_self_msat_offset += htlc.amount_msat as i64; + value_to_self_claimed_msat += htlc.amount_msat; } } }; @@ -4469,19 +4470,17 @@ where local_htlc_total_msat += htlc.amount_msat; } else { if htlc.state.preimage().is_some() { - value_to_self_msat_offset -= htlc.amount_msat as i64; + value_to_remote_claimed_msat += htlc.amount_msat; } } }; // # Panics // - // While we expect `value_to_self_msat_offset` to be negative in some cases, the value going - // to each party MUST be 0 or positive, even if all HTLCs pending in the commitment clear by - // failure. + // After all HTLC claims have been accounted for, the local balance MUST remain greater than or equal to 0. + + let mut value_to_self_msat = (funding.value_to_self_msat + value_to_self_claimed_msat).checked_sub(value_to_remote_claimed_msat).unwrap(); - // TODO: When MSRV >= 1.66.0, use u64::checked_add_signed - let mut value_to_self_msat = (funding.value_to_self_msat as i64 + value_to_self_msat_offset).try_into().unwrap(); let mut value_to_remote_msat = (funding.get_value_satoshis() * 1000).checked_sub(value_to_self_msat).unwrap(); value_to_self_msat = value_to_self_msat.checked_sub(local_htlc_total_msat).unwrap(); value_to_remote_msat = value_to_remote_msat.checked_sub(remote_htlc_total_msat).unwrap(); From ebf6b13cc3a97f63f351a2afd76b6bf34ea5f688 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Fri, 9 May 2025 23:09:46 +0000 Subject: [PATCH 2/8] Delete `CommitmentStats.{broadcaster_dust_limit_sat, feerate_per_kw}` In preparation for the upcoming `TxBuilder` API, we remove any fields in `CommitmentStats` which will not be set by `TxBuilder`. --- lightning/src/ln/channel.rs | 43 +++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b453b28c28b..d8843a941d9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1130,10 +1130,8 @@ struct CommitmentData<'a> { /// A struct gathering stats on a commitment transaction, either local or remote. struct CommitmentStats { - feerate_per_kw: u32, // the feerate of the commitment transaction - total_fee_sat: u64, // the total fee included in the transaction - total_anchors_sat: u64, // the sum of the anchors' amounts - broadcaster_dust_limit_sat: u64, // the broadcaster's dust limit + total_fee_sat: u64, // the total fee included in the transaction + total_anchors_sat: u64, // the sum of the anchors' amounts local_balance_before_fee_anchors_msat: u64, // local balance before fees and anchors *not* considering dust limits remote_balance_before_fee_anchors_msat: u64, // remote balance before fees and anchors *not* considering dust limits } @@ -4424,18 +4422,9 @@ where Ok(()) } - /// Builds stats on a potential commitment transaction build, without actually building the - /// commitment transaction. See `build_commitment_transaction` for further docs. #[inline] #[rustfmt::skip] - fn build_commitment_stats(&self, funding: &FundingScope, local: bool, generated_by_local: bool) -> CommitmentStats { - let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis }; - let mut non_dust_htlc_count = 0; - let mut remote_htlc_total_msat = 0; - let mut local_htlc_total_msat = 0; - let mut value_to_self_claimed_msat = 0; - let mut value_to_remote_claimed_msat = 0; - + fn get_commitment_feerate(&self, funding: &FundingScope, generated_by_local: bool) -> u32 { let mut feerate_per_kw = self.feerate_per_kw; if let Some((feerate, update_state)) = self.pending_update_fee { if match update_state { @@ -4443,12 +4432,29 @@ where // pending_inbound_htlcs below. FeeUpdateState::RemoteAnnounced => { debug_assert!(!funding.is_outbound()); !generated_by_local }, FeeUpdateState::AwaitingRemoteRevokeToAnnounce => { debug_assert!(!funding.is_outbound()); !generated_by_local }, - FeeUpdateState::Outbound => { assert!(funding.is_outbound()); generated_by_local }, + FeeUpdateState::Outbound => { assert!(funding.is_outbound()); generated_by_local }, } { feerate_per_kw = feerate; } } + feerate_per_kw + } + + /// Builds stats on a potential commitment transaction build, without actually building the + /// commitment transaction. See `build_commitment_transaction` for further docs. + #[inline] + #[rustfmt::skip] + fn build_commitment_stats(&self, funding: &FundingScope, local: bool, generated_by_local: bool) -> CommitmentStats { + let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis }; + let mut non_dust_htlc_count = 0; + let mut remote_htlc_total_msat = 0; + let mut local_htlc_total_msat = 0; + let mut value_to_self_claimed_msat = 0; + let mut value_to_remote_claimed_msat = 0; + + let feerate_per_kw = self.get_commitment_feerate(funding, generated_by_local); + for htlc in self.pending_inbound_htlcs.iter() { if htlc.state.included_in_commitment(generated_by_local) { if !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type()) { @@ -4504,10 +4510,8 @@ where let total_anchors_sat = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; CommitmentStats { - feerate_per_kw, total_fee_sat, total_anchors_sat, - broadcaster_dust_limit_sat, local_balance_before_fee_anchors_msat: value_to_self_msat, remote_balance_before_fee_anchors_msat: value_to_remote_msat, } @@ -4531,12 +4535,13 @@ where fn build_commitment_transaction(&self, funding: &FundingScope, commitment_number: u64, per_commitment_point: &PublicKey, local: bool, generated_by_local: bool, logger: &L) -> CommitmentData where L::Target: Logger { + let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis }; + let feerate_per_kw = self.get_commitment_feerate(funding, generated_by_local); + let stats = self.build_commitment_stats(funding, local, generated_by_local); let CommitmentStats { - feerate_per_kw, total_fee_sat, total_anchors_sat, - broadcaster_dust_limit_sat, local_balance_before_fee_anchors_msat, remote_balance_before_fee_anchors_msat } = stats; From 5688658d2b4d8202cc7e532e885b9334b0f5efaf Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Tue, 1 Jul 2025 01:21:13 +0000 Subject: [PATCH 3/8] Delete `CommitmentStats.total_anchors_sat` The upcoming `TxBuilder` API will sum the msat value of any anchors and custom outputs on commitment transactions into `{local, remote}_balance_before_fee_msat`. --- lightning/src/ln/channel.rs | 55 +++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index d8843a941d9..47b8e2eb723 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1130,10 +1130,12 @@ struct CommitmentData<'a> { /// A struct gathering stats on a commitment transaction, either local or remote. struct CommitmentStats { - total_fee_sat: u64, // the total fee included in the transaction - total_anchors_sat: u64, // the sum of the anchors' amounts - local_balance_before_fee_anchors_msat: u64, // local balance before fees and anchors *not* considering dust limits - remote_balance_before_fee_anchors_msat: u64, // remote balance before fees and anchors *not* considering dust limits + /// The total fee included in the commitment transaction + commit_tx_fee_sat: u64, + /// The local balance before fees *not* considering dust limits + local_balance_before_fee_msat: u64, + /// The remote balance before fees *not* considering dust limits + remote_balance_before_fee_msat: u64, } /// Used when calculating whether we or the remote can afford an additional HTLC. @@ -4235,7 +4237,7 @@ where if update_fee { debug_assert!(!funding.is_outbound()); let counterparty_reserve_we_require_msat = funding.holder_selected_channel_reserve_satoshis * 1000; - if commitment_data.stats.remote_balance_before_fee_anchors_msat < commitment_data.stats.total_fee_sat * 1000 + commitment_data.stats.total_anchors_sat * 1000 + counterparty_reserve_we_require_msat { + if commitment_data.stats.remote_balance_before_fee_msat < commitment_data.stats.commit_tx_fee_sat * 1000 + counterparty_reserve_we_require_msat { return Err(ChannelError::close("Funding remote cannot afford proposed new fee".to_owned())); } } @@ -4251,7 +4253,7 @@ where && info.next_holder_htlc_id == self.next_holder_htlc_id && info.next_counterparty_htlc_id == self.next_counterparty_htlc_id && info.feerate == self.feerate_per_kw { - assert_eq!(commitment_data.stats.total_fee_sat, info.fee / 1000); + assert_eq!(commitment_data.stats.commit_tx_fee_sat, info.fee / 1000); } } } @@ -4327,8 +4329,8 @@ where &holder_commitment_point.current_point(), true, true, logger, ); let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.tx.nondust_htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, funding.get_channel_type()) * 1000; - let holder_balance_msat = commitment_data.stats.local_balance_before_fee_anchors_msat - htlc_stats.outbound_holding_cell_msat; - if holder_balance_msat < buffer_fee_msat + commitment_data.stats.total_anchors_sat * 1000 + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { + let holder_balance_msat = commitment_data.stats.local_balance_before_fee_msat - htlc_stats.outbound_holding_cell_msat; + if holder_balance_msat < buffer_fee_msat + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { //TODO: auto-close after a number of failures? log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw); return false; @@ -4506,14 +4508,26 @@ where broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat); } - let total_fee_sat = commit_tx_fee_sat(feerate_per_kw, non_dust_htlc_count, &funding.channel_transaction_parameters.channel_type_features); + let commit_tx_fee_sat = commit_tx_fee_sat(feerate_per_kw, non_dust_htlc_count, &funding.channel_transaction_parameters.channel_type_features); let total_anchors_sat = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; + // We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater + // than or equal to `total_anchors_sat`. + // + // This is because when the remote party sends an `update_fee` message, we build the new + // commitment transaction *before* checking whether the remote party's balance is enough to + // cover the total anchor sum. + + if funding.is_outbound() { + value_to_self_msat = value_to_self_msat.saturating_sub(total_anchors_sat * 1000); + } else { + value_to_remote_msat = value_to_remote_msat.saturating_sub(total_anchors_sat * 1000); + } + CommitmentStats { - total_fee_sat, - total_anchors_sat, - local_balance_before_fee_anchors_msat: value_to_self_msat, - remote_balance_before_fee_anchors_msat: value_to_remote_msat, + commit_tx_fee_sat, + local_balance_before_fee_msat: value_to_self_msat, + remote_balance_before_fee_msat: value_to_remote_msat, } } @@ -4540,10 +4554,9 @@ where let stats = self.build_commitment_stats(funding, local, generated_by_local); let CommitmentStats { - total_fee_sat, - total_anchors_sat, - local_balance_before_fee_anchors_msat, - remote_balance_before_fee_anchors_msat + commit_tx_fee_sat, + local_balance_before_fee_msat, + remote_balance_before_fee_msat } = stats; let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len(); @@ -4607,16 +4620,16 @@ where }; // We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater - // than or equal to the sum of `total_fee_sat` and `total_anchors_sat`. + // than or equal to `commit_tx_fee_sat`. // // This is because when the remote party sends an `update_fee` message, we build the new // commitment transaction *before* checking whether the remote party's balance is enough to - // cover the total fee and the anchors. + // cover the total fee. let (value_to_self, value_to_remote) = if funding.is_outbound() { - ((local_balance_before_fee_anchors_msat / 1000).saturating_sub(total_anchors_sat).saturating_sub(total_fee_sat), remote_balance_before_fee_anchors_msat / 1000) + ((local_balance_before_fee_msat / 1000).saturating_sub(commit_tx_fee_sat), remote_balance_before_fee_msat / 1000) } else { - (local_balance_before_fee_anchors_msat / 1000, (remote_balance_before_fee_anchors_msat / 1000).saturating_sub(total_anchors_sat).saturating_sub(total_fee_sat)) + (local_balance_before_fee_msat / 1000, (remote_balance_before_fee_msat / 1000).saturating_sub(commit_tx_fee_sat)) }; let mut to_broadcaster_value_sat = if local { value_to_self } else { value_to_remote }; From aacaf774fa400031bd8463032dd78a930f5f3e21 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Sat, 28 Jun 2025 02:14:19 +0000 Subject: [PATCH 4/8] Account for newly trimmed HTLCs in `ChannelContext::can_send_update_fee` We previously were not accounting for the decrease in weight due to newly trimmed HTLCs at a proposed higher feerate, and hence overestimated the total fees required to set such feerate on the commitment transaction. While this only applied to `only_static_remote_key` channels, this could have led us to determine that we could not afford a proposed higher feerate when we in fact could afford it. --- lightning/src/ln/channel.rs | 24 ++-- lightning/src/ln/update_fee_tests.rs | 182 +++++++++++++++++++++++++++ 2 files changed, 192 insertions(+), 14 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 47b8e2eb723..207f8a7bcdb 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4314,8 +4314,7 @@ where #[rustfmt::skip] fn can_send_update_fee( - &self, funding: &FundingScope, holder_commitment_point: &HolderCommitmentPoint, - feerate_per_kw: u32, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, + &self, funding: &FundingScope, feerate_per_kw: u32, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) -> bool where F::Target: FeeEstimator, @@ -4324,13 +4323,10 @@ where // 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 htlc_stats = self.get_pending_htlc_stats(funding, Some(feerate_per_kw), dust_exposure_limiting_feerate); - let commitment_data = self.build_commitment_transaction( - funding, holder_commitment_point.transaction_number(), - &holder_commitment_point.current_point(), true, true, logger, - ); - let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.tx.nondust_htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, funding.get_channel_type()) * 1000; - let holder_balance_msat = commitment_data.stats.local_balance_before_fee_msat - htlc_stats.outbound_holding_cell_msat; - if holder_balance_msat < buffer_fee_msat + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { + 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; + // Note that `stats.commit_tx_fee_sat` accounts for any HTLCs that transition from non-dust to dust under a higher feerate (in the case where HTLC-transactions pay endogenous fees). + if holder_balance_msat < stats.commit_tx_fee_sat * 1000 + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { //TODO: auto-close after a number of failures? log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw); return false; @@ -4447,7 +4443,7 @@ where /// commitment transaction. See `build_commitment_transaction` for further docs. #[inline] #[rustfmt::skip] - fn build_commitment_stats(&self, funding: &FundingScope, local: bool, generated_by_local: bool) -> CommitmentStats { + fn build_commitment_stats(&self, funding: &FundingScope, local: bool, generated_by_local: bool, feerate_per_kw: Option, fee_buffer_nondust_htlcs: Option) -> CommitmentStats { let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis }; let mut non_dust_htlc_count = 0; let mut remote_htlc_total_msat = 0; @@ -4455,7 +4451,7 @@ where let mut value_to_self_claimed_msat = 0; let mut value_to_remote_claimed_msat = 0; - let feerate_per_kw = self.get_commitment_feerate(funding, generated_by_local); + let feerate_per_kw = feerate_per_kw.unwrap_or_else(|| self.get_commitment_feerate(funding, generated_by_local)); for htlc in self.pending_inbound_htlcs.iter() { if htlc.state.included_in_commitment(generated_by_local) { @@ -4508,7 +4504,7 @@ where broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat); } - let commit_tx_fee_sat = commit_tx_fee_sat(feerate_per_kw, non_dust_htlc_count, &funding.channel_transaction_parameters.channel_type_features); + let commit_tx_fee_sat = commit_tx_fee_sat(feerate_per_kw, non_dust_htlc_count + fee_buffer_nondust_htlcs.unwrap_or(0), &funding.channel_transaction_parameters.channel_type_features); let total_anchors_sat = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; // We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater @@ -4552,7 +4548,7 @@ where let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis }; let feerate_per_kw = self.get_commitment_feerate(funding, generated_by_local); - let stats = self.build_commitment_stats(funding, local, generated_by_local); + let stats = self.build_commitment_stats(funding, local, generated_by_local, None, None); let CommitmentStats { commit_tx_fee_sat, local_balance_before_fee_msat, @@ -7659,7 +7655,7 @@ where let can_send_update_fee = core::iter::once(&self.funding) .chain(self.pending_funding.iter()) - .all(|funding| self.context.can_send_update_fee(funding, &self.holder_commitment_point, feerate_per_kw, fee_estimator, logger)); + .all(|funding| self.context.can_send_update_fee(funding, feerate_per_kw, fee_estimator, logger)); if !can_send_update_fee { return None; } diff --git a/lightning/src/ln/update_fee_tests.rs b/lightning/src/ln/update_fee_tests.rs index 8fa508ecf93..3b324dd8892 100644 --- a/lightning/src/ln/update_fee_tests.rs +++ b/lightning/src/ln/update_fee_tests.rs @@ -1193,3 +1193,185 @@ pub fn do_cannot_afford_on_holding_cell_release( nodes[0].logger.assert_log("lightning::ln::channel", err, 1); } } + +#[xtest(feature = "_externalize_tests")] +pub fn can_afford_given_trimmed_htlcs() { + do_can_afford_given_trimmed_htlcs(core::cmp::Ordering::Equal); + do_can_afford_given_trimmed_htlcs(core::cmp::Ordering::Greater); + do_can_afford_given_trimmed_htlcs(core::cmp::Ordering::Less); +} + +pub fn do_can_afford_given_trimmed_htlcs(inequality_regions: core::cmp::Ordering) { + // Test that when we check whether we can afford a feerate update, we account for the + // decrease in the weight of the commitment transaction due to newly trimmed HTLCs at the higher feerate. + // + // Place a non-dust HTLC on the transaction, increase the feerate such that the HTLC + // gets trimmed, and finally check whether we were able to afford the new feerate. + + let channel_type = ChannelTypeFeatures::only_static_remote_key(); + let can_afford = match inequality_regions { + core::cmp::Ordering::Less => false, + core::cmp::Ordering::Equal => true, + core::cmp::Ordering::Greater => true, + }; + let inequality_boundary_offset = match inequality_regions { + core::cmp::Ordering::Less => 0, + core::cmp::Ordering::Equal => 1, + core::cmp::Ordering::Greater => 2, + }; + + let chanmon_cfgs = create_chanmon_cfgs(2); + + let mut default_config = test_default_channel_config(); + default_config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = + 100; + + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(default_config.clone()), Some(default_config)]); + + 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(); + + // We will update the feerate from 253sat/kw to 1000sat/kw + let target_feerate = 1000; + // Set a HTLC amount that is non-dust at 253sat/kw and dust at 1000sat/kw + let node_0_inbound_htlc_amount_sat = 750; + + // This is the number of HTLCs that `can_send_update_fee` will account for when checking + // whether node 0 can afford the target feerate. We do not include the inbound HTLC we will send, + // as that HTLC will be trimmed at the new feerate. + let buffer_tx_fee_sat = chan_utils::commit_tx_fee_sat( + target_feerate, + crate::ln::channel::CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, + &channel_type, + ); + let channel_reserve_satoshis = 1000; + + let channel_value_sat = 100_000; + let node_0_balance_sat = + (buffer_tx_fee_sat + channel_reserve_satoshis) - 1 + inequality_boundary_offset; + let node_1_balance_sat = channel_value_sat - node_0_balance_sat; + + let chan_id = + create_chan_between_nodes_with_value(&nodes[0], &nodes[1], channel_value_sat, 0).3; + { + // Double check the reserve here + let per_peer_state_lock; + let mut peer_state_lock; + let chan = + get_channel_ref!(nodes[1], nodes[0], per_peer_state_lock, peer_state_lock, chan_id); + assert_eq!( + chan.funding().holder_selected_channel_reserve_satoshis, + channel_reserve_satoshis + ); + } + + // Set node 0's balance at some offset from the inequality boundary + send_payment(&nodes[0], &[&nodes[1]], node_1_balance_sat * 1000); + + // Route the HTLC from node 1 to node 0 + route_payment(&nodes[1], &[&nodes[0]], node_0_inbound_htlc_amount_sat * 1000); + + // Confirm the feerate on node 0's commitment transaction + { + let expected_tx_fee_sat = chan_utils::commit_tx_fee_sat(253, 1, &channel_type); + let commitment_tx = get_local_commitment_txn!(nodes[0], chan_id)[0].clone(); + + let mut actual_fee = commitment_tx + .output + .iter() + .map(|output| output.value.to_sat()) + .reduce(|acc, value| acc + value) + .unwrap(); + actual_fee = channel_value_sat - actual_fee; + assert_eq!(expected_tx_fee_sat, actual_fee); + + // The HTLC is non-dust... + assert_eq!(commitment_tx.output.len(), 3); + } + + { + // Bump the feerate + let mut feerate_lock = chanmon_cfgs[0].fee_estimator.sat_per_kw.lock().unwrap(); + *feerate_lock = target_feerate; + } + nodes[0].node.timer_tick_occurred(); + check_added_monitors(&nodes[0], if can_afford { 1 } else { 0 }); + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + + if can_afford { + // We could afford the target feerate, sanity check everything + assert_eq!(events.len(), 1); + if let MessageSendEvent::UpdateHTLCs { node_id, channel_id, updates } = + events.pop().unwrap() + { + assert_eq!(node_id, node_b_id); + assert_eq!(channel_id, chan_id); + assert_eq!(updates.commitment_signed.len(), 1); + // The HTLC is now trimmed! + assert_eq!(updates.commitment_signed[0].htlc_signatures.len(), 0); + assert_eq!(updates.update_add_htlcs.len(), 0); + assert_eq!(updates.update_fulfill_htlcs.len(), 0); + assert_eq!(updates.update_fail_htlcs.len(), 0); + assert_eq!(updates.update_fail_malformed_htlcs.len(), 0); + let update_fee = updates.update_fee.unwrap(); + assert_eq!(update_fee.channel_id, chan_id); + assert_eq!(update_fee.feerate_per_kw, target_feerate); + + nodes[1].node.handle_update_fee(node_a_id, &update_fee); + commitment_signed_dance!(nodes[1], nodes[0], updates.commitment_signed, false); + + // Confirm the feerate on node 0's commitment transaction + { + // Also add the trimmed HTLC to the fees + let expected_tx_fee_sat = + chan_utils::commit_tx_fee_sat(target_feerate, 0, &channel_type) + + node_0_inbound_htlc_amount_sat; + let commitment_tx = get_local_commitment_txn!(nodes[0], channel_id)[0].clone(); + + let mut actual_fee = commitment_tx + .output + .iter() + .map(|output| output.value.to_sat()) + .reduce(|acc, value| acc + value) + .unwrap(); + actual_fee = channel_value_sat - actual_fee; + assert_eq!(expected_tx_fee_sat, actual_fee); + + // The HTLC is now trimmed! + assert_eq!(commitment_tx.output.len(), 2); + } + + // Confirm the feerate on node 1's commitment transaction + { + // Also add the trimmed HTLC to the fees + let expected_tx_fee_sat = + chan_utils::commit_tx_fee_sat(target_feerate, 0, &channel_type) + + node_0_inbound_htlc_amount_sat; + let commitment_tx = get_local_commitment_txn!(nodes[1], channel_id)[0].clone(); + + let mut actual_fee = commitment_tx + .output + .iter() + .map(|output| output.value.to_sat()) + .reduce(|acc, value| acc + value) + .unwrap(); + actual_fee = channel_value_sat - actual_fee; + assert_eq!(expected_tx_fee_sat, actual_fee); + + // The HTLC is now trimmed! + assert_eq!(commitment_tx.output.len(), 2); + } + } else { + panic!(); + } + } else { + // We could not afford the target feerate, no events should be generated + assert_eq!(events.len(), 0); + let err = format!("Cannot afford to send new feerate at {}", target_feerate); + nodes[0].logger.assert_log("lightning::ln::channel", err, 1); + } +} From f399c918637b11478fad86525b7bdbab3760c8cc Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Tue, 1 Jul 2025 01:06:56 +0000 Subject: [PATCH 5/8] Add `TxBuilder::commit_tx_fee_sat` When the `TxBuilder` API is made public in the future, this API will let creators of custom commitment transactions tell the lightning state machine the fees required to set a given feerate on a commitment transaction with a given number of nondust HTLCs. In upcoming commits, we plan to expand this call with the full list of HTLCs and let `TxBuilder` decide which HTLCs are dust, and which are nondust. --- lightning/src/ln/chan_utils.rs | 6 +-- lightning/src/ln/channel.rs | 64 +++++++++++++++++--------------- lightning/src/sign/mod.rs | 1 + lightning/src/sign/tx_builder.rs | 21 +++++++++++ 4 files changed, 59 insertions(+), 33 deletions(-) create mode 100644 lightning/src/sign/tx_builder.rs diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 13b74afdd64..56299d450c1 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -234,16 +234,14 @@ pub(crate) fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_t } #[rustfmt::skip] -pub(crate) fn commit_and_htlc_tx_fees_sat(feerate_per_kw: u32, num_accepted_htlcs: usize, num_offered_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 { - let num_htlcs = num_accepted_htlcs + num_offered_htlcs; - let commit_tx_fees_sat = commit_tx_fee_sat(feerate_per_kw, num_htlcs, channel_type_features); +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 }; - commit_tx_fees_sat + htlc_tx_fees_sat + htlc_tx_fees_sat } // Various functions for key derivation and transaction creation for use within channels. Primarily diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 207f8a7bcdb..8066c861061 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -40,7 +40,7 @@ use crate::ln::chan_utils; #[cfg(splicing)] use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT; use crate::ln::chan_utils::{ - commit_tx_fee_sat, get_commitment_transaction_number_obscure_factor, htlc_success_tx_weight, + get_commitment_transaction_number_obscure_factor, htlc_success_tx_weight, htlc_timeout_tx_weight, max_htlcs, ChannelPublicKeys, ChannelTransactionParameters, ClosingTransaction, CommitmentTransaction, CounterpartyChannelTransactionParameters, CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, @@ -69,6 +69,7 @@ use crate::ln::script::{self, ShutdownScript}; use crate::ln::types::ChannelId; use crate::routing::gossip::NodeId; use crate::sign::ecdsa::EcdsaChannelSigner; +use crate::sign::tx_builder::{SpecTxBuilder, TxBuilder}; use crate::sign::{ChannelSigner, EntropySource, NodeSigner, Recipient, SignerProvider}; use crate::types::features::{ChannelTypeFeatures, InitFeatures}; use crate::types::payment::{PaymentHash, PaymentPreimage}; @@ -3124,12 +3125,12 @@ where 0 }; let funders_amount_msat = open_channel_fields.funding_satoshis * 1000 - msg_push_msat; - let commitment_tx_fee = commit_tx_fee_sat(open_channel_fields.commitment_feerate_sat_per_1000_weight, MIN_AFFORDABLE_HTLC_COUNT, &channel_type); - if (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value) < commitment_tx_fee { - return Err(ChannelError::close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value), commitment_tx_fee))); + let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(open_channel_fields.commitment_feerate_sat_per_1000_weight, MIN_AFFORDABLE_HTLC_COUNT, &channel_type); + if (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value) < commit_tx_fee_sat { + return Err(ChannelError::close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value), commit_tx_fee_sat))); } - let to_remote_satoshis = funders_amount_msat / 1000 - commitment_tx_fee - anchor_outputs_value; + let to_remote_satoshis = funders_amount_msat / 1000 - commit_tx_fee_sat - anchor_outputs_value; // While it's reasonable for us to not meet the channel reserve initially (if they don't // want to push much to us), our counterparty should always have more than our reserve. if to_remote_satoshis < holder_selected_channel_reserve_satoshis { @@ -3402,9 +3403,9 @@ where }; let value_to_self_msat = channel_value_satoshis * 1000 - push_msat; - let commitment_tx_fee = commit_tx_fee_sat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) * 1000; - if value_to_self_msat.saturating_sub(anchor_outputs_value_msat) < commitment_tx_fee { - return Err(APIError::APIMisuseError{ err: format!("Funding amount ({}) can't even pay fee for initial commitment transaction fee of {}.", value_to_self_msat / 1000, commitment_tx_fee / 1000) }); + let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type); + if value_to_self_msat.saturating_sub(anchor_outputs_value_msat) < commit_tx_fee_sat * 1000 { + return Err(APIError::APIMisuseError{ err: format!("Funding amount ({}) can't even pay fee for initial commitment transaction fee of {}.", value_to_self_msat / 1000, commit_tx_fee_sat) }); } let mut secp_ctx = Secp256k1::new(); @@ -4504,7 +4505,7 @@ where broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat); } - let commit_tx_fee_sat = commit_tx_fee_sat(feerate_per_kw, non_dust_htlc_count + fee_buffer_nondust_htlcs.unwrap_or(0), &funding.channel_transaction_parameters.channel_type_features); + let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(feerate_per_kw, non_dust_htlc_count + fee_buffer_nondust_htlcs.unwrap_or(0), &funding.channel_transaction_parameters.channel_type_features); let total_anchors_sat = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; // We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater @@ -4753,7 +4754,7 @@ where { 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; - for ref htlc in context.pending_inbound_htlcs.iter() { + 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 { on_counterparty_tx_dust_exposure_msat += htlc.amount_msat; @@ -4773,7 +4774,7 @@ where { 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; - for ref htlc in context.pending_outbound_htlcs.iter() { + 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 { on_counterparty_tx_dust_exposure_msat += htlc.amount_msat; @@ -4810,10 +4811,14 @@ where .unwrap_or(self.feerate_per_kw) .checked_sub(dust_exposure_limiting_feerate); let extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat = excess_feerate_opt.map(|excess_feerate| { - let extra_htlc_dust_exposure = on_counterparty_tx_dust_exposure_msat - + chan_utils::commit_and_htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1, on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()) * 1000; - on_counterparty_tx_dust_exposure_msat - += chan_utils::commit_and_htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs, on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()) * 1000; + 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()); + + let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()); + let htlc_tx_fees_sat = chan_utils::htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs, on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()); + + let extra_htlc_dust_exposure = on_counterparty_tx_dust_exposure_msat + (extra_htlc_commit_tx_fee_sat + extra_htlc_htlc_tx_fees_sat) * 1000; + on_counterparty_tx_dust_exposure_msat += (commit_tx_fee_sat + htlc_tx_fees_sat) * 1000; extra_htlc_dust_exposure }); @@ -5158,12 +5163,12 @@ where } let num_htlcs = included_htlcs + addl_htlcs; - let res = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000; + let commit_tx_fee_msat = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000; #[cfg(any(test, fuzzing))] { - let mut fee = res; + let mut fee = commit_tx_fee_msat; if fee_spike_buffer_htlc.is_some() { - fee = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000; + fee = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000; } let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len() + context.holding_cell_htlc_updates.len(); @@ -5182,7 +5187,7 @@ where }; *funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info); } - res + commit_tx_fee_msat } /// Get the commitment tx fee for the remote's next commitment transaction based on the number of @@ -5256,12 +5261,12 @@ where } let num_htlcs = included_htlcs + addl_htlcs; - let res = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000; + let commit_tx_fee_msat = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000; #[cfg(any(test, fuzzing))] if let Some(htlc) = &htlc { - let mut fee = res; + let mut fee = commit_tx_fee_msat; if fee_spike_buffer_htlc.is_some() { - fee = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000; + fee = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000; } let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len(); let commitment_tx_info = CommitmentTxInfoCached { @@ -5279,7 +5284,7 @@ where }; *funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info); } - res + commit_tx_fee_msat } #[rustfmt::skip] @@ -10574,8 +10579,7 @@ where && info.next_holder_htlc_id == self.context.next_holder_htlc_id && info.next_counterparty_htlc_id == self.context.next_counterparty_htlc_id && info.feerate == self.context.feerate_per_kw { - let actual_fee = commit_tx_fee_sat(self.context.feerate_per_kw, counterparty_commitment_tx.nondust_htlcs().len(), self.funding.get_channel_type()) * 1000; - assert_eq!(actual_fee, info.fee); + assert_eq!(commitment_data.stats.commit_tx_fee_sat, info.fee); } } } @@ -13194,11 +13198,13 @@ mod tests { use crate::chain::chaininterface::LowerBoundedFeeEstimator; use crate::chain::transaction::OutPoint; use crate::chain::BestBlock; - use crate::ln::chan_utils::{self, htlc_success_tx_weight, htlc_timeout_tx_weight}; + use crate::ln::chan_utils::{ + self, commit_tx_fee_sat, htlc_success_tx_weight, htlc_timeout_tx_weight, + }; use crate::ln::channel::{ - commit_tx_fee_sat, AwaitingChannelReadyFlags, ChannelState, FundedChannel, HTLCCandidate, - HTLCInitiator, HTLCUpdateAwaitingACK, InboundHTLCOutput, InboundHTLCState, - InboundV1Channel, OutboundHTLCOutput, OutboundHTLCState, OutboundV1Channel, + AwaitingChannelReadyFlags, ChannelState, FundedChannel, HTLCCandidate, HTLCInitiator, + HTLCUpdateAwaitingACK, InboundHTLCOutput, InboundHTLCState, InboundV1Channel, + OutboundHTLCOutput, OutboundHTLCState, OutboundV1Channel, }; use crate::ln::channel::{ MAX_FUNDING_SATOSHIS_NO_WUMBO, MIN_THEIR_CHAN_RESERVE_SATOSHIS, diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 075cd2b644e..2610131d96f 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -78,6 +78,7 @@ pub(crate) mod type_resolver; pub mod ecdsa; #[cfg(taproot)] pub mod taproot; +pub mod tx_builder; /// Information about a spendable output to a P2WSH script. /// diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs new file mode 100644 index 00000000000..02b431575da --- /dev/null +++ b/lightning/src/sign/tx_builder.rs @@ -0,0 +1,21 @@ +//! Defines the `TxBuilder` trait, and the `SpecTxBuilder` type + +use types::features::ChannelTypeFeatures; + +use crate::ln::chan_utils::commit_tx_fee_sat; + +pub(crate) trait TxBuilder { + fn commit_tx_fee_sat( + &self, feerate_per_kw: u32, nondust_htlc_count: usize, channel_type: &ChannelTypeFeatures, + ) -> u64; +} + +pub(crate) struct SpecTxBuilder {} + +impl TxBuilder for SpecTxBuilder { + fn commit_tx_fee_sat( + &self, feerate_per_kw: u32, nondust_htlc_count: usize, channel_type: &ChannelTypeFeatures, + ) -> u64 { + commit_tx_fee_sat(feerate_per_kw, nondust_htlc_count, channel_type) + } +} From 85cc09ef453dc91aa06fb35dc765358fec4f3d17 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Wed, 25 Jun 2025 23:57:24 +0000 Subject: [PATCH 6/8] Include HTLC amts in `ClaimableOnChannelClose.transaction_fee_satoshis` In addition to the amount set by `commit_tx_fee_sat`, the transaction fee of a commitment transaction may also increase due to HTLC amounts that get burned to fees, and any elided anchors. We now include these amounts in the `transaction_fee_satoshis` field of `Balance::ClaimableOnChannelClose` too. This commit also makes the `transaction_fee_satoshis` field correct for custom transactions not encompassed in `chan_utils::commit_tx_fee_sat`. --- lightning/src/chain/channelmonitor.rs | 13 +++++-------- lightning/src/ln/monitor_tests.rs | 16 ++++++++++------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 54f170bcbe3..756808a1f28 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2736,15 +2736,11 @@ impl ChannelMonitor { } } else { let mut claimable_inbound_htlc_value_sat = 0; - let mut nondust_htlc_count = 0; let mut outbound_payment_htlc_rounded_msat = 0; let mut outbound_forwarded_htlc_rounded_msat = 0; let mut inbound_claiming_htlc_rounded_msat = 0; let mut inbound_htlc_rounded_msat = 0; for (htlc, source) in holder_commitment_htlcs!(us, CURRENT_WITH_SOURCES) { - if htlc.transaction_output_index.is_some() { - nondust_htlc_count += 1; - } let rounded_value_msat = if htlc.transaction_output_index.is_none() { htlc.amount_msat } else { htlc.amount_msat % 1000 }; @@ -2788,11 +2784,12 @@ impl ChannelMonitor { let to_self_value_sat = us.funding.current_holder_commitment_tx.to_broadcaster_value_sat(); res.push(Balance::ClaimableOnChannelClose { amount_satoshis: to_self_value_sat + claimable_inbound_htlc_value_sat, + // In addition to `commit_tx_fee_sat`, this can also include dust HTLCs, and the total msat amount rounded down from non-dust HTLCs transaction_fee_satoshis: if us.holder_pays_commitment_tx_fee.unwrap_or(true) { - chan_utils::commit_tx_fee_sat( - us.funding.current_holder_commitment_tx.feerate_per_kw(), nondust_htlc_count, - us.channel_type_features(), - ) + let transaction = &us.funding.current_holder_commitment_tx.trust().built_transaction().transaction; + // Unwrap here; commitment transactions always have at least one output + let output_value_sat = transaction.output.iter().map(|txout| txout.value).reduce(|sum, value| sum + value).unwrap().to_sat(); + us.funding.channel_parameters.channel_value_satoshis - output_value_sat } else { 0 }, outbound_payment_htlc_rounded_msat, outbound_forwarded_htlc_rounded_msat, diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index cdcb570af68..56be4e7d4bd 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -544,9 +544,11 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { let commitment_tx_fee = chan_feerate as u64 * (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let anchor_outputs_value = if anchors { 2 * channel::ANCHOR_OUTPUT_VALUE_SATOSHI } else { 0 }; + let amount_satoshis = 1_000_000 - 3_000 - 4_000 - 1_000 - 3 - commitment_tx_fee - anchor_outputs_value - 1; /* msat amount that is burned to fees */ assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { - amount_satoshis: 1_000_000 - 3_000 - 4_000 - 1_000 - 3 - commitment_tx_fee - anchor_outputs_value - 1 /* msat amount that is burned to fees */, - transaction_fee_satoshis: commitment_tx_fee, + amount_satoshis, + // In addition to `commitment_tx_fee`, this also includes the dust HTLC, and the total msat amount rounded down from non-dust HTLCs + transaction_fee_satoshis: 1_000_000 - 4_000 - 3_000 - 1_000 - amount_satoshis - anchor_outputs_value, outbound_payment_htlc_rounded_msat: 3300, outbound_forwarded_htlc_rounded_msat: 0, inbound_claiming_htlc_rounded_msat: 0, @@ -598,16 +600,18 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { let commitment_tx_fee = chan_feerate as u64 * (chan_utils::commitment_tx_base_weight(&channel_type_features) + if prev_commitment_tx { 1 } else { 2 } * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; - let mut a_expected_balances = vec![Balance::ClaimableOnChannelClose { - amount_satoshis: 1_000_000 - // Channel funding value in satoshis + let amount_satoshis = 1_000_000 - // Channel funding value in satoshis 4_000 - // The to-be-failed HTLC value in satoshis 3_000 - // The claimed HTLC value in satoshis 1_000 - // The push_msat value in satoshis 3 - // The dust HTLC value in satoshis commitment_tx_fee - // The commitment transaction fee with two HTLC outputs anchor_outputs_value - // The anchor outputs value in satoshis - 1, // The rounded up msat part of the one HTLC - transaction_fee_satoshis: commitment_tx_fee, + 1; // The rounded up msat part of the one HTLC + let mut a_expected_balances = vec![Balance::ClaimableOnChannelClose { + amount_satoshis, // Channel funding value in satoshis + // In addition to `commitment_tx_fee`, this also includes the dust HTLC, and the total msat amount rounded down from non-dust HTLCs + transaction_fee_satoshis: 1_000_000 - 4_000 - 3_000 - 1_000 - amount_satoshis - anchor_outputs_value, outbound_payment_htlc_rounded_msat: 3000 + if prev_commitment_tx { 200 /* 1 to-be-failed HTLC */ } else { 300 /* 2 HTLCs */ }, outbound_forwarded_htlc_rounded_msat: 0, From 07b794014ecec69689f4d8e1c225b624ace252c5 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Tue, 1 Jul 2025 02:13:48 +0000 Subject: [PATCH 7/8] Add `TxBuilder::subtract_non_htlc_outputs` When the `TxBuilder` API is made public in the future, this trait method will let creators of custom commitment transactions subtract any additional output amounts from the local and remote balances after the HTLCs have been accounted for. We leverage this API to account for today's 330 sat anchors. --- lightning/src/ln/channel.rs | 199 ++++++++++++++----------------- lightning/src/sign/tx_builder.rs | 35 ++++++ 2 files changed, 124 insertions(+), 110 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8066c861061..044658686f5 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3117,20 +3117,21 @@ where return Err(ChannelError::close(format!("Dust limit ({}) too high for the channel reserve we require the remote to keep ({})", open_channel_fields.dust_limit_satoshis, holder_selected_channel_reserve_satoshis))); } + // v1 channel opens set `our_funding_satoshis` to 0, and v2 channel opens set `msg_push_msat` to 0. + debug_assert!(our_funding_satoshis == 0 || msg_push_msat == 0); + let value_to_self_msat = our_funding_satoshis * 1000 + msg_push_msat; + // check if the funder's amount for the initial commitment tx is sufficient // for full fee payment plus a few HTLCs to ensure the channel will be useful. - let anchor_outputs_value = if channel_type.supports_anchors_zero_fee_htlc_tx() { - ANCHOR_OUTPUT_VALUE_SATOSHI * 2 - } else { - 0 - }; let funders_amount_msat = open_channel_fields.funding_satoshis * 1000 - msg_push_msat; let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(open_channel_fields.commitment_feerate_sat_per_1000_weight, MIN_AFFORDABLE_HTLC_COUNT, &channel_type); - if (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value) < commit_tx_fee_sat { - return Err(ChannelError::close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value), commit_tx_fee_sat))); + // Subtract any non-HTLC outputs from the remote balance + let (_, remote_balance_before_fee_msat) = SpecTxBuilder {}.subtract_non_htlc_outputs(false, value_to_self_msat, funders_amount_msat, &channel_type); + if remote_balance_before_fee_msat / 1000 < commit_tx_fee_sat { + return Err(ChannelError::close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", funders_amount_msat / 1000, commit_tx_fee_sat))); } - let to_remote_satoshis = funders_amount_msat / 1000 - commit_tx_fee_sat - anchor_outputs_value; + let to_remote_satoshis = remote_balance_before_fee_msat / 1000 - commit_tx_fee_sat; // While it's reasonable for us to not meet the channel reserve initially (if they don't // want to push much to us), our counterparty should always have more than our reserve. if to_remote_satoshis < holder_selected_channel_reserve_satoshis { @@ -3184,8 +3185,6 @@ where Some(cmp::max(config.channel_handshake_config.minimum_depth, 1)) }; - let value_to_self_msat = our_funding_satoshis * 1000 + msg_push_msat; - // TODO(dual_funding): Checks for `funding_feerate_sat_per_1000_weight`? let pubkeys = holder_signer.pubkeys(None, &secp_ctx); @@ -3389,22 +3388,27 @@ 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, anchor_outputs_value_msat) = - if channel_type.supports_anchor_zero_fee_commitments() { - (0, 0) - } else if channel_type.supports_anchors_zero_fee_htlc_tx() { - let feerate = fee_estimator - .bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee); - (feerate, ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000) + 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 { - let feerate = fee_estimator - .bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); - (feerate, 0) + ConfirmationTarget::NonAnchorChannelFee }; + fee_estimator.bounded_sat_per_1000_weight(commitment_conf_target) + }; 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); - if value_to_self_msat.saturating_sub(anchor_outputs_value_msat) < commit_tx_fee_sat * 1000 { + // Subtract any non-HTLC outputs from the local balance + let (local_balance_before_fee_msat, _) = SpecTxBuilder {}.subtract_non_htlc_outputs( + true, + value_to_self_msat, + push_msat, + &channel_type, + ); + if local_balance_before_fee_msat / 1000 < commit_tx_fee_sat { return Err(APIError::APIMisuseError{ err: format!("Funding amount ({}) can't even pay fee for initial commitment transaction fee of {}.", value_to_self_msat / 1000, commit_tx_fee_sat) }); } @@ -4125,20 +4129,22 @@ where // violate the reserve value if we do not do this (as we forget inbound HTLCs from the // Channel state once they will not be present in the next received commitment // transaction). - let mut removed_outbound_total_msat = 0; - for ref htlc in self.pending_outbound_htlcs.iter() { - if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) = htlc.state { - removed_outbound_total_msat += htlc.amount_msat; - } else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state { - removed_outbound_total_msat += htlc.amount_msat; + let (local_balance_before_fee_msat, remote_balance_before_fee_msat) = { + let mut removed_outbound_total_msat = 0; + for htlc in self.pending_outbound_htlcs.iter() { + if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) | OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state { + removed_outbound_total_msat += htlc.amount_msat; + } } - } + let pending_value_to_self_msat = + funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; + let pending_remote_value_msat = + funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; - let pending_value_to_self_msat = - funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; - let pending_remote_value_msat = - funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; - if pending_remote_value_msat < msg.amount_msat { + // Subtract any non-HTLC outputs from the local and remote balances + SpecTxBuilder {}.subtract_non_htlc_outputs(funding.is_outbound(), funding.value_to_self_msat, pending_remote_value_msat, funding.get_channel_type()) + }; + if remote_balance_before_fee_msat < msg.amount_msat { return Err(ChannelError::close("Remote HTLC add would overdraw remaining funds".to_owned())); } @@ -4149,29 +4155,19 @@ where let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); self.next_remote_commit_tx_fee_msat(funding, Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations }; - let anchor_outputs_value_msat = if !funding.is_outbound() && funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 - } else { - 0 - }; - if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(anchor_outputs_value_msat) < remote_commit_tx_fee_msat { + if remote_balance_before_fee_msat.saturating_sub(msg.amount_msat) < remote_commit_tx_fee_msat { return Err(ChannelError::close("Remote HTLC add would not leave enough to pay for fees".to_owned())); }; - if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat).saturating_sub(anchor_outputs_value_msat) < funding.holder_selected_channel_reserve_satoshis * 1000 { + if remote_balance_before_fee_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat) < funding.holder_selected_channel_reserve_satoshis * 1000 { return Err(ChannelError::close("Remote HTLC add would put them under remote reserve value".to_owned())); } } - let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 - } else { - 0 - }; if funding.is_outbound() { // Check that they won't violate our local required channel reserve by adding this HTLC. let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(funding, htlc_candidate, None); - if funding.value_to_self_msat < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat { + if local_balance_before_fee_msat < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat { return Err(ChannelError::close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned())); } } @@ -4380,27 +4376,26 @@ where } } - let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 - } else { - 0 - }; - - let mut removed_outbound_total_msat = 0; - for ref htlc in self.pending_outbound_htlcs.iter() { - if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) = htlc.state { - removed_outbound_total_msat += htlc.amount_msat; - } else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state { - removed_outbound_total_msat += htlc.amount_msat; + if !funding.is_outbound() { + let mut removed_outbound_total_msat = 0; + for htlc in self.pending_outbound_htlcs.iter() { + if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) | OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state { + removed_outbound_total_msat += htlc.amount_msat; + } } - } - let pending_value_to_self_msat = - funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; - let pending_remote_value_msat = - funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; + let pending_value_to_self_msat = + funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; + let pending_remote_value_msat = + funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; + // Subtract any non-HTLC outputs from the local and remote balances + let (_, remote_balance_before_fee_msat) = SpecTxBuilder {}.subtract_non_htlc_outputs( + funding.is_outbound(), + pending_value_to_self_msat, + pending_remote_value_msat, + funding.get_channel_type() + ); - if !funding.is_outbound() { // `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 @@ -4412,7 +4407,7 @@ where if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; } - if pending_remote_value_msat.saturating_sub(funding.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat { + if remote_balance_before_fee_msat.saturating_sub(funding.holder_selected_channel_reserve_satoshis * 1000) < remote_fee_cost_incl_stuck_buffer_msat { log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", &self.channel_id()); return Err(LocalHTLCFailureReason::FeeSpikeBuffer); } @@ -4446,7 +4441,7 @@ where #[rustfmt::skip] fn build_commitment_stats(&self, funding: &FundingScope, local: bool, generated_by_local: bool, feerate_per_kw: Option, fee_buffer_nondust_htlcs: Option) -> CommitmentStats { let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis }; - let mut non_dust_htlc_count = 0; + let mut nondust_htlc_count = 0; let mut remote_htlc_total_msat = 0; let mut local_htlc_total_msat = 0; let mut value_to_self_claimed_msat = 0; @@ -4457,7 +4452,7 @@ where for htlc in self.pending_inbound_htlcs.iter() { if htlc.state.included_in_commitment(generated_by_local) { if !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type()) { - non_dust_htlc_count += 1; + nondust_htlc_count += 1; } remote_htlc_total_msat += htlc.amount_msat; } else { @@ -4470,7 +4465,7 @@ where for htlc in self.pending_outbound_htlcs.iter() { if htlc.state.included_in_commitment(generated_by_local) { if !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type()) { - non_dust_htlc_count += 1; + nondust_htlc_count += 1; } local_htlc_total_msat += htlc.amount_msat; } else { @@ -4505,27 +4500,16 @@ where broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat); } - let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(feerate_per_kw, non_dust_htlc_count + fee_buffer_nondust_htlcs.unwrap_or(0), &funding.channel_transaction_parameters.channel_type_features); - let total_anchors_sat = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; - - // We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater - // than or equal to `total_anchors_sat`. - // - // This is because when the remote party sends an `update_fee` message, we build the new - // commitment transaction *before* checking whether the remote party's balance is enough to - // cover the total anchor sum. - - if funding.is_outbound() { - value_to_self_msat = value_to_self_msat.saturating_sub(total_anchors_sat * 1000); - } else { - value_to_remote_msat = value_to_remote_msat.saturating_sub(total_anchors_sat * 1000); - } + let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(feerate_per_kw, nondust_htlc_count + fee_buffer_nondust_htlcs.unwrap_or(0), funding.get_channel_type()); + // Subtract any non-HTLC outputs from the local and remote balances + let (local_balance_before_fee_msat, remote_balance_before_fee_msat) = SpecTxBuilder {}.subtract_non_htlc_outputs( + funding.is_outbound(), + value_to_self_msat, + value_to_remote_msat, + funding.get_channel_type(), + ); - CommitmentStats { - commit_tx_fee_sat, - local_balance_before_fee_msat: value_to_self_msat, - remote_balance_before_fee_msat: value_to_remote_msat, - } + CommitmentStats { commit_tx_fee_sat, local_balance_before_fee_msat, remote_balance_before_fee_msat } } /// Transaction nomenclature is somewhat confusing here as there are many different cases - a @@ -4944,18 +4928,20 @@ where let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator); let htlc_stats = context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); - let outbound_capacity_msat = funding.value_to_self_msat - .saturating_sub(htlc_stats.pending_outbound_htlcs_value_msat) + // Subtract any non-HTLC outputs from the local and remote balances + let (local_balance_before_fee_msat, remote_balance_before_fee_msat) = SpecTxBuilder {}.subtract_non_htlc_outputs( + funding.is_outbound(), + funding.value_to_self_msat.saturating_sub(htlc_stats.pending_outbound_htlcs_value_msat), + (funding.get_value_satoshis() * 1000).checked_sub(funding.value_to_self_msat).unwrap().saturating_sub(htlc_stats.pending_inbound_htlcs_value_msat), + funding.get_channel_type(), + ); + + let outbound_capacity_msat = local_balance_before_fee_msat .saturating_sub( funding.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) * 1000); let mut available_capacity_msat = outbound_capacity_msat; - let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 - } else { - 0 - }; if funding.is_outbound() { // We should mind channel commit tx fee when computing how much of the available capacity // can be used in the next htlc. Mirrors the logic in send_htlc. @@ -4970,9 +4956,9 @@ where } 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, Some(())); 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, Some(())); 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; @@ -4982,7 +4968,7 @@ where // value ends up being below dust, we have this fee available again. In that case, // match the value to right-below-dust. let mut capacity_minus_commitment_fee_msat: i64 = available_capacity_msat as i64 - - max_reserved_commit_tx_fee_msat as i64 - anchor_outputs_value_msat as i64; + max_reserved_commit_tx_fee_msat as i64; if capacity_minus_commitment_fee_msat < (real_dust_limit_timeout_sat as i64) * 1000 { let one_htlc_difference_msat = max_reserved_commit_tx_fee_msat - min_reserved_commit_tx_fee_msat; debug_assert!(one_htlc_difference_msat != 0); @@ -5001,13 +4987,10 @@ where } 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); + let max_reserved_commit_tx_fee_msat = context.next_remote_commit_tx_fee_msat(funding, Some(htlc_above_dust), None); let holder_selected_chan_reserve_msat = funding.holder_selected_channel_reserve_satoshis * 1000; - let remote_balance_msat = (funding.get_value_satoshis() * 1000 - funding.value_to_self_msat) - .saturating_sub(htlc_stats.pending_inbound_htlcs_value_msat); - - if remote_balance_msat < max_reserved_commit_tx_fee_msat + holder_selected_chan_reserve_msat + anchor_outputs_value_msat { + if remote_balance_before_fee_msat < max_reserved_commit_tx_fee_msat + holder_selected_chan_reserve_msat { // If another HTLC's fee would reduce the remote's balance below the reserve limit // we've selected for them, we can only send dust HTLCs. available_capacity_msat = cmp::min(available_capacity_msat, real_dust_limit_success_sat * 1000 - 1); @@ -5072,11 +5055,7 @@ where #[allow(deprecated)] // TODO: Remove once balance_msat is removed. AvailableBalances { - inbound_capacity_msat: cmp::max(funding.get_value_satoshis() as i64 * 1000 - - funding.value_to_self_msat as i64 - - htlc_stats.pending_inbound_htlcs_value_msat as i64 - - funding.holder_selected_channel_reserve_satoshis as i64 * 1000, - 0) as u64, + inbound_capacity_msat: remote_balance_before_fee_msat.saturating_sub(funding.holder_selected_channel_reserve_satoshis * 1000), outbound_capacity_msat, next_outbound_htlc_limit_msat: available_capacity_msat, next_outbound_htlc_minimum_msat, @@ -5097,7 +5076,7 @@ where fn next_local_commit_tx_fee_msat( &self, funding: &FundingScope, htlc: HTLCCandidate, fee_spike_buffer_htlc: Option<()>, ) -> u64 { - let context = &self; + 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() { @@ -5206,7 +5185,7 @@ where ) -> u64 { debug_assert!(htlc.is_some() || fee_spike_buffer_htlc.is_some(), "At least one of the options must be set"); - let context = &self; + 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() { diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index 02b431575da..46b77754572 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -3,11 +3,16 @@ use types::features::ChannelTypeFeatures; use crate::ln::chan_utils::commit_tx_fee_sat; +use crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI; pub(crate) trait TxBuilder { fn commit_tx_fee_sat( &self, feerate_per_kw: u32, nondust_htlc_count: usize, channel_type: &ChannelTypeFeatures, ) -> u64; + fn subtract_non_htlc_outputs( + &self, is_outbound_from_holder: bool, value_to_self_after_htlcs: u64, + value_to_remote_after_htlcs: u64, channel_type: &ChannelTypeFeatures, + ) -> (u64, u64); } pub(crate) struct SpecTxBuilder {} @@ -18,4 +23,34 @@ impl TxBuilder for SpecTxBuilder { ) -> u64 { commit_tx_fee_sat(feerate_per_kw, nondust_htlc_count, channel_type) } + fn subtract_non_htlc_outputs( + &self, is_outbound_from_holder: bool, value_to_self_after_htlcs: u64, + value_to_remote_after_htlcs: u64, channel_type: &ChannelTypeFeatures, + ) -> (u64, u64) { + let total_anchors_sat = if channel_type.supports_anchors_zero_fee_htlc_tx() { + ANCHOR_OUTPUT_VALUE_SATOSHI * 2 + } else { + 0 + }; + + let mut local_balance_before_fee_msat = value_to_self_after_htlcs; + let mut remote_balance_before_fee_msat = value_to_remote_after_htlcs; + + // We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater + // than or equal to `total_anchors_sat`. + // + // This is because when the remote party sends an `update_fee` message, we build the new + // commitment transaction *before* checking whether the remote party's balance is enough to + // cover the total anchor sum. + + if is_outbound_from_holder { + local_balance_before_fee_msat = + local_balance_before_fee_msat.saturating_sub(total_anchors_sat * 1000); + } else { + remote_balance_before_fee_msat = + remote_balance_before_fee_msat.saturating_sub(total_anchors_sat * 1000); + } + + (local_balance_before_fee_msat, remote_balance_before_fee_msat) + } } From 39cb0a3b31724e19cc2f99a48f206b7d5604cb8d Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Tue, 1 Jul 2025 02:32:56 +0000 Subject: [PATCH 8/8] Add `TxBuilder::build_commitment_transaction` When the `TxBuilder` API is made public in the future, this trait method will let implementers build custom commitment transactions as part of the lightning state machine. --- lightning/src/ln/channel.rs | 83 +++++++------------- lightning/src/sign/tx_builder.rs | 130 ++++++++++++++++++++++++++++++- 2 files changed, 155 insertions(+), 58 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 044658686f5..9dbbc7f493e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1130,13 +1130,14 @@ struct CommitmentData<'a> { } /// A struct gathering stats on a commitment transaction, either local or remote. -struct CommitmentStats { +#[derive(Debug, PartialEq)] +pub(crate) struct CommitmentStats { /// The total fee included in the commitment transaction - commit_tx_fee_sat: u64, + pub commit_tx_fee_sat: u64, /// The local balance before fees *not* considering dust limits - local_balance_before_fee_msat: u64, + pub local_balance_before_fee_msat: u64, /// The remote balance before fees *not* considering dust limits - remote_balance_before_fee_msat: u64, + pub remote_balance_before_fee_msat: u64, } /// Used when calculating whether we or the remote can afford an additional HTLC. @@ -4533,16 +4534,10 @@ where let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis }; let feerate_per_kw = self.get_commitment_feerate(funding, generated_by_local); - let stats = self.build_commitment_stats(funding, local, generated_by_local, None, None); - let CommitmentStats { - commit_tx_fee_sat, - local_balance_before_fee_msat, - remote_balance_before_fee_msat - } = stats; - let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len(); let mut htlcs_included: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs); - let mut nondust_htlcs: Vec = Vec::with_capacity(num_htlcs); + let mut value_to_self_claimed_msat = 0; + let mut value_to_remote_claimed_msat = 0; log_trace!(logger, "Building commitment transaction number {} (really {} xor {}) for channel {} for {}, generated by {} with fee {}...", commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number), @@ -4557,21 +4552,15 @@ where amount_msat: $htlc.amount_msat, cltv_expiry: $htlc.cltv_expiry, payment_hash: $htlc.payment_hash, - transaction_output_index: None + transaction_output_index: None, } } } macro_rules! add_htlc_output { ($htlc: expr, $outbound: expr, $source: expr) => { - let htlc_in_tx = get_htlc_in_commitment!($htlc, $outbound == local); - htlcs_included.push((htlc_in_tx.clone(), $source)); - if $htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type()) { - log_trace!(logger, " ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $htlc.state, $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat); - } else { - log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $htlc.state, $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat); - nondust_htlcs.push(htlc_in_tx); - } + let htlc = get_htlc_in_commitment!($htlc, $outbound == local); + htlcs_included.push((htlc, $source)); } } @@ -4580,11 +4569,13 @@ where for htlc in self.pending_inbound_htlcs.iter() { if htlc.state.included_in_commitment(generated_by_local) { + log_trace!(logger, " ...including inbound {} HTLC {} (hash {}) with value {}", htlc.state, htlc.htlc_id, htlc.payment_hash, htlc.amount_msat); add_htlc_output!(htlc, false, None); } else { log_trace!(logger, " ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state); if let Some(preimage) = htlc.state.preimage() { inbound_htlc_preimages.push(preimage); + value_to_self_claimed_msat += htlc.amount_msat; } } }; @@ -4594,53 +4585,35 @@ where outbound_htlc_preimages.push(preimage); } if htlc.state.included_in_commitment(generated_by_local) { + log_trace!(logger, " ...including outbound {} HTLC {} (hash {}) with value {}", htlc.state, htlc.htlc_id, htlc.payment_hash, htlc.amount_msat); add_htlc_output!(htlc, true, Some(&htlc.source)); } else { log_trace!(logger, " ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state); + if htlc.state.preimage().is_some() { + value_to_remote_claimed_msat += htlc.amount_msat; + } } }; - // We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater - // than or equal to `commit_tx_fee_sat`. + // # Panics // - // This is because when the remote party sends an `update_fee` message, we build the new - // commitment transaction *before* checking whether the remote party's balance is enough to - // cover the total fee. - - let (value_to_self, value_to_remote) = if funding.is_outbound() { - ((local_balance_before_fee_msat / 1000).saturating_sub(commit_tx_fee_sat), remote_balance_before_fee_msat / 1000) - } else { - (local_balance_before_fee_msat / 1000, (remote_balance_before_fee_msat / 1000).saturating_sub(commit_tx_fee_sat)) - }; - - let mut to_broadcaster_value_sat = if local { value_to_self } else { value_to_remote }; - let mut to_countersignatory_value_sat = if local { value_to_remote } else { value_to_self }; - - if to_broadcaster_value_sat >= broadcaster_dust_limit_sat { - log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, to_broadcaster_value_sat); - } else { - to_broadcaster_value_sat = 0; - } + // After all HTLC claims have been accounted for, the local balance MUST remain greater than or equal to 0. - if to_countersignatory_value_sat >= broadcaster_dust_limit_sat { - log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, to_countersignatory_value_sat); - } else { - to_countersignatory_value_sat = 0; - } + let value_to_self_msat = (funding.value_to_self_msat + value_to_self_claimed_msat).checked_sub(value_to_remote_claimed_msat).unwrap(); - let channel_parameters = - if local { funding.channel_transaction_parameters.as_holder_broadcastable() } - else { funding.channel_transaction_parameters.as_counterparty_broadcastable() }; - let tx = CommitmentTransaction::new( + let (tx, stats) = SpecTxBuilder {}.build_commitment_transaction( + local, commitment_number, per_commitment_point, - to_broadcaster_value_sat, - to_countersignatory_value_sat, - feerate_per_kw, - nondust_htlcs, - &channel_parameters, + &funding.channel_transaction_parameters, &self.secp_ctx, + value_to_self_msat, + htlcs_included.iter().map(|(htlc, _source)| htlc).cloned().collect(), + feerate_per_kw, + broadcaster_dust_limit_sat, + logger, ); + debug_assert_eq!(stats, self.build_commitment_stats(funding, local, generated_by_local, None, None)); // This populates the HTLC-source table with the indices from the HTLCs in the commitment // transaction. diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index 46b77754572..6e623d1a7db 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -1,9 +1,17 @@ //! Defines the `TxBuilder` trait, and the `SpecTxBuilder` type -use types::features::ChannelTypeFeatures; +use core::ops::Deref; -use crate::ln::chan_utils::commit_tx_fee_sat; -use crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI; +use bitcoin::secp256k1::{self, PublicKey, Secp256k1}; + +use crate::ln::chan_utils::{ + commit_tx_fee_sat, htlc_success_tx_weight, htlc_timeout_tx_weight, + ChannelTransactionParameters, CommitmentTransaction, HTLCOutputInCommitment, +}; +use crate::ln::channel::{CommitmentStats, ANCHOR_OUTPUT_VALUE_SATOSHI}; +use crate::prelude::*; +use crate::types::features::ChannelTypeFeatures; +use crate::util::logger::Logger; pub(crate) trait TxBuilder { fn commit_tx_fee_sat( @@ -13,6 +21,14 @@ pub(crate) trait TxBuilder { &self, is_outbound_from_holder: bool, value_to_self_after_htlcs: u64, value_to_remote_after_htlcs: u64, channel_type: &ChannelTypeFeatures, ) -> (u64, u64); + fn build_commitment_transaction( + &self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey, + channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1, + value_to_self_msat: u64, htlcs_in_tx: Vec, feerate_per_kw: u32, + broadcaster_dust_limit_sat: u64, logger: &L, + ) -> (CommitmentTransaction, CommitmentStats) + where + L::Target: Logger; } pub(crate) struct SpecTxBuilder {} @@ -53,4 +69,112 @@ impl TxBuilder for SpecTxBuilder { (local_balance_before_fee_msat, remote_balance_before_fee_msat) } + #[rustfmt::skip] + fn build_commitment_transaction( + &self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey, + channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1, + value_to_self_msat: u64, mut htlcs_in_tx: Vec, feerate_per_kw: u32, + broadcaster_dust_limit_sat: u64, logger: &L, + ) -> (CommitmentTransaction, CommitmentStats) + where + L::Target: Logger, + { + let mut local_htlc_total_msat = 0; + let mut remote_htlc_total_msat = 0; + let channel_type = &channel_parameters.channel_type_features; + + let is_dust = |offered: bool, amount_msat: u64| -> bool { + let htlc_tx_fee_sat = if channel_type.supports_anchors_zero_fee_htlc_tx() { + 0 + } else { + let htlc_tx_weight = if offered { + htlc_timeout_tx_weight(channel_type) + } else { + htlc_success_tx_weight(channel_type) + }; + // As required by the spec, round down + feerate_per_kw as u64 * htlc_tx_weight / 1000 + }; + amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee_sat + }; + + // Trim dust htlcs + htlcs_in_tx.retain(|htlc| { + if htlc.offered == local { + // This is an outbound htlc + local_htlc_total_msat += htlc.amount_msat; + } else { + remote_htlc_total_msat += htlc.amount_msat; + } + if is_dust(htlc.offered, htlc.amount_msat) { + log_trace!(logger, " ...trimming {} HTLC with value {}sat, hash {}, due to dust limit {}", if htlc.offered == local { "outbound" } else { "inbound" }, htlc.amount_msat / 1000, htlc.payment_hash, broadcaster_dust_limit_sat); + false + } else { + true + } + }); + + // # Panics + // + // The value going to each party MUST be 0 or positive, even if all HTLCs pending in the + // commitment clear by failure. + + let commit_tx_fee_sat = self.commit_tx_fee_sat(feerate_per_kw, htlcs_in_tx.len(), &channel_parameters.channel_type_features); + let value_to_self_after_htlcs_msat = value_to_self_msat.checked_sub(local_htlc_total_msat).unwrap(); + let value_to_remote_after_htlcs_msat = + (channel_parameters.channel_value_satoshis * 1000).checked_sub(value_to_self_msat).unwrap().checked_sub(remote_htlc_total_msat).unwrap(); + let (local_balance_before_fee_msat, remote_balance_before_fee_msat) = + self.subtract_non_htlc_outputs(channel_parameters.is_outbound_from_holder, value_to_self_after_htlcs_msat, value_to_remote_after_htlcs_msat, &channel_parameters.channel_type_features); + + // We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater + // than or equal to `commit_tx_fee_sat`. + // + // This is because when the remote party sends an `update_fee` message, we build the new + // commitment transaction *before* checking whether the remote party's balance is enough to + // cover the total fee. + + let (value_to_self, value_to_remote) = if channel_parameters.is_outbound_from_holder { + ((local_balance_before_fee_msat / 1000).saturating_sub(commit_tx_fee_sat), remote_balance_before_fee_msat / 1000) + } else { + (local_balance_before_fee_msat / 1000, (remote_balance_before_fee_msat / 1000).saturating_sub(commit_tx_fee_sat)) + }; + + let mut to_broadcaster_value_sat = if local { value_to_self } else { value_to_remote }; + let mut to_countersignatory_value_sat = if local { value_to_remote } else { value_to_self }; + + if to_broadcaster_value_sat >= broadcaster_dust_limit_sat { + log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, to_broadcaster_value_sat); + } else { + to_broadcaster_value_sat = 0; + } + + if to_countersignatory_value_sat >= broadcaster_dust_limit_sat { + log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, to_countersignatory_value_sat); + } else { + to_countersignatory_value_sat = 0; + } + + let directed_parameters = + if local { channel_parameters.as_holder_broadcastable() } + else { channel_parameters.as_counterparty_broadcastable() }; + let tx = CommitmentTransaction::new( + commitment_number, + per_commitment_point, + to_broadcaster_value_sat, + to_countersignatory_value_sat, + feerate_per_kw, + htlcs_in_tx, + &directed_parameters, + secp_ctx, + ); + + ( + tx, + CommitmentStats { + commit_tx_fee_sat, + local_balance_before_fee_msat, + remote_balance_before_fee_msat, + }, + ) + } }