Consider prior contributions when filtering unique inputs/outputs
What changed, and why it matters
This patch fixes a bug in the Lightning Dev Kit's splicing feature. When a user tries to add funds to a channel while another splice negotiation is already ongoing, the code could mistakenly treat coins that are already locked into the earlier splice as 'reclaimable.' That could let a user discard or reclaim UTXOs that should stay reserved, potentially causing inconsistent channel state or loss of funds in edge cases.
Review and merge the patch; add regression tests covering overlapping splice contributions where prior candidates hold UTXOs, and verify that `FailSplice`/`DiscardFunding` returns only genuinely unique/unused inputs and outputs.
Security signals we found
UTXO double-counting / incorrect release of locked funds during concurrent splice negotiations
State inconsistency between pending splice contributions and current negotiation context
Logic bug in channel funding/splicing lifecycle
Evidence from the diff
In lightning/src/ln/channel.rs, funding_contributed computes unique inputs/outputs via into_unique_contributions to decide what to return via FailSplice or DiscardFunding. Previously it only considered the current/existing contribution context, ignoring earlier negotiated candidates stored in PendingFunding::contributions. The patch adds contributed_inputs() and contributed_outputs() iterators over all prior contributions and chains them into the uniqueness check, so UTXOs already locked in earlier splice candidates are not incorrectly returned as reclaimable.
Changed components
lightning/src/ln/channel.rsPendingFundingFundingNegotiationSplice contribution handlingInspect captured patch +27 / −8
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 741da76..f5272e2 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -3074,6 +3074,14 @@ impl PendingFunding {
}
}
+ fn contributed_inputs(&self) -> impl Iterator<Item = bitcoin::OutPoint> + '_ {
+ self.contributions.iter().flat_map(|c| c.contributed_inputs())
+ }
+
+ fn contributed_outputs(&self) -> impl Iterator<Item = &TxOut> + '_ {
+ self.contributions.iter().flat_map(|c| c.contributed_outputs())
+ }
+
fn check_get_splice_locked<SP: SignerProvider>(
&mut self, context: &ChannelContext<SP>, confirmed_funding_index: usize, height: u32,
) -> Option<msgs::SpliceLocked> {
@@ -12040,9 +12048,16 @@ where
if let Some(QuiescentAction::Splice { contribution: existing, .. }) = &self.quiescent_action
{
+ let pending_splice = self.pending_splice.as_ref();
+ let prior_inputs = pending_splice
+ .into_iter()
+ .flat_map(|pending_splice| pending_splice.contributed_inputs());
+ let prior_outputs = pending_splice
+ .into_iter()
+ .flat_map(|pending_splice| pending_splice.contributed_outputs());
return match contribution.into_unique_contributions(
- existing.contributed_inputs(),
- existing.contributed_outputs(),
+ existing.contributed_inputs().chain(prior_inputs),
+ existing.contributed_outputs().chain(prior_outputs),
) {
None => Err(QuiescentError::DoNothing),
Some((inputs, outputs)) => Err(QuiescentError::DiscardFunding { inputs, outputs }),
@@ -12056,17 +12071,21 @@ where
.filter(|funding_negotiation| funding_negotiation.is_initiator());
if let Some(funding_negotiation) = initiated_funding_negotiation {
+ let pending_splice =
+ self.pending_splice.as_ref().expect("funding negotiation implies pending splice");
+ let prior_inputs = pending_splice.contributed_inputs();
+ let prior_outputs = pending_splice.contributed_outputs();
let unique_contributions = match funding_negotiation {
FundingNegotiation::AwaitingAck { context, .. } => contribution
.into_unique_contributions(
- context.contributed_inputs(),
- context.contributed_outputs(),
+ context.contributed_inputs().chain(prior_inputs),
+ context.contributed_outputs().chain(prior_outputs),
),
FundingNegotiation::ConstructingTransaction {
interactive_tx_constructor, ..
} => contribution.into_unique_contributions(
- interactive_tx_constructor.contributed_inputs(),
- interactive_tx_constructor.contributed_outputs(),
+ interactive_tx_constructor.contributed_inputs().chain(prior_inputs),
+ interactive_tx_constructor.contributed_outputs().chain(prior_outputs),
),
FundingNegotiation::AwaitingSignatures { .. } => {
let session = self
@@ -12075,8 +12094,8 @@ where
.as_ref()
.expect("pending splice awaiting signatures");
contribution.into_unique_contributions(
- session.contributed_inputs(),
- session.contributed_outputs(),
+ session.contributed_inputs().chain(prior_inputs),
+ session.contributed_outputs().chain(prior_outputs),
)
},
};
Why this scored 59/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.