Skip to content

Commit 08115fb

Browse files
jkczyzclaude
andcommitted
Filter prior contributions from SpliceFundingFailed events
SpliceFundingFailed events return contributed inputs and outputs to the user so they can unlock the associated UTXOs. When an RBF attempt is in progress, inputs/outputs already consumed by prior contributions must be excluded to avoid the user prematurely unlocking UTXOs that are still needed by the active funding negotiation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 20991c2 commit 08115fb

2 files changed

Lines changed: 173 additions & 37 deletions

File tree

lightning/src/ln/channel.rs

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2984,6 +2984,16 @@ impl PendingFunding {
29842984
self.contributions.iter().flat_map(|c| c.contributed_outputs())
29852985
}
29862986

2987+
fn prior_contributed_inputs(&self) -> impl Iterator<Item = bitcoin::OutPoint> + '_ {
2988+
let len = self.contributions.len();
2989+
self.contributions[..len.saturating_sub(1)].iter().flat_map(|c| c.contributed_inputs())
2990+
}
2991+
2992+
fn prior_contributed_outputs(&self) -> impl Iterator<Item = &TxOut> + '_ {
2993+
let len = self.contributions.len();
2994+
self.contributions[..len.saturating_sub(1)].iter().flat_map(|c| c.contributed_outputs())
2995+
}
2996+
29872997
fn check_get_splice_locked<SP: SignerProvider>(
29882998
&mut self, context: &ChannelContext<SP>, confirmed_funding_index: usize, height: u32,
29892999
) -> Option<msgs::SpliceLocked> {
@@ -3032,25 +3042,6 @@ pub(super) enum QuiescentError {
30323042
FailSplice(SpliceFundingFailed),
30333043
}
30343044

3035-
impl From<QuiescentAction> for QuiescentError {
3036-
fn from(action: QuiescentAction) -> Self {
3037-
match action {
3038-
QuiescentAction::Splice { contribution, .. } => {
3039-
let (contributed_inputs, contributed_outputs) =
3040-
contribution.into_contributed_inputs_and_outputs();
3041-
return QuiescentError::FailSplice(SpliceFundingFailed {
3042-
funding_txo: None,
3043-
channel_type: None,
3044-
contributed_inputs,
3045-
contributed_outputs,
3046-
});
3047-
},
3048-
#[cfg(any(test, fuzzing, feature = "_test_utils"))]
3049-
QuiescentAction::DoNothing => QuiescentError::DoNothing,
3050-
}
3051-
}
3052-
}
3053-
30543045
pub(crate) enum StfuResponse {
30553046
Stfu(msgs::Stfu),
30563047
SpliceInit(msgs::SpliceInit),
@@ -6592,7 +6583,7 @@ pub struct SpliceFundingFailed {
65926583
}
65936584

65946585
macro_rules! maybe_create_splice_funding_failed {
6595-
($funded_channel: expr, $pending_splice: expr, $get: ident, $contributed_inputs_and_outputs: ident) => {{
6586+
($funded_channel: expr, $pending_splice: expr, $pending_splice_ref: expr, $get: ident, $contributed_inputs_and_outputs: ident) => {{
65966587
$pending_splice
65976588
.and_then(|pending_splice| pending_splice.funding_negotiation.$get())
65986589
.filter(|funding_negotiation| funding_negotiation.is_initiator())
@@ -6606,7 +6597,7 @@ macro_rules! maybe_create_splice_funding_failed {
66066597
.as_funding()
66076598
.map(|funding| funding.get_channel_type().clone());
66086599

6609-
let (contributed_inputs, contributed_outputs) = match funding_negotiation {
6600+
let (mut contributed_inputs, mut contributed_outputs) = match funding_negotiation {
66106601
FundingNegotiation::AwaitingAck { context, .. } => {
66116602
context.$contributed_inputs_and_outputs()
66126603
},
@@ -6622,6 +6613,15 @@ macro_rules! maybe_create_splice_funding_failed {
66226613
.$contributed_inputs_and_outputs(),
66236614
};
66246615

6616+
if let Some(pending_splice) = $pending_splice_ref {
6617+
for input in pending_splice.prior_contributed_inputs() {
6618+
contributed_inputs.retain(|i| *i != input);
6619+
}
6620+
for output in pending_splice.prior_contributed_outputs() {
6621+
contributed_outputs.retain(|o| *o != *output);
6622+
}
6623+
}
6624+
66256625
SpliceFundingFailed {
66266626
funding_txo,
66276627
channel_type,
@@ -6655,23 +6655,40 @@ where
66556655
shutdown_result
66566656
}
66576657

6658-
fn abandon_quiescent_action(&mut self) -> Option<SpliceFundingFailed> {
6659-
match self.quiescent_action.take() {
6660-
Some(QuiescentAction::Splice { contribution, .. }) => {
6661-
let (inputs, outputs) = contribution.into_contributed_inputs_and_outputs();
6662-
Some(SpliceFundingFailed {
6658+
fn quiescent_action_into_error(&self, action: QuiescentAction) -> QuiescentError {
6659+
match action {
6660+
QuiescentAction::Splice { contribution, .. } => {
6661+
let (mut inputs, mut outputs) = contribution.into_contributed_inputs_and_outputs();
6662+
if let Some(ref pending_splice) = self.pending_splice {
6663+
for input in pending_splice.contributed_inputs() {
6664+
inputs.retain(|i| *i != input);
6665+
}
6666+
for output in pending_splice.contributed_outputs() {
6667+
outputs.retain(|o| *o != *output);
6668+
}
6669+
}
6670+
QuiescentError::FailSplice(SpliceFundingFailed {
66636671
funding_txo: None,
66646672
channel_type: None,
66656673
contributed_inputs: inputs,
66666674
contributed_outputs: outputs,
66676675
})
66686676
},
66696677
#[cfg(any(test, fuzzing, feature = "_test_utils"))]
6670-
Some(quiescent_action) => {
6671-
self.quiescent_action = Some(quiescent_action);
6678+
QuiescentAction::DoNothing => QuiescentError::DoNothing,
6679+
}
6680+
}
6681+
6682+
fn abandon_quiescent_action(&mut self) -> Option<SpliceFundingFailed> {
6683+
let action = self.quiescent_action.take()?;
6684+
match self.quiescent_action_into_error(action) {
6685+
QuiescentError::FailSplice(failed) => Some(failed),
6686+
#[cfg(any(test, fuzzing, feature = "_test_utils"))]
6687+
QuiescentError::DoNothing => None,
6688+
_ => {
6689+
debug_assert!(false);
66726690
None
66736691
},
6674-
None => None,
66756692
}
66766693
}
66776694

@@ -6795,6 +6812,7 @@ where
67956812
let splice_funding_failed = maybe_create_splice_funding_failed!(
67966813
self,
67976814
self.pending_splice.as_mut(),
6815+
self.pending_splice.as_ref(),
67986816
take,
67996817
into_contributed_inputs_and_outputs
68006818
);
@@ -6819,6 +6837,7 @@ where
68196837
maybe_create_splice_funding_failed!(
68206838
self,
68216839
self.pending_splice.as_ref(),
6840+
self.pending_splice.as_ref(),
68226841
as_ref,
68236842
to_contributed_inputs_and_outputs
68246843
)
@@ -13545,14 +13564,14 @@ where
1354513564

1354613565
if !self.context.is_usable() {
1354713566
log_debug!(logger, "Channel is not in a usable state to propose quiescence");
13548-
return Err(action.into());
13567+
return Err(self.quiescent_action_into_error(action));
1354913568
}
1355013569
if self.quiescent_action.is_some() {
1355113570
log_debug!(
1355213571
logger,
1355313572
"Channel already has a pending quiescent action and cannot start another",
1355413573
);
13555-
return Err(action.into());
13574+
return Err(self.quiescent_action_into_error(action));
1355613575
}
1355713576
// Since we don't have a pending quiescent action, we should never be in a state where we
1355813577
// sent `stfu` without already having become quiescent.

lightning/src/ln/splicing_tests.rs

Lines changed: 123 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2904,12 +2904,14 @@ fn fail_quiescent_action_on_channel_close() {
29042904

29052905
#[test]
29062906
fn abandon_splice_quiescent_action_on_shutdown() {
2907-
do_abandon_splice_quiescent_action_on_shutdown(true);
2908-
do_abandon_splice_quiescent_action_on_shutdown(false);
2907+
do_abandon_splice_quiescent_action_on_shutdown(true, false);
2908+
do_abandon_splice_quiescent_action_on_shutdown(false, false);
2909+
do_abandon_splice_quiescent_action_on_shutdown(true, true);
2910+
do_abandon_splice_quiescent_action_on_shutdown(false, true);
29092911
}
29102912

29112913
#[cfg(test)]
2912-
fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool) {
2914+
fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_splice: bool) {
29132915
let chanmon_cfgs = create_chanmon_cfgs(2);
29142916
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
29152917
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
@@ -2923,6 +2925,19 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool) {
29232925
let (_, _, channel_id, _) =
29242926
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0);
29252927

2928+
// When testing with a prior pending splice, complete splice A first so that
2929+
// `quiescent_action_into_error` filters against `pending_splice.contributed_inputs/outputs`.
2930+
if pending_splice {
2931+
let funding_contribution = do_initiate_splice_in(
2932+
&nodes[0],
2933+
&nodes[1],
2934+
channel_id,
2935+
Amount::from_sat(initial_channel_capacity / 2),
2936+
);
2937+
let (_splice_tx, _new_funding_script) =
2938+
splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution);
2939+
}
2940+
29262941
// Since we cannot close after having sent `stfu`, send an HTLC so that when we attempt to
29272942
// splice, the `stfu` message is held back.
29282943
let payment_amount = 1_000_000;
@@ -2935,17 +2950,22 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool) {
29352950
check_added_monitors(&nodes[0], 1);
29362951

29372952
nodes[1].node.handle_update_add_htlc(node_id_0, &update.update_add_htlcs[0]);
2938-
nodes[1].node.handle_commitment_signed(node_id_0, &update.commitment_signed[0]);
2953+
// After a splice, commitment_signed messages are batched across funding scopes.
2954+
nodes[1].node.handle_commitment_signed_batch_test(node_id_0, &update.commitment_signed);
29392955
check_added_monitors(&nodes[1], 1);
29402956
let (revoke_and_ack, _) = get_revoke_commit_msgs(&nodes[1], &node_id_0);
29412957

29422958
nodes[0].node.handle_revoke_and_ack(node_id_1, &revoke_and_ack);
29432959
check_added_monitors(&nodes[0], 1);
29442960

29452961
// Attempt the splice. `stfu` should not go out yet as the state machine is pending.
2946-
let splice_in_amount = initial_channel_capacity / 2;
2962+
// Use a different amount when there's a prior splice so the change output differs.
2963+
let splice_in_amount =
2964+
if pending_splice { initial_channel_capacity / 4 } else { initial_channel_capacity / 2 };
29472965
let funding_contribution =
29482966
initiate_splice_in(&nodes[0], &nodes[1], channel_id, Amount::from_sat(splice_in_amount));
2967+
let splice_b_change_output =
2968+
if pending_splice { funding_contribution.change_output().cloned() } else { None };
29492969
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
29502970

29512971
// Close the channel. We should see a `SpliceFailed` event for the pending splice
@@ -2959,7 +2979,33 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool) {
29592979
let shutdown = get_event_msg!(closer_node, MessageSendEvent::SendShutdown, closee_node_id);
29602980
closee_node.node.handle_shutdown(closer_node_id, &shutdown);
29612981

2962-
expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution);
2982+
if pending_splice {
2983+
// With a prior pending splice, contributions are filtered against committed inputs/outputs.
2984+
let events = nodes[0].node.get_and_clear_pending_events();
2985+
assert_eq!(events.len(), 2, "{events:?}");
2986+
match &events[0] {
2987+
Event::SpliceFailed { channel_id: cid, .. } => {
2988+
assert_eq!(*cid, channel_id);
2989+
},
2990+
other => panic!("Expected SpliceFailed, got {:?}", other),
2991+
}
2992+
match &events[1] {
2993+
Event::DiscardFunding {
2994+
funding_info: FundingInfo::Contribution { inputs, outputs },
2995+
..
2996+
} => {
2997+
// The UTXO was filtered: it's still committed to the prior splice.
2998+
assert!(inputs.is_empty(), "Expected empty inputs (filtered), got {:?}", inputs);
2999+
// The change output was NOT filtered: different splice-in amount produces a
3000+
// different change.
3001+
let expected_outputs: Vec<_> = splice_b_change_output.into_iter().collect();
3002+
assert_eq!(*outputs, expected_outputs);
3003+
},
3004+
other => panic!("Expected DiscardFunding with Contribution, got {:?}", other),
3005+
}
3006+
} else {
3007+
expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution);
3008+
}
29633009
let _ = get_event_msg!(closee_node, MessageSendEvent::SendShutdown, closer_node_id);
29643010
}
29653011

@@ -5318,3 +5364,74 @@ fn test_splice_rbf_sequential() {
53185364
node.chain_source.remove_watched_txn_and_outputs(outpoint_1, new_funding_script.clone());
53195365
}
53205366
}
5367+
5368+
#[test]
5369+
fn test_splice_rbf_disconnect_filters_prior_contributions() {
5370+
// When disconnecting during an RBF round that reuses the same UTXOs as a prior round,
5371+
// the SpliceFundingFailed event should filter out inputs/outputs still committed to the prior
5372+
// round. This exercises the `reset_pending_splice_state` → `maybe_create_splice_funding_failed`
5373+
// macro path.
5374+
let chanmon_cfgs = create_chanmon_cfgs(2);
5375+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
5376+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
5377+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
5378+
5379+
let node_id_0 = nodes[0].node.get_our_node_id();
5380+
let node_id_1 = nodes[1].node.get_our_node_id();
5381+
5382+
let initial_channel_value_sat = 100_000;
5383+
let (_, _, channel_id, _) =
5384+
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0);
5385+
5386+
let added_value = Amount::from_sat(50_000);
5387+
// Provide exactly 1 UTXO per node so coin selection is deterministic.
5388+
provide_utxo_reserves(&nodes, 1, added_value * 2);
5389+
5390+
// --- Round 0: Initial splice-in at floor feerate (253). ---
5391+
let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value);
5392+
let (_splice_tx_0, _new_funding_script) =
5393+
splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution);
5394+
5395+
// --- Round 1: RBF at higher feerate without providing new UTXOs. ---
5396+
// The wallet reselects the same UTXO since the splice tx hasn't been mined.
5397+
let feerate_1_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25 + 23) / 24;
5398+
let rbf_feerate = FeeRate::from_sat_per_kwu(feerate_1_sat_per_kwu);
5399+
let funding_contribution_1 =
5400+
do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate);
5401+
let rbf_change_output = funding_contribution_1.change_output().cloned();
5402+
5403+
// STFU exchange + RBF handshake to start interactive TX.
5404+
complete_rbf_handshake(&nodes[0], &nodes[1]);
5405+
5406+
// Disconnect mid-negotiation. Stale interactive TX messages are cleared by peer_disconnected.
5407+
nodes[0].node.peer_disconnected(node_id_1);
5408+
nodes[1].node.peer_disconnected(node_id_0);
5409+
5410+
// The initiator should get SpliceFailed + DiscardFunding with filtered contributions.
5411+
let events = nodes[0].node.get_and_clear_pending_events();
5412+
assert_eq!(events.len(), 2, "{events:?}");
5413+
match &events[0] {
5414+
Event::SpliceFailed { channel_id: cid, .. } => {
5415+
assert_eq!(*cid, channel_id);
5416+
},
5417+
other => panic!("Expected SpliceFailed, got {:?}", other),
5418+
}
5419+
match &events[1] {
5420+
Event::DiscardFunding {
5421+
funding_info: FundingInfo::Contribution { inputs, outputs },
5422+
..
5423+
} => {
5424+
// The UTXO was filtered out: it's still committed to round 0's splice.
5425+
assert!(inputs.is_empty(), "Expected empty inputs (filtered), got {:?}", inputs);
5426+
// The change output was NOT filtered: different feerate produces a different amount.
5427+
let expected_outputs: Vec<_> = rbf_change_output.into_iter().collect();
5428+
assert_eq!(*outputs, expected_outputs);
5429+
},
5430+
other => panic!("Expected DiscardFunding with Contribution, got {:?}", other),
5431+
}
5432+
5433+
// Reconnect. After a completed splice, channel_ready is not re-sent.
5434+
let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
5435+
reconnect_args.send_announcement_sigs = (true, true);
5436+
reconnect_nodes(reconnect_args);
5437+
}

0 commit comments

Comments
 (0)