Fix output filtering in into_unique_contributions
What changed, and why it matters
This commit fixes a bug in how the Lightning Dev Kit filters duplicate transaction outputs when multiple funding or splicing attempts overlap. Previously, it treated two outputs as duplicates only if both the amount and the destination address matched exactly. The fix now treats outputs as duplicates if only the destination address matches, even if the amounts differ. This prevents a user from accidentally losing control of funds when a second splicing attempt reuses a change address with a different amount, such as after a fee-bump (RBF).
Review whether the script_pubkey-only filtering could inadvertently suppress legitimate outputs in any edge cases, and confirm that wallet change-address reuse across splicing rounds is handled safely. Consider whether additional tests are needed for multi-party or adversarial scenarios.
Security signals we found
Change in output-deduplication semantics from full TxOut equality to script_pubkey-only equality
Test updates demonstrate the intended behavior: same-script outputs filtered, different-script outputs retained
Commit message describes the change as a fix for output filtering in funding/splicing contributions
Scenario involves overlapping funding contributions and DiscardFunding event handling
No explicit security framing or CVE reference in commit or supplied materials
Evidence from the diff
In FundingContribution::into_unique_contributions, the output-deduplication loop previously compared full TxOut equality (*output != *existing). The patch changes the comparison to only compare script_pubkey. This means outputs that reuse the same script/address are filtered as already-committed, even if their value differs. The change is accompanied by test updates that construct second splice contributions with a distinct splice-out script_pubkey and verify that only the non-overlapping (by script) outputs survive filtering in DiscardFunding events. The commit message explicitly frames this as a correctness fix for RBF-like scenarios where change amounts differ across rounds.
Changed components
lightning/src/ln/funding.rslightning/src/ln/splicing_tests.rsFundingContribution::into_unique_contributionsDiscardFunding event generationInspect captured patch +31 / −21
diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs
index e954149..2f4e89d 100644
--- a/lightning/src/ln/funding.rs
+++ b/lightning/src/ln/funding.rs
@@ -766,7 +766,7 @@ impl FundingContribution {
inputs.retain(|input| *input != existing);
}
for existing in existing_outputs {
- outputs.retain(|output| *output != *existing);
+ outputs.retain(|output| output.script_pubkey != existing.script_pubkey);
}
if inputs.is_empty() && outputs.is_empty() {
None
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index 4135f2b..623a151 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -3915,11 +3915,11 @@ fn test_funding_contributed_splice_already_pending() {
.build()
.unwrap();
- // Initiate a second splice with a DIFFERENT output to test that different outputs
- // are included in DiscardFunding (not filtered out)
+ // Initiate a second splice with a DIFFERENT output (different script_pubkey) to test that
+ // non-overlapping outputs are included in DiscardFunding (not filtered out).
let second_splice_out = TxOut {
- value: Amount::from_sat(6_000), // Different amount
- script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())),
+ value: Amount::from_sat(6_000),
+ script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(),
};
// Clear UTXOs and add a LARGER one for the second contribution to ensure
@@ -3950,8 +3950,7 @@ fn test_funding_contributed_splice_already_pending() {
// Second funding_contributed with a different contribution - this should trigger
// DiscardFunding because there's already a pending quiescent action (splice contribution).
// Only inputs/outputs NOT in the existing contribution should be discarded.
- let (expected_inputs, expected_outputs) =
- second_contribution.clone().into_contributed_inputs_and_outputs();
+ let expected_inputs: Vec<_> = second_contribution.contributed_inputs().collect();
// Returns Err(APIMisuseError) and emits DiscardFunding for the non-duplicate parts of the second contribution
assert_eq!(
@@ -3971,11 +3970,10 @@ fn test_funding_contributed_splice_already_pending() {
if let FundingInfo::Contribution { inputs, outputs } = funding_info {
// The input is different, so it should be in the discard event
assert_eq!(*inputs, expected_inputs);
- // The splice-out output is different (6000 vs 5000), so it should be in discard event
- assert!(expected_outputs.contains(&second_splice_out));
- assert!(!expected_outputs.contains(&first_splice_out));
- // The different outputs should NOT be filtered out
- assert_eq!(*outputs, expected_outputs);
+ // The splice-out output (different script_pubkey) survives filtering;
+ // the change output (same script_pubkey as first contribution) is filtered.
+ assert_eq!(outputs.len(), 1);
+ assert!(outputs.contains(&second_splice_out));
} else {
panic!("Expected FundingInfo::Contribution");
}
@@ -4068,14 +4066,24 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) {
let first_contribution =
funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap();
- // Build second contribution with different UTXOs so inputs/outputs don't overlap
+ // Build second contribution with different UTXOs and a splice-out output using a different
+ // script_pubkey (node 1's address) so it survives script_pubkey-based filtering.
nodes[0].wallet_source.clear_utxos();
provide_utxo_reserves(&nodes, 1, splice_in_amount * 3);
+ let splice_out_output = TxOut {
+ value: Amount::from_sat(1_000),
+ script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(),
+ };
let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap();
let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger);
- let second_contribution =
- funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap();
+ let second_contribution = funding_template
+ .without_prior_contribution(feerate, FeeRate::MAX)
+ .with_coin_selection_source_sync(&wallet)
+ .add_value(splice_in_amount)
+ .add_outputs(vec![splice_out_output.clone()])
+ .build()
+ .unwrap();
// First funding_contributed - sets up the quiescent action and queues STFU
nodes[0]
@@ -4120,10 +4128,10 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) {
}
}
- // Call funding_contributed with a different contribution (non-overlapping inputs/outputs).
- // This hits the funding_negotiation path and returns DiscardFunding.
- let (expected_inputs, expected_outputs) =
- second_contribution.clone().into_contributed_inputs_and_outputs();
+ // Call funding_contributed with the second contribution. Inputs don't overlap (different
+ // UTXOs) so they all survive. The splice-out output (different script_pubkey) survives
+ // while the change output (same script_pubkey as first contribution) is filtered.
+ let expected_inputs: Vec<_> = second_contribution.contributed_inputs().collect();
assert_eq!(
nodes[0].node.funding_contributed(&channel_id, &node_id_1, second_contribution, None),
Err(APIError::APIMisuseError {
@@ -4131,15 +4139,17 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) {
})
);
- // Assert DiscardFunding event with the non-duplicate inputs/outputs
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1, "{events:?}");
match &events[0] {
Event::DiscardFunding { channel_id: event_channel_id, funding_info } => {
assert_eq!(*event_channel_id, channel_id);
if let FundingInfo::Contribution { inputs, outputs } = funding_info {
+ // Inputs are unique (different UTXOs) so none are filtered.
assert_eq!(*inputs, expected_inputs);
- assert_eq!(*outputs, expected_outputs);
+ // Only the splice-out output survives; the change output is filtered
+ // (same script_pubkey as first contribution's change).
+ assert_eq!(*outputs, vec![splice_out_output]);
} else {
panic!("Expected FundingInfo::Contribution");
}
Why this scored 44/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.