From 48d6c835419f3cc4f8c0cd31b81968e071120657 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 fde56a9adcb..e1192041517 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3250,16 +3250,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), @@ -7217,27 +7207,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, } } @@ -12734,22 +12710,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 @@ -14396,9 +14378,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() @@ -14406,15 +14385,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 a6823e39aed..519d52a479b 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3847,7 +3847,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 2dec452b3d800b36199fcbc92e0d39b47a6a51b1 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 a15f8ce8cd9..271e135d51d 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 f4139dbfac17b87f818cdc2bef5702440c6d3756 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 | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 3a0b4fb0630..f73b4870166 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -761,6 +761,13 @@ impl FundingContribution { (contributed_inputs, contributed_outputs.map(|output| output.script_pubkey).collect()) } + /// Returns this contribution's inputs and outputs after removing any that overlap + /// with the provided `existing_inputs`/`existing_outputs`. + /// + /// Multiple contribution outputs sharing a `script_pubkey` are all dropped when any + /// existing output uses the same script. + /// + /// Returns `None` if every input and output was filtered as overlapping. pub(crate) fn into_unique_contributions<'a>( self, existing_inputs: impl Iterator, existing_outputs: impl Iterator,