Skip to content

Don't trim HTLCs when calculating the commit tx fee including the fee spike multiple #4574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tankyleo wants to merge 6 commits into
base: main
Choose a base branch
from
+554 −86
Open

Don't trim HTLCs when calculating the commit tx fee including the fee spike multiple #4574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a430756
As a channel fundee, keep a fee spike buffer from the no-outputs case
tankyleo Apr 23, 2026
7efe9c5
Don't trim HTLCs when calculating the reserved commit tx fee
tankyleo Apr 17, 2026
def7abc
Add test coverage for `tx_builder::get_available_balances`
tankyleo Apr 18, 2026
33edd17
Move the calculation of the spiked feerate to `tx_builder`
tankyleo Apr 21, 2026
11cbc95
Don't trim HTLCs when calculating the fee spike commit tx fee
tankyleo Apr 21, 2026
6d8b153
Test the fee spike buffer multiple affordability check
tankyleo Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 33 additions & 29 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4198,6 +4198,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
addl_nondust_htlc_count,
channel_context.feerate_per_kw,
false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
Expand Down Expand Up @@ -4494,6 +4495,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
addl_nondust_htlc_count,
channel_context.feerate_per_kw,
false,
dust_exposure_limiting_feerate,
)
.map_err(|()| APIError::APIMisuseError {
Expand Down Expand Up @@ -5284,7 +5286,8 @@ impl<SP: SignerProvider> ChannelContext<SP> {
fn get_next_local_commitment_stats(
&self, funding: &FundingScope, htlc_candidate: Option<HTLCAmountDirection>,
include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize,
feerate_per_kw: u32, dust_exposure_limiting_feerate: Option<u32>,
feerate_per_kw: u32, include_fee_spike_multiple: bool,
dust_exposure_limiting_feerate: Option<u32>,
) -> Result<(ChannelStats, Vec<HTLCAmountDirection>), ()> {
let next_commitment_htlcs = self.get_next_commitment_htlcs(
true,
Expand All @@ -5306,6 +5309,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
&next_commitment_htlcs,
addl_nondust_htlc_count,
feerate_per_kw,
include_fee_spike_multiple,
dust_exposure_limiting_feerate,
max_dust_htlc_exposure_msat,
channel_constraints,
Expand All @@ -5330,6 +5334,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
&next_commitment_htlcs,
0,
feerate_per_kw,
false,
dust_exposure_limiting_feerate,
max_dust_htlc_exposure_msat,
channel_constraints,
Expand All @@ -5351,7 +5356,8 @@ impl<SP: SignerProvider> ChannelContext<SP> {
fn get_next_remote_commitment_stats(
&self, funding: &FundingScope, htlc_candidate: Option<HTLCAmountDirection>,
include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize,
feerate_per_kw: u32, dust_exposure_limiting_feerate: Option<u32>,
feerate_per_kw: u32, include_fee_spike_multiple: bool,
dust_exposure_limiting_feerate: Option<u32>,
) -> Result<(ChannelStats, Vec<HTLCAmountDirection>), ()> {
let next_commitment_htlcs = self.get_next_commitment_htlcs(
false,
Expand All @@ -5373,6 +5379,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
&next_commitment_htlcs,
addl_nondust_htlc_count,
feerate_per_kw,
include_fee_spike_multiple,
dust_exposure_limiting_feerate,
max_dust_htlc_exposure_msat,
channel_constraints,
Expand All @@ -5397,6 +5404,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
&next_commitment_htlcs,
0,
feerate_per_kw,
false,
dust_exposure_limiting_feerate,
max_dust_htlc_exposure_msat,
channel_constraints,
Expand Down Expand Up @@ -5439,6 +5447,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
fee_spike_buffer_htlc,
self.feerate_per_kw,
false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
Expand Down Expand Up @@ -5497,6 +5506,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
fee_spike_buffer_htlc,
self.feerate_per_kw,
false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
Expand All @@ -5523,6 +5533,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
0,
new_feerate_per_kw,
false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
Expand All @@ -5544,6 +5555,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
0,
new_feerate_per_kw,
false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
Expand Down Expand Up @@ -5724,6 +5736,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize,
feerate_per_kw,
false,
dust_exposure_limiting_feerate,
) {
stats
Expand Down Expand Up @@ -5763,6 +5776,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize,
feerate_per_kw,
false,
dust_exposure_limiting_feerate,
) {
stats
Expand Down Expand Up @@ -5810,6 +5824,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
fee_spike_buffer_htlc,
feerate,
false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
Expand All @@ -5826,6 +5841,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
fee_spike_buffer_htlc,
feerate,
false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
Expand Down Expand Up @@ -5862,21 +5878,14 @@ impl<SP: SignerProvider> ChannelContext<SP> {
if !funding.is_outbound() {
// Note that with anchor outputs we are no longer as sensitive to fee spikes, so we don't need
// to account for them.
let fee_spike_multiple =
if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32
} else {
1
};
// Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop
let spiked_feerate = feerate.saturating_mul(fee_spike_multiple);
let (remote_stats, _remote_htlcs) = self
.get_next_remote_commitment_stats(
funding,
None,
include_counterparty_unknown_htlcs,
fee_spike_buffer_htlc,
spiked_feerate,
feerate,
true,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
Expand Down Expand Up @@ -6231,6 +6240,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
addl_nondust_htlc_count,
self.feerate_per_kw,
false,
dust_exposure_limiting_feerate,
)
.map(|(remote_stats, _)| remote_stats.available_balances)?;
Expand All @@ -6252,6 +6262,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
addl_nondust_htlc_count,
self.feerate_per_kw,
false,
dust_exposure_limiting_feerate,
)
.unwrap();
Expand Down Expand Up @@ -13372,16 +13383,6 @@ where
// We are not interested in dust exposure
let dust_exposure_limiting_feerate = None;

// Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop
let feerate_per_kw = if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
// Similar to HTLC additions, require the funder to have enough funds reserved for
// fees such that the feerate can jump without rendering the channel useless.
let spike_mul = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
self.context.feerate_per_kw.saturating_mul(spike_mul)
} else {
self.context.feerate_per_kw
};

// Different dust limits on the local and remote commitments cause the commitment
// transaction fee to be different depending on the commitment, so we grab the floor
// of both balances across both commitments here.
Expand All @@ -13399,7 +13400,8 @@ where
None, // htlc_candidate
include_counterparty_unknown_htlcs,
addl_nondust_htlc_count,
feerate_per_kw,
self.context.feerate_per_kw,
true,
dust_exposure_limiting_feerate,
)
.map_err(|()| "Balance exhausted on local commitment")?;
Expand All @@ -13411,7 +13413,8 @@ where
None, // htlc_candidate
include_counterparty_unknown_htlcs,
addl_nondust_htlc_count,
feerate_per_kw,
self.context.feerate_per_kw,
true,
dust_exposure_limiting_feerate,
)
.map_err(|()| "Balance exhausted on remote commitment")?;
Expand Down Expand Up @@ -13451,6 +13454,7 @@ where
include_counterparty_unknown_htlcs,
0,
self.context.feerate_per_kw,
false,
dust_exposure_limiting_feerate,
)
.map_err(|()| "Balance exhausted on remote commitment")?;
Expand Down Expand Up @@ -17186,7 +17190,7 @@ mod tests {
// Make sure when Node A calculates their local commitment transaction, none of the HTLCs pass
// the dust limit check.
let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amount_msat, outbound: true };
let local_commit_tx_fee = node_a_chan.context.get_next_local_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
let local_commit_tx_fee = node_a_chan.context.get_next_local_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
let local_commit_fee_0_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 0, node_a_chan.funding.get_channel_type()) * 1000;
assert_eq!(local_commit_tx_fee, local_commit_fee_0_htlcs);

Expand All @@ -17195,7 +17199,7 @@ mod tests {
node_a_chan.funding.channel_transaction_parameters.is_outbound_from_holder = false;
let remote_commit_fee_3_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 3, node_a_chan.funding.get_channel_type()) * 1000;
let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amount_msat, outbound: true };
let remote_commit_tx_fee = node_a_chan.context.get_next_remote_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
let remote_commit_tx_fee = node_a_chan.context.get_next_remote_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
assert_eq!(remote_commit_tx_fee, remote_commit_fee_3_htlcs);
}

Expand Down Expand Up @@ -17230,27 +17234,27 @@ mod tests {
// counted as dust when it shouldn't be.
let htlc_amt_above_timeout = (htlc_timeout_tx_fee_sat + chan.context.holder_dust_limit_satoshis + 1) * 1000;
let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amt_above_timeout, outbound: true };
let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
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 = (htlc_success_tx_fee_sat + chan.context.holder_dust_limit_satoshis - 1) * 1000;
let htlc_candidate = HTLCAmountDirection { amount_msat: dust_htlc_amt_below_success, outbound: false };
let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs);

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 = (htlc_timeout_tx_fee_sat + chan.context.counterparty_dust_limit_satoshis + 1) * 1000;
let htlc_candidate = HTLCAmountDirection { amount_msat: dust_htlc_amt_above_timeout, outbound: true };
let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
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 = (htlc_success_tx_fee_sat + chan.context.counterparty_dust_limit_satoshis - 1) * 1000;
let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amt_below_success, outbound: false };
let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000;
assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc);
}

Expand Down
Loading
Loading