Rework max commitment transaction balance debug assertions
What changed, and why it matters
This commit changes internal bookkeeping checks that only run in debug builds. The old checks could incorrectly fire and crash a developer/test build when a Lightning channel was spliced (resized) in a way that left one party temporarily below the required reserve balance. The new checks relax that rule for splicing by only requiring balances move upward when below reserve, rather than always staying above reserve. It is a robustness fix for an assertion, not a fix for a live-network security vulnerability.
No immediate action required. Treat as a normal correctness/robustness patch. Reviewers may want to confirm the new monotonicity assertion still catches actual balance-drift bugs in debug builds and that the splicing test covers both holder and counterparty below-reserve cases.
Security signals we found
Debug-only assertion relaxation for splicing correctness
Channel reserve invariant adjusted to allow temporary below-reserve state after splice
New regression test covering splice where counterparty balance falls below reserve
Evidence from the diff
The patch reworks #[cfg(debug_assertions)] assertions in FundingScope that tracked the maximum to_local/to_remote commitment transaction outputs. Previously the code asserted balances either already met the channel reserve or only grew toward it. With splicing, a non-contributing party can end up below the post-splice reserve legitimately, so the assertions were too strict. The renamed holder_prev_commitment_tx_balance / counterparty_prev_commitment_tx_balance fields now store the previous commitment balance and assert monotonic increase only while below reserve. A new test, test_splice_balance_falls_below_reserve, exercises the scenario. These assertions are compiled out in release builds, so they cannot be exploited against production nodes.
Changed components
lightning/src/ln/channel.rslightning/src/ln/splicing_tests.rsInspect captured patch +84 / −24
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 096a101..3dee029 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2583,10 +2583,10 @@ pub(super) struct FundingScope {
#[cfg(debug_assertions)]
/// Max to_local and to_remote outputs in a locally-generated commitment transaction
- holder_max_commitment_tx_output: Mutex<(u64, u64)>,
+ holder_prev_commitment_tx_balance: Mutex<(u64, u64)>,
#[cfg(debug_assertions)]
/// Max to_local and to_remote outputs in a remote-generated commitment transaction
- counterparty_max_commitment_tx_output: Mutex<(u64, u64)>,
+ counterparty_prev_commitment_tx_balance: Mutex<(u64, u64)>,
// 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
@@ -2658,9 +2658,9 @@ impl Readable for FundingScope {
counterparty_selected_channel_reserve_satoshis,
holder_selected_channel_reserve_satoshis: holder_selected_channel_reserve_satoshis.0.unwrap(),
#[cfg(debug_assertions)]
- holder_max_commitment_tx_output: Mutex::new((0, 0)),
+ holder_prev_commitment_tx_balance: Mutex::new((0, 0)),
#[cfg(debug_assertions)]
- counterparty_max_commitment_tx_output: Mutex::new((0, 0)),
+ counterparty_prev_commitment_tx_balance: Mutex::new((0, 0)),
channel_transaction_parameters: channel_transaction_parameters.0.unwrap(),
funding_transaction,
funding_tx_confirmed_in,
@@ -2847,16 +2847,16 @@ impl FundingScope {
counterparty_selected_channel_reserve_satoshis,
holder_selected_channel_reserve_satoshis,
#[cfg(debug_assertions)]
- holder_max_commitment_tx_output: {
- let prev = *prev_funding.holder_max_commitment_tx_output.lock().unwrap();
+ holder_prev_commitment_tx_balance: {
+ let prev = *prev_funding.holder_prev_commitment_tx_balance.lock().unwrap();
Mutex::new((
prev.0.saturating_add_signed(our_funding_contribution.to_sat() * 1000),
prev.1.saturating_add_signed(their_funding_contribution.to_sat() * 1000),
))
},
#[cfg(debug_assertions)]
- counterparty_max_commitment_tx_output: {
- let prev = *prev_funding.counterparty_max_commitment_tx_output.lock().unwrap();
+ counterparty_prev_commitment_tx_balance: {
+ let prev = *prev_funding.counterparty_prev_commitment_tx_balance.lock().unwrap();
Mutex::new((
prev.0.saturating_add_signed(our_funding_contribution.to_sat() * 1000),
prev.1.saturating_add_signed(their_funding_contribution.to_sat() * 1000),
@@ -3805,9 +3805,9 @@ impl<SP: SignerProvider> ChannelContext<SP> {
holder_selected_channel_reserve_satoshis,
#[cfg(debug_assertions)]
- holder_max_commitment_tx_output: Mutex::new((value_to_self_msat, (channel_value_satoshis * 1000 - msg_push_msat).saturating_sub(value_to_self_msat))),
+ holder_prev_commitment_tx_balance: Mutex::new((value_to_self_msat, (channel_value_satoshis * 1000 - msg_push_msat).saturating_sub(value_to_self_msat))),
#[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))),
+ counterparty_prev_commitment_tx_balance: 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_fee: Mutex::new(PredictedNextFee::default()),
@@ -4043,9 +4043,9 @@ impl<SP: SignerProvider> ChannelContext<SP> {
// We'll add our counterparty's `funding_satoshis` to these max commitment output assertions
// when we receive `accept_channel2`.
#[cfg(debug_assertions)]
- holder_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
+ holder_prev_commitment_tx_balance: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
#[cfg(debug_assertions)]
- counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
+ counterparty_prev_commitment_tx_balance: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
#[cfg(any(test, fuzzing))]
next_local_fee: Mutex::new(PredictedNextFee::default()),
@@ -5594,17 +5594,26 @@ impl<SP: SignerProvider> ChannelContext<SP> {
{
// Make sure that the to_self/to_remote is always either past the appropriate
// channel_reserve *or* it is making progress towards it.
- let mut broadcaster_max_commitment_tx_output = if generated_by_local {
- funding.holder_max_commitment_tx_output.lock().unwrap()
+ let mut broadcaster_prev_commitment_balance = if generated_by_local {
+ funding.holder_prev_commitment_tx_balance.lock().unwrap()
} else {
- funding.counterparty_max_commitment_tx_output.lock().unwrap()
+ funding.counterparty_prev_commitment_tx_balance.lock().unwrap()
};
- debug_assert!(broadcaster_max_commitment_tx_output.0 <= stats.local_balance_before_fee_msat || stats.local_balance_before_fee_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap());
- broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, stats.local_balance_before_fee_msat);
- debug_assert!(broadcaster_max_commitment_tx_output.1 <= stats.remote_balance_before_fee_msat || stats.remote_balance_before_fee_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis);
- broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, stats.remote_balance_before_fee_msat);
- }
+ if stats.local_balance_before_fee_msat / 1000 < funding.counterparty_selected_channel_reserve_satoshis.unwrap() {
+ // If the local balance is below the reserve on this new commitment, it MUST be
+ // greater than or equal to the one on the previous commitment.
+ debug_assert!(broadcaster_prev_commitment_balance.0 <= stats.local_balance_before_fee_msat);
+ }
+ broadcaster_prev_commitment_balance.0 = stats.local_balance_before_fee_msat;
+
+ if stats.remote_balance_before_fee_msat / 1000 < funding.holder_selected_channel_reserve_satoshis {
+ // If the remote balance is below the reserve on this new commitment, it MUST be
+ // greater than or equal to the one on the previous commitment.
+ debug_assert!(broadcaster_prev_commitment_balance.1 <= stats.remote_balance_before_fee_msat);
+ }
+ broadcaster_prev_commitment_balance.1 = stats.remote_balance_before_fee_msat;
+ }
// This populates the HTLC-source table with the indices from the HTLCs in the commitment
// transaction.
@@ -15983,9 +15992,9 @@ impl<'a, 'b, 'c, ES: EntropySource, SP: SignerProvider>
.unwrap(),
#[cfg(debug_assertions)]
- holder_max_commitment_tx_output: Mutex::new((0, 0)),
+ holder_prev_commitment_tx_balance: Mutex::new((0, 0)),
#[cfg(debug_assertions)]
- counterparty_max_commitment_tx_output: Mutex::new((0, 0)),
+ counterparty_prev_commitment_tx_balance: Mutex::new((0, 0)),
#[cfg(any(test, fuzzing))]
next_local_fee: Mutex::new(PredictedNextFee::default()),
@@ -18548,9 +18557,9 @@ mod tests {
holder_selected_channel_reserve_satoshis: 0,
#[cfg(debug_assertions)]
- holder_max_commitment_tx_output: Mutex::new((0, 0)),
+ holder_prev_commitment_tx_balance: Mutex::new((0, 0)),
#[cfg(debug_assertions)]
- counterparty_max_commitment_tx_output: Mutex::new((0, 0)),
+ counterparty_prev_commitment_tx_balance: Mutex::new((0, 0)),
#[cfg(any(test, fuzzing))]
next_local_fee: Mutex::new(PredictedNextFee::default()),
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index 6727437..086454d 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -2730,3 +2730,54 @@ fn test_splice_buffer_invalid_commitment_signed_closes_channel() {
);
check_added_monitors(&nodes[0], 1);
}
+
+#[test]
+fn test_splice_balance_falls_below_reserve() {
+ // Test that we're able to proceed with a splice where the acceptor does not contribute
+ // anything, but the initiator does, resulting in an increased channel reserve that the
+ // counterparty does not meet but is still valid.
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let mut config = test_default_channel_config();
+ config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100;
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ let initial_channel_value_sat = 100_000;
+ // Push 10k sat to node 1 so it has balance to send HTLCs back.
+ let push_msat = 10_000_000;
+ let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value(
+ &nodes,
+ 0,
+ 1,
+ initial_channel_value_sat,
+ push_msat,
+ );
+
+ let _ = provide_anchor_reserves(&nodes);
+
+ // Create bidirectional pending HTLCs (routed but not claimed).
+ // Outbound HTLC from node 0 to node 1.
+ let (preimage_0_to_1, _hash_0_to_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+ // Large inbound HTLC from node 1 to node 0, bringing node 1's remaining balance down to
+ // 2000 sat. The old reserve (1% of 100k) is 1000 sat so this is still above reserve.
+ let (preimage_1_to_0, _hash_1_to_0, ..) = route_payment(&nodes[1], &[&nodes[0]], 8_000_000);
+
+ // Splice-in 200k sat. The new channel value becomes 300k sat, raising the reserve to 3000
+ // sat. Node 1's remaining 2000 sat is now below the new reserve.
+ let initiator_contribution =
+ initiate_splice_in(&nodes[0], &nodes[1], channel_id, Amount::from_sat(200_000));
+ let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution);
+
+ // Confirm and lock the splice.
+ mine_transaction(&nodes[0], &splice_tx);
+ mine_transaction(&nodes[1], &splice_tx);
+ lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1);
+
+ // Claim both pending HTLCs to verify the channel is fully functional after the splice.
+ claim_payment(&nodes[0], &[&nodes[1]], preimage_0_to_1);
+ claim_payment(&nodes[1], &[&nodes[0]], preimage_1_to_0);
+
+ // Final sanity check: send a payment using the new spliced capacity.
+ let _ = send_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+}
Why this scored 19/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.