← Watch feed
Move the calculation of the spiked feerate to `tx_builder`
Public commit record
73/100 · Adequate
What the developer wrote
Authored by Leo Nash
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.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a code-cleanup refactor that moves the calculation of a 'spiked feerate' (a safety buffer against sudden Bitcoin fee increases) from one part of the code to another. It does not change the actual multiplier value or the security policy; it only changes where the multiplication happens. The commit message explicitly says this is preparation for a future change, not a fix itself. There is no direct evidence in the commit that it repairs a vulnerability.
Recommended action
No immediate action required. Treat as a normal refactor. Review the follow-up commit mentioned in the message to assess whether the planned fee-spike-buffer calculation change has security implications.
Security signals we found
01
Refactor of fee-spike-buffer calculation location
02
No change to the FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE constant or its value
03
No change to anchor/non-anchor channel handling logic
04
Commit message frames change as preparation for future work, not as a security fix
05
No test changes that indicate a bug being fixed
Evidence from the diff
The change relocates the FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE application from channel.rs into tx_builder.rs’s get_next_commitment_stats. A new boolean parameter assume_fee_spike is threaded through the call chain. Callers that previously computed spiked_feerate themselves now pass the raw feerate_per_kw plus assume_fee_spike=true, while most other callers pass false. The logic remains the same: for non-anchor channels, the feerate is multiplied by the spike buffer; for anchor channels it is not. This is a pure refactor in preparation for a subsequent commit that will change the fee-spike-buffer calculation.
Changed components
lightning/src/ln/channel.rslightning/src/sign/tx_builder.rsInspect captured patch +50 / −41
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index e07ee7f..e52c5b2 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -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(|()| {
@@ -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 {
@@ -5284,7 +5286,7 @@ 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, assume_fee_spike: bool, dust_exposure_limiting_feerate: Option<u32>,
) -> Result<(ChannelStats, Vec<HTLCAmountDirection>), ()> {
let next_commitment_htlcs = self.get_next_commitment_htlcs(
true,
@@ -5306,6 +5308,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
&next_commitment_htlcs,
addl_nondust_htlc_count,
feerate_per_kw,
+ assume_fee_spike,
dust_exposure_limiting_feerate,
max_dust_htlc_exposure_msat,
channel_constraints,
@@ -5330,6 +5333,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,
@@ -5351,7 +5355,7 @@ 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, assume_fee_spike: bool, dust_exposure_limiting_feerate: Option<u32>,
) -> Result<(ChannelStats, Vec<HTLCAmountDirection>), ()> {
let next_commitment_htlcs = self.get_next_commitment_htlcs(
false,
@@ -5373,6 +5377,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
&next_commitment_htlcs,
addl_nondust_htlc_count,
feerate_per_kw,
+ assume_fee_spike,
dust_exposure_limiting_feerate,
max_dust_htlc_exposure_msat,
channel_constraints,
@@ -5397,6 +5402,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,
@@ -5439,6 +5445,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(|()| {
@@ -5497,6 +5504,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(|()| {
@@ -5523,6 +5531,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
0,
new_feerate_per_kw,
+ false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
@@ -5544,6 +5553,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
0,
new_feerate_per_kw,
+ false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
@@ -5724,6 +5734,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
@@ -5763,6 +5774,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
@@ -5810,6 +5822,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
fee_spike_buffer_htlc,
feerate,
+ false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
@@ -5826,6 +5839,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
fee_spike_buffer_htlc,
feerate,
+ false,
dust_exposure_limiting_feerate,
)
.map_err(|()| {
@@ -5862,21 +5876,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(|()| {
@@ -6231,6 +6238,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)?;
@@ -6252,6 +6260,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
include_counterparty_unknown_htlcs,
addl_nondust_htlc_count,
self.feerate_per_kw,
+ false,
dust_exposure_limiting_feerate,
)
.unwrap();
@@ -13372,16 +13381,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 +13398,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 +13411,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 +13452,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 +17188,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 +17197,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 +17232,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 +17246,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 6c70f6e..e1b9521 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, assume_fee_spike: bool,
dust_exposure_limiting_feerate: Option<u32>, broadcaster_dust_limit_satoshis: u64,
channel_type: &ChannelTypeFeatures,
) -> Result<NextCommitmentStats, ()> {
@@ -270,11 +270,16 @@ fn get_next_commitment_stats(
channel_type,
);
- // Calculate fees on commitment transaction
- let nondust_htlc_count = next_commitment_htlcs
+ let spiked_feerate = if assume_fee_spike && !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 +289,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 +301,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 +317,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,
})
@@ -802,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, assume_fee_spike: bool,
dust_exposure_limiting_feerate: Option<u32>, max_dust_htlc_exposure_msat: u64,
channel_constraints: ChannelConstraints, channel_type: &ChannelTypeFeatures,
) -> Result<ChannelStats, ()>;
@@ -820,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, assume_fee_spike: bool,
dust_exposure_limiting_feerate: Option<u32>, max_dust_htlc_exposure_msat: u64,
channel_constraints: ChannelConstraints, channel_type: &ChannelTypeFeatures,
) -> Result<ChannelStats, ()> {
@@ -833,6 +838,7 @@ impl TxBuilder for SpecTxBuilder {
pending_htlcs,
addl_nondust_htlc_count,
feerate_per_kw,
+ assume_fee_spike,
dust_exposure_limiting_feerate,
channel_constraints.holder_dust_limit_satoshis,
channel_type,
@@ -846,6 +852,7 @@ impl TxBuilder for SpecTxBuilder {
pending_htlcs,
addl_nondust_htlc_count,
feerate_per_kw,
+ assume_fee_spike,
dust_exposure_limiting_feerate,
channel_constraints.counterparty_dust_limit_satoshis,
channel_type,
Risk score
Our methodology →Why this scored 28/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.