Skip to content

Commit f94d53e

Browse files
jkczyzclaude
andcommitted
Pick NegotiationFailureReason at error construction
QuiescentError::FailSplice was built with a placeholder NegotiationFailureReason::Unknown and expected callers to chain a with_negotiation_failure_reason builder. Sites that forgot the chain leaked Unknown into Event::SpliceNegotiationFailed, and the pattern forced splice-specific reason vocabulary into the generic QuiescentAction helper. Each call site in propose_quiescence now picks the reason at construction. The pending-quiescent-action branch is unreachable, so it asserts unconditionally; the match retains arms for both action variants so release builds return a sensible error if the invariant is violated. abandon_quiescent_action returns SpliceFundingFailed directly without round-tripping through QuiescentError, since the reason was always discarded there. Make funding_contributed's pending-quiescent-action check exhaustive on QuiescentAction. A future variant produces a compile error here and at the matching arm in propose_quiescence, forcing the author to decide how it interacts with funding contribution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5e8c2fc commit f94d53e

2 files changed

Lines changed: 50 additions & 52 deletions

File tree

lightning/src/ln/channel.rs

Lines changed: 49 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -3195,16 +3195,6 @@ pub(super) enum QuiescentError {
31953195
FailSplice(SpliceFundingFailed, NegotiationFailureReason),
31963196
}
31973197

3198-
impl QuiescentError {
3199-
fn with_negotiation_failure_reason(mut self, reason: NegotiationFailureReason) -> Self {
3200-
match self {
3201-
QuiescentError::FailSplice(_, ref mut r) => *r = reason,
3202-
_ => debug_assert!(false, "Expected FailSplice variant"),
3203-
}
3204-
self
3205-
}
3206-
}
3207-
32083198
pub(crate) enum StfuResponse {
32093199
Stfu(msgs::Stfu),
32103200
SpliceInit(msgs::SpliceInit),
@@ -7133,27 +7123,13 @@ where
71337123
.expect("is_initiator is true so this always returns Some")
71347124
}
71357125

