Skip to content

Commit 40d1b26

Browse files
jkczyzclaude
jkczyz
and
claude
committed
Emit DiscardFunding before SpliceFailed
Reverse the event ordering at all emission sites so that Event::DiscardFunding is emitted before Event::SpliceFailed. If the user retries the splice when handling SpliceFailed, the contributed inputs would still be locked. A subsequent DiscardFunding would then incorrectly unlock inputs that are now committed to the new attempt. Emitting DiscardFunding first avoids this by ensuring inputs are unlocked before any retry occurs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e11cbe0 commit 40d1b26

3 files changed

Lines changed: 131 additions & 115 deletions

File tree

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 97 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -4109,6 +4109,15 @@ impl<
41094109
if let Some(splice_funding_failed) = splice_funding_failed {
41104110
let (funding_info, contribution) = splice_funding_failed.into_parts();
41114111
let mut pending_events = self.pending_events.lock().unwrap();
4112+
if let Some(funding_info) = funding_info {
4113+
pending_events.push_back((
4114+
events::Event::DiscardFunding {
4115+
channel_id: *chan_id,
4116+
funding_info,
4117+
},
4118+
None,
4119+
));
4120+
}
41124121
pending_events.push_back((
41134122
events::Event::SpliceFailed {
41144123
channel_id: *chan_id,
@@ -4119,15 +4128,6 @@ impl<
41194128
},
41204129
None,
41214130
));
4122-
if let Some(funding_info) = funding_info {
4123-
pending_events.push_back((
4124-
events::Event::DiscardFunding {
4125-
channel_id: *chan_id,
4126-
funding_info,
4127-
},
4128-
None,
4129-
));
4130-
}
41314131
}
41324132

41334133
// We can send the `shutdown` message before updating the `ChannelMonitor`
@@ -4415,6 +4415,15 @@ impl<
44154415

44164416
if let Some(splice_funding_failed) = shutdown_res.splice_funding_failed.take() {
44174417
let (funding_info, contribution) = splice_funding_failed.into_parts();
4418+
if let Some(funding_info) = funding_info {
4419+
pending_events.push_back((
4420+
events::Event::DiscardFunding {
4421+
channel_id: shutdown_res.channel_id,
4422+
funding_info,
4423+
},
4424+
None,
4425+
));
4426+
}
44184427
pending_events.push_back((
44194428
events::Event::SpliceFailed {
44204429
channel_id: shutdown_res.channel_id,
@@ -4425,15 +4434,6 @@ impl<
44254434
},
44264435
None,
44274436
));
4428-
if let Some(funding_info) = funding_info {
4429-
pending_events.push_back((
4430-
events::Event::DiscardFunding {
4431-
channel_id: shutdown_res.channel_id,
4432-
funding_info,
4433-
},
4434-
None,
4435-
));
4436-
}
44374437
}
44384438

44394439
if let Some(transaction) = shutdown_res.unbroadcasted_funding_tx {
@@ -4921,6 +4921,15 @@ impl<
49214921
if let Some(splice_funding_failed) = splice_funding_failed {
49224922
let (funding_info, contribution) = splice_funding_failed.into_parts();
49234923
let pending_events = &mut self.pending_events.lock().unwrap();
4924+
if let Some(funding_info) = funding_info {
4925+
pending_events.push_back((
4926+
events::Event::DiscardFunding {
4927+
channel_id: *channel_id,
4928+
funding_info,
4929+
},
4930+
None,
4931+
));
4932+
}
49244933
pending_events.push_back((
49254934
events::Event::SpliceFailed {
49264935
channel_id: *channel_id,
@@ -4931,15 +4940,6 @@ impl<
49314940
},
49324941
None,
49334942
));
4934-
if let Some(funding_info) = funding_info {
4935-
pending_events.push_back((
4936-
events::Event::DiscardFunding {
4937-
channel_id: *channel_id,
4938-
funding_info,
4939-
},
4940-
None,
4941-
));
4942-
}
49434943
}
49444944

49454945
Ok(())
@@ -6612,6 +6612,12 @@ impl<
66126612
QuiescentError::FailSplice(splice_funding_failed, reason) => {
66136613
let (funding_info, contribution) = splice_funding_failed.into_parts();
66146614
let pending_events = &mut self.pending_events.lock().unwrap();
6615+
if let Some(funding_info) = funding_info {
6616+
pending_events.push_back((
6617+
events::Event::DiscardFunding { channel_id, funding_info },
6618+
None,
6619+
));
6620+
}
66156621
pending_events.push_back((
66166622
events::Event::SpliceFailed {
66176623
channel_id,
@@ -6622,12 +6628,6 @@ impl<
66226628
},
66236629
None,
66246630
));
6625-
if let Some(funding_info) = funding_info {
6626-
pending_events.push_back((
6627-
events::Event::DiscardFunding { channel_id, funding_info },
6628-
None,
6629-
));
6630-
}
66316631
},
66326632
}
66336633
}
@@ -11842,6 +11842,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1184211842
if let Some(splice_funding_failed) = splice_funding_failed {
1184311843
let (funding_info, contribution) = splice_funding_failed.into_parts();
1184411844
let pending_events = &mut self.pending_events.lock().unwrap();
11845+
if let Some(funding_info) = funding_info {
11846+
pending_events.push_back((
11847+
events::Event::DiscardFunding { channel_id, funding_info },
11848+
None,
11849+
));
11850+
}
1184511851
pending_events.push_back((
1184611852
events::Event::SpliceFailed {
1184711853
channel_id,
@@ -11854,12 +11860,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1185411860
},
1185511861
None,
1185611862
));
11857-
if let Some(funding_info) = funding_info {
11858-
pending_events.push_back((
11859-
events::Event::DiscardFunding { channel_id, funding_info },
11860-
None,
11861-
));
11862-
}
1186311863
}
1186411864
debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_)));
1186511865

