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 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/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, 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],