Skip to content

Commit 0c71b13

Browse files
jkczyzclaude
andcommitted
Filter duplicate inputs/outputs in funding_contributed
When funding_contributed is called with a contribution that has inputs/outputs already present in an existing pending splice contribution, filter them out before emitting DiscardFunding. If all inputs and outputs are duplicates, return DoNothing and emit no event. Add helper methods to FundingContribution for iterating over contributed inputs and outputs to support the filtering logic. Add test coverage for both the filtering behavior and the DoNothing path when an identical contribution is provided twice. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 2fad9dc commit 0c71b13

3 files changed

Lines changed: 152 additions & 22 deletions

File tree

lightning/src/ln/channel.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11939,9 +11939,24 @@ where
1193911939
) -> Result<Option<msgs::Stfu>, QuiescentError> {
1194011940
debug_assert!(contribution.is_splice());
1194111941

11942-
if let Some(QuiescentAction::Splice { contribution: _, .. }) = &self.quiescent_action {
11943-
// TODO(splicing): Filter duplicates
11944-
let (inputs, outputs) = contribution.into_contributed_inputs_and_outputs();
11942+
if let Some(QuiescentAction::Splice { contribution: existing, .. }) = &self.quiescent_action
11943+
{
11944+
let (new_inputs, new_outputs) = contribution.into_contributed_inputs_and_outputs();
11945+
11946+
// Filter out inputs/outputs already in the existing contribution
11947+
let inputs: Vec<_> = new_inputs
11948+
.into_iter()
11949+
.filter(|input| !existing.contributed_inputs().any(|e| e == *input))
11950+
.collect();
11951+
let outputs: Vec<_> = new_outputs
11952+
.into_iter()
11953+
.filter(|output| !existing.contributed_outputs().any(|e| *e == *output))
11954+
.collect();
11955+
11956+
if inputs.is_empty() && outputs.is_empty() {
11957+
return Err(QuiescentError::DoNothing);
11958+
}
11959+
1194511960
return Err(QuiescentError::DiscardFunding { inputs, outputs });
1194611961
}
1194711962

lightning/src/ln/funding.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,14 @@ impl FundingContribution {
308308
self.is_splice
309309
}
310310

311+
pub(super) fn contributed_inputs(&self) -> impl Iterator<Item = OutPoint> + '_ {
312+
self.inputs.iter().map(|input| input.utxo.outpoint)
313+
}
314+
315+
pub(super) fn contributed_outputs(&self) -> impl Iterator<Item = &TxOut> + '_ {
316+
self.outputs.iter().chain(self.change_output.iter())
317+
}
318+
311319
pub(super) fn into_tx_parts(self) -> (Vec<FundingTxInput>, Vec<TxOut>) {
312320
let FundingContribution { inputs, mut outputs, change_output, .. } = self;
313321

lightning/src/ln/splicing_tests.rs

Lines changed: 126 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@ use crate::util::wallet_utils::{WalletSourceSync, WalletSync};
2929

3030
use crate::sync::Arc;
3131

32+
use bitcoin::hashes::Hash;
3233
use bitcoin::secp256k1::ecdsa::Signature;
3334
use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
34-
use bitcoin::{Amount, FeeRate, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut};
35+
use bitcoin::{
36+
Amount, FeeRate, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut, WPubkeyHash,
37+
};
3538

3639
#[test]
3740
fn test_splicing_not_supported_api_error() {
@@ -2603,7 +2606,8 @@ fn test_funding_contributed_channel_not_found() {
26032606
#[test]
26042607
fn test_funding_contributed_splice_already_pending() {
26052608
// Tests that calling funding_contributed when there's already a pending splice
2606-
// contribution returns Ok(()) but emits a DiscardFunding event for the duplicate.
2609+
// contribution returns Ok(()) but emits a DiscardFunding event containing only the
2610+
// inputs/outputs that are NOT already in the existing contribution.
26072611
let chanmon_cfgs = create_chanmon_cfgs(2);
26082612
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
26092613
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
@@ -2618,27 +2622,58 @@ fn test_funding_contributed_splice_already_pending() {
26182622
// Provide enough UTXOs for two contributions
26192623
provide_utxo_reserves(&nodes, 2, splice_in_amount * 2);
26202624

2621-
let contribution = SpliceContribution::splice_in(splice_in_amount);
2625+
// Use splice_in_and_out with an output so we can test output filtering
2626+
let first_splice_out = TxOut {
2627+
value: Amount::from_sat(5_000),
2628+
script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())),
2629+
};
2630+
let first_contribution_spec =
2631+
SpliceContribution::splice_in_and_out(splice_in_amount, vec![first_splice_out.clone()]);
26222632
let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64);
26232633

2624-
// Initiate splice to get a FundingNeeded event
2625-
nodes[0].node.splice_channel(&channel_id, &node_id_1, contribution, feerate).unwrap();
2634+
// Initiate first splice to get a FundingNeeded event
2635+
nodes[0]
2636+
.node
2637+
.splice_channel(&channel_id, &node_id_1, first_contribution_spec, feerate)
2638+
.unwrap();
26262639

26272640
let event = get_event!(nodes[0], Event::FundingNeeded);
2628-
let funding_template = match event {
2641+
let first_funding_template = match event {
26292642
Event::FundingNeeded { funding_template, .. } => funding_template,
26302643
_ => panic!("Expected Event::FundingNeeded"),
26312644
};
26322645

26332646
// Build the first contribution
26342647
let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger);
2635-
let first_contribution = funding_template.clone().build_sync(&wallet).unwrap();
2648+
let first_contribution = first_funding_template.build_sync(&wallet).unwrap();
26362649

2637-
// Clear UTXOs and add fresh ones for the second contribution
2650+
// Initiate a second splice with a DIFFERENT output to test that different outputs
2651+
// are included in DiscardFunding (not filtered out)
2652+
let second_splice_out = TxOut {
2653+
value: Amount::from_sat(6_000), // Different amount
2654+
script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())),
2655+
};
2656+
let second_contribution_spec =
2657+
SpliceContribution::splice_in_and_out(splice_in_amount, vec![second_splice_out.clone()]);
2658+
2659+
// Clear UTXOs and add a LARGER one for the second contribution to ensure
2660+
// the change output will be different from the first contribution's change
26382661
nodes[0].wallet_source.clear_utxos();
2639-
provide_utxo_reserves(&nodes, 1, splice_in_amount * 2);
2662+
provide_utxo_reserves(&nodes, 1, splice_in_amount * 3);
2663+
2664+
nodes[0]
2665+
.node
2666+
.splice_channel(&channel_id, &node_id_1, second_contribution_spec, feerate)
2667+
.unwrap();
2668+
2669+
let event = get_event!(nodes[0], Event::FundingNeeded);
2670+
let second_funding_template = match event {
2671+
Event::FundingNeeded { funding_template, .. } => funding_template,
2672+
_ => panic!("Expected Event::FundingNeeded"),
2673+
};
2674+
26402675
let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger);
2641-
let second_contribution = funding_template.build_sync(&wallet).unwrap();
2676+
let second_contribution = second_funding_template.build_sync(&wallet).unwrap();
26422677

