← Watch feed
Error if the calculated v2 reserve is greater than the channel value
Public commit record
85/100 · Strong
What the developer wrote
Authored by Leo Nash
Error if the calculated v2 reserve is greater than the channel value
In 0FC channels, capping the reserve to the total value of the channel
allowed a splice initiator to withdraw past their reserve in case the
acceptor had no balance in the channel.
This is because the post-splice value of the channel was equal to the
initiator's post splice balance. Hence, this post splice balance always
matched the reserve, even though the reserve was below the dust limit.
The only thing that prevented the initiator from withdrawing all their
balance was the script dust limit check in
`interactivetxs::NegotiationContext::receive_tx_add_output`.
In case the splice acceptor had any balance in the channel, or there
were HTLCs in the channel, or the channel was not 0FC, the
splice initiator's post-splice balance was always below the full channel
value. Hence when the reserve was capped at the channel value, the
post-splice balance was always below the reserve, and the splice was
rejected.
Also, in `validate_splice_contributions`, to determine the
`counterparty_selected_channel_reserve`, we now read the holder's dust
limit from the context, instead of the current global constant.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This commit fixes a bug in Lightning Dev Kit's splicing logic for zero-fee-commitment (0FC) channels. Previously, when calculating the required channel reserve after a splice, the code would cap the reserve at the total channel value. In a special case where the splice acceptor had no balance and no HTLCs existed, this cap allowed the splice initiator to withdraw more than they should—effectively reducing their balance below the required reserve. The only thing stopping a full drain was a separate dust-limit check. The fix makes the reserve calculation return an error if the computed reserve would exceed the post-splice channel value, and it uses each side's actual dust limit instead of a global constant when validating splice contributions.
Recommended action
Review and merge the fix, then run the new regression tests. Consider whether any deployed nodes need to be checked for exposure to 0FC splice-out reserve bypasses. No CVE or advisory is referenced in the commit materials.
Security signals we found
01
Channel reserve bypass in 0FC splice-out
02
Reserve calculation capped to channel value instead of dust limit
03
Use of per-party dust limit rather than global constant
04
New regression test for reserve breach scenarios
05
Error propagation added to reserve computation
Evidence from the diff
The patch changes get_v2_channel_reserve_satoshis to return Result<u64, ()> and errors when channel_value_satoshis < dust_limit_satoshis. Previously it returned min(channel_value, max(q, dust_limit)), which could produce a reserve below the dust limit when the channel value was very small. In FundingScope::validate_splice_contributions, the reserve is now computed using the context-specific holder/counterparty dust limits, and errors are propagated. A new test, test_splice_out_initiator_reserve_breach_zero_fee_commitments, demonstrates that a 0FC splice initiator could previously splice out past their reserve when the acceptor had no balance, and verifies the fix. The tx_builder.rs helper is updated to handle the new Result type and adds a debug assertion that splicing out one more satoshi would violate the reserve.
Changed components
lightning/src/ln/channel.rslightning/src/ln/splicing_tests.rslightning/src/sign/tx_builder.rsInspect captured patch +312 / −20
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index e07ee7f..f2c5b3b 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2752,16 +2752,14 @@ impl FundingScope {
) -> Result<Self, String> {
if our_funding_contribution.unsigned_abs() > Amount::MAX_MONEY {
return Err(format!(
- "Channel {} cannot be spliced; our {} contribution exceeds the total bitcoin supply",
- context.channel_id(),
+ "Our {} contribution exceeds the total bitcoin supply",
our_funding_contribution,
));
}
if their_funding_contribution.unsigned_abs() > Amount::MAX_MONEY {
return Err(format!(
- "Channel {} cannot be spliced; their {} contribution exceeds the total bitcoin supply",
- context.channel_id(),
+ "Their {} contribution exceeds the total bitcoin supply",
their_funding_contribution,
));
}
@@ -2821,17 +2819,31 @@ impl FundingScope {
// New reserve values are based on the new channel value and are v2-specific
let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
post_channel_value_sat,
- MIN_CHAN_DUST_LIMIT_SATOSHIS,
+ context.holder_dust_limit_satoshis,
prev_funding
.counterparty_selected_channel_reserve_satoshis
.expect("counterparty reserve is set")
== 0,
- );
+ )
+ .map_err(|()| {
+ format!(
+ "The post-splice channel value {post_channel_value_sat} is smaller \
+ than our dust limit {}",
+ context.holder_dust_limit_satoshis
+ )
+ })?;
let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
post_channel_value_sat,
context.counterparty_dust_limit_satoshis,
prev_funding.holder_selected_channel_reserve_satoshis == 0,
- );
+ )
+ .map_err(|()| {
+ format!(
+ "The post-splice channel value {post_channel_value_sat} is smaller \
+ than their dust limit {}",
+ context.counterparty_dust_limit_satoshis,
+ )
+ })?;
Ok(Self {
channel_transaction_parameters: post_channel_transaction_parameters,
@@ -3384,6 +3396,9 @@ pub(super) struct ChannelContext<SP: SignerProvider> {
/// We use this to close if funding is never broadcasted.
pub(super) channel_creation_height: u32,
+ #[cfg(any(test, feature = "_test_utils"))]
+ pub(crate) counterparty_dust_limit_satoshis: u64,
+ #[cfg(not(any(test, feature = "_test_utils")))]
counterparty_dust_limit_satoshis: u64,
#[cfg(any(test, feature = "_test_utils"))]
@@ -6776,19 +6791,24 @@ pub(crate) fn get_legacy_default_holder_selected_channel_reserve_satoshis(
/// Returns a minimum channel reserve value each party needs to maintain, fixed in the spec to a
/// default of 1% of the total channel value.
///
-/// Guaranteed to return a value no larger than channel_value_satoshis
+/// Guaranteed to return a value no larger than `channel_value_satoshis`
///
/// This is used both for outbound and inbound channels and has lower bound
/// of `dust_limit_satoshis`.
+///
+/// Returns `Err` if `channel_value_satoshis` is smaller than `dust_limit_satoshis`.
pub(crate) fn get_v2_channel_reserve_satoshis(
channel_value_satoshis: u64, dust_limit_satoshis: u64, is_0reserve: bool,
-) -> u64 {
+) -> Result<u64, ()> {
+ if channel_value_satoshis < dust_limit_satoshis {
+ return Err(());
+ }
if is_0reserve {
- return 0;
+ return Ok(0);
}
// Fixed at 1% of channel value by spec.
let (q, _) = channel_value_satoshis.overflowing_div(100);
- cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis))
+ Ok(cmp::max(q, dust_limit_satoshis))
}
/// Returns the minimum feerate for RBF attempts given a previous feerate.
@@ -12824,7 +12844,8 @@ where
their_funding_contribution,
counterparty_funding_pubkey,
our_new_holder_keys,
- )?;
+ )
+ .map_err(|e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e))?;
let (post_splice_holder_balance, post_splice_counterparty_balance) =
self.get_holder_counterparty_balances_floor_incl_fee(&candidate_scope).map_err(
@@ -15117,8 +15138,13 @@ impl<SP: SignerProvider> PendingV2Channel<SP> {
});
let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
- funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, trusted_channel_features.is_some_and(|f| f.is_0reserve()));
-
+ funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, trusted_channel_features.is_some_and(|f| f.is_0reserve())
+ ).map_err(|()| APIError::APIMisuseError {
+ err: format!(
+ "The channel value {funding_satoshis} is smaller than their dust \
+ limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}"
+ )
+ })?;
let funding_feerate_sat_per_1000_weight = fee_estimator.bounded_sat_per_1000_weight(funding_confirmation_target);
let funding_tx_locktime = LockTime::from_height(current_chain_height)
.map_err(|_| APIError::APIMisuseError {
@@ -15257,9 +15283,16 @@ impl<SP: SignerProvider> PendingV2Channel<SP> {
let channel_value_satoshis =
our_funding_contribution_sats.saturating_add(msg.common_fields.funding_satoshis);
let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
- channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, msg.disable_channel_reserve.is_some());
+ channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, msg.disable_channel_reserve.is_some()
+ ).map_err(|()| ChannelError::close(format!(
+ "The channel value {channel_value_satoshis} is smaller than our dust limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}"
+ )))?;
+ let their_dust_limit_satoshis = msg.common_fields.dust_limit_satoshis;
let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
- channel_value_satoshis, msg.common_fields.dust_limit_satoshis, trusted_channel_features.is_some_and(|f| f.is_0reserve()));
+ channel_value_satoshis, their_dust_limit_satoshis, trusted_channel_features.is_some_and(|f| f.is_0reserve())
+ ).map_err(|()| ChannelError::close(format!(
+ "The channel value {channel_value_satoshis} is smaller than their dust limit {their_dust_limit_satoshis}"
+ )))?;
let channel_type = channel_type_from_open_channel(&msg.common_fields, our_supported_features)?;
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index a536135..0de7574 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -7865,13 +7865,15 @@ fn do_test_0reserve_splice_counterparty_validation(
// They obviously can't afford their contribution, so we fail before even
// querying `TxBuilder`
format!(
- "Got non-closing error: Their contribution candidate {funding_contribution_sat}sat \
+ "Got non-closing error: Channel {channel_id} cannot be spliced; \
+ Their contribution candidate {funding_contribution_sat}sat \
is greater than their total balance in the channel {initiator_value_to_self_sat}sat"
)
} else if post_channel_value_sat < MIN_CHANNEL_VALUE_SATOSHIS {
// We require all spliced channels to have a value of at least 1000 satoshis after the splice
format!(
- "Got non-closing error: Spliced channel value must be at least {MIN_CHANNEL_VALUE_SATOSHIS} satoshis. \
+ "Got non-closing error: Channel {channel_id} cannot be spliced; \
+ Spliced channel value must be at least {MIN_CHANNEL_VALUE_SATOSHIS} satoshis. \
It would be {post_channel_value_sat}"
)
} else {
@@ -7888,3 +7890,244 @@ fn do_test_0reserve_splice_counterparty_validation(
channel_type
}
+
+/// We previously allowed a splice initiator to splice out funds past their channel reserve if the
+/// the acceptor had no balance in the channel, and there were no HTLCs in the channel
+#[cfg(test)]
+enum AcceptorBalance {
+ NoBalance,
+ BalanceInHTLC,
+ SettledBalance,
+}
+
+#[cfg(test)]
+enum ValidationCase {
+ Passes,
+ FailsAtHolder,
+ FailsAtCounterparty,
+}
+
+#[test]
+fn test_splice_out_initiator_reserve_breach_zero_fee_commitments() {
+ do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ AcceptorBalance::NoBalance,
+ ValidationCase::Passes,
+ );
+ do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ AcceptorBalance::BalanceInHTLC,
+ ValidationCase::Passes,
+ );
+ do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ AcceptorBalance::SettledBalance,
+ ValidationCase::Passes,
+ );
+
+ // We used to fail this case here
+ do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ AcceptorBalance::NoBalance,
+ ValidationCase::FailsAtHolder,
+ );
+
+ do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ AcceptorBalance::BalanceInHTLC,
+ ValidationCase::FailsAtHolder,
+ );
+ do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ AcceptorBalance::SettledBalance,
+ ValidationCase::FailsAtHolder,
+ );
+
+ // We used to fail this case here
+ do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ AcceptorBalance::NoBalance,
+ ValidationCase::FailsAtCounterparty,
+ );
+
+ do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ AcceptorBalance::BalanceInHTLC,
+ ValidationCase::FailsAtCounterparty,
+ );
+ do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ AcceptorBalance::SettledBalance,
+ ValidationCase::FailsAtCounterparty,
+ );
+}
+
+#[cfg(test)]
+fn do_test_splice_out_initiator_reserve_breach_zero_fee_commitments(
+ acceptor_balance: AcceptorBalance, validation_case: ValidationCase,
+) {
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let mut config = test_default_channel_config();
+ // This reserve breach was only possible in 0FC channels
+ config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true;
+ config.channel_handshake_config.our_htlc_minimum_msat = 1;
+ let node_chanmgrs =
+ create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config.clone())]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ // Node 0 is initiator, node 1 is acceptor
+ let _node_id_0 = nodes[0].node.get_our_node_id();
+ let _node_id_1 = nodes[1].node.get_our_node_id();
+
+ let channel_value_sat = 100_000;
+ let node_1_settled_balance_msat =
+ if matches!(acceptor_balance, AcceptorBalance::SettledBalance) { 1 } else { 0 };
+ let node_1_htlc_balance_msat =
+ if matches!(acceptor_balance, AcceptorBalance::BalanceInHTLC) { 1 } else { 0 };
+ let node_0_balance_msat =
+ channel_value_sat * 1000 - node_1_settled_balance_msat - node_1_htlc_balance_msat;
+
+ // Bump initiator's dust limit to the highest value we allow in anchor channels
+ let high_dust_limit_satoshis = 10_000;
+
+ let (_, _, channel_id, _tx) = create_announced_chan_between_nodes_with_value(
+ &nodes,
+ 0,
+ 1,
+ channel_value_sat,
+ node_1_settled_balance_msat,
+ );
+
+ if matches!(acceptor_balance, AcceptorBalance::BalanceInHTLC) {
+ let _ = route_payment(&nodes[0], &[&nodes[1]], node_1_htlc_balance_msat);
+ }
+
+ {
+ let per_peer_lock;
+ let mut peer_state_lock;
+ let channel =
+ get_channel_ref!(nodes[0], nodes[1], per_peer_lock, peer_state_lock, channel_id);
+ if let Some(chan) = channel.as_funded_mut() {
+ chan.context.holder_dust_limit_satoshis = high_dust_limit_satoshis;
+ } else {
+ panic!("Unexpected Channel phase");
+ }
+ }
+
+ {
+ 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);
+ if let Some(chan) = channel.as_funded_mut() {
+ chan.context.counterparty_dust_limit_satoshis = high_dust_limit_satoshis;
+ } else {
+ panic!("Unexpected Channel phase");
+ }
+ }
+
+ if matches!(validation_case, ValidationCase::Passes) {
+ let node_0_balance_leftover_amount = Amount::from_sat(high_dust_limit_satoshis);
+ // Estimated fees of a splice_out at 253sat/kw
+ let estimated_fees = 183;
+ // Note in 0FC we've got no fee spike buffer, no commit tx fee, no anchors
+ let splice_out_output_sat =
+ node_0_balance_msat / 1000 - node_0_balance_leftover_amount.to_sat() - estimated_fees;
+ let splice_out_output_amount = Amount::from_sat(splice_out_output_sat);
+ let outputs = vec![TxOut {
+ value: splice_out_output_amount,
+ script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
+ }];
+ let contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs).unwrap();
+
+ let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, contribution);
+ mine_transaction(&nodes[0], &splice_tx);
+ mine_transaction(&nodes[1], &splice_tx);
+ lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1);
+ } else {
+ let node_0_balance_leftover_amount = Amount::from_sat(high_dust_limit_satoshis - 1);
+ // Note in 0FC we've got no fee spike buffer, no commit tx fee, no anchors
+ let funding_contribution_sat =
+ -((node_0_balance_msat / 1000 - node_0_balance_leftover_amount.to_sat()) as i64);
+ let value = if matches!(validation_case, ValidationCase::FailsAtHolder) {
+ Amount::from_sat(funding_contribution_sat.unsigned_abs() - 183)
+ } else if matches!(validation_case, ValidationCase::FailsAtCounterparty) {
+ // Splice out some dummy amount to get past the initiator's validation,
+ // we'll modify the message in-flight.
+ Amount::from_sat(1000)
+ } else {
+ panic!("Unexpected test case");
+ };
+ let outputs = vec![TxOut {
+ value,
+ script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
+ }];
+ let contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs);
+
+ if matches!(validation_case, ValidationCase::FailsAtHolder) {
+ assert_eq!(
+ contribution.unwrap_err(),
+ APIError::APIMisuseError {
+ err: format!("Channel {channel_id} cannot accept funding contribution"),
+ }
+ );
+ let splice_out_value = value + Amount::from_sat(183);
+ let splice_out_max = splice_out_value - Amount::ONE_SAT;
+ let cannot_splice_out = format!(
+ "Channel {channel_id} cannot be funded: \
+ Our splice-out value of {splice_out_value} is greater than the \
+ maximum {splice_out_max}"
+ );
+ nodes[0].logger.assert_log("lightning::ln::channel", cannot_splice_out, 1);
+ return;
+ }
+
+ // The dummy contribution should have passed the holder's validation
+ assert!(contribution.is_ok());
+
+ // When acceptor has no balance, the reserve the initiator should keep should remain
+ // clamped at its dust limit. We previously allowed the initiator to withdraw past
+ // this point.
+ let v2_channel_reserve = Amount::from_sat(high_dust_limit_satoshis);
+
+ let initiator = &nodes[0];
+ let acceptor = &nodes[1];
+ let node_id_initiator = initiator.node.get_our_node_id();
+ let node_id_acceptor = acceptor.node.get_our_node_id();
+
+ let stfu_init = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor);
+ acceptor.node.handle_stfu(node_id_initiator, &stfu_init);
+ let stfu_ack = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator);
+ initiator.node.handle_stfu(node_id_acceptor, &stfu_ack);
+
+ let mut splice_init =
+ get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor);
+ // Make the modification here, acceptor should now complain. If the acceptor has no
+ // balance, we previously would not complain.
+ splice_init.funding_contribution_satoshis = funding_contribution_sat;
+ acceptor.node.handle_splice_init(node_id_initiator, &splice_init);
+ let msg_events = acceptor.node.get_and_clear_pending_msg_events();
+ assert_eq!(msg_events.len(), 1);
+ if let MessageSendEvent::HandleError { action, .. } = &msg_events[0] {
+ assert!(matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. }));
+ } else {
+ panic!("Expected MessageSendEvent::HandleError");
+ }
+ let post_splice_channel_value_sat = node_0_balance_leftover_amount.to_sat();
+ let cannot_splice_out = if matches!(acceptor_balance, AcceptorBalance::NoBalance) {
+ format!(
+ "Got non-closing error: Channel {channel_id} cannot \
+ be spliced; The post-splice channel value {post_splice_channel_value_sat} \
+ is smaller than their dust limit {high_dust_limit_satoshis}"
+ )
+ } else {
+ // As soon as we've pushed any sats out of our balance, the channel value
+ // is now at the dust limit, so we don't complain when determining the new
+ // dust limits, but later when we check the balances against those new
+ // dust limits
+ assert_eq!(
+ channel_value_sat.checked_add_signed(funding_contribution_sat).unwrap(),
+ high_dust_limit_satoshis
+ );
+ format!(
+ "Got non-closing error: Channel {channel_id} cannot \
+ be spliced out; their post-splice channel balance \
+ {node_0_balance_leftover_amount} is smaller than our selected v2 reserve \
+ {v2_channel_reserve}"
+ )
+ };
+ acceptor.logger.assert_log("lightning::ln::channelmanager", cannot_splice_out, 1);
+ }
+}
diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs
index ffb01c5..98a64e8 100644
--- a/lightning/src/sign/tx_builder.rs
+++ b/lightning/src/sign/tx_builder.rs
@@ -365,7 +365,8 @@ fn get_next_splice_out_maximum_sat(
channel_value_satoshis,
channel_constraints.holder_dust_limit_satoshis,
false,
- );
+ )
+ .unwrap();
// If the holder cannot splice out anything, they must be at or
// below the v2 reserve
debug_assert!(current_balance_sat <= v2_reserve_sat);
@@ -374,7 +375,8 @@ fn get_next_splice_out_maximum_sat(
channel_value_satoshis.saturating_sub(max_splice_out_sat),
channel_constraints.holder_dust_limit_satoshis,
false,
- );
+ )
+ .unwrap();
// If the holder can splice out some maximum, splicing out that
// maximum lands them at exactly the new v2 reserve + the
// `post_splice_delta_above_reserve_sat`
@@ -382,6 +384,20 @@ fn get_next_splice_out_maximum_sat(
local_balance_before_fee_sat.saturating_sub(max_splice_out_sat),
post_splice_reserve_sat.saturating_add(post_splice_delta_above_reserve_sat)
);
+ // Splice out an additional satoshi, and check that we are offside
+ let offside_splice_out_sat = max_splice_out_sat + 1;
+ let post_splice_reserve_sat_result = get_v2_channel_reserve_satoshis(
+ channel_value_satoshis.saturating_sub(offside_splice_out_sat),
+ channel_constraints.holder_dust_limit_satoshis,
+ false,
+ );
+ match post_splice_reserve_sat_result {
+ Ok(reserve) => debug_assert!(
+ local_balance_before_fee_sat.saturating_sub(offside_splice_out_sat)
+ < reserve.saturating_add(post_splice_delta_above_reserve_sat)
+ ),
+ Err(()) => (),
+ }
}
max_splice_out_sat
} else {
Risk score
Our methodology →Why this scored 66/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.