@@ -12000,6 +12000,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1200012000
if let Some(splice_funding_failed) = splice_funding_failed {
1200112001
let (funding_info, contribution) = splice_funding_failed.into_parts();
1200212002
let pending_events = &mut self.pending_events.lock().unwrap();
12003+
if let Some(funding_info) = funding_info {
12004+
pending_events.push_back((
12005+
events::Event::DiscardFunding {
12006+
channel_id: msg.channel_id,
12007+
funding_info,
12008+
},
12009+
None,
12010+
));
12011+
}
1200312012
pending_events.push_back((
1200412013
events::Event::SpliceFailed {
1200512014
channel_id: msg.channel_id,
@@ -12012,15 +12021,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1201212021
},
1201312022
None,
1201412023
));
12015-
if let Some(funding_info) = funding_info {
12016-
pending_events.push_back((
12017-
events::Event::DiscardFunding {
12018-
channel_id: msg.channel_id,
12019-
funding_info,
12020-
},
12021-
None,
12022-
));
12023-
}
1202412024
}
1202512025
debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_)));
1202612026

@@ -12172,6 +12172,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1217212172
if let Some(splice_funding_failed) = splice_failed {
1217312173
let (funding_info, contribution) = splice_funding_failed.into_parts();
1217412174
let pending_events = &mut self.pending_events.lock().unwrap();
12175+
if let Some(funding_info) = funding_info {
12176+
pending_events.push_back((
12177+
events::Event::DiscardFunding {
12178+
channel_id: msg.channel_id,
12179+
funding_info,
12180+
},
12181+
None,
12182+
));
12183+
}
1217512184
pending_events.push_back((
1217612185
events::Event::SpliceFailed {
1217712186
channel_id: msg.channel_id,
@@ -12186,15 +12195,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1218612195
},
1218712196
None,
1218812197
));
12189-
if let Some(funding_info) = funding_info {
12190-
pending_events.push_back((
12191-
events::Event::DiscardFunding {
12192-
channel_id: msg.channel_id,
12193-
funding_info,
12194-
},
12195-
None,
12196-
));
12197-
}
1219812198
}
1219912199

