From f94d53e3f88e24cdfc70f6c0c57fc2eca180a9f6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 8 May 2026 10:49:28 -0500 Subject: [PATCH 1/3] 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) --- lightning/src/ln/channel.rs | 100 ++++++++++++++--------------- lightning/src/ln/splicing_tests.rs | 2 +- 2 files changed, 50 insertions(+), 52 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 3f3a6feb414..c437874b2e5 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3195,16 +3195,6 @@ pub(super) enum QuiescentError { FailSplice(SpliceFundingFailed, NegotiationFailureReason), } -impl QuiescentError { - fn with_negotiation_failure_reason(mut self, reason: NegotiationFailureReason) -> Self { - match self { - QuiescentError::FailSplice(_, ref mut r) => *r = reason, - _ => debug_assert!(false, "Expected FailSplice variant"), - } - self - } -} - pub(crate) enum StfuResponse { Stfu(msgs::Stfu), SpliceInit(msgs::SpliceInit), @@ -7133,27 +7123,13 @@ where .expect("is_initiator is true so this always returns Some") } - fn quiescent_action_into_error(&self, action: QuiescentAction) -> QuiescentError { - match action { - QuiescentAction::Splice { contribution, .. } => QuiescentError::FailSplice( - self.splice_funding_failed_for(contribution), - NegotiationFailureReason::Unknown, - ), - #[cfg(any(test, fuzzing, feature = "_test_utils"))] - QuiescentAction::DoNothing => QuiescentError::DoNothing, - } - } - fn abandon_quiescent_action(&mut self) -> Option { - let action = self.quiescent_action.take()?; - match self.quiescent_action_into_error(action) { - QuiescentError::FailSplice(failed, _) => Some(failed), - #[cfg(any(test, fuzzing, feature = "_test_utils"))] - QuiescentError::DoNothing => None, - _ => { - debug_assert!(false); - None + match self.quiescent_action.take()? { + QuiescentAction::Splice { contribution, .. } => { + Some(self.splice_funding_failed_for(contribution)) }, + #[cfg(any(test, fuzzing, feature = "_test_utils"))] + QuiescentAction::DoNothing => None, } } @@ -12588,22 +12564,28 @@ where ) -> Result, QuiescentError> { debug_assert!(contribution.is_splice()); - if let Some(QuiescentAction::Splice { contribution: existing, .. }) = &self.quiescent_action - { - let pending_splice = self.pending_splice.as_ref(); - let prior_inputs = pending_splice - .into_iter() - .flat_map(|pending_splice| pending_splice.contributed_inputs()); - let prior_outputs = pending_splice - .into_iter() - .flat_map(|pending_splice| pending_splice.contributed_outputs()); - return match contribution.into_unique_contributions( - existing.contributed_inputs().chain(prior_inputs), - existing.contributed_outputs().chain(prior_outputs), - ) { - None => Err(QuiescentError::DoNothing), - Some((inputs, outputs)) => Err(QuiescentError::DiscardFunding { inputs, outputs }), - }; + match self.quiescent_action.as_ref() { + Some(QuiescentAction::Splice { contribution: existing, .. }) => { + let pending_splice = self.pending_splice.as_ref(); + let prior_inputs = pending_splice + .into_iter() + .flat_map(|pending_splice| pending_splice.contributed_inputs()); + let prior_outputs = pending_splice + .into_iter() + .flat_map(|pending_splice| pending_splice.contributed_outputs()); + return match contribution.into_unique_contributions( + existing.contributed_inputs().chain(prior_inputs), + existing.contributed_outputs().chain(prior_outputs), + ) { + None => Err(QuiescentError::DoNothing), + Some((inputs, outputs)) => { + Err(QuiescentError::DiscardFunding { inputs, outputs }) + }, + }; + }, + #[cfg(any(test, fuzzing, feature = "_test_utils"))] + Some(QuiescentAction::DoNothing) => unreachable!(), + None => {}, } let initiated_funding_negotiation = self @@ -14238,9 +14220,6 @@ where ) -> Result, QuiescentError> { log_debug!(logger, "Attempting to initiate quiescence"); - // TODO: NegotiationFailureReason is splice-specific, but propose_quiescence is - // generic. The reason should be selected by the caller, but it currently can't - // distinguish why quiescence failed. Revisit when a second quiescent protocol is added. if !self.context.is_usable() { debug_assert!( self.context.channel_state.is_local_shutdown_sent() @@ -14248,15 +14227,34 @@ where "splice_channel should have prevented reaching propose_quiescence on a non-ready channel" ); log_debug!(logger, "Channel is not in a usable state to propose quiescence"); - return Err(self.quiescent_action_into_error(action) - .with_negotiation_failure_reason(NegotiationFailureReason::ChannelClosing)); + return Err(match action { + QuiescentAction::Splice { contribution, .. } => QuiescentError::FailSplice( + self.splice_funding_failed_for(contribution), + NegotiationFailureReason::ChannelClosing, + ), + #[cfg(any(test, fuzzing, feature = "_test_utils"))] + QuiescentAction::DoNothing => QuiescentError::DoNothing, + }); } + if self.quiescent_action.is_some() { + debug_assert!( + false, + "callers must not invoke propose_quiescence with {:?} while quiescent_action is set", + action, + ); log_debug!( logger, "Channel already has a pending quiescent action and cannot start another", ); - return Err(self.quiescent_action_into_error(action)); + return Err(match action { + #[cfg(any(test, fuzzing, feature = "_test_utils"))] + QuiescentAction::DoNothing => QuiescentError::DoNothing, + QuiescentAction::Splice { contribution, .. } => QuiescentError::FailSplice( + self.splice_funding_failed_for(contribution), + NegotiationFailureReason::Unknown, + ), + }); } // Since we don't have a pending quiescent action, we should never be in a state where we // sent `stfu` without already having become quiescent. diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index b0ca6e494c5..8799970d694 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3540,7 +3540,7 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); // When testing with a prior pending splice, complete splice A first so that - // `quiescent_action_into_error` filters against `pending_splice.contributed_inputs/outputs`. + // `splice_funding_failed_for` filters against `pending_splice.contributed_inputs/outputs`. if pending_splice { let funding_contribution = do_initiate_splice_in( &nodes[0], From 2bde91e43e28f8ab86122073a2f8c500e7c027f1 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 8 May 2026 11:14:25 -0500 Subject: [PATCH 2/3] Document SpliceNegotiationFailed contribution / DiscardFunding overlap The contribution returned in Event::SpliceNegotiationFailed may include inputs and outputs already committed to a prior negotiated (but not yet locked) splice transaction. Those overlapping items are intentionally omitted from the preceding Event::DiscardFunding to avoid prompting the user to reclaim UTXOs that are still in use elsewhere. The relationship was documented on the internal SpliceFundingFailed fields but lost when they were made private; surface it on the public event field doc. Co-Authored-By: Claude Opus 4.7 (1M context) --- lightning/src/events/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 2be6ef13965..020357a5e8b 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1706,6 +1706,12 @@ pub enum Event { /// Alternatively, call [`ChannelManager::splice_channel`] to obtain a fresh /// [`FundingTemplate`] and build a new contribution. /// + /// The contribution preserves the full set of inputs and outputs from the failed round, + /// including any that were also committed to a prior negotiated (but not yet locked) + /// splice transaction. Those overlapping inputs and outputs are intentionally omitted + /// from the preceding [`Event::DiscardFunding`], since they remain committed to that + /// prior splice. + /// /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel /// [`FundingTemplate`]: crate::ln::funding::FundingTemplate From b2b0d4397a91418227c6ae0a7ea0bfd7dce4ccb7 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 8 May 2026 11:41:22 -0500 Subject: [PATCH 3/3] Document script_pubkey-only matching in into_unique_contributions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function compares outputs by script_pubkey alone, not full TxOut, so any contribution output sharing a script with an existing output is filtered regardless of value. This is intentional — a change output's value may shift between rounds (e.g., for a new feerate) and should still match. But the consequence isn't obvious: multiple contribution outputs sharing a script are all filtered together when any existing output uses that script. Document it. Co-Authored-By: Claude Opus 4.7 (1M context) --- lightning/src/ln/funding.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 2f4e89db926..2eb8f9b0414 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -757,6 +757,15 @@ impl FundingContribution { (inputs.into_iter().map(|input| input.utxo.outpoint).collect(), outputs) } + /// Returns this contribution's inputs and outputs after removing any that overlap with + /// the provided `existing_inputs`/`existing_outputs`. + /// + /// Outputs are compared by `script_pubkey` alone (not full `TxOut`), since values may + /// differ between rounds (e.g., a change output adjusted for a new feerate). As a + /// consequence, multiple contribution outputs sharing a `script_pubkey` are all + /// dropped together when any existing output uses the same script. + /// + /// Returns `None` if every input and output was filtered as overlapping. pub(super) fn into_unique_contributions<'a>( self, existing_inputs: impl Iterator, existing_outputs: impl Iterator,