← Watch feed
Add validation of the fees predicted by `next_commitment_stats`
Public commit record
73/100 · Adequate
What the developer wrote
Authored by Leo Nash
Add validation of the fees predicted by `next_commitment_stats`
Anytime we build a (feerate, nondust-htlc-count, fee) pair, cache it,
and check that the fee matches if the feerate and nondust-htlc-count
match when building a commitment transaction.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit adds internal bookkeeping and consistency checks to make sure the fee predicted when planning a Lightning channel commitment transaction matches the fee actually used when the transaction is later built. It only runs during tests and fuzzing, so it does not directly change production behavior. It is a defensive hardening/debugging patch rather than a fix for an active exploit.
Recommended action
Treat as low-priority defensive hardening. Review whether the invariant should also be enforced in production builds (e.g., as a non-panicking consistency check or error return) rather than only under test/fuzzing. No immediate patch or incident response is indicated by the diff alone.
Security signals we found
01
New test/fuzz-only assertion comparing predicted vs actual commitment transaction fees
02
Addition of cached predicted-fee state in FundingScope
03
No production runtime enforcement; changes are cfg-gated to test and fuzzing
04
Commit title frames change as validation, but diff shows it is debug/test instrumentation
05
No mention of CVE, advisory, or security bug in commit message
Evidence from the diff
The change introduces a PredictedNextFee struct (gated by #[cfg(any(test, fuzzing))]) that caches the predicted (feerate, nondust_htlc_count, commit_tx_fee_sat) from get_next_commitment_stats. When a local or remote commitment transaction is later validated, an assert_eq! checks that the actual fee matches the cached prediction if the feerate and nondust HTLC count match. The assertion is also test/fuzz-only. The production code path gains no runtime validation; the commit is effectively an invariant check to catch fee-calculation mismatches during testing.
Changed components
lightning/src/ln/channel.rsFundingScopeget_next_local_commitment_statsget_next_remote_commitment_statslocal commitment validationremote commitment validationInspect captured patch +114 / −5
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index a0d241a..e7410fc 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2062,6 +2062,10 @@ pub(super) struct FundingScope {
next_local_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
#[cfg(any(test, fuzzing))]
next_remote_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
+ #[cfg(any(test, fuzzing))]
+ next_local_fee: Mutex<PredictedNextFee>,
+ #[cfg(any(test, fuzzing))]
+ next_remote_fee: Mutex<PredictedNextFee>,
pub(super) channel_transaction_parameters: ChannelTransactionParameters,
@@ -2138,6 +2142,10 @@ impl Readable for FundingScope {
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()),
})
}
}
@@ -2314,6 +2322,10 @@ impl FundingScope {
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()),
funding_tx_confirmation_height: 0,
funding_tx_confirmed_in: None,
minimum_depth_override: None,
@@ -3197,6 +3209,10 @@ where
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()),
channel_transaction_parameters: ChannelTransactionParameters {
holder_pubkeys: pubkeys,
@@ -3437,6 +3453,10 @@ where
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()),
channel_transaction_parameters: ChannelTransactionParameters {
holder_pubkeys: pubkeys,
@@ -4216,7 +4236,8 @@ where
include_counterparty_unknown_htlcs,
);
let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(true, funding);
- SpecTxBuilder {}.get_next_commitment_stats(
+
+ let ret = SpecTxBuilder {}.get_next_commitment_stats(
true,
funding.is_outbound(),
funding.get_value_satoshis(),
@@ -4227,7 +4248,38 @@ where
dust_exposure_limiting_feerate,
self.holder_dust_limit_satoshis,
funding.get_channel_type(),
- )
+ );
+
+ #[cfg(any(test, fuzzing))]
+ {
+ if addl_nondust_htlc_count == 0 {
+ *funding.next_local_fee.lock().unwrap() = PredictedNextFee {
+ predicted_feerate: feerate_per_kw,
+ predicted_nondust_htlc_count: ret.nondust_htlc_count,
+ predicted_fee_sat: ret.commit_tx_fee_sat,
+ };
+ } else {
+ let predicted_stats = SpecTxBuilder {}.get_next_commitment_stats(
+ true,
+ funding.is_outbound(),
+ funding.get_value_satoshis(),
+ next_value_to_self_msat,
+ &next_commitment_htlcs,
+ 0,
+ feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ self.holder_dust_limit_satoshis,
+ funding.get_channel_type(),
+ );
+ *funding.next_local_fee.lock().unwrap() = PredictedNextFee {
+ predicted_feerate: feerate_per_kw,
+ predicted_nondust_htlc_count: predicted_stats.nondust_htlc_count,
+ predicted_fee_sat: predicted_stats.commit_tx_fee_sat,
+ };
+ }
+ }
+
+ ret
}
fn get_next_remote_commitment_stats(
@@ -4241,7 +4293,8 @@ where
include_counterparty_unknown_htlcs,
);
let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(false, funding);
- SpecTxBuilder {}.get_next_commitment_stats(
+
+ let ret = SpecTxBuilder {}.get_next_commitment_stats(
false,
funding.is_outbound(),
funding.get_value_satoshis(),
@@ -4252,7 +4305,38 @@ where
dust_exposure_limiting_feerate,
self.counterparty_dust_limit_satoshis,
funding.get_channel_type(),
- )
+ );
+
+ #[cfg(any(test, fuzzing))]
+ {
+ if addl_nondust_htlc_count == 0 {
+ *funding.next_remote_fee.lock().unwrap() = PredictedNextFee {
+ predicted_feerate: feerate_per_kw,
+ predicted_nondust_htlc_count: ret.nondust_htlc_count,
+ predicted_fee_sat: ret.commit_tx_fee_sat,
+ };
+ } else {
+ let predicted_stats = SpecTxBuilder {}.get_next_commitment_stats(
+ false,
+ funding.is_outbound(),
+ funding.get_value_satoshis(),
+ next_value_to_self_msat,
+ &next_commitment_htlcs,
+ 0,
+ feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ self.counterparty_dust_limit_satoshis,
+ funding.get_channel_type(),
+ );
+ *funding.next_remote_fee.lock().unwrap() = PredictedNextFee {
+ predicted_feerate: feerate_per_kw,
+ predicted_nondust_htlc_count: predicted_stats.nondust_htlc_count,
+ predicted_fee_sat: predicted_stats.commit_tx_fee_sat,
+ };
+ }
+ }
+
+ ret
}
#[rustfmt::skip]
@@ -4410,6 +4494,10 @@ where
}
}
}
+ 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);
+ }
}
if msg.htlc_signatures.len() != commitment_data.tx.nondust_htlcs().len() {
@@ -6216,6 +6304,14 @@ struct CommitmentTxInfoCached {
feerate: u32,
}
+#[cfg(any(test, fuzzing))]
+#[derive(Clone, Copy, Default)]
+struct PredictedNextFee {
+ predicted_feerate: u32,
+ predicted_nondust_htlc_count: usize,
+ predicted_fee_sat: u64,
+}
+
/// Contents of a wire message that fails an HTLC backwards. Useful for [`FundedChannel::fail_htlc`] to
/// fail with either [`msgs::UpdateFailMalformedHTLC`] or [`msgs::UpdateFailHTLC`] as needed.
trait FailHTLCContents {
@@ -11386,6 +11482,10 @@ where
}
}
}
+ 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);
+ }
}
(commitment_data.htlcs_included, counterparty_commitment_tx)
@@ -14032,6 +14132,10 @@ where
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()),
channel_transaction_parameters: channel_parameters,
funding_transaction,
@@ -16092,7 +16196,7 @@ mod tests {
fn get_pre_and_post(
pre_channel_value: u64, our_funding_contribution: i64, their_funding_contribution: i64,
) -> (u64, u64) {
- use crate::ln::channel::FundingScope;
+ use crate::ln::channel::{FundingScope, PredictedNextFee};
let funding = FundingScope {
value_to_self_msat: 0,
@@ -16109,6 +16213,11 @@ mod tests {
#[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()),
+
channel_transaction_parameters: ChannelTransactionParameters::test_dummy(
pre_channel_value,
),
Risk score
Our methodology →Why this scored 27/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.