Don't trim HTLCs when calculating the fee spike commit tx fee
What changed, and why it matters
This commit fixes a bug in how Lightning Dev Kit calculates whether a channel funder can afford a sudden spike in Bitcoin transaction fees. Previously, the code assumed HTLCs (small conditional payments) that would become uneconomical at a doubled feerate were already trimmed away when checking the funder's buffer. This made the check too optimistic: it only verified the funder could survive an exact 2x fee jump, not any jump between normal and 2x. The fix counts HTLCs at the current feerate instead of the spiked one, ensuring the reserve covers the full range of possible fee increases. The commit is framed by its author as a correctness fix for fee-spike buffer accounting.
Treat as a security-relevant correctness fix. Review related fee-spike buffer logic across the codebase, run the new regression test, and consider whether any deployed nodes need to update to avoid accepting HTLCs that could leave the funder unable to cover commitment transaction fees during fee spikes.
Security signals we found
Fee-spike buffer accounting bug
HTLC dust trimming at spiked feerate
Channel funder reserve/balance validation
Potential channel force-close or fund loss if funder cannot cover commitment tx fee
New regression test added for the fixed behavior
Evidence from the diff
In lightning/src/sign/tx_builder.rs, get_next_commitment_stats previously computed spiked_nondust_htlc_count using the spiked feerate and used that count to compute commit_tx_fee_sat at the spiked feerate. Because HTLCs that are non-dust at the base feerate can become dust at the spiked feerate, this trimmed them from the fee calculation at the higher feerate, underestimating the worst-case commitment transaction fee for intermediate fee increases. The patch instead counts nondust_htlc_count at the current/base feerate and adds any additional non-dust HTLCs, then computes the fee at the spiked feerate with that count. This ensures the funder can afford any feerate increase from 1x to the configured spike multiple (e.g., 2x). A new unit test test_fail_cannot_afford_dust_htlcs_at_spike_multiple_if_nondust_at_base_feerate demonstrates the scenario: an HTLC is non-dust at the base feerate but dust at 2x, and a 5th such HTLC is now rejected by the counterparty due to a fee-spike buffer violation.
Changed components
lightning/src/sign/tx_builder.rslightning/src/ln/htlc_reserve_unit_tests.rsChannel commitment transaction fee calculationFee spike buffer check in `can_accept_incoming_htlc`Inspect captured patch +244 / −37
diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs
index cbb67e9..54b27ca 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<UserConfig>, 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<Node<'b, 'c, 'd>>, channel_id: ChannelId, channel_value_sat: u64,
- dust_limit_satoshis: u64, receiver_amount_msat: u64, htlc_cltv: u32, payment_hash: PaymentHash,
+ nodes: &'a Vec<Node<'b, 'c, 'd>>, channel_id: ChannelId, value_to_self_msat: u64,
+ dust_limit_satoshis: u64, payment_hash: PaymentHash,
+ htlcs_in_commitment: Vec<HTLCOutputInCommitment>, 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);
}
@@ -3549,3 +3551,199 @@ fn test_outbound_vs_available_capacity_outbound_htlc_limit_spiked_feerate() {
);
}
}
+
+/// 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,
+ );
+}
diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs
index e1b9521..4bf6962 100644
--- a/lightning/src/sign/tx_builder.rs
+++ b/lightning/src/sign/tx_builder.rs
@@ -300,9 +300,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(
@@ -317,7 +326,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,
})
Why this scored 60/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.