← Watch feed
Delete dead `next_{local, remote}_commitment_tx_fee_info_cached`
Public commit record
75/100 · Adequate
What the developer wrote
Authored by Leo Nash
Delete dead `next_{local, remote}_commitment_tx_fee_info_cached`
The cached fee is never checked in the current test suite.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides an explanatory body✓ Mentions testing or verification
What changed, and why it matters
This commit removes unused test-only code that cached predicted commitment transaction fees. The removed fields were only compiled under test/fuzzing configurations and were never checked by the current test suite. The commit does not change production behavior or fix any security issue.
Recommended action
No security action required. This is a code-cleanup/refactoring change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit deletes the next_local_commitment_tx_fee_info_cached and next_remote_commitment_tx_fee_info_cached fields from FundingScope, along with all code that populated, cleared, or asserted against them. These fields were guarded by #[cfg(any(test, fuzzing))] and were described as dead code. A newer PredictedNextFee-based mechanism remains in place for fee prediction validation. No runtime logic, serialization format, or consensus-critical code is affected.
Changed components
lightning/src/ln/channel.rsInspect captured patch +5 / −128
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index e7410fc..86972a2 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2054,14 +2054,9 @@ pub(super) struct FundingScope {
/// Max to_local and to_remote outputs in a remote-generated commitment transaction
counterparty_max_commitment_tx_output: Mutex<(u64, u64)>,
- // We save these values so we can make sure `next_local_commit_tx_fee_msat` and
- // `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will
- // be, by comparing the cached values to the fee of the transaction generated by
- // `build_commitment_transaction`.
- #[cfg(any(test, fuzzing))]
- next_local_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
- #[cfg(any(test, fuzzing))]
- next_remote_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
+ // We save these values so we can make sure validation of channel updates properly predicts
+ // what the next commitment transaction fee will be, by comparing the cached values to the
+ // fee of the transaction generated by `build_commitment_transaction`.
#[cfg(any(test, fuzzing))]
next_local_fee: Mutex<PredictedNextFee>,
#[cfg(any(test, fuzzing))]
@@ -2139,10 +2134,6 @@ impl Readable for FundingScope {
short_channel_id,
minimum_depth_override,
#[cfg(any(test, fuzzing))]
- next_local_commitment_tx_fee_info_cached: Mutex::new(None),
- #[cfg(any(test, fuzzing))]
- next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
- #[cfg(any(test, fuzzing))]
next_local_fee: Mutex::new(PredictedNextFee::default()),
#[cfg(any(test, fuzzing))]
next_remote_fee: Mutex::new(PredictedNextFee::default()),
@@ -2319,10 +2310,6 @@ impl FundingScope {
(post_channel_value * 1000).saturating_sub(post_value_to_self_msat),
)),
#[cfg(any(test, fuzzing))]
- next_local_commitment_tx_fee_info_cached: Mutex::new(None),
- #[cfg(any(test, fuzzing))]
- next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
- #[cfg(any(test, fuzzing))]
next_local_fee: Mutex::new(PredictedNextFee::default()),
#[cfg(any(test, fuzzing))]
next_remote_fee: Mutex::new(PredictedNextFee::default()),
@@ -3205,10 +3192,6 @@ where
#[cfg(debug_assertions)]
counterparty_max_commitment_tx_output: Mutex::new((value_to_self_msat, (channel_value_satoshis * 1000 - msg_push_msat).saturating_sub(value_to_self_msat))),
- #[cfg(any(test, fuzzing))]
- next_local_commitment_tx_fee_info_cached: Mutex::new(None),
- #[cfg(any(test, fuzzing))]
- next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
#[cfg(any(test, fuzzing))]
next_local_fee: Mutex::new(PredictedNextFee::default()),
#[cfg(any(test, fuzzing))]
@@ -3449,10 +3432,6 @@ where
#[cfg(debug_assertions)]
counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
- #[cfg(any(test, fuzzing))]
- next_local_commitment_tx_fee_info_cached: Mutex::new(None),
- #[cfg(any(test, fuzzing))]
- next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
#[cfg(any(test, fuzzing))]
next_local_fee: Mutex::new(PredictedNextFee::default()),
#[cfg(any(test, fuzzing))]
@@ -4480,20 +4459,6 @@ where
}
#[cfg(any(test, fuzzing))]
{
- if funding.is_outbound() {
- let projected_commit_tx_info = funding.next_local_commitment_tx_fee_info_cached.lock().unwrap().take();
- *funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
- if let Some(info) = projected_commit_tx_info {
- let total_pending_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len()
- + self.holding_cell_htlc_updates.len();
- if info.total_pending_htlcs == total_pending_htlcs
- && info.next_holder_htlc_id == self.next_holder_htlc_id
- && info.next_counterparty_htlc_id == self.next_counterparty_htlc_id
- && info.feerate == self.feerate_per_kw {
- assert_eq!(commitment_data.stats.commit_tx_fee_sat, info.fee / 1000);
- }
- }
- }
let PredictedNextFee { predicted_feerate, predicted_nondust_htlc_count, predicted_fee_sat } = *funding.next_local_fee.lock().unwrap();
if predicted_feerate == commitment_data.tx.feerate_per_kw() && predicted_nondust_htlc_count == commitment_data.tx.nondust_htlcs().len() {
assert_eq!(predicted_fee_sat, commitment_data.stats.commit_tx_fee_sat);
@@ -5327,31 +5292,7 @@ where
}
let num_htlcs = included_htlcs + addl_htlcs;
- let commit_tx_fee_msat = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000;
- #[cfg(any(test, fuzzing))]
- {
- let mut fee = commit_tx_fee_msat;
- if fee_spike_buffer_htlc.is_some() {
- fee = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000;
- }
- let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len()
- + context.holding_cell_htlc_updates.len();
- let commitment_tx_info = CommitmentTxInfoCached {
- fee,
- total_pending_htlcs,
- next_holder_htlc_id: match htlc.origin {
- HTLCInitiator::LocalOffered => context.next_holder_htlc_id + 1,
- HTLCInitiator::RemoteOffered => context.next_holder_htlc_id,
- },
- next_counterparty_htlc_id: match htlc.origin {
- HTLCInitiator::LocalOffered => context.next_counterparty_htlc_id,
- HTLCInitiator::RemoteOffered => context.next_counterparty_htlc_id + 1,
- },
- feerate: context.feerate_per_kw,
- };
- *funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
- }
- commit_tx_fee_msat
+ SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000
}
/// Get the commitment tx fee for the remote's next commitment transaction based on the number of
@@ -5428,30 +5369,7 @@ where
}
let num_htlcs = included_htlcs + addl_htlcs;
- let commit_tx_fee_msat = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000;
- #[cfg(any(test, fuzzing))]
- if let Some(htlc) = &htlc {
- let mut fee = commit_tx_fee_msat;
- if fee_spike_buffer_htlc.is_some() {
- fee = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000;
- }
- let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len();
- let commitment_tx_info = CommitmentTxInfoCached {
- fee,
- total_pending_htlcs,
- next_holder_htlc_id: match htlc.origin {
- HTLCInitiator::LocalOffered => context.next_holder_htlc_id + 1,
- HTLCInitiator::RemoteOffered => context.next_holder_htlc_id,
- },
- next_counterparty_htlc_id: match htlc.origin {
- HTLCInitiator::LocalOffered => context.next_counterparty_htlc_id,
- HTLCInitiator::RemoteOffered => context.next_counterparty_htlc_id + 1,
- },
- feerate: context.feerate_per_kw,
- };
- *funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
- }
- commit_tx_fee_msat
+ SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000
}
#[rustfmt::skip]
@@ -6295,15 +6213,6 @@ macro_rules! promote_splice_funding {
};
}
-#[cfg(any(test, fuzzing))]
-struct CommitmentTxInfoCached {
- fee: u64,
- total_pending_htlcs: usize,
- next_holder_htlc_id: u64,
- next_counterparty_htlc_id: u64,
- feerate: u32,
-}
-
#[cfg(any(test, fuzzing))]
#[derive(Clone, Copy, Default)]
struct PredictedNextFee {
@@ -7839,16 +7748,6 @@ where
return Err(ChannelError::close("Received an unexpected revoke_and_ack".to_owned()));
}
- #[cfg(any(test, fuzzing))]
- {
- for funding in
- core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut())
- {
- *funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
- *funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
- }
- }
-
match &self.context.holder_signer {
ChannelSignerType::Ecdsa(ecdsa) => {
ecdsa
@@ -11469,19 +11368,6 @@ where
#[cfg(any(test, fuzzing))]
{
- if !funding.is_outbound() {
- let projected_commit_tx_info = funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take();
- *funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
- if let Some(info) = projected_commit_tx_info {
- let total_pending_htlcs = self.context.pending_inbound_htlcs.len() + self.context.pending_outbound_htlcs.len();
- if info.total_pending_htlcs == total_pending_htlcs
- && info.next_holder_htlc_id == self.context.next_holder_htlc_id
- && info.next_counterparty_htlc_id == self.context.next_counterparty_htlc_id
- && info.feerate == self.context.feerate_per_kw {
- assert_eq!(commitment_data.stats.commit_tx_fee_sat, info.fee);
- }
- }
- }
let PredictedNextFee { predicted_feerate, predicted_nondust_htlc_count, predicted_fee_sat } = *funding.next_remote_fee.lock().unwrap();
if predicted_feerate == counterparty_commitment_tx.feerate_per_kw() && predicted_nondust_htlc_count == counterparty_commitment_tx.nondust_htlcs().len() {
assert_eq!(predicted_fee_sat, commitment_data.stats.commit_tx_fee_sat);
@@ -14128,10 +14014,6 @@ where
#[cfg(debug_assertions)]
counterparty_max_commitment_tx_output: Mutex::new((0, 0)),
- #[cfg(any(test, fuzzing))]
- next_local_commitment_tx_fee_info_cached: Mutex::new(None),
- #[cfg(any(test, fuzzing))]
- next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
#[cfg(any(test, fuzzing))]
next_local_fee: Mutex::new(PredictedNextFee::default()),
#[cfg(any(test, fuzzing))]
@@ -16208,11 +16090,6 @@ mod tests {
#[cfg(debug_assertions)]
counterparty_max_commitment_tx_output: Mutex::new((0, 0)),
- #[cfg(any(test, fuzzing))]
- next_local_commitment_tx_fee_info_cached: Mutex::new(None),
- #[cfg(any(test, fuzzing))]
- next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
-
#[cfg(any(test, fuzzing))]
next_local_fee: Mutex::new(PredictedNextFee::default()),
#[cfg(any(test, fuzzing))]
Risk score
Our methodology →Why this scored 14/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.