1220012200
let holding_cell_res = if exited_quiescence {
@@ -12324,6 +12324,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1232412324
if let Some(splice_funding_failed) = splice_funding_failed {
1232512325
let (funding_info, contribution) = splice_funding_failed.into_parts();
1232612326
let mut pending_events = self.pending_events.lock().unwrap();
12327+
if let Some(funding_info) = funding_info {
12328+
pending_events.push_back((
12329+
events::Event::DiscardFunding {
12330+
channel_id: msg.channel_id,
12331+
funding_info,
12332+
},
12333+
None,
12334+
));
12335+
}
1232712336
pending_events.push_back((
1232812337
events::Event::SpliceFailed {
1232912338
channel_id: msg.channel_id,
@@ -12334,15 +12343,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1233412343
},
1233512344
None,
1233612345
));
12337-
if let Some(funding_info) = funding_info {
12338-
pending_events.push_back((
12339-
events::Event::DiscardFunding {
12340-
channel_id: msg.channel_id,
12341-
funding_info,
12342-
},
12343-
None,
12344-
));
12345-
}
1234612346
}
1234712347

1234812348
if let Some(msg) = shutdown {
@@ -15167,6 +15167,22 @@ impl<
1516715167
self.process_pending_events(&event_handler);
1516815168
let collected_events = events.into_inner();
1516915169

15170+
// When both DiscardFunding and SpliceFailed are emitted for the same channel,
15171+
// DiscardFunding must come first so that inputs are unlocked before any retry.
15172+
// Each pair is emitted adjacently under a single lock, so checking adjacent
15173+
// events is sufficient.
15174+
for window in collected_events.windows(2) {
15175+
if let events::Event::SpliceFailed { channel_id, .. } = &window[0] {
15176+
if let events::Event::DiscardFunding { channel_id: cid, .. } = &window[1] {
15177+
assert!(
15178+
channel_id != cid,
15179+
"DiscardFunding must precede SpliceFailed for channel {}",
15180+
channel_id,
15181+
);
15182+
}
15183+
}
15184+
}
15185+
1517015186
// To expand the coverage and make sure all events are properly serialised and deserialised,
1517115187
// we test all generated events round-trip:
1517215188
for event in &collected_events {
@@ -15441,19 +15457,19 @@ impl<
1544115457

1544215458
if let Some(splice_funding_failed) = splice_funding_failed {
1544315459
let (funding_info, contribution) = splice_funding_failed.into_parts();
15460+
if let Some(funding_info) = funding_info {
15461+
splice_failed_events.push(events::Event::DiscardFunding {
15462+
channel_id: chan.context().channel_id(),
15463+
funding_info,
15464+
});
15465+
}
1544415466
splice_failed_events.push(events::Event::SpliceFailed {
1544515467
channel_id: chan.context().channel_id(),
1544615468
counterparty_node_id,
1544715469
user_channel_id: chan.context().get_user_id(),
1544815470
contribution,
1544915471
reason: events::NegotiationFailureReason::PeerDisconnected,
1545015472
});
15451-
if let Some(funding_info) = funding_info {
15452-
splice_failed_events.push(events::Event::DiscardFunding {
15453-
channel_id: chan.context().channel_id(),
15454-
funding_info,
15455-
});
15456-
}
1545715473
}
1545815474

1545915475
if is_resumable {
@@ -18112,6 +18128,15 @@ impl<
1811218128
for chan in peer_state.channel_by_id.values().filter_map(Channel::as_funded) {
1811318129
if let Some(splice_funding_failed) = chan.maybe_splice_funding_failed() {
1811418130
let (funding_info, contribution) = splice_funding_failed.into_parts();
18131+
if let Some(funding_info) = funding_info {
18132+
events.push_back((
18133+
events::Event::DiscardFunding {
18134+
channel_id: chan.context().channel_id(),
18135+
funding_info,
18136+
},
18137+
None,
18138+
));
18139+
}
1811518140
events.push_back((
1811618141
events::Event::SpliceFailed {
1811718142
channel_id: chan.context.channel_id(),
@@ -18122,15 +18147,6 @@ impl<
1812218147
},
1812318148
None,
1812418149
));
18125-
if let Some(funding_info) = funding_info {
18126-
events.push_back((
18127-
events::Event::DiscardFunding {
18128-
channel_id: chan.context().channel_id(),
18129-
funding_info,
18130-
},
18131-
None,
18132-
));
18133-
}
1813418150
}
1813518151
}
1813618152
}

‎lightning/src/ln/functional_test_utils.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3248,18 +3248,10 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>(
32483248
let events = node.node.get_and_clear_pending_events();
32493249
assert_eq!(events.len(), 2);
32503250
match &events[0] {
3251-
Event::SpliceFailed { channel_id, reason, contribution, .. } => {
3252-
assert_eq!(*expected_channel_id, *channel_id);
3253-
assert_eq!(expected_reason, *reason);
3254-
assert_eq!(contribution.as_ref(), Some(&funding_contribution));
3255-
},
3256-
_ => panic!("Unexpected event"),
3257-
}
3258-
match &events[1] {
32593251
Event::DiscardFunding { funding_info, .. } => {
32603252
if let FundingInfo::Contribution { inputs, outputs } = &funding_info {
32613253
let (expected_inputs, expected_outputs) =
3262-
funding_contribution.into_contributed_inputs_and_outputs();
3254+
funding_contribution.clone().into_contributed_inputs_and_outputs();
32633255
assert_eq!(*inputs, expected_inputs);
32643256
assert_eq!(*outputs, expected_outputs);
32653257
} else {
@@ -3268,6 +3260,14 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>(
32683260
},
32693261
_ => panic!("Unexpected event"),
32703262
}
3263+
match &events[1] {
3264+
Event::SpliceFailed { channel_id, reason, contribution, .. } => {
3265+
assert_eq!(*expected_channel_id, *channel_id);
3266+
assert_eq!(expected_reason, *reason);
3267+
assert_eq!(contribution.as_ref(), Some(&funding_contribution));
3268+
},
3269+
_ => panic!("Unexpected event"),
3270+
}
32713271
}
32723272

32733273
#[cfg(any(test, ldk_bench, feature = "_test_utils"))]

0 commit comments

Comments
 (0)