26432678
// First funding_contributed - this sets up the quiescent action
26442679
nodes[0].node.funding_contributed(&channel_id, &node_id_1, first_contribution, None).unwrap();
@@ -2647,18 +2682,90 @@ fn test_funding_contributed_splice_already_pending() {
26472682
let _ = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1);
26482683

26492684
// Second funding_contributed with a different contribution - this should trigger
2650-
// DiscardFunding because there's already a pending quiescent action (splice contribution)
2651-
let result = nodes[0].node.funding_contributed(
2652-
&channel_id,
2653-
&node_id_1,
2654-
second_contribution.clone(),
2655-
None,
2656-
);
2685+
// DiscardFunding because there's already a pending quiescent action (splice contribution).
2686+
// Only inputs/outputs NOT in the existing contribution should be discarded.
2687+
let (expected_inputs, expected_outputs) =
2688+
second_contribution.clone().into_contributed_inputs_and_outputs();
2689+
let result =
2690+
nodes[0].node.funding_contributed(&channel_id, &node_id_1, second_contribution, None);
2691+
2692+
// Returns Ok(()) but emits DiscardFunding for the non-duplicate parts of the second contribution
2693+
assert!(result.is_ok());
2694+
2695+
// The second contribution has different outputs (second_splice_out differs from first_splice_out),
2696+
// so those outputs should NOT be filtered out - they should appear in DiscardFunding.
2697+
let events = nodes[0].node.get_and_clear_pending_events();
2698+
assert_eq!(events.len(), 1);
2699+
match &events[0] {
2700+
Event::DiscardFunding { channel_id: event_channel_id, funding_info } => {
2701+
assert_eq!(event_channel_id, &channel_id);
2702+
if let FundingInfo::Contribution { inputs, outputs } = funding_info {
2703+
// The input is different, so it should be in the discard event
2704+
assert_eq!(*inputs, expected_inputs);
2705+
// The splice-out output is different (6000 vs 5000), so it should be in discard event
2706+
assert!(expected_outputs.contains(&second_splice_out));
2707+
assert!(!expected_outputs.contains(&first_splice_out));
2708+
// The different outputs should NOT be filtered out
2709+
assert_eq!(*outputs, expected_outputs);
2710+
} else {
2711+
panic!("Expected FundingInfo::Contribution");
2712+
}
2713+
},
2714+
_ => panic!("Expected DiscardFunding event"),
2715+
}
2716+
}
26572717