7136-
fn quiescent_action_into_error(&self, action: QuiescentAction) -> QuiescentError {
7137-
match action {
7138-
QuiescentAction::Splice { contribution, .. } => QuiescentError::FailSplice(
7139-
self.splice_funding_failed_for(contribution),
7140-
NegotiationFailureReason::Unknown,
7141-
),
7142-
#[cfg(any(test, fuzzing, feature = "_test_utils"))]
7143-
QuiescentAction::DoNothing => QuiescentError::DoNothing,
7144-
}
7145-
}
7146-
71477126
fn abandon_quiescent_action(&mut self) -> Option<SpliceFundingFailed> {
7148-
let action = self.quiescent_action.take()?;
7149-
match self.quiescent_action_into_error(action) {
7150-
QuiescentError::FailSplice(failed, _) => Some(failed),
7151-
#[cfg(any(test, fuzzing, feature = "_test_utils"))]
7152-
QuiescentError::DoNothing => None,
7153-
_ => {
7154-
debug_assert!(false);
7155-
None
7127+
match self.quiescent_action.take()? {
7128+
QuiescentAction::Splice { contribution, .. } => {
7129+
Some(self.splice_funding_failed_for(contribution))
71567130
},
7131+
#[cfg(any(test, fuzzing, feature = "_test_utils"))]
7132+
QuiescentAction::DoNothing => None,
71577133
}
71587134
}
71597135

@@ -12588,22 +12564,28 @@ where
1258812564
) -> Result<Option<msgs::Stfu>, QuiescentError> {
1258912565
debug_assert!(contribution.is_splice());
1259012566

12591-
if let Some(QuiescentAction::Splice { contribution: existing, .. }) = &self.quiescent_action
12592-
{
12593-
let pending_splice = self.pending_splice.as_ref();
12594-
let prior_inputs = pending_splice
12595-
.into_iter()
12596-
.flat_map(|pending_splice| pending_splice.contributed_inputs());
12597-
let prior_outputs = pending_splice
12598-
.into_iter()
12599-
.flat_map(|pending_splice| pending_splice.contributed_outputs());
12600-
return match contribution.into_unique_contributions(
12601-
existing.contributed_inputs().chain(prior_inputs),
12602-
existing.contributed_outputs().chain(prior_outputs),
12603-
) {
12604-
None => Err(QuiescentError::DoNothing),
12605-
Some((inputs, outputs)) => Err(QuiescentError::DiscardFunding { inputs, outputs }),
12606-
};
12567+
match self.quiescent_action.as_ref() {
12568+
Some(QuiescentAction::Splice { contribution: existing, .. }) => {
12569+
let pending_splice = self.pending_splice.as_ref();
12570+
let prior_inputs = pending_splice
12571+
.into_iter()
12572+
.flat_map(|pending_splice| pending_splice.contributed_inputs());
12573+
let prior_outputs = pending_splice
12574+
.into_iter()
12575+
.flat_map(|pending_splice| pending_splice.contributed_outputs());
12576+
return match contribution.into_unique_contributions(
12577+
existing.contributed_inputs().chain(prior_inputs),
12578+
existing.contributed_outputs().chain(prior_outputs),
12579+
) {
12580+
None => Err(QuiescentError::DoNothing),
12581+
Some((inputs, outputs)) => {
12582+
Err(QuiescentError::DiscardFunding { inputs, outputs })
12583+
},
12584+
};
12585+
},
12586+
#[cfg(any(test, fuzzing, feature = "_test_utils"))]
12587+
Some(QuiescentAction::DoNothing) => unreachable!(),
12588+
None => {},
1260712589
}
1260812590

1260912591
let initiated_funding_negotiation = self
@@ -14238,25 +14220,41 @@ where
1423814220
) -> Result<Option<msgs::Stfu>, QuiescentError> {
1423914221
log_debug!(logger, "Attempting to initiate quiescence");
1424014222

14241-
// TODO: NegotiationFailureReason is splice-specific, but propose_quiescence is
14242-
// generic. The reason should be selected by the caller, but it currently can't
14243-
// distinguish why quiescence failed. Revisit when a second quiescent protocol is added.
1424414223
if !self.context.is_usable() {
1424514224
debug_assert!(
1424614225
self.context.channel_state.is_local_shutdown_sent()
1424714226
|| self.context.channel_state.is_remote_shutdown_sent(),
1424814227
"splice_channel should have prevented reaching propose_quiescence on a non-ready channel"
1424914228
);
1425014229
log_debug!(logger, "Channel is not in a usable state to propose quiescence");
14251-
return Err(self.quiescent_action_into_error(action)
14252-
.with_negotiation_failure_reason(NegotiationFailureReason::ChannelClosing));
14230+
return Err(match action {
14231+
QuiescentAction::Splice { contribution, .. } => QuiescentError::FailSplice(
14232+
self.splice_funding_failed_for(contribution),
14233+
NegotiationFailureReason::ChannelClosing,
14234+
),
14235+
#[cfg(any(test, fuzzing, feature = "_test_utils"))]
14236+
QuiescentAction::DoNothing => QuiescentError::DoNothing,
14237+
});
1425314238
}
14239+
1425414240
if self.quiescent_action.is_some() {
14241+
debug_assert!(
14242+
false,
14243+
"callers must not invoke propose_quiescence with {:?} while quiescent_action is set",
14244+
action,
14245+
);
1425514246
log_debug!(
1425614247
logger,
1425714248
"Channel already has a pending quiescent action and cannot start another",
1425814249
);
14259-
return Err(self.quiescent_action_into_error(action));
14250+
return Err(match action {
14251+
#[cfg(any(test, fuzzing, feature = "_test_utils"))]
14252+
QuiescentAction::DoNothing => QuiescentError::DoNothing,
14253+
QuiescentAction::Splice { contribution, .. } => QuiescentError::FailSplice(
14254+
self.splice_funding_failed_for(contribution),
14255+
NegotiationFailureReason::Unknown,
14256+
),
14257+
});
1426014258
}
1426114259
// Since we don't have a pending quiescent action, we should never be in a state where we
1426214260
// sent `stfu` without already having become quiescent.

lightning/src/ln/splicing_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3540,7 +3540,7 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_
35403540
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0);
35413541

35423542
// When testing with a prior pending splice, complete splice A first so that
3543-
// `quiescent_action_into_error` filters against `pending_splice.contributed_inputs/outputs`.
3543+
// `splice_funding_failed_for` filters against `pending_splice.contributed_inputs/outputs`.
35443544
if pending_splice {
35453545
let funding_contribution = do_initiate_splice_in(
35463546
&nodes[0],

0 commit comments

Comments
 (0)