From a430756861a13c577ca697416448ddc9a81aab27 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Thu, 23 Apr 2026 05:41:36 +0000 Subject: [PATCH 1/6] As a channel fundee, keep a fee spike buffer from the no-outputs case As a channel fundee, we previously could send HTLCs such that a fee spike in a legacy channel triggered the no-outputs case in zero-reserve channels. We now also maintain a buffer from the no-outputs case when we are the channel fundee. --- lightning/src/sign/tx_builder.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index ffb01c571b7..ea27adc441e 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -446,15 +446,14 @@ fn get_available_balances( if channel_type.supports_anchor_zero_fee_commitments() { 0 } else { 1 }; // Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop - let spiked_feerate = feerate_per_kw.saturating_mul( - if is_outbound_from_holder && !channel_type.supports_anchors_zero_fee_htlc_tx() { + let spiked_feerate = + feerate_per_kw.saturating_mul(if !channel_type.supports_anchors_zero_fee_htlc_tx() { crate::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 } else { 1 - }, - ); + }); - let local_nondust_htlc_count = pending_htlcs + let local_spiked_nondust_htlc_count = pending_htlcs .iter() .filter(|htlc| { !htlc.is_dust( @@ -467,12 +466,12 @@ fn get_available_balances( .count(); let local_max_commit_tx_fee_sat = commit_tx_fee_sat( spiked_feerate, - local_nondust_htlc_count + fee_spike_buffer_htlc + 1, + local_spiked_nondust_htlc_count + fee_spike_buffer_htlc + 1, channel_type, ); let local_min_commit_tx_fee_sat = commit_tx_fee_sat( spiked_feerate, - local_nondust_htlc_count + fee_spike_buffer_htlc, + local_spiked_nondust_htlc_count + fee_spike_buffer_htlc, channel_type, ); let (local_dust_exposure_msat, _) = get_dust_exposure_stats( @@ -528,7 +527,7 @@ fn get_available_balances( remote_balance_before_fee_msat, spiked_feerate, // The number of non-dust HTLCs on the local commitment at the spiked feerate - local_nondust_htlc_count, + local_spiked_nondust_htlc_count, // The post-splice minimum balance of the holder if is_outbound_from_holder { local_min_commit_tx_fee_sat } else { 0 }, &channel_constraints, @@ -661,7 +660,7 @@ fn get_available_balances( // Now adjust our min and max size HTLC to make sure both the local and the remote commitments still have // at least one output at the spiked feerate. - let remote_nondust_htlc_count = pending_htlcs + let remote_spiked_nondust_htlc_count = pending_htlcs .iter() .filter(|htlc| { !htlc.is_dust( @@ -679,8 +678,8 @@ fn get_available_balances( is_outbound_from_holder, local_balance_before_fee_msat, remote_balance_before_fee_msat, - local_nondust_htlc_count, spiked_feerate, + local_spiked_nondust_htlc_count, channel_constraints.holder_dust_limit_satoshis, channel_type, next_outbound_htlc_minimum_msat, @@ -693,8 +692,8 @@ fn get_available_balances( is_outbound_from_holder, local_balance_before_fee_msat, remote_balance_before_fee_msat, - remote_nondust_htlc_count, spiked_feerate, + remote_spiked_nondust_htlc_count, channel_constraints.counterparty_dust_limit_satoshis, channel_type, next_outbound_htlc_minimum_msat, @@ -715,9 +714,10 @@ fn get_available_balances( fn adjust_boundaries_if_max_dust_htlc_produces_no_output( local: bool, is_outbound_from_holder: bool, holder_balance_before_fee_msat: u64, - counterparty_balance_before_fee_msat: u64, nondust_htlc_count: usize, spiked_feerate: u32, - dust_limit_satoshis: u64, channel_type: &ChannelTypeFeatures, - next_outbound_htlc_minimum_msat: u64, available_capacity_msat: u64, + counterparty_balance_before_fee_msat: u64, spiked_feerate: u32, + spiked_feerate_nondust_htlc_count: usize, dust_limit_satoshis: u64, + channel_type: &ChannelTypeFeatures, next_outbound_htlc_minimum_msat: u64, + available_capacity_msat: u64, ) -> (u64, u64) { // First, determine the biggest dust HTLC we could send let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = @@ -733,7 +733,7 @@ fn adjust_boundaries_if_max_dust_htlc_produces_no_output( holder_balance_before_fee_msat.saturating_sub(max_dust_htlc_msat), counterparty_balance_before_fee_msat, spiked_feerate, - nondust_htlc_count, + spiked_feerate_nondust_htlc_count, dust_limit_satoshis, channel_type, ) { From 7efe9c5126ad7b7bc1ea260814d23e6ccb13e39e Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Fri, 17 Apr 2026 16:45:34 +0000 Subject: [PATCH 2/6] Don't trim HTLCs when calculating the reserved commit tx fee We previously accounted for HTLC trims at the spiked feerate when calculating the reserved commitment transaction fees. This could cause an underestimate of the real current commitment fee at the current channel feerate. This is because a 2x increase in the feerate could trim enough HTLCs to result in a smaller commitment transaction fee. Also, the previous code only reserved the fee for an exact 2x increase in the feerate, instead of reserving the fee for any increase in the feerate between 1x to 2x. Fixes #4563. --- lightning/src/sign/tx_builder.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index ea27adc441e..51ef294d5ef 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -453,6 +453,17 @@ fn get_available_balances( 1 }); + let local_nondust_htlc_count = pending_htlcs + .iter() + .filter(|htlc| { + !htlc.is_dust( + true, + feerate_per_kw, + channel_constraints.holder_dust_limit_satoshis, + channel_type, + ) + }) + .count(); let local_spiked_nondust_htlc_count = pending_htlcs .iter() .filter(|htlc| { @@ -464,14 +475,18 @@ fn get_available_balances( ) }) .count(); + + // Note here we use the htlc count at the current feerate together with the spiked feerate; + // this makes sure that the holder can afford any fee bump between 1x to 2x from the current + // feerate. let local_max_commit_tx_fee_sat = commit_tx_fee_sat( spiked_feerate, - local_spiked_nondust_htlc_count + fee_spike_buffer_htlc + 1, + local_nondust_htlc_count + fee_spike_buffer_htlc + 1, channel_type, ); let local_min_commit_tx_fee_sat = commit_tx_fee_sat( spiked_feerate, - local_spiked_nondust_htlc_count + fee_spike_buffer_htlc, + local_nondust_htlc_count + fee_spike_buffer_htlc, channel_type, ); let (local_dust_exposure_msat, _) = get_dust_exposure_stats( From def7abc994c313af21de0e0f63ff03a4826f2103 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Sat, 18 Apr 2026 15:48:42 +0000 Subject: [PATCH 3/6] Add test coverage for `tx_builder::get_available_balances` Focus on reserved commitment transaction fees, and available capacity in the 0-reserve, fundee case. --- lightning/src/ln/htlc_reserve_unit_tests.rs | 234 ++++++++++++++++++++ 1 file changed, 234 insertions(+) diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs index 45d3cf5950f..03c8b20646e 100644 --- a/lightning/src/ln/htlc_reserve_unit_tests.rs +++ b/lightning/src/ln/htlc_reserve_unit_tests.rs @@ -11,6 +11,7 @@ use crate::ln::channel::{ FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, MIN_CHAN_DUST_LIMIT_SATOSHIS, }; +use crate::ln::channel_state::ChannelDetails; use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, TrustedChannelFeatures}; use crate::ln::functional_test_utils::*; use crate::ln::msgs::{self, BaseMessageHandler, ChannelMessageHandler, MessageSendEvent}; @@ -3406,3 +3407,236 @@ fn test_0reserve_zero_conf_combined() { assert_eq!(node_1_max_htlc, node_0_max_htlc - node_1_reserve * 1000); send_payment(&nodes[1], &[&nodes[0]], node_1_max_htlc); } + +#[xtest(feature = "_externalize_tests")] +fn test_0reserve_outbound_vs_available_capacity_outbound_htlc_limit() { + let mut config = test_default_channel_config(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage = + 100; + + let channel_type = ChannelTypeFeatures::only_static_remote_key(); + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); + let 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(); + + const FEERATE: u32 = 253; + const MULTIPLE: u32 = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32; + const SPIKED_FEERATE: u32 = FEERATE * MULTIPLE; + const DUST_LIMIT_MSAT: u64 = 546 * 1000; + const CHANNEL_VALUE_MSAT: u64 = 10_000 * 1000; + const NODE_0_VALUE_TO_SELF_MSAT: u64 = 5000 * 1000; + const NODE_1_VALUE_TO_SELF_MSAT: u64 = 5000 * 1000; + + // Find the HTLC amount that will be non-dust at the current feerate, but dust at the spiked feerate + const SPIKED_DUST_HTLC_MSAT: u64 = 880 * 1000; + const HTLC_SPIKE_DUST_LIMIT_MSAT: u64 = 881 * 1000; + let htlc_timeout_spike_tx_fee_msat = + second_stage_tx_fees_sat(&channel_type, SPIKED_FEERATE).1 * 1000; + assert_eq!(HTLC_SPIKE_DUST_LIMIT_MSAT, DUST_LIMIT_MSAT + htlc_timeout_spike_tx_fee_msat); + + let real_htlc_timeout_tx_fee_msat = second_stage_tx_fees_sat(&channel_type, FEERATE).1 * 1000; + let real_htlc_timeout_dust_limit_msat = DUST_LIMIT_MSAT + real_htlc_timeout_tx_fee_msat; + + let (channel_id, _funding_tx) = setup_0reserve_no_outputs_channels( + &nodes, + CHANNEL_VALUE_MSAT / 1000, + DUST_LIMIT_MSAT / 1000, + ); + assert_eq!(nodes[0].node.list_channels()[0].channel_type.as_ref().unwrap(), &channel_type); + + // Balance the channel so each side has 5_000 sats + send_payment(&nodes[0], &[&nodes[1]], NODE_1_VALUE_TO_SELF_MSAT); + + let count_total_htlcs = |details: &ChannelDetails| { + details.pending_outbound_htlcs.len() + details.pending_inbound_htlcs.len() + }; + let count_node_0_nondust_htlcs = || { + let mut txs = get_local_commitment_txn!(nodes[0], channel_id); + let commitment_tx = &txs[0]; + commitment_tx + .output + .iter() + .filter(|output| output.value.to_sat() * 1000 == SPIKED_DUST_HTLC_MSAT) + .count() + }; + let count_node_1_nondust_htlcs = || { + let mut txs = get_local_commitment_txn!(nodes[1], channel_id); + let commitment_tx = &txs[0]; + commitment_tx + .output + .iter() + .filter(|output| output.value.to_sat() * 1000 == SPIKED_DUST_HTLC_MSAT) + .count() + }; + + // Sanity check + { + let reserved_fee_sat = commit_tx_fee_sat(SPIKED_FEERATE, 2, &channel_type); + let node_0_outbound_capacity_msat = NODE_0_VALUE_TO_SELF_MSAT; + let node_0_available_capacity_msat = + node_0_outbound_capacity_msat - reserved_fee_sat * 1000; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + assert_eq!(count_total_htlcs(&node_0_details), 0); + assert_eq!(count_node_0_nondust_htlcs(), 0); + } + + // Route 3 880sat HTLCs from node 0 to node 1 + for i in 1..4 { + route_payment(&nodes[0], &[&nodes[1]], SPIKED_DUST_HTLC_MSAT); + + let max_reserved_fee_msat = commit_tx_fee_sat(SPIKED_FEERATE, 2 + i, &channel_type) * 1000; + let node_0_outbound_capacity_msat = + NODE_0_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * i as u64; + let node_0_available_capacity_msat = node_0_outbound_capacity_msat - max_reserved_fee_msat; + // Node 0 can send non-dust HTLCs throughout + assert!(node_0_available_capacity_msat >= HTLC_SPIKE_DUST_LIMIT_MSAT); + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + assert_eq!(count_total_htlcs(&node_0_details), i); + assert_eq!(count_node_0_nondust_htlcs(), i); + } + + // Route one last 880sat HTLC, after which node 0 can only send dust HTLCs + route_payment(&nodes[0], &[&nodes[1]], SPIKED_DUST_HTLC_MSAT); + + let node_0_details = &nodes[0].node.list_channels()[0]; + let local_nondust_htlc_count = 4; + assert_eq!(count_total_htlcs(&node_0_details), local_nondust_htlc_count); + assert_eq!(count_node_0_nondust_htlcs(), local_nondust_htlc_count); + + let node_0_outbound_capacity_msat = + NODE_0_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * local_nondust_htlc_count as u64; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + + // Node 0 can only send dust HTLCs + let min_reserved_fee_msat = + commit_tx_fee_sat(SPIKED_FEERATE, local_nondust_htlc_count + 1, &channel_type) * 1000; + let node_0_available_capacity_msat = node_0_outbound_capacity_msat - min_reserved_fee_msat; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert!(node_0_details.next_outbound_htlc_limit_msat < real_htlc_timeout_dust_limit_msat); + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + + // Route a dust HTLC, and confirm node 0's main output is now below its dust limit + let current_commit_tx_fee_msat = + commit_tx_fee_sat(FEERATE, local_nondust_htlc_count, &channel_type) * 1000; + let to_local_msat = NODE_0_VALUE_TO_SELF_MSAT + - SPIKED_DUST_HTLC_MSAT * local_nondust_htlc_count as u64 + - current_commit_tx_fee_msat; + assert!(to_local_msat > DUST_LIMIT_MSAT); + assert!(to_local_msat - 600_000 < DUST_LIMIT_MSAT); + assert!(600_000 <= node_0_available_capacity_msat); + + route_payment(&nodes[0], &[&nodes[1]], 600_000); + + let node_0_balance_before_fee_msat = + NODE_0_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * 4 - 600_000; + assert_eq!( + nodes[0].node.list_channels()[0].outbound_capacity_msat, + node_0_balance_before_fee_msat + ); + assert_eq!( + nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat, + node_0_balance_before_fee_msat - min_reserved_fee_msat + ); + assert_eq!(count_node_0_nondust_htlcs(), local_nondust_htlc_count); + assert_eq!(count_node_1_nondust_htlcs(), local_nondust_htlc_count); + + // Route 4 880sat HTLCs from node 1 to node 0 + for i in 1..5 { + route_payment(&nodes[1], &[&nodes[0]], SPIKED_DUST_HTLC_MSAT); + + let node_1_outbound_capacity_msat = + NODE_1_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * i as u64; + assert!(node_1_outbound_capacity_msat >= HTLC_SPIKE_DUST_LIMIT_MSAT); + let node_1_details = &nodes[1].node.list_channels()[0]; + assert_eq!(node_1_details.outbound_capacity_msat, node_1_outbound_capacity_msat); + assert_eq!(node_1_details.next_outbound_htlc_limit_msat, node_1_outbound_capacity_msat); + + // Sending the greatest dust HTLC still lands node 1's main output above its dust limit + assert!( + node_1_outbound_capacity_msat - (HTLC_SPIKE_DUST_LIMIT_MSAT - 1) >= DUST_LIMIT_MSAT + ); + + let nondust_htlc_count = 4 + i; + // At the current feerate, 880sat HTLCs are present on both commitments + assert_eq!(count_node_0_nondust_htlcs(), nondust_htlc_count); + assert_eq!(count_node_1_nondust_htlcs(), nondust_htlc_count); + + // Node 0's outbound capacity does not budge, and its available capacity is 0 + // TODO: Node 0 should really be rejecting HTLCs here, but it only checks against the spiked buffer + // if it is the fundee, and here it is the funder. + assert!( + node_0_balance_before_fee_msat + >= commit_tx_fee_sat(FEERATE, nondust_htlc_count, &channel_type) + ); + assert_eq!( + nodes[0].node.list_channels()[0].outbound_capacity_msat, + node_0_balance_before_fee_msat + ); + assert_eq!(nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat, 0); + } + + // Route another 880sat HTLC + let (preimage, _hash, _secret, _id) = + route_payment(&nodes[1], &[&nodes[0]], SPIKED_DUST_HTLC_MSAT); + + let node_1_outbound_capacity_msat = NODE_1_VALUE_TO_SELF_MSAT - 5 * SPIKED_DUST_HTLC_MSAT; + let node_1_details = &nodes[1].node.list_channels()[0]; + assert_eq!(node_1_details.outbound_capacity_msat, node_1_outbound_capacity_msat); + // At this point, sending the greatest dust HTLC pushes node 1's main output below the dust limit + assert!( + node_1_outbound_capacity_msat.saturating_sub(HTLC_SPIKE_DUST_LIMIT_MSAT - 1) + < DUST_LIMIT_MSAT + ); + // So we subtract the dust limit from node 1's outbound capacity to arrive at node 1's available capacity + let node_1_available_capacity_msat = node_1_outbound_capacity_msat - DUST_LIMIT_MSAT; + assert_eq!( + nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat, + node_1_available_capacity_msat + ); + + // A few sanity checks + let nondust_htlc_count = 9; + assert_eq!(count_node_0_nondust_htlcs(), nondust_htlc_count); + assert_eq!(count_node_1_nondust_htlcs(), nondust_htlc_count); + // Node 0, the funder, can still afford all the HTLCs on the commitments + let node_0_balance_before_fee_msat = nodes[0].node.list_channels()[0].outbound_capacity_msat; + assert!( + node_0_balance_before_fee_msat + >= commit_tx_fee_sat(FEERATE, nondust_htlc_count, &channel_type) + ); + + // Node 0 claims an 880sat HTLC, which brings its output back above its dust limit + claim_payment(&nodes[1], &[&nodes[0]], preimage); + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!( + node_0_details.outbound_capacity_msat, + node_0_balance_before_fee_msat + SPIKED_DUST_HTLC_MSAT + ); + + // Node 1 is now free to withdraw all of its channel balance + let node_1_details = &nodes[1].node.list_channels()[0]; + assert_eq!(node_1_details.outbound_capacity_msat, node_1_outbound_capacity_msat); + assert_eq!( + nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat, + node_1_outbound_capacity_msat + ); + + send_payment(&nodes[1], &[&nodes[0]], node_1_outbound_capacity_msat); + + let node_1_details = &nodes[1].node.list_channels()[0]; + assert_eq!(node_1_details.outbound_capacity_msat, 0); + assert_eq!(nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat, 0); +} From 33edd17cfd067ce4253eeb2593c030ee0a66e105 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Tue, 21 Apr 2026 16:42:27 +0000 Subject: [PATCH 4/6] Move the calculation of the spiked feerate to `tx_builder` In the next commit, we will make changes to how the fee spike buffer is calculated which require the real feerate to always be passed to `tx_builder::get_next_commitment_stats`, even in the case where we include a fee spike multiple. --- lightning/src/ln/channel.rs | 62 +++++++++++++++++--------------- lightning/src/sign/tx_builder.rs | 32 ++++++++++------- 2 files changed, 53 insertions(+), 41 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e07ee7fceab..ff807739d5f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4198,6 +4198,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, channel_context.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -4494,6 +4495,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, channel_context.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| APIError::APIMisuseError { @@ -5284,7 +5286,8 @@ impl ChannelContext { fn get_next_local_commitment_stats( &self, funding: &FundingScope, htlc_candidate: Option, include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize, - feerate_per_kw: u32, dust_exposure_limiting_feerate: Option, + feerate_per_kw: u32, include_fee_spike_multiple: bool, + dust_exposure_limiting_feerate: Option, ) -> Result<(ChannelStats, Vec), ()> { let next_commitment_htlcs = self.get_next_commitment_htlcs( true, @@ -5306,6 +5309,7 @@ impl ChannelContext { &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, @@ -5330,6 +5334,7 @@ impl ChannelContext { &next_commitment_htlcs, 0, feerate_per_kw, + false, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5351,7 +5356,8 @@ impl ChannelContext { fn get_next_remote_commitment_stats( &self, funding: &FundingScope, htlc_candidate: Option, include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize, - feerate_per_kw: u32, dust_exposure_limiting_feerate: Option, + feerate_per_kw: u32, include_fee_spike_multiple: bool, + dust_exposure_limiting_feerate: Option, ) -> Result<(ChannelStats, Vec), ()> { let next_commitment_htlcs = self.get_next_commitment_htlcs( false, @@ -5373,6 +5379,7 @@ impl ChannelContext { &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, @@ -5397,6 +5404,7 @@ impl ChannelContext { &next_commitment_htlcs, 0, feerate_per_kw, + false, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5439,6 +5447,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5497,6 +5506,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5523,6 +5533,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, 0, new_feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5544,6 +5555,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, 0, new_feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5724,6 +5736,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, feerate_per_kw, + false, dust_exposure_limiting_feerate, ) { stats @@ -5763,6 +5776,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, feerate_per_kw, + false, dust_exposure_limiting_feerate, ) { stats @@ -5810,6 +5824,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, feerate, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5826,6 +5841,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, feerate, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5862,21 +5878,14 @@ impl ChannelContext { 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(|()| { @@ -6231,6 +6240,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map(|(remote_stats, _)| remote_stats.available_balances)?; @@ -6252,6 +6262,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .unwrap(); @@ -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. @@ -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")?; @@ -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")?; @@ -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")?; @@ -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); @@ -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); } @@ -17230,13 +17234,13 @@ 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; @@ -17244,13 +17248,13 @@ mod tests { // 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); } diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index 51ef294d5ef..a96539c6ace 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -11,7 +11,7 @@ use crate::ln::chan_utils::{ }; use crate::ln::channel::{ get_v2_channel_reserve_satoshis, CommitmentStats, ANCHOR_OUTPUT_VALUE_SATOSHI, - MIN_CHANNEL_VALUE_SATOSHIS, + FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_CHANNEL_VALUE_SATOSHIS, }; use crate::prelude::*; use crate::types::features::ChannelTypeFeatures; @@ -219,7 +219,7 @@ fn has_output( fn get_next_commitment_stats( local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, value_to_holder_msat: u64, next_commitment_htlcs: &[HTLCAmountDirection], - addl_nondust_htlc_count: usize, feerate_per_kw: u32, + addl_nondust_htlc_count: usize, feerate_per_kw: u32, include_fee_spike_multiple: bool, dust_exposure_limiting_feerate: Option, broadcaster_dust_limit_satoshis: u64, channel_type: &ChannelTypeFeatures, ) -> Result { @@ -270,11 +270,17 @@ fn get_next_commitment_stats( channel_type, ); - // Calculate fees on commitment transaction - let nondust_htlc_count = next_commitment_htlcs + let spiked_feerate = + if include_fee_spike_multiple && !channel_type.supports_anchors_zero_fee_htlc_tx() { + feerate_per_kw.saturating_mul(FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32) + } else { + feerate_per_kw + }; + + let spiked_nondust_htlc_count = next_commitment_htlcs .iter() .filter(|htlc| { - !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_satoshis, channel_type) + !htlc.is_dust(local, spiked_feerate, broadcaster_dust_limit_satoshis, channel_type) }) .count(); @@ -284,8 +290,8 @@ fn get_next_commitment_stats( is_outbound_from_holder, holder_balance_before_fee_msat, counterparty_balance_before_fee_msat, - feerate_per_kw, - nondust_htlc_count, + spiked_feerate, + spiked_nondust_htlc_count, broadcaster_dust_limit_satoshis, channel_type, ) { @@ -296,8 +302,8 @@ fn get_next_commitment_stats( // this bigger transaction fee ? The funder can dip below their dust limit to cover this case, as the // commitment will have at least one output: the non-dust fee spike buffer HTLC offered by the counterparty. let commit_tx_fee_sat = commit_tx_fee_sat( - feerate_per_kw, - nondust_htlc_count + addl_nondust_htlc_count, + spiked_feerate, + spiked_nondust_htlc_count + addl_nondust_htlc_count, channel_type, ); let (holder_balance_msat, counterparty_balance_msat) = checked_sub_from_funder( @@ -312,7 +318,7 @@ fn get_next_commitment_stats( counterparty_balance_msat, dust_exposure_msat, #[cfg(any(test, fuzzing))] - nondust_htlc_count: nondust_htlc_count + addl_nondust_htlc_count, + nondust_htlc_count: spiked_nondust_htlc_count + addl_nondust_htlc_count, #[cfg(any(test, fuzzing))] commit_tx_fee_sat, }) @@ -801,7 +807,7 @@ pub(crate) trait TxBuilder { fn get_channel_stats( &self, local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, value_to_holder_msat: u64, pending_htlcs: &[HTLCAmountDirection], - addl_nondust_htlc_count: usize, feerate_per_kw: u32, + addl_nondust_htlc_count: usize, feerate_per_kw: u32, include_fee_spike_multiple: bool, dust_exposure_limiting_feerate: Option, max_dust_htlc_exposure_msat: u64, channel_constraints: ChannelConstraints, channel_type: &ChannelTypeFeatures, ) -> Result; @@ -819,7 +825,7 @@ impl TxBuilder for SpecTxBuilder { fn get_channel_stats( &self, local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, value_to_holder_msat: u64, pending_htlcs: &[HTLCAmountDirection], - addl_nondust_htlc_count: usize, feerate_per_kw: u32, + addl_nondust_htlc_count: usize, feerate_per_kw: u32, include_fee_spike_multiple: bool, dust_exposure_limiting_feerate: Option, max_dust_htlc_exposure_msat: u64, channel_constraints: ChannelConstraints, channel_type: &ChannelTypeFeatures, ) -> Result { @@ -832,6 +838,7 @@ impl TxBuilder for SpecTxBuilder { pending_htlcs, addl_nondust_htlc_count, feerate_per_kw, + include_fee_spike_multiple, dust_exposure_limiting_feerate, channel_constraints.holder_dust_limit_satoshis, channel_type, @@ -845,6 +852,7 @@ impl TxBuilder for SpecTxBuilder { pending_htlcs, addl_nondust_htlc_count, feerate_per_kw, + include_fee_spike_multiple, dust_exposure_limiting_feerate, channel_constraints.counterparty_dust_limit_satoshis, channel_type, From 11cbc95856f20e654e124951a64af035d2509828 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Tue, 21 Apr 2026 16:46:11 +0000 Subject: [PATCH 5/6] Don't trim HTLCs when calculating the fee spike commit tx fee We previously accounted for HTLC trims at the spiked feerate when calculating the commitment transaction fee including the fee spike multiple. This only ensured that the funder of the channel could afford the commitment transaction fee for an exact 2x increase in the feerate. Now, we check that the funder can cover any increase in the feerate between 1x to 2x. --- lightning/src/sign/tx_builder.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index a96539c6ace..6e136ddd7ed 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -301,9 +301,18 @@ fn get_next_commitment_stats( // 2) Now including any additional non-dust HTLCs (usually the fee spike buffer HTLC), does the funder cover // this bigger transaction fee ? The funder can dip below their dust limit to cover this case, as the // commitment will have at least one output: the non-dust fee spike buffer HTLC offered by the counterparty. + let nondust_htlc_count = next_commitment_htlcs + .iter() + .filter(|htlc| { + !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_satoshis, channel_type) + }) + .count(); + // Note here we use the htlc count at the current feerate together with the spiked feerate; + // this makes sure that the holder can afford any fee bump between 1x to 2x from the current + // feerate if the fee spike multiple is included. let commit_tx_fee_sat = commit_tx_fee_sat( spiked_feerate, - spiked_nondust_htlc_count + addl_nondust_htlc_count, + nondust_htlc_count + addl_nondust_htlc_count, channel_type, ); let (holder_balance_msat, counterparty_balance_msat) = checked_sub_from_funder( @@ -318,7 +327,7 @@ fn get_next_commitment_stats( counterparty_balance_msat, dust_exposure_msat, #[cfg(any(test, fuzzing))] - nondust_htlc_count: spiked_nondust_htlc_count + addl_nondust_htlc_count, + nondust_htlc_count: nondust_htlc_count + addl_nondust_htlc_count, #[cfg(any(test, fuzzing))] commit_tx_fee_sat, }) From 6d8b153e5c3b31f2ca7f9f025a7b705238ffbb9f Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Tue, 21 Apr 2026 23:21:09 +0000 Subject: [PATCH 6/6] Test the fee spike buffer multiple affordability check Make sure it correctly does not trim HTLCs at the spiked feerate, see the previous commit for further details. --- lightning/src/ln/htlc_reserve_unit_tests.rs | 268 +++++++++++++++++--- 1 file changed, 233 insertions(+), 35 deletions(-) diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs index 03c8b20646e..e288c4ec34d 100644 --- a/lightning/src/ln/htlc_reserve_unit_tests.rs +++ b/lightning/src/ln/htlc_reserve_unit_tests.rs @@ -3,8 +3,8 @@ use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose}; use crate::ln::chan_utils::{ self, commit_tx_fee_sat, commitment_tx_base_weight, second_stage_tx_fees_sat, - shared_anchor_script_pubkey, CommitmentTransaction, COMMITMENT_TX_WEIGHT_PER_HTLC, - TRUC_CHILD_MAX_WEIGHT, + shared_anchor_script_pubkey, CommitmentTransaction, HTLCOutputInCommitment, + COMMITMENT_TX_WEIGHT_PER_HTLC, TRUC_CHILD_MAX_WEIGHT, }; use crate::ln::channel::{ get_holder_selected_channel_reserve_satoshis, Channel, ANCHOR_OUTPUT_VALUE_SATOSHI, @@ -888,7 +888,7 @@ pub fn do_test_fee_spike_buffer(cfg: Option, htlc_fails: bool) { // Build the remote commitment transaction so we can sign it, and then later use the // signature for the commitment_signed message. - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { + let accepted_htlc_info = HTLCOutputInCommitment { offered: false, amount_msat: payment_amt_msat, cltv_expiry: htlc_cltv, @@ -2143,7 +2143,7 @@ pub fn do_test_dust_limit_fee_accounting(can_afford: bool) { let (_payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], HTLC_AMT_SAT * 1000); // Grab a snapshot of these HTLCs to manually build the commitment transaction later... - let accepted_htlc = chan_utils::HTLCOutputInCommitment { + let accepted_htlc = HTLCOutputInCommitment { offered: false, amount_msat: HTLC_AMT_SAT * 1000, // Hard-coded to match the expected value @@ -2257,7 +2257,7 @@ pub fn do_test_dust_limit_fee_accounting(can_afford: bool) { &channel_type, ); - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { + let accepted_htlc_info = HTLCOutputInCommitment { offered: false, amount_msat: HTLC_AMT_SAT * 1000, cltv_expiry, @@ -2838,21 +2838,30 @@ fn do_test_0reserve_no_outputs_legacy(no_outputs_case: LegacyChannelsNoOutputs) return; } + let htlcs_in_commitment = vec![HTLCOutputInCommitment { + offered: false, + amount_msat: receiver_amount_msat, + cltv_expiry: htlc_cltv, + payment_hash, + transaction_output_index: Some(1), + }]; + manually_trigger_update_fail_htlc( &nodes, channel_id, - channel_value_sat, + channel_value_sat * 1000, dust_limit_satoshis, - receiver_amount_msat, - htlc_cltv, payment_hash, + htlcs_in_commitment, + false, ); } } fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( - nodes: &'a Vec>, channel_id: ChannelId, channel_value_sat: u64, - dust_limit_satoshis: u64, receiver_amount_msat: u64, htlc_cltv: u32, payment_hash: PaymentHash, + nodes: &'a Vec>, channel_id: ChannelId, value_to_self_msat: u64, + dust_limit_satoshis: u64, payment_hash: PaymentHash, + htlcs_in_commitment: Vec, can_afford_but_reserve_is_breached: bool, ) { let node_a_id = nodes[0].node.get_our_node_id(); let node_b_id = nodes[1].node.get_our_node_id(); @@ -2864,8 +2873,6 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( let feerate_per_kw = get_feerate!(nodes[0], nodes[1], channel_id); - const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1; - let (local_secret, next_local_point) = { let per_peer_state = nodes[0].node.per_peer_state.read().unwrap(); let chan_lock = per_peer_state.get(&node_b_id).unwrap().lock().unwrap(); @@ -2873,36 +2880,29 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( chan_lock.channel_by_id.get(&channel_id).and_then(Channel::as_funded).unwrap(); let chan_signer = local_chan.get_signer(); // Make the signer believe we validated another commitment, so we can release the secret + let commit_number = chan_signer.get_enforcement_state().last_holder_commitment; chan_signer.get_enforcement_state().last_holder_commitment -= 1; ( - chan_signer.release_commitment_secret(INITIAL_COMMITMENT_NUMBER).unwrap(), - chan_signer.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 2, &secp_ctx).unwrap(), + chan_signer.release_commitment_secret(commit_number).unwrap(), + chan_signer.get_per_commitment_point(commit_number - 2, &secp_ctx).unwrap(), ) }; - let remote_point = { + let (remote_commit_number, remote_point) = { let per_peer_lock; let mut peer_state_lock; let channel = get_channel_ref!(nodes[1], nodes[0], per_peer_lock, peer_state_lock, channel_id); let chan_signer = channel.as_funded().unwrap().get_signer(); - chan_signer.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx).unwrap() + let commit_number = chan_signer.get_enforcement_state().last_holder_commitment; + let remote_point = + chan_signer.get_per_commitment_point(commit_number - 1, &secp_ctx).unwrap(); + (commit_number - 1, remote_point) }; // Build the remote commitment transaction so we can sign it, and then later use the // signature for the commitment_signed message. - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { - offered: false, - amount_msat: receiver_amount_msat, - cltv_expiry: htlc_cltv, - payment_hash, - transaction_output_index: Some(1), - }; - - let local_chan_balance_msat = channel_value_sat * 1000; - let commitment_number = INITIAL_COMMITMENT_NUMBER - 1; - let res = { let per_peer_lock; let mut peer_state_lock; @@ -2913,12 +2913,12 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( let (commitment_tx, _stats) = SpecTxBuilder {}.build_commitment_transaction( false, - commitment_number, + remote_commit_number, &remote_point, &channel.funding().channel_transaction_parameters, &secp_ctx, - local_chan_balance_msat, - vec![accepted_htlc_info], + value_to_self_msat, + htlcs_in_commitment, feerate_per_kw, dust_limit_satoshis, &nodes[0].logger, @@ -2969,11 +2969,13 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( }, _ => panic!("Unexpected event"), }; - nodes[1].logger.assert_log( - "lightning::ln::channel", - "Attempting to fail HTLC due to balance exhausted on remote commitment".to_string(), - 1, - ); + let log_string = + if can_afford_but_reserve_is_breached { + String::from("Attempting to fail HTLC due to fee spike buffer violation. Rebalancing is required.") + } else { + String::from("Attempting to fail HTLC due to balance exhausted on remote commitment") + }; + nodes[1].logger.assert_log("lightning::ln::channel", log_string, 1); check_added_monitors(&nodes[1], 3); } @@ -3640,3 +3642,199 @@ fn test_0reserve_outbound_vs_available_capacity_outbound_htlc_limit() { assert_eq!(node_1_details.outbound_capacity_msat, 0); assert_eq!(nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat, 0); } + +/// Make sure that we do not account for HTLCs going from non-dust to dust at the spiked feerate +/// when checking the fee spike buffer in `can_accept_incoming_htlc`. This is required to make sure +/// that we can afford *any* increase in the feerate between 1x to 2x, instead of checking whether +/// we can afford only the 2x increase in the feerate. +#[xtest(feature = "_externalize_tests")] +fn test_fail_cannot_afford_dust_htlcs_at_spike_multiple_if_nondust_at_base_feerate() { + let mut config = test_default_channel_config(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage = + 100; + + let channel_type = ChannelTypeFeatures::only_static_remote_key(); + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); + let 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(); + + const FEERATE: u32 = 253; + const MULTIPLE: u32 = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32; + const SPIKED_FEERATE: u32 = FEERATE * MULTIPLE; + const DUST_LIMIT_MSAT: u64 = 354 * 1000; + const CHANNEL_VALUE_MSAT: u64 = 10_000 * 1000; + const NODE_0_VALUE_TO_SELF_MSAT: u64 = 5_000 * 1000; + const NODE_1_VALUE_TO_SELF_MSAT: u64 = 5_000 * 1000; + const CHANNEL_RESERVE_MSAT: u64 = 1_000 * 1_000; + + let channel_id = create_announced_chan_between_nodes_with_value( + &nodes, + 0, + 1, + CHANNEL_VALUE_MSAT / 1000, + NODE_1_VALUE_TO_SELF_MSAT, + ) + .2; + assert_eq!(nodes[0].node.list_channels()[0].channel_type.as_ref().unwrap(), &channel_type); + + // Find the HTLC amount that will be non-dust at the current feerate, + // but dust at the spiked feerate. + const SPIKED_DUST_HTLC_MSAT: u64 = 688 * 1000; + const HTLC_SPIKE_DUST_LIMIT_MSAT: u64 = 689 * 1000; + // When checking the fee spike buffer in `can_accept_incoming_htlc`, we check the remote + // commitment, hence inbound HTLCs will be offered HTLCs, and use the timeout dust limit. + let htlc_timeout_spike_tx_fee_msat = + second_stage_tx_fees_sat(&channel_type, SPIKED_FEERATE).1 * 1000; + assert_eq!(HTLC_SPIKE_DUST_LIMIT_MSAT, DUST_LIMIT_MSAT + htlc_timeout_spike_tx_fee_msat); + + // Calculate here the dust limit at the current feerate so we know when node 0 cannot send + // any further non-dust HTLCs at the current feerate. + let htlc_timeout_tx_fee_msat = second_stage_tx_fees_sat(&channel_type, FEERATE).1 * 1000; + let htlc_dust_limit_msat = DUST_LIMIT_MSAT + htlc_timeout_tx_fee_msat; + // Make sure the HTLC will be non-dust at the current feerate + assert!(SPIKED_DUST_HTLC_MSAT > htlc_dust_limit_msat); + + // Place a few non-dust HTLCs on the commitment, these HTLCs would get trimmed upon a 2x + // increase in the feerate. + let mut sent_htlcs_count: usize = 0; + let mut payment_hashes = Vec::new(); + while nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat >= htlc_dust_limit_msat { + let (_preimage, hash, _secret, _id) = + route_payment(&nodes[0], &[&nodes[1]], SPIKED_DUST_HTLC_MSAT); + payment_hashes.push(hash); + sent_htlcs_count += 1; + } + assert_eq!(sent_htlcs_count, 4); + + // Check the outbound and available capacities + let node_0_outbound_capacity_msat = NODE_0_VALUE_TO_SELF_MSAT + - sent_htlcs_count as u64 * SPIKED_DUST_HTLC_MSAT + - CHANNEL_RESERVE_MSAT; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + // Node 0 can now only send dust HTLCs, so we reserve the fees for a single additional + // inbound non-dust HTLC. + let min_reserved_fee_msat = + commit_tx_fee_sat(SPIKED_FEERATE, sent_htlcs_count + 1, &channel_type) * 1000; + let node_0_available_capacity_msat = node_0_outbound_capacity_msat - min_reserved_fee_msat; + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + + // Then send an identical, 5th non-dust HTLC, bypass the validation from the holder, and + // check that the counterparty fails it due to a fee spike buffer violation. + + // First check the maths + + // Node 0 can afford an exact 2x increase in the feerate + let spiked_commit_tx_fee_msat = commit_tx_fee_sat(SPIKED_FEERATE, 0, &channel_type) * 1000; + assert!((node_0_outbound_capacity_msat - SPIKED_DUST_HTLC_MSAT) + .checked_sub(spiked_commit_tx_fee_msat) + .is_some()); + // Node 0 can afford a 5th non-dust HTLC at the current feerate, so `update_add_htlc` + // validation will pass. + let real_commit_tx_fee_msat = commit_tx_fee_sat(FEERATE, 5, &channel_type) * 1000; + assert!((node_0_outbound_capacity_msat - SPIKED_DUST_HTLC_MSAT) + .checked_sub(real_commit_tx_fee_msat) + .is_some()); + // But we don't account for the HTLC trimming effect of the spike multiple feerate increase, + // so the 5th HTLC should be rejected at `can_accept_incoming_htlc`! + let expected_commit_tx_fee_msat = commit_tx_fee_sat(SPIKED_FEERATE, 5, &channel_type) * 1000; + assert!((node_0_outbound_capacity_msat - SPIKED_DUST_HTLC_MSAT) + .checked_sub(expected_commit_tx_fee_msat) + .is_none()); + + // Then run the experiment + + let sender_amount_msat = node_0_available_capacity_msat; + let receiver_amount_msat = SPIKED_DUST_HTLC_MSAT; + let (route, payment_hash, _, payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[1], sender_amount_msat); + let secp_ctx = Secp256k1::new(); + let session_priv = SecretKey::from_slice(&[42; 32]).unwrap(); + let cur_height = nodes[0].node.best_block.read().unwrap().height + 1; + let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv); + let recipient_onion_fields = + RecipientOnionFields::secret_only(payment_secret, sender_amount_msat); + let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::test_build_onion_payloads( + &route.paths[0], + &recipient_onion_fields, + cur_height, + &None, + None, + None, + ) + .unwrap(); + assert_eq!(htlc_msat, sender_amount_msat); + let onion_packet = + onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash) + .unwrap(); + let msg = msgs::UpdateAddHTLC { + channel_id, + htlc_id: sent_htlcs_count as u64, + amount_msat: receiver_amount_msat, + payment_hash, + cltv_expiry: htlc_cltv, + onion_routing_packet: onion_packet, + skimmed_fee_msat: None, + blinding_point: None, + hold_htlc: None, + accountable: None, + }; + + nodes[1].node.handle_update_add_htlc(node_a_id, &msg); + + let htlcs_in_tx = vec![ + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x75).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(0), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x64).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(1), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash, + amount_msat: 688_000, + transaction_output_index: Some(2), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x72).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(3), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x66).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(4), + }, + ]; + + manually_trigger_update_fail_htlc( + &nodes, + channel_id, + NODE_0_VALUE_TO_SELF_MSAT, + DUST_LIMIT_MSAT / 1000, + payment_hash, + htlcs_in_tx, + true, + ); +}