2658-
// Returns Ok(()) but emits DiscardFunding for the second contribution
2718+
#[test]
2719+
fn test_funding_contributed_duplicate_contribution_no_event() {
2720+
// Tests that calling funding_contributed with the exact same contribution twice
2721+
// returns Ok(()) but emits no events on the second call (DoNothing path).
2722+
// This tests the case where all inputs/outputs in the second contribution
2723+
// are already present in the existing contribution.
2724+
let chanmon_cfgs = create_chanmon_cfgs(2);
2725+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
2726+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
2727+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
2728+
2729+
let node_id_1 = nodes[1].node.get_our_node_id();
2730+
2731+
let (_, _, channel_id, _) =
2732+
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0);
2733+
2734+
let splice_in_amount = Amount::from_sat(20_000);
2735+
provide_utxo_reserves(&nodes, 1, splice_in_amount * 2);
2736+
2737+
let contribution_spec = SpliceContribution::splice_in(splice_in_amount);
2738+
let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64);
2739+
2740+
// Initiate splice to get a FundingNeeded event
2741+
nodes[0].node.splice_channel(&channel_id, &node_id_1, contribution_spec, feerate).unwrap();
2742+
2743+
let event = get_event!(nodes[0], Event::FundingNeeded);
2744+
let funding_template = match event {
2745+
Event::FundingNeeded { funding_template, .. } => funding_template,
2746+
_ => panic!("Expected Event::FundingNeeded"),
2747+
};
2748+
2749+
// Build the contribution
2750+
let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger);
2751+
let contribution = funding_template.build_sync(&wallet).unwrap();
2752+
2753+
// First funding_contributed - this sets up the quiescent action
2754+
nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution.clone(), None).unwrap();
2755+
2756+
// Drain the pending stfu message
2757+
let _ = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1);
2758+
2759+
// Second funding_contributed with the SAME contribution (same inputs/outputs)
2760+
// This should trigger the DoNothing path because all inputs/outputs are duplicates
2761+
let result = nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution, None);
2762+
2763+
// Returns Ok(()) and emits NO events (DoNothing path)
26592764
assert!(result.is_ok());
26602765

2661-
expect_discard_funding_event(&nodes[0], &channel_id, second_contribution);
2766+
// Verify no events were emitted - the duplicate contribution is silently ignored
2767+
let events = nodes[0].node.get_and_clear_pending_events();
2768+
assert!(events.is_empty(), "Expected no events for duplicate contribution, got {:?}", events);
26622769
}
26632770

26642771
#[test]

0 commit comments

Comments
 (0)