From f0a8cebbc1e879f892012b7cfabbed80d376bc1b Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 17:56:42 -0500 Subject: [PATCH 01/11] Add NegotiationFailureReason to SpliceFailed event Each splice negotiation round can fail for different reasons, but Event::SpliceFailed previously gave no indication of what went wrong. Add a NegotiationFailureReason enum so users can distinguish failures and take appropriate action (e.g., retry with a higher feerate vs. wait for the channel to become usable). The reason is determined at each channelmanager emission site based on context rather than threaded through channel.rs internals, since the channelmanager knows the triggering context (disconnect, tx_abort, shutdown, etc.) while channel.rs functions like abandon_quiescent_action handle both splice and non-splice quiescent actions. The one exception is QuiescentError::FailSplice, which carries a reason alongside the SpliceFundingFailed. This is appropriate because FailSplice is already splice-specific, and the channel.rs code that constructs it (e.g., contribution validation, feerate checks) knows the specific failure cause. A with_negotiation_failure_reason method on QuiescentError allows callers to override the default when needed. Older serializations that lack the reason field default to Unknown via default_value in deserialization. The persistence reload path uses PeerDisconnected since a reload implies the peer connection was lost. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/events/mod.rs | 110 ++++++++++++++++ lightning/src/ln/channel.rs | 47 +++++-- lightning/src/ln/channelmanager.rs | 32 ++++- lightning/src/ln/functional_test_utils.rs | 9 +- lightning/src/ln/splicing_tests.rs | 154 ++++++++++++++++++---- 5 files changed, 308 insertions(+), 44 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 73c4a39c76f..0c99ee02c79 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -99,6 +99,110 @@ impl_writeable_tlv_based_enum!(FundingInfo, } ); +/// The reason a funding negotiation round failed. +/// +/// Each negotiation attempt (initial or RBF) resolves to either success or failure. This enum +/// indicates what caused the failure. Use [`is_retriable`] to determine whether the splice can +/// be reattempted on this channel by calling [`ChannelManager::splice_channel`]. +/// +/// [`is_retriable`]: Self::is_retriable +/// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum NegotiationFailureReason { + /// The reason was not available (e.g., from an older serialization). + Unknown, + /// The peer disconnected during negotiation. Wait for the peer to reconnect, then retry. + PeerDisconnected, + /// The counterparty explicitly aborted the negotiation by sending `tx_abort`. Retrying with + /// the same parameters is unlikely to succeed — consider adjusting the contribution or + /// waiting for the counterparty to initiate. + CounterpartyAborted { + /// The counterparty's abort message. + /// + /// This is counterparty-provided data. Use `Display` on [`UntrustedString`] for safe + /// logging. + msg: UntrustedString, + }, + /// An error occurred during interactive transaction negotiation (e.g., the counterparty sent + /// an invalid message). The negotiation was aborted. + NegotiationError { + /// A developer-readable error message. + msg: String, + }, + /// The funding contribution was invalid (e.g., insufficient balance for the splice amount). + /// Call [`ChannelManager::splice_channel`] for a fresh [`FundingTemplate`] and build a new + /// contribution with adjusted parameters. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + /// [`FundingTemplate`]: crate::ln::funding::FundingTemplate + ContributionInvalid, + /// The negotiation was locally abandoned via `ChannelManager::abandon_splice`. + LocallyAbandoned, + /// The channel is closing, so the negotiation cannot continue. See [`Event::ChannelClosed`] + /// for the closure reason. + ChannelClosing, + /// The contribution's feerate was too low for RBF. Call [`ChannelManager::splice_channel`] + /// for a fresh [`FundingTemplate`] (which includes the updated minimum feerate) and build a + /// new contribution with a higher feerate. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + /// [`FundingTemplate`]: crate::ln::funding::FundingTemplate + FeeRateTooLow, +} + +impl NegotiationFailureReason { + /// Whether the splice negotiation is likely to succeed if retried on this channel. When `true`, + /// call [`ChannelManager::splice_channel`] to obtain a fresh [`FundingTemplate`] and retry. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + /// [`FundingTemplate`]: crate::ln::funding::FundingTemplate + pub fn is_retriable(&self) -> bool { + match self { + Self::Unknown + | Self::PeerDisconnected + | Self::ContributionInvalid + | Self::FeeRateTooLow => true, + Self::CounterpartyAborted { .. } + | Self::NegotiationError { .. } + | Self::LocallyAbandoned + | Self::ChannelClosing => false, + } + } +} + +impl core::fmt::Display for NegotiationFailureReason { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Unknown => f.write_str("unknown reason"), + Self::PeerDisconnected => f.write_str("peer disconnected during negotiation"), + Self::CounterpartyAborted { msg } => { + write!(f, "counterparty aborted: {}", msg) + }, + Self::NegotiationError { msg } => write!(f, "negotiation error: {}", msg), + Self::ContributionInvalid => f.write_str("funding contribution was invalid"), + Self::LocallyAbandoned => f.write_str("splice locally abandoned"), + + Self::ChannelClosing => f.write_str("channel is closing"), + Self::FeeRateTooLow => f.write_str("feerate too low for RBF"), + } + } +} + +impl_writeable_tlv_based_enum_upgradable!(NegotiationFailureReason, + (1, Unknown) => {}, + (3, PeerDisconnected) => {}, + (5, CounterpartyAborted) => { + (1, msg, required), + }, + (7, NegotiationError) => { + (1, msg, required), + }, + (9, ContributionInvalid) => {}, + (11, LocallyAbandoned) => {}, + (13, ChannelClosing) => {}, + (15, FeeRateTooLow) => {}, +); + /// Some information provided on receipt of payment depends on whether the payment received is a /// spontaneous payment or a "conventional" lightning payment that's paying an invoice. #[derive(Clone, Debug, PartialEq, Eq)] @@ -1586,6 +1690,8 @@ pub enum Event { abandoned_funding_txo: Option, /// The features that this channel will operate with, if available. channel_type: Option, + /// The reason the splice negotiation failed. + reason: NegotiationFailureReason, }, /// Used to indicate to the user that they can abandon the funding transaction and recycle the /// inputs for another purpose. @@ -2379,6 +2485,7 @@ impl Writeable for Event { ref counterparty_node_id, ref abandoned_funding_txo, ref channel_type, + ref reason, } => { 52u8.write(writer)?; write_tlv_fields!(writer, { @@ -2387,6 +2494,7 @@ impl Writeable for Event { (5, user_channel_id, required), (7, counterparty_node_id, required), (9, abandoned_funding_txo, option), + (11, reason, required), }); }, // Note that, going forward, all new events must only write data inside of @@ -3031,6 +3139,7 @@ impl MaybeReadable for Event { (5, user_channel_id, required), (7, counterparty_node_id, required), (9, abandoned_funding_txo, option), + (11, reason, upgradable_option), }); Ok(Some(Event::SpliceFailed { @@ -3039,6 +3148,7 @@ impl MaybeReadable for Event { counterparty_node_id: counterparty_node_id.0.unwrap(), abandoned_funding_txo, channel_type, + reason: reason.unwrap_or(NegotiationFailureReason::Unknown), })) }; f() diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8c74fa6753a..ad643a192ad 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -37,7 +37,7 @@ use crate::chain::channelmonitor::{ }; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::chain::BlockLocator; -use crate::events::{ClosureReason, FundingInfo}; +use crate::events::{ClosureReason, FundingInfo, NegotiationFailureReason}; use crate::ln::chan_utils; use crate::ln::chan_utils::{ get_commitment_transaction_number_obscure_factor, max_htlcs, second_stage_tx_fees_sat, @@ -3192,7 +3192,17 @@ pub(crate) enum QuiescentAction { pub(super) enum QuiescentError { DoNothing, DiscardFunding { inputs: Vec, outputs: Vec }, - FailSplice(SpliceFundingFailed), + 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 { @@ -7155,9 +7165,10 @@ where fn quiescent_action_into_error(&self, action: QuiescentAction) -> QuiescentError { match action { - QuiescentAction::Splice { contribution, .. } => { - QuiescentError::FailSplice(self.splice_funding_failed_for(contribution)) - }, + QuiescentAction::Splice { contribution, .. } => QuiescentError::FailSplice( + self.splice_funding_failed_for(contribution), + NegotiationFailureReason::Unknown, + ), #[cfg(any(test, fuzzing, feature = "_test_utils"))] QuiescentAction::DoNothing => QuiescentError::DoNothing, } @@ -7166,7 +7177,7 @@ where 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), + QuiescentError::FailSplice(failed, _) => Some(failed), #[cfg(any(test, fuzzing, feature = "_test_utils"))] QuiescentError::DoNothing => None, _ => { @@ -10446,7 +10457,7 @@ where tx_abort = Some(msgs::TxAbort { channel_id: self.context.channel_id(), data: - "No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() }); + "Signing was not completed for this funding transaction; it may be forgotten.".as_bytes().to_vec() }); } } if let Some(funding_txid) = retransmit_funding_commit_sig { @@ -12652,7 +12663,10 @@ where ) { log_error!(logger, "Channel {} cannot be funded: {}", self.context.channel_id(), e); - return Err(QuiescentError::FailSplice(self.splice_funding_failed_for(contribution))); + return Err(QuiescentError::FailSplice( + self.splice_funding_failed_for(contribution), + NegotiationFailureReason::ContributionInvalid, + )); } if let Some(pending_splice) = self.pending_splice.as_ref() { @@ -12668,6 +12682,7 @@ where ); return Err(QuiescentError::FailSplice( self.splice_funding_failed_for(contribution), + NegotiationFailureReason::FeeRateTooLow, )); } } @@ -14165,9 +14180,18 @@ 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() + || self.context.channel_state.is_remote_shutdown_sent(), + "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)); + return Err(self.quiescent_action_into_error(action) + .with_negotiation_failure_reason(NegotiationFailureReason::ChannelClosing)); } if self.quiescent_action.is_some() { log_debug!( @@ -14286,7 +14310,10 @@ where self.context.channel_id(), e, )), - QuiescentError::FailSplice(failed), + QuiescentError::FailSplice( + failed, + NegotiationFailureReason::ContributionInvalid, + ), )); } let prior_contribution = contribution.clone(); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 64486598005..ef2ce9a7d14 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4169,6 +4169,7 @@ impl< user_channel_id: chan.context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); @@ -4475,6 +4476,7 @@ impl< user_channel_id: shutdown_res.user_channel_id, abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); @@ -4981,6 +4983,7 @@ impl< user_channel_id: chan.context.get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::LocallyAbandoned, }, None, )); @@ -6673,12 +6676,15 @@ impl< )); } }, - QuiescentError::FailSplice(SpliceFundingFailed { - funding_txo, - channel_type, - contributed_inputs, - contributed_outputs, - }) => { + QuiescentError::FailSplice( + SpliceFundingFailed { + funding_txo, + channel_type, + contributed_inputs, + contributed_outputs, + }, + reason, + ) => { let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { @@ -6687,6 +6693,7 @@ impl< user_channel_id, abandoned_funding_txo: funding_txo, channel_type, + reason, }, None, )); @@ -6840,7 +6847,7 @@ impl< "Channel {} already has a pending funding contribution", channel_id, ), - QuiescentError::FailSplice(_) => format!( + QuiescentError::FailSplice(..) => format!( "Channel {} cannot accept funding contribution", channel_id, ), @@ -11983,6 +11990,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ user_channel_id, abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type.clone(), + reason: events::NegotiationFailureReason::NegotiationError { + msg: format!("{:?}", err.err), + }, }, None, )); @@ -12319,6 +12329,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ user_channel_id: chan_entry.get().context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::CounterpartyAborted { + msg: UntrustedString( + String::from_utf8_lossy(&msg.data).to_string(), + ), + }, }, None, )); @@ -12467,6 +12482,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ user_channel_id: chan.context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); @@ -15543,6 +15559,7 @@ impl< user_channel_id: chan.context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::PeerDisconnected, }); splice_failed_events.push(events::Event::DiscardFunding { channel_id: chan.context().channel_id(), @@ -18171,6 +18188,7 @@ impl< user_channel_id: chan.context.get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::PeerDisconnected, }, None, )); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index b48d76d646d..b8ef5890899 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -19,8 +19,8 @@ use crate::chain::{BlockLocator, ChannelMonitorUpdateStatus, Confirm, Listen, Wa use crate::events::bump_transaction::sync::BumpTransactionEventHandlerSync; use crate::events::bump_transaction::BumpTransactionEvent; use crate::events::{ - ClaimedHTLC, ClosureReason, Event, FundingInfo, HTLCHandlingFailureType, PaidBolt12Invoice, - PathFailure, PaymentFailureReason, PaymentPurpose, + ClaimedHTLC, ClosureReason, Event, FundingInfo, HTLCHandlingFailureType, + NegotiationFailureReason, PaidBolt12Invoice, PathFailure, PaymentFailureReason, PaymentPurpose, }; use crate::ln::chan_utils::{ commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, TRUC_MAX_WEIGHT, @@ -3232,13 +3232,14 @@ pub fn expect_splice_pending_event<'a, 'b, 'c, 'd>( #[cfg(any(test, ldk_bench, feature = "_test_utils"))] pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( node: &'a Node<'b, 'c, 'd>, expected_channel_id: &ChannelId, - funding_contribution: FundingContribution, + funding_contribution: FundingContribution, expected_reason: NegotiationFailureReason, ) { let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match &events[0] { - Event::SpliceFailed { channel_id, .. } => { + Event::SpliceFailed { channel_id, reason, .. } => { assert_eq!(*expected_channel_id, *channel_id); + assert_eq!(expected_reason, *reason); }, _ => panic!("Unexpected event"), } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 9a3813904e1..34b51e2696c 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -13,7 +13,9 @@ use crate::chain::chaininterface::{FundingPurpose, TransactionType, FEERATE_FLOO use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::OutPoint; use crate::chain::ChannelMonitorUpdateStatus; -use crate::events::{ClosureReason, Event, FundingInfo, HTLCHandlingFailureType}; +use crate::events::{ + ClosureReason, Event, FundingInfo, HTLCHandlingFailureType, NegotiationFailureReason, +}; use crate::ln::chan_utils; use crate::ln::channel::{ ANCHOR_OUTPUT_VALUE_SATOSHI, CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY, @@ -28,6 +30,7 @@ use crate::ln::outbound_payment::RecipientOnionFields; use crate::ln::types::ChannelId; use crate::routing::router::{PaymentParameters, RouteParameters}; use crate::types::features::ChannelTypeFeatures; +use crate::types::string::UntrustedString; use crate::util::config::UserConfig; use crate::util::errors::APIError; use crate::util::ser::Writeable; @@ -252,7 +255,12 @@ pub fn initiate_splice_out<'a, 'b, 'c, 'd>( ) { Ok(()) => Ok(funding_contribution), Err(e) => { - expect_splice_failed_events(initiator, &channel_id, funding_contribution); + expect_splice_failed_events( + initiator, + &channel_id, + funding_contribution, + NegotiationFailureReason::ContributionInvalid, + ); Err(e) }, } @@ -884,7 +892,12 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { nodes[1].node.peer_disconnected(node_id_0); } - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_channel_ready = (true, true); @@ -935,7 +948,12 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { nodes[1].node.peer_disconnected(node_id_0); } - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_channel_ready = (true, true); @@ -1017,7 +1035,17 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { let tx_abort = get_event_msg!(nodes[0], MessageSendEvent::SendTxAbort, node_id_1); nodes[1].node.handle_tx_abort(node_id_0, &tx_abort); - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::CounterpartyAborted { + msg: UntrustedString( + "Signing was not completed for this funding transaction; it may be forgotten." + .to_string(), + ), + }, + ); // Attempt a splice negotiation that completes, (i.e. `tx_signatures` are exchanged). Reconnecting // should not abort the negotiation or reset the splice state. @@ -1101,7 +1129,12 @@ fn test_config_reject_inbound_splices() { nodes[0].node.peer_disconnected(node_id_1); nodes[1].node.peer_disconnected(node_id_0); - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_channel_ready = (true, true); @@ -2696,7 +2729,14 @@ fn fail_splice_on_interactive_tx_error() { get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); initiator.node.handle_tx_add_input(node_id_acceptor, &tx_add_input); - expect_splice_failed_events(initiator, &channel_id, funding_contribution); + expect_splice_failed_events( + initiator, + &channel_id, + funding_contribution, + NegotiationFailureReason::NegotiationError { + msg: "Abort: Parity for `serial_id` was incorrect".to_string(), + }, + ); // We exit quiescence upon sending `tx_abort`, so we should see the holding cell be immediately // freed. @@ -2767,7 +2807,12 @@ fn fail_splice_on_tx_abort() { let tx_abort = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); initiator.node.handle_tx_abort(node_id_acceptor, &tx_abort); - expect_splice_failed_events(initiator, &channel_id, funding_contribution); + expect_splice_failed_events( + initiator, + &channel_id, + funding_contribution, + NegotiationFailureReason::CounterpartyAborted { msg: UntrustedString(String::new()) }, + ); // We exit quiescence upon receiving `tx_abort`, so we should see our `tx_abort` echo and the // holding cell be immediately freed. @@ -2863,7 +2908,16 @@ fn fail_splice_on_tx_complete_error() { }; initiator.node.handle_tx_abort(node_id_acceptor, tx_abort); - expect_splice_failed_events(initiator, &channel_id, funding_contribution); + expect_splice_failed_events( + initiator, + &channel_id, + funding_contribution, + NegotiationFailureReason::CounterpartyAborted { + msg: UntrustedString( + "Total value of outputs exceeds total value of inputs".to_string(), + ), + }, + ); let tx_abort = get_event_msg!(initiator, MessageSendEvent::SendTxAbort, node_id_acceptor); acceptor.node.handle_tx_abort(node_id_initiator, &tx_abort); @@ -3153,8 +3207,9 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => { + Event::SpliceFailed { channel_id: cid, reason, .. } => { assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::ChannelClosing); }, other => panic!("Expected SpliceFailed, got {:?}", other), } @@ -3173,7 +3228,12 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), } } else { - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::ChannelClosing, + ); } let _ = get_event_msg!(closee_node, MessageSendEvent::SendShutdown, closer_node_id); } @@ -4146,7 +4206,12 @@ fn test_funding_contributed_channel_shutdown() { }) ); - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::ChannelClosing, + ); } #[test] @@ -4327,7 +4392,12 @@ fn do_test_splice_pending_htlcs(config: UserConfig) { let reconnect_args = ReconnectArgs::new(initiator, acceptor); reconnect_nodes(reconnect_args); - expect_splice_failed_events(initiator, &channel_id, contribution); + expect_splice_failed_events( + initiator, + &channel_id, + contribution, + NegotiationFailureReason::PeerDisconnected, + ); // 4) Try again with the additional satoshi removed from the splice-out message, and check that it passes // validation on the receiver's side. @@ -4362,7 +4432,12 @@ fn do_test_splice_pending_htlcs(config: UserConfig) { nodes[1].node.peer_disconnected(node_id_0); let reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_nodes(reconnect_args); - expect_splice_failed_events(&nodes[1], &channel_id, contribution); + expect_splice_failed_events( + &nodes[1], + &channel_id, + contribution, + NegotiationFailureReason::PeerDisconnected, + ); let details = &nodes[1].node.list_channels()[0]; let expected_outbound_htlc_max = (pre_splice_balance.to_sat() - details.unspendable_punishment_reserve.unwrap()) * 1000; @@ -4513,7 +4588,12 @@ fn test_splice_acceptor_disconnect_emits_events() { nodes[1].node.peer_disconnected(node_id_0); // The initiator should get SpliceFailed + DiscardFunding. - expect_splice_failed_events(&nodes[0], &channel_id, node_0_funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + node_0_funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); // The acceptor should also get SpliceFailed + DiscardFunding with its contributions // so it can reclaim its UTXOs. The contribution is feerate-adjusted by handle_splice_init, @@ -4521,7 +4601,10 @@ fn test_splice_acceptor_disconnect_emits_events() { let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => assert_eq!(*cid, channel_id), + Event::SpliceFailed { channel_id: cid, reason, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + }, other => panic!("Expected SpliceFailed, got {:?}", other), } match &events[1] { @@ -6444,7 +6527,10 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => assert_eq!(*cid, channel_id), + Event::SpliceFailed { channel_id: cid, reason, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + }, other => panic!("Expected SpliceFailed, got {:?}", other), } match &events[1] { @@ -6523,8 +6609,9 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => { + Event::SpliceFailed { channel_id: cid, reason, .. } => { assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); }, other => panic!("Expected SpliceFailed, got {:?}", other), } @@ -6564,7 +6651,10 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => assert_eq!(*cid, channel_id), + Event::SpliceFailed { channel_id: cid, reason, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + }, other => panic!("Expected SpliceFailed, got {:?}", other), } match &events[1] { @@ -7101,7 +7191,12 @@ fn test_splice_revalidation_at_quiescence() { assert_eq!(msg_events.len(), 1, "{msg_events:?}"); assert!(matches!(msg_events[0], MessageSendEvent::HandleError { .. })); - expect_splice_failed_events(&nodes[0], &channel_id, contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + contribution, + NegotiationFailureReason::ContributionInvalid, + ); } #[test] @@ -7351,7 +7446,10 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => assert_eq!(*cid, channel_id), + Event::SpliceFailed { channel_id: cid, reason, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::FeeRateTooLow); + }, other => panic!("Expected SpliceFailed, got {:?}", other), } } @@ -7447,7 +7545,12 @@ fn test_no_disconnect_after_splice_aborted() { // Abort the splice, which should clear the timer when exiting quiescence. nodes[0].node.abandon_splice(&channel_id, &node_id_1).unwrap(); - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::LocallyAbandoned, + ); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); let tx_abort = msg_events @@ -7519,7 +7622,12 @@ fn test_no_disconnect_after_quiescence_on_reconnect() { nodes[0].node.peer_disconnected(node_id_1); nodes[1].node.peer_disconnected(node_id_0); - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_channel_ready = (true, true); From 450eb6453106a90419323d189467fdaa56249dfe Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 18:20:35 -0500 Subject: [PATCH 02/11] Add FundingContribution to SpliceFailed event Replace the abandoned_funding_txo and channel_type fields on Event::SpliceFailed with an Option from the failed round. Users can feed this back to funding_contributed to retry or use it to inform a fresh attempt via splice_channel. Also makes FundingContribution::feerate() public so users can inspect the feerate when deciding whether to retry or bump. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/events/mod.rs | 39 ++--- lightning/src/ln/channel.rs | 55 ++++--- lightning/src/ln/channelmanager.rs | 189 ++++++++++------------ lightning/src/ln/functional_test_utils.rs | 3 +- lightning/src/ln/splicing_tests.rs | 149 ++++++++--------- 5 files changed, 201 insertions(+), 234 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 0c99ee02c79..5a52be026fb 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -25,6 +25,7 @@ use crate::blinded_path::payment::{ use crate::chain::transaction; use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS; use crate::ln::channelmanager::{InterceptId, PaymentId}; +use crate::ln::funding::FundingContribution; use crate::ln::msgs; use crate::ln::onion_utils::LocalHTLCFailureReason; use crate::ln::outbound_payment::RecipientOnionFields; @@ -1664,19 +1665,20 @@ pub enum Event { /// The witness script that is used to lock the channel's funding output to commitment transactions. new_funding_redeem_script: ScriptBuf, }, - /// Used to indicate that a splice for the given `channel_id` has failed. + /// Used to indicate that a splice negotiation round for the given `channel_id` has failed. /// - /// This event may be emitted if a splice fails after it has been initiated but prior to signing - /// any negotiated funding transaction. + /// Each splice attempt (initial or RBF) resolves to either [`Event::SplicePending`] on + /// success or this event on failure. Prior successfully negotiated splice transactions are + /// unaffected. /// - /// Any UTXOs contributed to be spent by the funding transaction may be reused and will be - /// given in `contributed_inputs`. + /// Any UTXOs contributed to the failed round that are not committed to a prior negotiated + /// splice transaction will be returned via a preceding [`Event::DiscardFunding`]. /// /// # Failure Behavior and Persistence /// This event will eventually be replayed after failures-to-handle (i.e., the event handler /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. SpliceFailed { - /// The `channel_id` of the channel for which the splice failed. + /// The `channel_id` of the channel for which the splice negotiation round failed. channel_id: ChannelId, /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound /// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels. @@ -1686,12 +1688,17 @@ pub enum Event { user_channel_id: u128, /// The `node_id` of the channel counterparty. counterparty_node_id: PublicKey, - /// The outpoint of the channel's splice funding transaction, if one was created. - abandoned_funding_txo: Option, - /// The features that this channel will operate with, if available. - channel_type: Option, /// The reason the splice negotiation failed. reason: NegotiationFailureReason, + /// The funding contribution from the failed negotiation round, if available. This can be + /// fed back to [`ChannelManager::funding_contributed`] to retry with the same parameters. + /// Alternatively, call [`ChannelManager::splice_channel`] to obtain a fresh + /// [`FundingTemplate`] and build a new contribution. + /// + /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + /// [`FundingTemplate`]: crate::ln::funding::FundingTemplate + contribution: Option, }, /// Used to indicate to the user that they can abandon the funding transaction and recycle the /// inputs for another purpose. @@ -2483,18 +2490,16 @@ impl Writeable for Event { ref channel_id, ref user_channel_id, ref counterparty_node_id, - ref abandoned_funding_txo, - ref channel_type, ref reason, + ref contribution, } => { 52u8.write(writer)?; write_tlv_fields!(writer, { (1, channel_id, required), - (3, channel_type, option), (5, user_channel_id, required), (7, counterparty_node_id, required), - (9, abandoned_funding_txo, option), (11, reason, required), + (13, contribution, option), }); }, // Note that, going forward, all new events must only write data inside of @@ -3135,20 +3140,18 @@ impl MaybeReadable for Event { let mut f = || { _init_and_read_len_prefixed_tlv_fields!(reader, { (1, channel_id, required), - (3, channel_type, option), (5, user_channel_id, required), (7, counterparty_node_id, required), - (9, abandoned_funding_txo, option), (11, reason, upgradable_option), + (13, contribution, option), }); Ok(Some(Event::SpliceFailed { channel_id: channel_id.0.unwrap(), user_channel_id: user_channel_id.0.unwrap(), counterparty_node_id: counterparty_node_id.0.unwrap(), - abandoned_funding_txo, - channel_type, reason: reason.unwrap_or(NegotiationFailureReason::Unknown), + contribution, })) }; f() diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ad643a192ad..473fc6b46f3 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7051,17 +7051,33 @@ pub struct SpliceFundingNegotiated { /// Information about a splice funding negotiation that has failed. pub struct SpliceFundingFailed { - /// The outpoint of the channel's splice funding transaction, if one was created. - pub funding_txo: Option, + /// UTXOs spent as inputs contributed to the splice transaction. Excludes inputs already + /// contributed in prior rounds, which may be included in `contribution`. + contributed_inputs: Vec, - /// The features that this channel will operate with, if available. - pub channel_type: Option, + /// Outputs contributed to the splice transaction. Excludes outputs already contributed + /// in prior rounds, which may be included in `contribution`. + contributed_outputs: Vec, - /// UTXOs spent as inputs contributed to the splice transaction. - pub contributed_inputs: Vec, + /// The funding contribution from the failed round, if available. + contribution: Option, +} - /// Outputs contributed to the splice transaction. - pub contributed_outputs: Vec, +impl SpliceFundingFailed { + /// Splits into the funding info for `DiscardFunding` (if there are inputs or outputs to + /// discard) and the contribution for `SpliceFailed`. + pub(super) fn into_parts(self) -> (Option, Option) { + let funding_info = + if !self.contributed_inputs.is_empty() || !self.contributed_outputs.is_empty() { + Some(FundingInfo::Contribution { + inputs: self.contributed_inputs, + outputs: self.contributed_outputs, + }) + } else { + None + }; + (funding_info, self.contribution) + } } macro_rules! maybe_create_splice_funding_failed { @@ -7071,15 +7087,6 @@ macro_rules! maybe_create_splice_funding_failed { .and_then(|funding_negotiation| { let is_initiator = funding_negotiation.is_initiator(); - let funding_txo = funding_negotiation - .as_funding() - .and_then(|funding| funding.get_funding_txo()) - .map(|txo| txo.into_bitcoin_outpoint()); - - let channel_type = funding_negotiation - .as_funding() - .map(|funding| funding.get_channel_type().clone()); - let (mut contributed_inputs, mut contributed_outputs) = match funding_negotiation { FundingNegotiation::AwaitingAck { context, .. } => { context.$contributed_inputs_and_outputs() @@ -7110,12 +7117,10 @@ macro_rules! maybe_create_splice_funding_failed { return None; } - Some(SpliceFundingFailed { - funding_txo, - channel_type, - contributed_inputs, - contributed_outputs, - }) + let contribution = + $pending_splice_ref.and_then(|ps| ps.contributions.last().cloned()); + + Some(SpliceFundingFailed { contributed_inputs, contributed_outputs, contribution }) }) }}; } @@ -7146,6 +7151,7 @@ where /// Builds a [`SpliceFundingFailed`] from a contribution, filtering out inputs/outputs /// that are still committed to a prior splice round. fn splice_funding_failed_for(&self, contribution: FundingContribution) -> SpliceFundingFailed { + let cloned_contribution = contribution.clone(); let (mut inputs, mut outputs) = contribution.into_contributed_inputs_and_outputs(); if let Some(ref pending_splice) = self.pending_splice { for input in pending_splice.contributed_inputs() { @@ -7156,10 +7162,9 @@ where } } SpliceFundingFailed { - funding_txo: None, - channel_type: None, contributed_inputs: inputs, contributed_outputs: outputs, + contribution: Some(cloned_contribution), } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ef2ce9a7d14..2d6aaa56c5f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -61,8 +61,8 @@ use crate::ln::channel::QuiescentError; use crate::ln::channel::{ self, hold_time_since, Channel, ChannelError, ChannelUpdateStatus, DisconnectResult, FundedChannel, FundingTxSigned, InboundV1Channel, InteractiveTxMsgError, OutboundHop, - OutboundV1Channel, PendingV2Channel, ReconnectionMsg, ShutdownResult, SpliceFundingFailed, - StfuResponse, UpdateFulfillCommitFetch, WithChannelContext, + OutboundV1Channel, PendingV2Channel, ReconnectionMsg, ShutdownResult, StfuResponse, + UpdateFulfillCommitFetch, WithChannelContext, }; use crate::ln::channel_state::ChannelDetails; use crate::ln::funding::{FundingContribution, FundingTemplate}; @@ -4161,28 +4161,27 @@ impl< failed_htlcs = htlcs; if let Some(splice_funding_failed) = splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: *chan_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *chan_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: *chan_id, + funding_info, }, - }, - None, - )); + None, + )); + } } // We can send the `shutdown` message before updating the `ChannelMonitor` @@ -4469,27 +4468,26 @@ impl< )); if let Some(splice_funding_failed) = shutdown_res.splice_funding_failed.take() { + let (funding_info, contribution) = splice_funding_failed.into_parts(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: shutdown_res.channel_id, counterparty_node_id: shutdown_res.counterparty_node_id, user_channel_id: shutdown_res.user_channel_id, - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: shutdown_res.channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: shutdown_res.channel_id, + funding_info, }, - }, - None, - )); + None, + )); + } } if let Some(transaction) = shutdown_res.unbroadcasted_funding_tx { @@ -4975,28 +4973,27 @@ impl< }); if let Some(splice_funding_failed) = splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: *channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context.get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::LocallyAbandoned, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: *channel_id, + funding_info, }, - }, - None, - )); + None, + )); + } } Ok(()) @@ -6676,36 +6673,22 @@ impl< )); } }, - QuiescentError::FailSplice( - SpliceFundingFailed { - funding_txo, - channel_type, - contributed_inputs, - contributed_outputs, - }, - reason, - ) => { + QuiescentError::FailSplice(splice_funding_failed, reason) => { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id, counterparty_node_id, user_channel_id, - abandoned_funding_txo: funding_txo, - channel_type, reason, + contribution, }, None, )); - if !contributed_inputs.is_empty() || !contributed_outputs.is_empty() { + if let Some(funding_info) = funding_info { pending_events.push_back(( - events::Event::DiscardFunding { - channel_id, - funding_info: FundingInfo::Contribution { - inputs: contributed_inputs, - outputs: contributed_outputs, - }, - }, + events::Event::DiscardFunding { channel_id, funding_info }, None, )); } @@ -11982,30 +11965,24 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ user_channel_id: u128, ) -> MsgHandleErrInternal { if let Some(splice_funding_failed) = err.splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id, - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type.clone(), + contribution, reason: events::NegotiationFailureReason::NegotiationError { msg: format!("{:?}", err.err), }, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, - }, - }, - None, - )); + if let Some(funding_info) = funding_info { + pending_events + .push_back((events::Event::DiscardFunding { channel_id, funding_info }, None)); + } } MsgHandleErrInternal::from_chan_no_close(err.err, channel_id) } @@ -12321,14 +12298,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } if let Some(splice_funding_failed) = splice_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: msg.channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan_entry.get().context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::CounterpartyAborted { msg: UntrustedString( String::from_utf8_lossy(&msg.data).to_string(), @@ -12337,16 +12314,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: msg.channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: msg.channel_id, + funding_info, }, - }, - None, - )); + None, + )); + } } let holding_cell_res = if needs_holding_cell_release { @@ -12474,28 +12450,27 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ dropped_htlcs = htlcs; if let Some(splice_funding_failed) = splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: msg.channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: msg.channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: msg.channel_id, + funding_info, }, - }, - None, - )); + None, + )); + } } if let Some(msg) = shutdown { @@ -15553,21 +15528,20 @@ impl< chan.peer_disconnected_is_resumable(&&logger); if let Some(splice_funding_failed) = splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); splice_failed_events.push(events::Event::SpliceFailed { channel_id: chan.context().channel_id(), counterparty_node_id, user_channel_id: chan.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::PeerDisconnected, }); - splice_failed_events.push(events::Event::DiscardFunding { - channel_id: chan.context().channel_id(), - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, - }, - }); + if let Some(funding_info) = funding_info { + splice_failed_events.push(events::Event::DiscardFunding { + channel_id: chan.context().channel_id(), + funding_info, + }); + } } if is_resumable { @@ -18181,27 +18155,26 @@ impl< for peer_state in peer_states.iter() { for chan in peer_state.channel_by_id.values().filter_map(Channel::as_funded) { if let Some(splice_funding_failed) = chan.maybe_splice_funding_failed() { + let (funding_info, contribution) = splice_funding_failed.into_parts(); events.push_back(( events::Event::SpliceFailed { channel_id: chan.context.channel_id(), counterparty_node_id: chan.context.get_counterparty_node_id(), user_channel_id: chan.context.get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, reason: events::NegotiationFailureReason::PeerDisconnected, + contribution, }, None, )); - events.push_back(( - events::Event::DiscardFunding { - channel_id: chan.context().channel_id(), - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + events.push_back(( + events::Event::DiscardFunding { + channel_id: chan.context().channel_id(), + funding_info, }, - }, - None, - )); + None, + )); + } } } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index b8ef5890899..df161715152 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3237,9 +3237,10 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match &events[0] { - Event::SpliceFailed { channel_id, reason, .. } => { + Event::SpliceFailed { channel_id, reason, contribution, .. } => { assert_eq!(*expected_channel_id, *channel_id); assert_eq!(expected_reason, *reason); + assert_eq!(contribution.as_ref(), Some(&funding_contribution)); }, _ => panic!("Unexpected event"), } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 34b51e2696c..63d0b32f1fd 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -205,13 +205,12 @@ pub fn do_initiate_splice_in<'a, 'b, 'c, 'd>( pub fn do_initiate_rbf_splice_in<'a, 'b, 'c, 'd>( node: &'a Node<'b, 'c, 'd>, counterparty: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - value_added: Amount, feerate: FeeRate, + feerate: FeeRate, ) -> FundingContribution { let node_id_counterparty = counterparty.node.get_our_node_id(); let funding_template = node.node.splice_channel(&channel_id, &node_id_counterparty).unwrap(); - let wallet = WalletSync::new(Arc::clone(&node.wallet_source), node.logger); let funding_contribution = - funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap(); + funding_template.with_prior_contribution(feerate, FeeRate::MAX).build().unwrap(); node.node .funding_contributed(&channel_id, &node_id_counterparty, funding_contribution.clone(), None) .unwrap(); @@ -220,15 +219,12 @@ pub fn do_initiate_rbf_splice_in<'a, 'b, 'c, 'd>( pub fn do_initiate_rbf_splice_in_and_out<'a, 'b, 'c, 'd>( node: &'a Node<'b, 'c, 'd>, counterparty: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - value_added: Amount, outputs: Vec, feerate: FeeRate, + outputs: Vec, feerate: FeeRate, ) -> FundingContribution { let node_id_counterparty = counterparty.node.get_our_node_id(); let funding_template = node.node.splice_channel(&channel_id, &node_id_counterparty).unwrap(); - let wallet = WalletSync::new(Arc::clone(&node.wallet_source), node.logger); let funding_contribution = funding_template - .without_prior_contribution(feerate, FeeRate::MAX) - .with_coin_selection_source_sync(&wallet) - .add_value(value_added) + .with_prior_contribution(feerate, FeeRate::MAX) .add_outputs(outputs) .build() .unwrap(); @@ -238,6 +234,22 @@ pub fn do_initiate_rbf_splice_in_and_out<'a, 'b, 'c, 'd>( funding_contribution } +pub fn do_initiate_splice_in_at_feerate<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, + value_added: Amount, feerate: FeeRate, +) -> FundingContribution { + let node_id_acceptor = acceptor.node.get_our_node_id(); + let funding_template = initiator.node.splice_channel(&channel_id, &node_id_acceptor).unwrap(); + let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); + let funding_contribution = + funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap(); + initiator + .node + .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) + .unwrap(); + funding_contribution +} + pub fn initiate_splice_out<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, outputs: Vec, @@ -3207,9 +3219,10 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, .. } => { + Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::ChannelClosing); + assert!(contribution.is_some()); }, other => panic!("Expected SpliceFailed, got {:?}", other), } @@ -4601,9 +4614,10 @@ fn test_splice_acceptor_disconnect_emits_events() { let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, .. } => { + Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + assert!(contribution.is_some()); }, other => panic!("Expected SpliceFailed, got {:?}", other), } @@ -4660,7 +4674,7 @@ fn test_splice_rbf_acceptor_basic() { let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); // Steps 4-8: STFU exchange → tx_init_rbf → tx_ack_rbf. complete_rbf_handshake(&nodes[0], &nodes[1]); @@ -4724,8 +4738,7 @@ fn test_splice_rbf_at_high_feerate() { // Step 2: RBF to a high feerate (1000 sat/kwu, well above the 600 crossover point). provide_utxo_reserves(&nodes, 2, added_value * 2); let high_feerate = FeeRate::from_sat_per_kwu(1000); - let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, high_feerate); + let contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, high_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], @@ -4750,8 +4763,7 @@ fn test_splice_rbf_at_high_feerate() { let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); funding_template.min_rbf_feerate().unwrap() }; - let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + let contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], @@ -4816,7 +4828,7 @@ fn test_splice_rbf_insufficient_feerate() { // Node 0 initiates a proper RBF but we tamper the feerate to be insufficient. provide_utxo_reserves(&nodes, 2, added_value * 2); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -4842,17 +4854,14 @@ fn test_splice_rbf_insufficient_feerate() { // Node 0 echoes tx_abort and exits quiescence, freeing the holding cell. nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); - // TODO: the RBF round's inputs are partially filtered against the prior round's committed - // UTXOs, so the DiscardFunding carries coin-selection-dependent residue. Revisit once - // #4514 lands to see if its semantics change what DiscardFunding contains here. + // The RBF round contributed the same inputs and outputs as the prior round, so after + // filtering against the prior round's committed UTXOs nothing remains to discard and + // `DiscardFunding` is suppressed; only `SpliceFailed` is emitted. let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2, "{events:?}"); + assert_eq!(events.len(), 1, "{events:?}"); assert!( matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) ); - assert!( - matches!(&events[1], Event::DiscardFunding { channel_id: cid, .. } if *cid == channel_id) - ); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 2, "{msg_events:?}"); @@ -4885,7 +4894,7 @@ fn test_splice_rbf_insufficient_feerate() { // Node 0 initiates another proper RBF but we tamper the feerate to the 25/24 value. provide_utxo_reserves(&nodes, 2, added_value * 2); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -4903,22 +4912,19 @@ fn test_splice_rbf_insufficient_feerate() { nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); let tx_abort_echo = get_event_msg!(nodes[0], MessageSendEvent::SendTxAbort, node_id_1); - // TODO: same as above — revisit once #4514 lands. + // As above: nothing remains after filtering, so `DiscardFunding` is suppressed. let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2); + assert_eq!(events.len(), 1); assert!( matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) ); - assert!( - matches!(&events[1], Event::DiscardFunding { channel_id: cid, .. } if *cid == channel_id) - ); nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); // Acceptor-side: prev + 25 = 278 satisfies the combined BIP125 rule and is accepted. provide_utxo_reserves(&nodes, 2, added_value * 2); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -4958,8 +4964,7 @@ fn test_splice_rbf_insufficient_feerate_high() { provide_utxo_reserves(&nodes, 2, added_value * 2); let high_feerate = FeeRate::from_sat_per_kwu(1000); - let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, high_feerate); + let contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, high_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], @@ -4980,7 +4985,7 @@ fn test_splice_rbf_insufficient_feerate_high() { provide_utxo_reserves(&nodes, 2, added_value * 2); let min_rbf_feerate = FeeRate::from_sat_per_kwu(1041); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -4997,33 +5002,20 @@ fn test_splice_rbf_insufficient_feerate_high() { nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); let tx_abort_echo = get_event_msg!(nodes[0], MessageSendEvent::SendTxAbort, node_id_1); - // TODO: the RBF round's inputs are fully filtered against the prior round's committed - // UTXOs, so this DiscardFunding is emitted with empty inputs and outputs. Once #4514 - // lands, a fully-drained DiscardFunding should be suppressed entirely — expect - // `events.len() == 1`. + // The RBF round's inputs and outputs are fully filtered against the prior round's + // committed UTXOs, so `DiscardFunding` is suppressed. let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2); + assert_eq!(events.len(), 1); assert!( matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) ); - match &events[1] { - Event::DiscardFunding { - channel_id: cid, - funding_info: FundingInfo::Contribution { inputs, outputs }, - } => { - assert_eq!(*cid, channel_id); - assert!(inputs.is_empty(), "Expected inputs filtered, got {inputs:?}"); - assert!(outputs.is_empty(), "Expected outputs filtered, got {outputs:?}"); - }, - other => panic!("Expected DiscardFunding with Contribution, got {other:?}"), - } nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); // Feerate 1041 satisfies both rules — accepted. provide_utxo_reserves(&nodes, 2, added_value * 2); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -5315,7 +5307,7 @@ fn test_splice_rbf_not_quiescence_initiator() { let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); // STFU exchange: node 0 initiates quiescence. let stfu_init = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); @@ -5425,10 +5417,10 @@ pub fn do_test_splice_rbf_tiebreak( // Node 0 calls splice_channel + funding_contributed. let node_0_funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate_0); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate_0); // Node 1 calls splice_channel + funding_contributed. - let node_1_funding_contribution = do_initiate_rbf_splice_in( + let node_1_funding_contribution = do_initiate_splice_in_at_feerate( &nodes[1], &nodes[0], channel_id, @@ -5838,7 +5830,7 @@ fn test_splice_rbf_acceptor_recontributes() { let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let rbf_funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); // Steps 6-9: STFU exchange → tx_init_rbf → tx_ack_rbf. // Node 1 should re-contribute via our_prior_contribution. @@ -5967,7 +5959,7 @@ fn test_splice_rbf_after_counterparty_rbf_aborted() { let rbf_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); let _rbf_funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); let tx_ack_rbf = complete_rbf_handshake(&nodes[0], &nodes[1]); assert!(tx_ack_rbf.funding_output_contribution.is_some()); @@ -6163,7 +6155,7 @@ fn test_splice_rbf_sequential() { let rbf_feerate_1 = FeeRate::from_sat_per_kwu(feerate_1_sat_per_kwu); let funding_contribution_1 = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate_1); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate_1); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( @@ -6184,7 +6176,7 @@ fn test_splice_rbf_sequential() { let rbf_feerate_2 = FeeRate::from_sat_per_kwu(feerate_2_sat_per_kwu); let funding_contribution_2 = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate_2); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate_2); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( @@ -6511,7 +6503,7 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let _rbf_funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); let tx_ack_rbf = complete_rbf_handshake(&nodes[0], &nodes[1]); assert!( @@ -6523,24 +6515,22 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { nodes[0].node.peer_disconnected(node_id_1); nodes[1].node.peer_disconnected(node_id_0); - // The initiator should get SpliceFailed + DiscardFunding. + // The initiator re-used the same UTXOs as round 0. Since those UTXOs are still committed + // to round 0's splice, they are filtered and no DiscardFunding is emitted. let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2, "{events:?}"); + assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, .. } => { + Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + assert!(contribution.is_some()); }, other => panic!("Expected SpliceFailed, got {:?}", other), } - match &events[1] { - Event::DiscardFunding { funding_info: FundingInfo::Contribution { .. }, .. } => {}, - other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), - } // The acceptor re-contributed the same UTXOs as round 0 (via prior contribution // adjustment). Since those UTXOs are still committed to round 0's splice, they are - // filtered from the DiscardFunding event. With all inputs/outputs filtered, no events + // filtered and no DiscardFunding is emitted. With all inputs/outputs filtered, no events // are emitted for the acceptor. let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 0, "{events:?}"); @@ -6593,7 +6583,6 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { &nodes[0], &nodes[1], channel_id, - added_value, vec![splice_out_output.clone()], rbf_feerate, ); @@ -6609,9 +6598,10 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, .. } => { + Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + assert!(contribution.is_some()); }, other => panic!("Expected SpliceFailed, got {:?}", other), } @@ -6641,7 +6631,7 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { let rbf_feerate_2 = FeeRate::from_sat_per_kwu(feerate_1_sat_per_kwu); let _funding_contribution_2 = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate_2); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate_2); complete_rbf_handshake(&nodes[0], &nodes[1]); // Disconnect again to clean up the in-progress interactive TX negotiation. @@ -6649,18 +6639,15 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { nodes[1].node.peer_disconnected(node_id_0); let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2, "{events:?}"); + assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, .. } => { + Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + assert!(contribution.is_some()); }, other => panic!("Expected SpliceFailed, got {:?}", other), } - match &events[1] { - Event::DiscardFunding { .. } => {}, - other => panic!("Expected DiscardFunding, got {:?}", other), - } let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_announcement_sigs = (true, true); @@ -7326,8 +7313,7 @@ fn test_splice_rbf_rejects_low_feerate_after_several_attempts() { let feerate = prev_feerate + 25; provide_utxo_reserves(&nodes, 2, added_value * 2); let rbf_feerate = FeeRate::from_sat_per_kwu(feerate); - let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + let contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], @@ -7353,8 +7339,7 @@ fn test_splice_rbf_rejects_low_feerate_after_several_attempts() { let next_feerate = prev_feerate + 25; provide_utxo_reserves(&nodes, 2, added_value * 2); let rbf_feerate = FeeRate::from_sat_per_kwu(next_feerate); - let _contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + let _contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); let stfu_1 = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); @@ -7403,8 +7388,7 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let feerate = prev_feerate + 25; provide_utxo_reserves(&nodes, 2, added_value * 2); let rbf_feerate = FeeRate::from_sat_per_kwu(feerate); - let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + let contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], @@ -7446,9 +7430,10 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, .. } => { + Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::FeeRateTooLow); + assert!(contribution.is_some()); }, other => panic!("Expected SpliceFailed, got {:?}", other), } From 14c6819990fefbda725f548466958f1a9c611256 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 18:44:24 -0500 Subject: [PATCH 03/11] 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) --- lightning/src/ln/channelmanager.rs | 156 ++++++++++++---------- lightning/src/ln/functional_test_utils.rs | 18 +-- lightning/src/ln/splicing_tests.rs | 50 +++---- 3 files changed, 120 insertions(+), 104 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2d6aaa56c5f..6a3be0c8673 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4163,6 +4163,15 @@ impl< if let Some(splice_funding_failed) = splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let mut pending_events = self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: *chan_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: *chan_id, @@ -4173,15 +4182,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *chan_id, - funding_info, - }, - None, - )); - } } // We can send the `shutdown` message before updating the `ChannelMonitor` @@ -4469,6 +4469,15 @@ impl< if let Some(splice_funding_failed) = shutdown_res.splice_funding_failed.take() { let (funding_info, contribution) = splice_funding_failed.into_parts(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: shutdown_res.channel_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: shutdown_res.channel_id, @@ -4479,15 +4488,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: shutdown_res.channel_id, - funding_info, - }, - None, - )); - } } if let Some(transaction) = shutdown_res.unbroadcasted_funding_tx { @@ -4975,6 +4975,15 @@ impl< if let Some(splice_funding_failed) = splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: *channel_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: *channel_id, @@ -4985,15 +4994,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *channel_id, - funding_info, - }, - None, - )); - } } Ok(()) @@ -6676,6 +6676,12 @@ impl< QuiescentError::FailSplice(splice_funding_failed, reason) => { let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { channel_id, funding_info }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id, @@ -6686,12 +6692,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { channel_id, funding_info }, - None, - )); - } }, } } @@ -11967,6 +11967,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(splice_funding_failed) = err.splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events + .push_back((events::Event::DiscardFunding { channel_id, funding_info }, None)); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id, @@ -11979,10 +11983,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); - if let Some(funding_info) = funding_info { - pending_events - .push_back((events::Event::DiscardFunding { channel_id, funding_info }, None)); - } } MsgHandleErrInternal::from_chan_no_close(err.err, channel_id) } @@ -12300,6 +12300,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(splice_funding_failed) = splice_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: msg.channel_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: msg.channel_id, @@ -12314,15 +12323,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: msg.channel_id, - funding_info, - }, - None, - )); - } } let holding_cell_res = if needs_holding_cell_release { @@ -12452,6 +12452,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(splice_funding_failed) = splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let mut pending_events = self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: msg.channel_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: msg.channel_id, @@ -12462,15 +12471,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: msg.channel_id, - funding_info, - }, - None, - )); - } } if let Some(msg) = shutdown { @@ -15253,6 +15253,22 @@ impl< self.process_pending_events(&event_handler); let collected_events = events.into_inner(); + // When both DiscardFunding and SpliceFailed are emitted for the same channel, + // DiscardFunding must come first so that inputs are unlocked before any retry. + // Each pair is emitted adjacently under a single lock, so checking adjacent + // events is sufficient. + for window in collected_events.windows(2) { + if let events::Event::SpliceFailed { channel_id, .. } = &window[0] { + if let events::Event::DiscardFunding { channel_id: cid, .. } = &window[1] { + assert!( + channel_id != cid, + "DiscardFunding must precede SpliceFailed for channel {}", + channel_id, + ); + } + } + } + // To expand the coverage and make sure all events are properly serialised and deserialised, // we test all generated events round-trip: for event in &collected_events { @@ -15529,6 +15545,12 @@ impl< if let Some(splice_funding_failed) = splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); + if let Some(funding_info) = funding_info { + splice_failed_events.push(events::Event::DiscardFunding { + channel_id: chan.context().channel_id(), + funding_info, + }); + } splice_failed_events.push(events::Event::SpliceFailed { channel_id: chan.context().channel_id(), counterparty_node_id, @@ -15536,12 +15558,6 @@ impl< contribution, reason: events::NegotiationFailureReason::PeerDisconnected, }); - if let Some(funding_info) = funding_info { - splice_failed_events.push(events::Event::DiscardFunding { - channel_id: chan.context().channel_id(), - funding_info, - }); - } } if is_resumable { @@ -18156,6 +18172,15 @@ impl< for chan in peer_state.channel_by_id.values().filter_map(Channel::as_funded) { if let Some(splice_funding_failed) = chan.maybe_splice_funding_failed() { let (funding_info, contribution) = splice_funding_failed.into_parts(); + if let Some(funding_info) = funding_info { + events.push_back(( + events::Event::DiscardFunding { + channel_id: chan.context().channel_id(), + funding_info, + }, + None, + )); + } events.push_back(( events::Event::SpliceFailed { channel_id: chan.context.channel_id(), @@ -18166,15 +18191,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - events.push_back(( - events::Event::DiscardFunding { - channel_id: chan.context().channel_id(), - funding_info, - }, - None, - )); - } } } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index df161715152..c5b1104cc64 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3237,18 +3237,10 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match &events[0] { - Event::SpliceFailed { channel_id, reason, contribution, .. } => { - assert_eq!(*expected_channel_id, *channel_id); - assert_eq!(expected_reason, *reason); - assert_eq!(contribution.as_ref(), Some(&funding_contribution)); - }, - _ => panic!("Unexpected event"), - } - match &events[1] { Event::DiscardFunding { funding_info, .. } => { if let FundingInfo::Contribution { inputs, outputs } = &funding_info { let (expected_inputs, expected_outputs) = - funding_contribution.into_contributed_inputs_and_outputs(); + funding_contribution.clone().into_contributed_inputs_and_outputs(); assert_eq!(*inputs, expected_inputs); assert_eq!(*outputs, expected_outputs); } else { @@ -3257,6 +3249,14 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( }, _ => panic!("Unexpected event"), } + match &events[1] { + Event::SpliceFailed { channel_id, reason, contribution, .. } => { + assert_eq!(*expected_channel_id, *channel_id); + assert_eq!(expected_reason, *reason); + assert_eq!(contribution.as_ref(), Some(&funding_contribution)); + }, + _ => panic!("Unexpected event"), + } } #[cfg(any(test, ldk_bench, feature = "_test_utils"))] diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 63d0b32f1fd..1c6ad835ce0 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3219,14 +3219,6 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { - assert_eq!(*cid, channel_id); - assert_eq!(*reason, NegotiationFailureReason::ChannelClosing); - assert!(contribution.is_some()); - }, - other => panic!("Expected SpliceFailed, got {:?}", other), - } - match &events[1] { Event::DiscardFunding { funding_info: FundingInfo::Contribution { inputs, outputs }, .. @@ -3240,6 +3232,14 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ }, other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), } + match &events[1] { + Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::ChannelClosing); + assert!(contribution.is_some()); + }, + other => panic!("Expected SpliceFailed, got {:?}", other), + } } else { expect_splice_failed_events( &nodes[0], @@ -4614,14 +4614,6 @@ fn test_splice_acceptor_disconnect_emits_events() { let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { - assert_eq!(*cid, channel_id); - assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); - assert!(contribution.is_some()); - }, - other => panic!("Expected SpliceFailed, got {:?}", other), - } - match &events[1] { Event::DiscardFunding { funding_info: FundingInfo::Contribution { inputs, outputs }, .. @@ -4631,6 +4623,14 @@ fn test_splice_acceptor_disconnect_emits_events() { }, other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), } + match &events[1] { + Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + assert!(contribution.is_some()); + }, + other => panic!("Expected SpliceFailed, got {:?}", other), + } // Reconnect and verify the channel is still operational. let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); @@ -6594,18 +6594,10 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { nodes[0].node.peer_disconnected(node_id_1); nodes[1].node.peer_disconnected(node_id_0); - // The initiator should get SpliceFailed + DiscardFunding with filtered contributions. + // The initiator should get DiscardFunding + SpliceFailed with filtered contributions. let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { - assert_eq!(*cid, channel_id); - assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); - assert!(contribution.is_some()); - }, - other => panic!("Expected SpliceFailed, got {:?}", other), - } - match &events[1] { Event::DiscardFunding { funding_info: FundingInfo::Contribution { inputs, outputs }, .. @@ -6618,6 +6610,14 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { }, other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), } + match &events[1] { + Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + assert!(contribution.is_some()); + }, + other => panic!("Expected SpliceFailed, got {:?}", other), + } // Reconnect. After a completed splice, channel_ready is not re-sent. let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); From cc7fb0f5104d88240605a7f8b25fc58090bd1469 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 18:58:50 -0500 Subject: [PATCH 04/11] Rename SplicePending and SpliceFailed events Rename Event::SplicePending to Event::SpliceNegotiated and Event::SpliceFailed to Event::SpliceNegotiationFailed. These names better reflect the per-round semantics: each negotiation attempt resolves to one of these two outcomes, independent of the overall splice lifecycle. Co-Authored-By: Claude Opus 4.6 (1M context) --- fuzz/src/chanmon_consistency.rs | 4 +- fuzz/src/full_stack.rs | 4 +- lightning/src/events/mod.rs | 16 +++--- lightning/src/ln/async_signer_tests.rs | 8 +-- lightning/src/ln/channel.rs | 10 ++-- lightning/src/ln/channelmanager.rs | 53 +++++++++--------- lightning/src/ln/functional_test_utils.rs | 6 +-- lightning/src/ln/funding.rs | 4 +- lightning/src/ln/splicing_tests.rs | 54 +++++++++---------- .../4388-splice-failed-discard-funding.txt | 8 +-- 10 files changed, 84 insertions(+), 83 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index d678d97918f..678e6a6fc61 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -2091,7 +2091,7 @@ pub fn do_test(data: &[u8], out: Out) { ) .unwrap(); }, - events::Event::SplicePending { new_funding_txo, .. } => { + events::Event::SpliceNegotiated { new_funding_txo, .. } => { let broadcaster = match $node { 0 => &broadcast_a, 1 => &broadcast_b, @@ -2103,7 +2103,7 @@ pub fn do_test(data: &[u8], out: Out) { assert_eq!(new_funding_txo.txid, splice_tx.compute_txid()); chain_state.add_pending_tx(splice_tx); }, - events::Event::SpliceFailed { .. } => {}, + events::Event::SpliceNegotiationFailed { .. } => {}, events::Event::DiscardFunding { funding_info: events::FundingInfo::Contribution { .. } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 405d615e6f0..e79bef7c5ec 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1137,10 +1137,10 @@ pub fn do_test(mut data: &[u8], logger: &Arc signed_tx, ); }, - Event::SplicePending { .. } => { + Event::SpliceNegotiated { .. } => { // Splice negotiation completed, waiting for confirmation }, - Event::SpliceFailed { .. } => { + Event::SpliceNegotiationFailed { .. } => { // Splice failed, inputs can be re-spent }, Event::OpenChannelRequest { diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 5a52be026fb..9d00273cb46 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1646,8 +1646,8 @@ pub enum Event { /// # Failure Behavior and Persistence /// This event will eventually be replayed after failures-to-handle (i.e., the event handler /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. - SplicePending { - /// The `channel_id` of the channel that has a pending splice funding transaction. + SpliceNegotiated { + /// The `channel_id` of the channel with the negotiated splice funding transaction. channel_id: ChannelId, /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound /// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels. @@ -1667,7 +1667,7 @@ pub enum Event { }, /// Used to indicate that a splice negotiation round for the given `channel_id` has failed. /// - /// Each splice attempt (initial or RBF) resolves to either [`Event::SplicePending`] on + /// Each splice attempt (initial or RBF) resolves to either [`Event::SpliceNegotiated`] on /// success or this event on failure. Prior successfully negotiated splice transactions are /// unaffected. /// @@ -1677,7 +1677,7 @@ pub enum Event { /// # Failure Behavior and Persistence /// This event will eventually be replayed after failures-to-handle (i.e., the event handler /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. - SpliceFailed { + SpliceNegotiationFailed { /// The `channel_id` of the channel for which the splice negotiation round failed. channel_id: ChannelId, /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound @@ -2468,7 +2468,7 @@ impl Writeable for Event { // We never write out FundingTransactionReadyForSigning events as they will be regenerated when // necessary. }, - &Event::SplicePending { + &Event::SpliceNegotiated { ref channel_id, ref user_channel_id, ref counterparty_node_id, @@ -2486,7 +2486,7 @@ impl Writeable for Event { (11, new_funding_redeem_script, required), }); }, - &Event::SpliceFailed { + &Event::SpliceNegotiationFailed { ref channel_id, ref user_channel_id, ref counterparty_node_id, @@ -3125,7 +3125,7 @@ impl MaybeReadable for Event { (11, new_funding_redeem_script, required), }); - Ok(Some(Event::SplicePending { + Ok(Some(Event::SpliceNegotiated { channel_id: channel_id.0.unwrap(), user_channel_id: user_channel_id.0.unwrap(), counterparty_node_id: counterparty_node_id.0.unwrap(), @@ -3146,7 +3146,7 @@ impl MaybeReadable for Event { (13, contribution, option), }); - Ok(Some(Event::SpliceFailed { + Ok(Some(Event::SpliceNegotiationFailed { channel_id: channel_id.0.unwrap(), user_channel_id: user_channel_id.0.unwrap(), counterparty_node_id: counterparty_node_id.0.unwrap(), diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index f238c1db060..ae73dd830ac 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -1647,8 +1647,8 @@ fn test_async_splice_initial_commit_sig() { get_event_msg!(initiator, MessageSendEvent::SendTxSignatures, acceptor_node_id); acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); - let _ = get_event!(initiator, Event::SplicePending); - let _ = get_event!(acceptor, Event::SplicePending); + let _ = get_event!(initiator, Event::SpliceNegotiated); + let _ = get_event!(acceptor, Event::SpliceNegotiated); } #[test] @@ -1739,6 +1739,6 @@ fn test_async_splice_initial_commit_sig_waits_for_monitor_before_tx_signatures() get_event_msg!(initiator, MessageSendEvent::SendTxSignatures, acceptor_node_id); acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); - let _ = get_event!(initiator, Event::SplicePending); - let _ = get_event!(acceptor, Event::SplicePending); + let _ = get_event!(initiator, Event::SpliceNegotiated); + let _ = get_event!(acceptor, Event::SpliceNegotiated); } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 473fc6b46f3..b26ec70c5b8 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1185,7 +1185,7 @@ pub(super) struct InteractiveTxMsgError { /// The underlying error. pub(super) err: ChannelError, /// If a splice was in progress when processing the message, this contains the splice funding - /// information for emitting a `SpliceFailed` event. + /// information for emitting a `SpliceNegotiationFailed` event. pub(super) splice_funding_failed: Option, } @@ -1270,7 +1270,7 @@ pub(crate) struct ShutdownResult { pub(crate) channel_funding_txo: Option, pub(crate) last_local_balance_msat: u64, /// If a splice was in progress when the channel was shut down, this contains - /// the splice funding information for emitting a SpliceFailed event. + /// the splice funding information for emitting a SpliceNegotiationFailed event. pub(crate) splice_funding_failed: Option, } @@ -1278,7 +1278,7 @@ pub(crate) struct ShutdownResult { pub(crate) struct DisconnectResult { pub(crate) is_resumable: bool, /// If a splice was in progress when the channel was shut down, this contains - /// the splice funding information for emitting a SpliceFailed event. + /// the splice funding information for emitting a SpliceNegotiationFailed event. pub(crate) splice_funding_failed: Option, } @@ -7065,7 +7065,7 @@ pub struct SpliceFundingFailed { impl SpliceFundingFailed { /// Splits into the funding info for `DiscardFunding` (if there are inputs or outputs to - /// discard) and the contribution for `SpliceFailed`. + /// discard) and the contribution for `SpliceNegotiationFailed`. pub(super) fn into_parts(self) -> (Option, Option) { let funding_info = if !self.contributed_inputs.is_empty() || !self.contributed_outputs.is_empty() { @@ -12436,7 +12436,7 @@ where // // If the in-progress negotiation later fails (e.g., tx_abort), the derived // min_rbf_feerate becomes stale, causing a slightly higher feerate than - // necessary. Call splice_channel again after receiving SpliceFailed to get a + // necessary. Call splice_channel again after receiving SpliceNegotiationFailed to get a // fresh template without the stale RBF constraint. let prev_feerate = pending_splice.last_funding_feerate_sat_per_1000_weight.or_else(|| { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6a3be0c8673..7a3a5bd1ee4 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4173,7 +4173,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: *chan_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), @@ -4479,7 +4479,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: shutdown_res.channel_id, counterparty_node_id: shutdown_res.counterparty_node_id, user_channel_id: shutdown_res.user_channel_id, @@ -4985,7 +4985,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: *channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context.get_user_id(), @@ -6683,7 +6683,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id, counterparty_node_id, user_channel_id, @@ -6741,14 +6741,14 @@ impl< /// # Events /// /// Calling this method will commence the process of creating a new funding transaction for the - /// channel. Once the funding transaction has been constructed, an [`Event::SplicePending`] + /// channel. Once the funding transaction has been constructed, an [`Event::SpliceNegotiated`] /// will be emitted. At this point, any inputs contributed to the splice can only be re-spent /// if an [`Event::DiscardFunding`] is seen. /// - /// If any failures occur while negotiating the funding transaction, an [`Event::SpliceFailed`] - /// will be emitted. Any contributed inputs no longer used will be included in an - /// [`Event::DiscardFunding`] and thus can be re-spent. If a [`FundingTemplate`] was obtained - /// while a previous splice was still being negotiated, its + /// If any failures occur while negotiating the funding transaction, an + /// [`Event::SpliceNegotiationFailed`] will be emitted. Any contributed inputs no longer used + /// will be included in an [`Event::DiscardFunding`] and thus can be re-spent. If a + /// [`FundingTemplate`] was obtained while a previous splice was still being negotiated, its /// [`min_rbf_feerate`][FundingTemplate::min_rbf_feerate] may be stale after the failure. /// Call [`ChannelManager::splice_channel`] again to get a fresh template. /// @@ -6967,7 +6967,7 @@ impl< } if let Some(splice_negotiated) = splice_negotiated { self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { + events::Event::SpliceNegotiated { channel_id: *channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), @@ -11131,7 +11131,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ .and_then(|v| v.splice_negotiated.take()) { pending_events.push_back(( - events::Event::SplicePending { + events::Event::SpliceNegotiated { channel_id: channel.context.channel_id(), counterparty_node_id, user_channel_id: channel.context.get_user_id(), @@ -11972,7 +11972,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ .push_back((events::Event::DiscardFunding { channel_id, funding_info }, None)); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id, @@ -12224,7 +12224,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let needs_holding_cell_release = splice_negotiated.is_some(); if let Some(splice_negotiated) = splice_negotiated { self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { + events::Event::SpliceNegotiated { channel_id: msg.channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context.get_user_id(), @@ -12310,7 +12310,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: msg.channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan_entry.get().context().get_user_id(), @@ -12462,7 +12462,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: msg.channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), @@ -15253,16 +15253,16 @@ impl< self.process_pending_events(&event_handler); let collected_events = events.into_inner(); - // When both DiscardFunding and SpliceFailed are emitted for the same channel, - // DiscardFunding must come first so that inputs are unlocked before any retry. - // Each pair is emitted adjacently under a single lock, so checking adjacent - // events is sufficient. + // When both DiscardFunding and SpliceNegotiationFailed are emitted for the same + // channel, DiscardFunding must come first so that inputs are unlocked before any + // retry. Each pair is emitted adjacently under a single lock, so checking + // adjacent events is sufficient. for window in collected_events.windows(2) { - if let events::Event::SpliceFailed { channel_id, .. } = &window[0] { + if let events::Event::SpliceNegotiationFailed { channel_id, .. } = &window[0] { if let events::Event::DiscardFunding { channel_id: cid, .. } = &window[1] { assert!( channel_id != cid, - "DiscardFunding must precede SpliceFailed for channel {}", + "DiscardFunding must precede SpliceNegotiationFailed for channel {}", channel_id, ); } @@ -15551,7 +15551,7 @@ impl< funding_info, }); } - splice_failed_events.push(events::Event::SpliceFailed { + splice_failed_events.push(events::Event::SpliceNegotiationFailed { channel_id: chan.context().channel_id(), counterparty_node_id, user_channel_id: chan.context().get_user_id(), @@ -18163,8 +18163,9 @@ impl< let our_pending_intercepts = self.pending_intercepted_htlcs.lock().unwrap(); // Since some FundingNegotiation variants are not persisted, any splice in such state must - // be failed upon reload. However, as the necessary information for the SpliceFailed and - // DiscardFunding events is not persisted, the events need to be persisted even though they + // be failed upon reload. However, as the necessary information for the + // SpliceNegotiationFailed and DiscardFunding events is not persisted, the events need to + // be persisted even though they // haven't been emitted yet. These are removed after the events are written. let mut events = self.pending_events.lock().unwrap(); let event_count = events.len(); @@ -18182,7 +18183,7 @@ impl< )); } events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: chan.context.channel_id(), counterparty_node_id: chan.context.get_counterparty_node_id(), user_channel_id: chan.context.get_user_id(), @@ -18311,7 +18312,7 @@ impl< (23, self.best_block.read().unwrap().previous_blocks, required), }); - // Remove the SpliceFailed and DiscardFunding events added earlier. + // Remove the SpliceNegotiationFailed and DiscardFunding events added earlier. events.truncate(event_count); Ok(()) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index c5b1104cc64..f89fdd0572b 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2378,7 +2378,7 @@ pub fn check_closed_events(node: &Node, expected_close_events: &[ExpectedCloseEv discard_events_count ); assert_eq!( - events.iter().filter(|e| matches!(e, Event::SpliceFailed { .. },)).count(), + events.iter().filter(|e| matches!(e, Event::SpliceNegotiationFailed { .. },)).count(), splice_events_count ); } @@ -3221,7 +3221,7 @@ pub fn expect_splice_pending_event<'a, 'b, 'c, 'd>( let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match &events[0] { - crate::events::Event::SplicePending { channel_id, counterparty_node_id, .. } => { + crate::events::Event::SpliceNegotiated { channel_id, counterparty_node_id, .. } => { assert_eq!(*expected_counterparty_node_id, *counterparty_node_id); *channel_id }, @@ -3250,7 +3250,7 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( _ => panic!("Unexpected event"), } match &events[1] { - Event::SpliceFailed { channel_id, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id, reason, contribution, .. } => { assert_eq!(*expected_channel_id, *channel_id); assert_eq!(expected_reason, *reason); assert_eq!(contribution.as_ref(), Some(&funding_contribution)); diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 2867a03add7..e954149fd6c 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -121,10 +121,10 @@ pub enum FundingContributionError { /// /// Note: [`FundingTemplate::min_rbf_feerate`] may be derived from an in-progress /// negotiation that later aborts, leaving a stale (higher than necessary) minimum. If - /// this error occurs after receiving [`Event::SpliceFailed`], call + /// this error occurs after receiving [`Event::SpliceNegotiationFailed`], call /// [`ChannelManager::splice_channel`] again to get a fresh template. /// - /// [`Event::SpliceFailed`]: crate::events::Event::SpliceFailed + /// [`Event::SpliceNegotiationFailed`]: crate::events::Event::SpliceNegotiationFailed /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel FeeRateBelowRbfMinimum { /// The requested feerate. diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 1c6ad835ce0..4135f2b5604 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3203,7 +3203,7 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ }; assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - // Close the channel. We should see a `SpliceFailed` event for the pending splice + // Close the channel. We should see a `SpliceNegotiationFailed` event for the pending splice // `QuiescentAction`. let (closer_node, closee_node) = if local_shutdown { (&nodes[0], &nodes[1]) } else { (&nodes[1], &nodes[0]) }; @@ -3233,12 +3233,12 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), } match &events[1] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::ChannelClosing); assert!(contribution.is_some()); }, - other => panic!("Expected SpliceFailed, got {:?}", other), + other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), } } else { expect_splice_failed_events( @@ -4041,7 +4041,7 @@ fn test_funding_contributed_active_funding_negotiation() { fn do_test_funding_contributed_active_funding_negotiation(state: u8) { // Tests that calling funding_contributed when a splice is already being actively negotiated // (pending_splice.funding_negotiation exists and is_initiator()) returns Err(APIMisuseError) - // and emits SpliceFailed + DiscardFunding events for non-duplicate contributions, or + // and emits SpliceNegotiationFailed + DiscardFunding events for non-duplicate contributions, or // returns Err(APIMisuseError) with no events for duplicate contributions. // // State 0: AwaitingAck (splice_init sent, splice_ack not yet received) @@ -4177,7 +4177,7 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { #[test] fn test_funding_contributed_channel_shutdown() { // Tests that calling funding_contributed after initiating channel shutdown returns Err(APIMisuseError) - // and emits both SpliceFailed and DiscardFunding events. The channel is no longer usable + // and emits both SpliceNegotiationFailed and DiscardFunding events. The channel is no longer usable // after shutdown is initiated, so quiescence cannot be proposed. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); @@ -4206,7 +4206,7 @@ fn test_funding_contributed_channel_shutdown() { // Now call funding_contributed - this should trigger FailSplice because // propose_quiescence() will fail when is_usable() returns false. - // Returns Err(APIMisuseError) and emits both SpliceFailed and DiscardFunding. + // Returns Err(APIMisuseError) and emits both SpliceNegotiationFailed and DiscardFunding. assert_eq!( nodes[0].node.funding_contributed( &channel_id, @@ -4561,7 +4561,7 @@ pub fn reenter_quiescence<'a, 'b, 'c>( #[test] fn test_splice_acceptor_disconnect_emits_events() { // When both nodes contribute to a splice and the negotiation fails due to disconnect, - // both the initiator and acceptor should receive SpliceFailed + DiscardFunding events + // both the initiator and acceptor should receive SpliceNegotiationFailed + DiscardFunding events // so each can reclaim their UTXOs. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); @@ -4600,7 +4600,7 @@ fn test_splice_acceptor_disconnect_emits_events() { nodes[0].node.peer_disconnected(node_id_1); nodes[1].node.peer_disconnected(node_id_0); - // The initiator should get SpliceFailed + DiscardFunding. + // The initiator should get SpliceNegotiationFailed + DiscardFunding. expect_splice_failed_events( &nodes[0], &channel_id, @@ -4608,7 +4608,7 @@ fn test_splice_acceptor_disconnect_emits_events() { NegotiationFailureReason::PeerDisconnected, ); - // The acceptor should also get SpliceFailed + DiscardFunding with its contributions + // The acceptor should also get SpliceNegotiationFailed + DiscardFunding with its contributions // so it can reclaim its UTXOs. The contribution is feerate-adjusted by handle_splice_init, // so we check for non-empty inputs/outputs rather than exact values. let events = nodes[1].node.get_and_clear_pending_events(); @@ -4624,12 +4624,12 @@ fn test_splice_acceptor_disconnect_emits_events() { other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), } match &events[1] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); assert!(contribution.is_some()); }, - other => panic!("Expected SpliceFailed, got {:?}", other), + other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), } // Reconnect and verify the channel is still operational. @@ -4856,11 +4856,11 @@ fn test_splice_rbf_insufficient_feerate() { // The RBF round contributed the same inputs and outputs as the prior round, so after // filtering against the prior round's committed UTXOs nothing remains to discard and - // `DiscardFunding` is suppressed; only `SpliceFailed` is emitted. + // `DiscardFunding` is suppressed; only `SpliceNegotiationFailed` is emitted. let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); assert!( - matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) + matches!(&events[0], Event::SpliceNegotiationFailed { channel_id: cid, .. } if *cid == channel_id) ); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); @@ -4916,7 +4916,7 @@ fn test_splice_rbf_insufficient_feerate() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); assert!( - matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) + matches!(&events[0], Event::SpliceNegotiationFailed { channel_id: cid, .. } if *cid == channel_id) ); nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); @@ -5007,7 +5007,7 @@ fn test_splice_rbf_insufficient_feerate_high() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); assert!( - matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) + matches!(&events[0], Event::SpliceNegotiationFailed { channel_id: cid, .. } if *cid == channel_id) ); nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); @@ -6439,7 +6439,7 @@ fn test_splice_rbf_amends_prior_net_negative_contribution_request() { fn test_splice_rbf_acceptor_contributes_then_disconnects() { // When both nodes contribute to a splice and the initiator RBFs (with the acceptor // re-contributing via prior contribution), disconnecting mid-interactive-TX should emit - // SpliceFailed + DiscardFunding for both nodes so each can reclaim their UTXOs. + // SpliceNegotiationFailed + DiscardFunding for both nodes so each can reclaim their UTXOs. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -6520,12 +6520,12 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); assert!(contribution.is_some()); }, - other => panic!("Expected SpliceFailed, got {:?}", other), + other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), } // The acceptor re-contributed the same UTXOs as round 0 (via prior contribution @@ -6594,7 +6594,7 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { nodes[0].node.peer_disconnected(node_id_1); nodes[1].node.peer_disconnected(node_id_0); - // The initiator should get DiscardFunding + SpliceFailed with filtered contributions. + // The initiator should get DiscardFunding + SpliceNegotiationFailed with filtered contributions. let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { @@ -6611,12 +6611,12 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), } match &events[1] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); assert!(contribution.is_some()); }, - other => panic!("Expected SpliceFailed, got {:?}", other), + other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), } // Reconnect. After a completed splice, channel_ready is not re-sent. @@ -6641,12 +6641,12 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); assert!(contribution.is_some()); }, - other => panic!("Expected SpliceFailed, got {:?}", other), + other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), } let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); @@ -7050,7 +7050,7 @@ fn test_rbf_sync_returns_err_when_max_feerate_below_min_rbf() { fn test_splice_revalidation_at_quiescence() { // When an outbound HTLC is committed between funding_contributed and quiescence, the // holder's balance decreases. If the splice-out was marginal at funding_contributed time, - // the re-validation at quiescence should fail and emit SpliceFailed + DiscardFunding. + // the re-validation at quiescence should fail and emit SpliceNegotiationFailed + DiscardFunding. // // Flow: // 1. Send payment #1 (update_add + CS) → node 0 awaits RAA @@ -7425,17 +7425,17 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let result = nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution, None); assert!(result.is_err(), "Expected rejection for low feerate: {:?}", result); - // SpliceFailed is emitted. DiscardFunding is not emitted because all inputs/outputs + // SpliceNegotiationFailed is emitted. DiscardFunding is not emitted because all inputs/outputs // are filtered out (same UTXOs reused for RBF, still committed to the prior splice tx). let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::FeeRateTooLow); assert!(contribution.is_some()); }, - other => panic!("Expected SpliceFailed, got {:?}", other), + other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), } } diff --git a/pending_changelog/4388-splice-failed-discard-funding.txt b/pending_changelog/4388-splice-failed-discard-funding.txt index 64fc4ab4e26..67680f49cb1 100644 --- a/pending_changelog/4388-splice-failed-discard-funding.txt +++ b/pending_changelog/4388-splice-failed-discard-funding.txt @@ -1,21 +1,21 @@ # API Updates - * `Event::SpliceFailed` no longer carries `contributed_inputs` or `contributed_outputs` fields. + * `Event::SpliceNegotiationFailed` no longer carries `contributed_inputs` or `contributed_outputs` fields. Instead, a separate `Event::DiscardFunding` event with `FundingInfo::Contribution` is emitted for UTXO cleanup. * `Event::DiscardFunding` with `FundingInfo::Contribution` is also emitted without a - corresponding `Event::SpliceFailed` when `ChannelManager::funding_contributed` returns an + corresponding `Event::SpliceNegotiationFailed` when `ChannelManager::funding_contributed` returns an error (e.g., channel or peer not found, wrong channel state, duplicate contribution). # Backwards Compatibility * Older serializations that included `contributed_inputs` and `contributed_outputs` in - `SpliceFailed` will have those fields silently ignored on deserialization (they were odd TLV + `SpliceNegotiationFailed` will have those fields silently ignored on deserialization (they were odd TLV fields). A `DiscardFunding` event will not be produced when reading these older serializations. # Forward Compatibility * Downgrading will not set the removed `contributed_inputs`/`contributed_outputs` fields on - `SpliceFailed`, so older code expecting those fields will see empty vectors for splice + `SpliceNegotiationFailed`, so older code expecting those fields will see empty vectors for splice failures. From 2eb939b10646532754a471f2585b6e92e4e12cf6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 16:05:31 -0500 Subject: [PATCH 05/11] Simplify contribution pop in reset_pending_splice_state The was_negotiated check is unnecessary because reset_pending_splice_state only runs when funding_negotiation is present, meaning on_tx_signatures_exchange hasn't been called yet. Since the feerate is only recorded in last_funding_feerate_sat_per_1000_weight during on_tx_signatures_exchange, the current round's feerate can never match it. So the contribution can always be unconditionally popped. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b26ec70c5b8..455a5ae80d2 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7317,20 +7317,20 @@ where into_contributed_inputs_and_outputs ); - // Pop the current round's contribution if it wasn't from a negotiated round. Each round - // pushes a new entry to `contributions`; if the round aborts, we undo the push so that - // `contributions.last()` reflects the most recent negotiated round's contribution. This - // must happen after `maybe_create_splice_funding_failed` so that - // `prior_contributed_inputs` still includes the prior rounds' entries for filtering. - if let Some(pending_splice) = self.pending_splice.as_mut() { - if let Some(last) = pending_splice.contributions.last() { - let was_negotiated = pending_splice + // Pop the current round's contribution, if any (acceptors may not have one). This + // must happen after `maybe_create_splice_funding_failed` for correct filtering. + let pending_splice = self + .pending_splice + .as_mut() + .expect("reset_pending_splice_state requires pending_splice"); + if let Some(contribution) = pending_splice.contributions.pop() { + debug_assert!( + pending_splice .last_funding_feerate_sat_per_1000_weight - .is_some_and(|f| last.feerate() == FeeRate::from_sat_per_kwu(f as u64)); - if !was_negotiated { - pending_splice.contributions.pop(); - } - } + .map(|f| contribution.feerate() > FeeRate::from_sat_per_kwu(f as u64)) + .unwrap_or(true), + "current round's feerate should be greater than the last negotiated feerate", + ); } if self.pending_funding().is_empty() { From 9c6cca6b8ef69ba9ea55e6257cb4ba8eec28cc4a Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 18:52:27 -0500 Subject: [PATCH 06/11] Fix output filtering in into_unique_contributions Filter outputs by script_pubkey rather than full TxOut equality. Outputs reusing the same address as a prior round are still considered committed even if the value differs (e.g., different change amounts across RBF rounds with different feerates). Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/funding.rs | 2 +- lightning/src/ln/splicing_tests.rs | 50 ++++++++++++++++++------------ 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index e954149fd6c..2f4e89db926 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -766,7 +766,7 @@ impl FundingContribution { inputs.retain(|input| *input != existing); } for existing in existing_outputs { - outputs.retain(|output| *output != *existing); + outputs.retain(|output| output.script_pubkey != existing.script_pubkey); } if inputs.is_empty() && outputs.is_empty() { None diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 4135f2b5604..623a15180ea 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3915,11 +3915,11 @@ fn test_funding_contributed_splice_already_pending() { .build() .unwrap(); - // Initiate a second splice with a DIFFERENT output to test that different outputs - // are included in DiscardFunding (not filtered out) + // Initiate a second splice with a DIFFERENT output (different script_pubkey) to test that + // non-overlapping outputs are included in DiscardFunding (not filtered out). let second_splice_out = TxOut { - value: Amount::from_sat(6_000), // Different amount - script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())), + value: Amount::from_sat(6_000), + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }; // Clear UTXOs and add a LARGER one for the second contribution to ensure @@ -3950,8 +3950,7 @@ fn test_funding_contributed_splice_already_pending() { // Second funding_contributed with a different contribution - this should trigger // DiscardFunding because there's already a pending quiescent action (splice contribution). // Only inputs/outputs NOT in the existing contribution should be discarded. - let (expected_inputs, expected_outputs) = - second_contribution.clone().into_contributed_inputs_and_outputs(); + let expected_inputs: Vec<_> = second_contribution.contributed_inputs().collect(); // Returns Err(APIMisuseError) and emits DiscardFunding for the non-duplicate parts of the second contribution assert_eq!( @@ -3971,11 +3970,10 @@ fn test_funding_contributed_splice_already_pending() { if let FundingInfo::Contribution { inputs, outputs } = funding_info { // The input is different, so it should be in the discard event assert_eq!(*inputs, expected_inputs); - // The splice-out output is different (6000 vs 5000), so it should be in discard event - assert!(expected_outputs.contains(&second_splice_out)); - assert!(!expected_outputs.contains(&first_splice_out)); - // The different outputs should NOT be filtered out - assert_eq!(*outputs, expected_outputs); + // The splice-out output (different script_pubkey) survives filtering; + // the change output (same script_pubkey as first contribution) is filtered. + assert_eq!(outputs.len(), 1); + assert!(outputs.contains(&second_splice_out)); } else { panic!("Expected FundingInfo::Contribution"); } @@ -4068,14 +4066,24 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { let first_contribution = funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); - // Build second contribution with different UTXOs so inputs/outputs don't overlap + // Build second contribution with different UTXOs and a splice-out output using a different + // script_pubkey (node 1's address) so it survives script_pubkey-based filtering. nodes[0].wallet_source.clear_utxos(); provide_utxo_reserves(&nodes, 1, splice_in_amount * 3); + let splice_out_output = TxOut { + value: Amount::from_sat(1_000), + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), + }; let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let second_contribution = - funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); + let second_contribution = funding_template + .without_prior_contribution(feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(splice_in_amount) + .add_outputs(vec![splice_out_output.clone()]) + .build() + .unwrap(); // First funding_contributed - sets up the quiescent action and queues STFU nodes[0] @@ -4120,10 +4128,10 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { } } - // Call funding_contributed with a different contribution (non-overlapping inputs/outputs). - // This hits the funding_negotiation path and returns DiscardFunding. - let (expected_inputs, expected_outputs) = - second_contribution.clone().into_contributed_inputs_and_outputs(); + // Call funding_contributed with the second contribution. Inputs don't overlap (different + // UTXOs) so they all survive. The splice-out output (different script_pubkey) survives + // while the change output (same script_pubkey as first contribution) is filtered. + let expected_inputs: Vec<_> = second_contribution.contributed_inputs().collect(); assert_eq!( nodes[0].node.funding_contributed(&channel_id, &node_id_1, second_contribution, None), Err(APIError::APIMisuseError { @@ -4131,15 +4139,17 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { }) ); - // Assert DiscardFunding event with the non-duplicate inputs/outputs let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { Event::DiscardFunding { channel_id: event_channel_id, funding_info } => { assert_eq!(*event_channel_id, channel_id); if let FundingInfo::Contribution { inputs, outputs } = funding_info { + // Inputs are unique (different UTXOs) so none are filtered. assert_eq!(*inputs, expected_inputs); - assert_eq!(*outputs, expected_outputs); + // Only the splice-out output survives; the change output is filtered + // (same script_pubkey as first contribution's change). + assert_eq!(*outputs, vec![splice_out_output]); } else { panic!("Expected FundingInfo::Contribution"); } From db45c8335a722f12b97dd53dfdd6f0d497507924 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 16:42:20 -0500 Subject: [PATCH 07/11] Derive SpliceFundingFailed inputs from FundingContribution Replace the maybe_create_splice_funding_failed! macro and splice_funding_failed_for method with a unified splice_funding_failed_for! macro that derives contributed inputs and outputs from the FundingContribution rather than extracting them from the negotiation state. Callers pass ident parameters for which PendingSplice filtering methods to use: contributed_inputs/contributed_outputs when the current round's contribution has been popped or was never pushed, and prior_contributed_inputs/prior_contributed_outputs for the read-only persistence path where the contribution is cloned instead. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 158 +++++++++++++++++------------------- 1 file changed, 74 insertions(+), 84 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 455a5ae80d2..82c8835fee7 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6883,13 +6883,6 @@ impl FundingNegotiationContext { } } - fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - let contributed_inputs = - self.our_funding_inputs.into_iter().map(|input| input.utxo.outpoint).collect(); - let contributed_outputs = self.our_funding_outputs; - (contributed_inputs, contributed_outputs) - } - fn contributed_inputs(&self) -> impl Iterator + '_ { self.our_funding_inputs.iter().map(|input| input.utxo.outpoint) } @@ -6897,10 +6890,6 @@ impl FundingNegotiationContext { fn contributed_outputs(&self) -> impl Iterator + '_ { self.our_funding_outputs.iter() } - - fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } } // Holder designates channel data owned for the benefit of the user client. @@ -7080,48 +7069,29 @@ impl SpliceFundingFailed { } } -macro_rules! maybe_create_splice_funding_failed { - ($funded_channel: expr, $pending_splice: expr, $pending_splice_ref: expr, $get: ident, $contributed_inputs_and_outputs: ident) => {{ - $pending_splice - .and_then(|pending_splice| pending_splice.funding_negotiation.$get()) - .and_then(|funding_negotiation| { - let is_initiator = funding_negotiation.is_initiator(); - - let (mut contributed_inputs, mut contributed_outputs) = match funding_negotiation { - FundingNegotiation::AwaitingAck { context, .. } => { - context.$contributed_inputs_and_outputs() - }, - FundingNegotiation::ConstructingTransaction { - interactive_tx_constructor, - .. - } => interactive_tx_constructor.$contributed_inputs_and_outputs(), - FundingNegotiation::AwaitingSignatures { .. } => $funded_channel - .context - .interactive_tx_signing_session - .$get() - .expect("We have a pending splice awaiting signatures") - .$contributed_inputs_and_outputs(), - }; - - if let Some(pending_splice) = $pending_splice_ref { - for input in pending_splice.prior_contributed_inputs() { - contributed_inputs.retain(|i| *i != input); - } - for output in pending_splice.prior_contributed_outputs() { - contributed_outputs.retain(|o| o.script_pubkey != output.script_pubkey); - } - } - - if !is_initiator && contributed_inputs.is_empty() && contributed_outputs.is_empty() - { - return None; - } - - let contribution = - $pending_splice_ref.and_then(|ps| ps.contributions.last().cloned()); - - Some(SpliceFundingFailed { contributed_inputs, contributed_outputs, contribution }) - }) +macro_rules! splice_funding_failed_for { + ($self: expr, $is_initiator: expr, $contribution: expr, + $contributed_inputs: ident, $contributed_outputs: ident) => {{ + let contribution = $contribution; + let existing_inputs = + $self.pending_splice.as_ref().into_iter().flat_map(|ps| ps.$contributed_inputs()); + let existing_outputs = + $self.pending_splice.as_ref().into_iter().flat_map(|ps| ps.$contributed_outputs()); + let filtered = + contribution.clone().into_unique_contributions(existing_inputs, existing_outputs); + match filtered { + None if !$is_initiator => None, + None => Some(SpliceFundingFailed { + contributed_inputs: vec![], + contributed_outputs: vec![], + contribution: Some(contribution), + }), + Some((contributed_inputs, contributed_outputs)) => Some(SpliceFundingFailed { + contributed_inputs, + contributed_outputs, + contribution: Some(contribution), + }), + } }}; } @@ -7151,21 +7121,16 @@ where /// Builds a [`SpliceFundingFailed`] from a contribution, filtering out inputs/outputs /// that are still committed to a prior splice round. fn splice_funding_failed_for(&self, contribution: FundingContribution) -> SpliceFundingFailed { - let cloned_contribution = contribution.clone(); - let (mut inputs, mut outputs) = contribution.into_contributed_inputs_and_outputs(); - if let Some(ref pending_splice) = self.pending_splice { - for input in pending_splice.contributed_inputs() { - inputs.retain(|i| *i != input); - } - for output in pending_splice.contributed_outputs() { - outputs.retain(|o| o.script_pubkey != output.script_pubkey); - } - } - SpliceFundingFailed { - contributed_inputs: inputs, - contributed_outputs: outputs, - contribution: Some(cloned_contribution), - } + // The contribution was never pushed to `contributions`, so `contributed_inputs()` and + // `contributed_outputs()` return only prior rounds' entries for filtering. + splice_funding_failed_for!( + self, + true, + contribution, + contributed_inputs, + contributed_outputs + ) + .expect("is_initiator is true so this always returns Some") } fn quiescent_action_into_error(&self, action: QuiescentAction) -> QuiescentError { @@ -7309,21 +7274,23 @@ where ); } - let splice_funding_failed = maybe_create_splice_funding_failed!( - self, - self.pending_splice.as_mut(), - self.pending_splice.as_ref(), - take, - into_contributed_inputs_and_outputs - ); - - // Pop the current round's contribution, if any (acceptors may not have one). This - // must happen after `maybe_create_splice_funding_failed` for correct filtering. + // Take the funding negotiation and pop the current round's contribution, if any + // (acceptors may not have one). let pending_splice = self .pending_splice .as_mut() .expect("reset_pending_splice_state requires pending_splice"); - if let Some(contribution) = pending_splice.contributions.pop() { + debug_assert!( + pending_splice.funding_negotiation.is_some(), + "reset_pending_splice_state requires an active funding negotiation" + ); + let is_initiator = pending_splice + .funding_negotiation + .take() + .map(|negotiation| negotiation.is_initiator()) + .unwrap_or(false); + let contribution = pending_splice.contributions.pop(); + if let Some(ref contribution) = contribution { debug_assert!( pending_splice .last_funding_feerate_sat_per_1000_weight @@ -7333,6 +7300,18 @@ where ); } + // After pop, `contributed_inputs()` / `contributed_outputs()` return only prior + // rounds for filtering. + let splice_funding_failed = contribution.and_then(|contribution| { + splice_funding_failed_for!( + self, + is_initiator, + contribution, + contributed_inputs, + contributed_outputs + ) + }); + if self.pending_funding().is_empty() { self.pending_splice.take(); } @@ -7350,12 +7329,23 @@ where return None; } - maybe_create_splice_funding_failed!( + let pending_splice = self.pending_splice.as_ref()?; + debug_assert!( + pending_splice.funding_negotiation.is_some(), + "maybe_splice_funding_failed requires an active funding negotiation" + ); + let is_initiator = pending_splice + .funding_negotiation + .as_ref() + .map(|negotiation| negotiation.is_initiator()) + .unwrap_or(false); + let contribution = pending_splice.contributions.last().cloned()?; + splice_funding_failed_for!( self, - self.pending_splice.as_ref(), - self.pending_splice.as_ref(), - as_ref, - to_contributed_inputs_and_outputs + is_initiator, + contribution, + prior_contributed_inputs, + prior_contributed_outputs ) } From 367ffaa132dfdbe88d2d768346f3f376e4e8d728 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 17:01:30 -0500 Subject: [PATCH 08/11] Remove unused NegotiationError and contributed_inputs_and_outputs methods Now that splice_funding_failed_for! derives inputs and outputs from FundingContribution directly, remove the unused NegotiationError struct and into_negotiation_error methods from the interactive tx types, along with the into/to_contributed_inputs_and_outputs methods on ConstructedTransaction. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/interactivetxs.rs | 83 ------------------------------ 1 file changed, 83 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index ca8c4450012..10dae95cefa 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -90,13 +90,6 @@ impl SerialIdExt for SerialId { } } -#[derive(Clone, Debug)] -pub(crate) struct NegotiationError { - pub reason: AbortReason, - pub contributed_inputs: Vec, - pub contributed_outputs: Vec, -} - #[derive(Debug, Clone, Copy, PartialEq)] pub(crate) enum AbortReason { InvalidStateTransition, @@ -370,11 +363,6 @@ impl ConstructedTransaction { Ok(tx) } - fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - let (contributed_inputs, contributed_outputs) = self.into_contributed_inputs_and_outputs(); - NegotiationError { reason, contributed_inputs, contributed_outputs } - } - fn contributed_inputs(&self) -> impl Iterator + '_ { self.tx .input @@ -401,40 +389,6 @@ impl ConstructedTransaction { .map(|(_, (txout, _))| txout) } - fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } - - fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - let contributed_inputs = self - .tx - .input - .into_iter() - .zip(self.input_metadata.iter()) - .enumerate() - .filter(|(_, (_, input))| input.is_local(self.holder_is_initiator)) - .filter(|(index, _)| { - self.shared_input_index - .map(|shared_index| *index != shared_index as usize) - .unwrap_or(true) - }) - .map(|(_, (txin, _))| txin.previous_output) - .collect(); - - let contributed_outputs = self - .tx - .output - .into_iter() - .zip(self.output_metadata.iter()) - .enumerate() - .filter(|(_, (_, output))| output.is_local(self.holder_is_initiator)) - .filter(|(index, _)| *index != self.shared_output_index as usize) - .map(|(_, (txout, _))| txout) - .collect(); - - (contributed_inputs, contributed_outputs) - } - pub fn tx(&self) -> &Transaction { &self.tx } @@ -921,10 +875,6 @@ impl InteractiveTxSigningSession { Ok(()) } - pub(crate) fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - self.unsigned_tx.into_negotiation_error(reason) - } - pub(super) fn contributed_inputs(&self) -> impl Iterator + '_ { self.unsigned_tx.contributed_inputs() } @@ -932,14 +882,6 @@ impl InteractiveTxSigningSession { pub(super) fn contributed_outputs(&self) -> impl Iterator + '_ { self.unsigned_tx.contributed_outputs() } - - pub(super) fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } - - pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - self.unsigned_tx.into_contributed_inputs_and_outputs() - } } impl_writeable_tlv_based!(InteractiveTxSigningSession, { @@ -2172,27 +2114,6 @@ impl InteractiveTxConstructor { Self::new(args, false) } - fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - let (contributed_inputs, contributed_outputs) = self.into_contributed_inputs_and_outputs(); - NegotiationError { reason, contributed_inputs, contributed_outputs } - } - - pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - let contributed_inputs = self - .inputs_to_contribute - .into_iter() - .filter(|(_, input)| !input.is_shared()) - .map(|(_, input)| input.into_tx_in().previous_output) - .collect(); - let contributed_outputs = self - .outputs_to_contribute - .into_iter() - .filter(|(_, output)| !output.is_shared()) - .map(|(_, output)| output.into_tx_out()) - .collect(); - (contributed_inputs, contributed_outputs) - } - pub(super) fn contributed_inputs(&self) -> impl Iterator + '_ { self.inputs_to_contribute .iter() @@ -2207,10 +2128,6 @@ impl InteractiveTxConstructor { .map(|(_, output)| output.tx_out()) } - pub(super) fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } - pub fn is_initiator(&self) -> bool { self.is_initiator } From b44076e0484f3abca1aaae87b65a6d2b5debe68e Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 3 Apr 2026 10:28:45 -0500 Subject: [PATCH 09/11] Add pending changelog for splice negotiation event changes Co-Authored-By: Claude Opus 4.6 (1M context) --- pending_changelog/4514-splice-negotiation-failed.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 pending_changelog/4514-splice-negotiation-failed.txt diff --git a/pending_changelog/4514-splice-negotiation-failed.txt b/pending_changelog/4514-splice-negotiation-failed.txt new file mode 100644 index 00000000000..809bf7cb86d --- /dev/null +++ b/pending_changelog/4514-splice-negotiation-failed.txt @@ -0,0 +1,11 @@ +# API Updates + + * `Event::SplicePending` has been renamed to `Event::SpliceNegotiated`. + + * `Event::SpliceFailed` has been renamed to `Event::SpliceNegotiationFailed`. + + * `Event::SpliceNegotiationFailed` now includes a `reason` field + (`NegotiationFailureReason`) indicating why the negotiation round failed, + and a `contribution` field returning the `FundingContribution` for retry. + + * `FundingContribution` now exposes `feerate()` and `inputs()` accessor methods. From ffbb8fe69b80608c329075e7746c5e51346ac50d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 13 Apr 2026 17:48:52 -0500 Subject: [PATCH 10/11] Check can_initiate_rbf in stfu handler before sending tx_init_rbf If splice_locked is sent between our outgoing STFU and the counterparty's STFU response, the stfu() handler would proceed to send tx_init_rbf for an already-confirmed splice. Guard against this by re-checking can_initiate_rbf when entering quiescence. Disconnect because there is no way to cancel quiescence after both sides have exchanged STFU. Co-Authored-By: Claude Opus 4.6 (1M context) --- fuzz/src/chanmon_consistency.rs | 1 + lightning/src/events/mod.rs | 11 ++- lightning/src/ln/channel.rs | 10 +++ lightning/src/ln/splicing_tests.rs | 109 +++++++++++++++++++++++++++++ 4 files changed, 130 insertions(+), 1 deletion(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 678e6a6fc61..55b2a681725 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -930,6 +930,7 @@ fn assert_action_timeout_awaiting_response(action: &msgs::ErrorAction) { action, msgs::ErrorAction::DisconnectPeerWithWarning { msg } if msg.data.contains("Disconnecting due to timeout awaiting response") + || msg.data.contains("already sent splice_locked, cannot RBF") ), "Expected timeout disconnect, got: {:?}", action, diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 9d00273cb46..0d5b8f757ab 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -149,6 +149,12 @@ pub enum NegotiationFailureReason { /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel /// [`FundingTemplate`]: crate::ln::funding::FundingTemplate FeeRateTooLow, + /// An RBF attempt could not be initiated (e.g., a prior splice transaction already + /// confirmed). The channel remains operational — start a new splice with + /// [`ChannelManager::splice_channel`] if further changes are needed. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + CannotInitiateRbf, } impl NegotiationFailureReason { @@ -166,7 +172,8 @@ impl NegotiationFailureReason { Self::CounterpartyAborted { .. } | Self::NegotiationError { .. } | Self::LocallyAbandoned - | Self::ChannelClosing => false, + | Self::ChannelClosing + | Self::CannotInitiateRbf => false, } } } @@ -185,6 +192,7 @@ impl core::fmt::Display for NegotiationFailureReason { Self::ChannelClosing => f.write_str("channel is closing"), Self::FeeRateTooLow => f.write_str("feerate too low for RBF"), + Self::CannotInitiateRbf => f.write_str("cannot initiate RBF"), } } } @@ -202,6 +210,7 @@ impl_writeable_tlv_based_enum_upgradable!(NegotiationFailureReason, (11, LocallyAbandoned) => {}, (13, ChannelClosing) => {}, (15, FeeRateTooLow) => {}, + (17, CannotInitiateRbf) => {}, ); /// Some information provided on receipt of payment depends on whether the payment received is a diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 82c8835fee7..cff3466c808 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -14328,6 +14328,16 @@ where }; if self.pending_splice.is_some() { + if let Err(e) = self.can_initiate_rbf() { + let failed = self.splice_funding_failed_for(prior_contribution); + return Err(( + ChannelError::WarnAndDisconnect(e), + QuiescentError::FailSplice( + failed, + NegotiationFailureReason::CannotInitiateRbf, + ), + )); + } let tx_init_rbf = self.send_tx_init_rbf(context); self.pending_splice.as_mut().unwrap() .contributions.push(prior_contribution); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 623a15180ea..a3396d7e84a 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -5198,6 +5198,115 @@ fn test_splice_rbf_after_splice_locked() { } } +#[test] +fn test_splice_rbf_stfu_after_splice_locked() { + // Test that we don't send tx_init_rbf when we've already sent splice_locked. + // + // Scenario: node 0 initiates an RBF and sends STFU, but before receiving the counterparty's + // STFU response, it mines enough blocks to send splice_locked (setting sent_funding_txid). + // When node 1's STFU arrives, the stfu() handler should detect that RBF is no longer valid + // and return WarnAndDisconnect instead of sending tx_init_rbf. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Complete a splice-in from node 0. + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + + // Mine the splice tx on both nodes (not enough for splice_locked yet). + mine_transaction(&nodes[0], &splice_tx); + mine_transaction(&nodes[1], &splice_tx); + + // Provide more UTXOs for the RBF attempt. + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Initiate RBF from node 0 with fresh inputs so the RBF round has a unique input that + // survives filtering when the failure cleanup runs. + let rbf_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let funding_contribution = funding_template + .without_prior_contribution(rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(added_value) + .build() + .unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, funding_contribution.clone(), None) + .unwrap(); + + // Node 0 sends STFU (can_initiate_rbf passes since no splice_locked yet). + let stfu_init = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + + // Deliver STFU to node 1; extract node 1's STFU response but don't deliver it yet. + nodes[1].node.handle_stfu(node_id_0, &stfu_init); + let stfu_ack = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + + // Mine enough blocks on node 0 so it sends splice_locked (sets sent_funding_txid). + connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1); + let _splice_locked = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceLocked, node_id_1); + + // Now deliver node 1's STFU to node 0. The stfu() handler should detect that RBF is no + // longer valid (we already sent splice_locked) and return WarnAndDisconnect. + nodes[0].node.handle_stfu(node_id_1, &stfu_ack); + + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + match &msg_events[0] { + MessageSendEvent::HandleError { action, .. } => { + assert_eq!( + *action, + msgs::ErrorAction::DisconnectPeerWithWarning { + msg: msgs::WarningMessage { + channel_id, + data: format!( + "Channel {} already sent splice_locked, cannot RBF", + channel_id, + ), + }, + } + ); + }, + _ => panic!("Expected HandleError, got {:?}", msg_events[0]), + } + + // Node 0 should emit DiscardFunding + SpliceNegotiationFailed for the RBF contribution. + // The change output is filtered (same script_pubkey as the first splice's change output), + // but the input survives because it's a different UTXO from the first splice. + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2, "{events:?}"); + match &events[0] { + Event::DiscardFunding { + funding_info: FundingInfo::Contribution { inputs, outputs }, + .. + } => { + assert!(!inputs.is_empty()); + assert!(outputs.is_empty()); + }, + other => panic!("Expected DiscardFunding, got {:?}", other), + } + match &events[1] { + Event::SpliceNegotiationFailed { channel_id: cid, reason, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::CannotInitiateRbf); + }, + other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), + } +} + #[test] fn test_splice_zeroconf_no_rbf_feerate() { // Test that splice_channel returns a FundingTemplate with min_rbf_feerate = None for a From 51e6079878ac45b7f66d15683ef797b3213a23c6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 15 Apr 2026 21:04:46 -0500 Subject: [PATCH 11/11] Derive DiscardFunding inputs and outputs from contributions on promotion When a splice funding is promoted, produce FundingInfo::Contribution instead of FundingInfo::Tx for the discarded funding events. Each contribution is filtered against the promoted funding transaction's inputs and outputs, so only inputs and outputs unique to the discarded round are reported. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 30 ++--- lightning/src/ln/splicing_tests.rs | 198 +++++++++++++++++++++++------ 2 files changed, 176 insertions(+), 52 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index cff3466c808..18236d761a0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -11671,7 +11671,6 @@ where .iter_mut() .find(|funding| funding.get_funding_txid() == Some(splice_txid)) .unwrap(); - let prev_funding_txid = self.funding.get_funding_txid(); if let Some(scid) = self.funding.short_channel_id { self.context.historical_scids.push(scid); @@ -11679,22 +11678,21 @@ where core::mem::swap(&mut self.funding, funding); - // The swap above places the previous `FundingScope` into `pending_funding`. - pending_splice - .negotiated_candidates - .drain(..) - .filter(|funding| funding.get_funding_txid() != prev_funding_txid) - .map(|mut funding| { - funding - .funding_transaction - .take() - .map(|tx| FundingInfo::Tx { transaction: tx }) - .unwrap_or_else(|| FundingInfo::OutPoint { - outpoint: funding - .get_funding_txo() - .expect("Negotiated splices must have a known funding outpoint"), - }) + let promoted_tx = self + .funding + .funding_transaction + .as_ref() + .expect("Promoted splice funding should have a funding transaction"); + let contributions = core::mem::take(&mut pending_splice.contributions); + contributions + .into_iter() + .filter_map(|contribution| { + contribution.into_unique_contributions( + promoted_tx.input.iter().map(|i| i.previous_output), + promoted_tx.output.iter(), + ) }) + .map(|(inputs, outputs)| FundingInfo::Contribution { inputs, outputs }) .collect::>() }; diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index a3396d7e84a..1b6879e69f0 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -683,9 +683,15 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( (splice_tx, new_funding_script) } +pub struct SpliceLockedResult { + pub stfu: Option, + pub node_a_discarded: Vec<(Vec, Vec)>, + pub node_b_discarded: Vec<(Vec, Vec)>, +} + pub fn lock_splice_after_blocks<'a, 'b, 'c, 'd>( node_a: &'a Node<'b, 'c, 'd>, node_b: &'a Node<'b, 'c, 'd>, num_blocks: u32, -) -> Option { +) -> SpliceLockedResult { connect_blocks(node_a, num_blocks); connect_blocks(node_b, num_blocks); @@ -698,7 +704,7 @@ pub fn lock_splice_after_blocks<'a, 'b, 'c, 'd>( pub fn lock_splice<'a, 'b, 'c, 'd>( node_a: &'a Node<'b, 'c, 'd>, node_b: &'a Node<'b, 'c, 'd>, splice_locked_for_node_b: &msgs::SpliceLocked, is_0conf: bool, expected_discard_txids: &[Txid], -) -> Option { +) -> SpliceLockedResult { let prev_funding_txid = node_a .chain_monitor .chain_monitor @@ -735,29 +741,23 @@ pub fn lock_splice<'a, 'b, 'c, 'd>( } } - let mut all_discard_txids = Vec::new(); - let expected_num_events = 1 + expected_discard_txids.len(); - for node in [node_a, node_b] { + let mut node_a_discarded = Vec::new(); + let mut node_b_discarded = Vec::new(); + for (idx, node) in [node_a, node_b].into_iter().enumerate() { let events = node.node.get_and_clear_pending_events(); - assert_eq!(events.len(), expected_num_events, "{events:?}"); + assert!(!events.is_empty(), "Expected at least ChannelReady, got {events:?}"); assert!(matches!(events[0], Event::ChannelReady { .. })); - let discard_txids: Vec<_> = events[1..] - .iter() - .map(|e| match e { - Event::DiscardFunding { funding_info: FundingInfo::Tx { transaction }, .. } => { - transaction.compute_txid() - }, + let discarded = if idx == 0 { &mut node_a_discarded } else { &mut node_b_discarded }; + for event in &events[1..] { + match event { Event::DiscardFunding { - funding_info: FundingInfo::OutPoint { outpoint }, .. - } => outpoint.txid, - other => panic!("Expected DiscardFunding, got {:?}", other), - }) - .collect(); - for txid in expected_discard_txids { - assert!(discard_txids.contains(txid), "Missing DiscardFunding for txid {}", txid); - } - if all_discard_txids.is_empty() { - all_discard_txids = discard_txids; + funding_info: FundingInfo::Contribution { inputs, outputs }, + .. + } => { + discarded.push((inputs.clone(), outputs.clone())); + }, + other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), + } } check_added_monitors(node, 1); } @@ -795,18 +795,18 @@ pub fn lock_splice<'a, 'b, 'c, 'd>( // old funding as it is no longer being tracked. for node in [node_a, node_b] { node.chain_source.remove_watched_by_txid(prev_funding_txid); - for txid in &all_discard_txids { + for txid in expected_discard_txids { node.chain_source.remove_watched_by_txid(*txid); } } - node_a_stfu.or(node_b_stfu) + SpliceLockedResult { stfu: node_a_stfu.or(node_b_stfu), node_a_discarded, node_b_discarded } } pub fn lock_rbf_splice_after_blocks<'a, 'b, 'c, 'd>( node_a: &'a Node<'b, 'c, 'd>, node_b: &'a Node<'b, 'c, 'd>, tx: &Transaction, num_blocks: u32, expected_discard_txids: &[Txid], -) -> Option { +) -> SpliceLockedResult { mine_transaction(node_a, tx); mine_transaction(node_b, tx); @@ -1400,7 +1400,7 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { mine_transaction(&nodes[0], &splice_tx); mine_transaction(&nodes[1], &splice_tx); - let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1).stfu; // Node 0 had called splice_channel (line above) but never funding_contributed, so no stfu // is expected from node 0 at this point. assert!(stfu.is_none()); @@ -1428,7 +1428,7 @@ fn test_initiating_splice_holds_stfu_with_pending_splice() { // Mine and lock the splice. mine_transaction(&nodes[0], &splice_tx); mine_transaction(&nodes[1], &splice_tx); - let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], 5); + let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], 5).stfu; assert!(stfu.is_none()); } @@ -1664,7 +1664,7 @@ fn do_test_splice_tiebreak( mine_transaction(&nodes[1], &tx); // After splice_locked, node 1's preserved QuiescentAction triggers STFU for retry. - let node_1_stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + let node_1_stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1).stfu; let stfu_1 = if let Some(MessageSendEvent::SendStfu { msg, .. }) = node_1_stfu { assert!(msg.initiator); msg @@ -4712,13 +4712,124 @@ fn test_splice_rbf_acceptor_basic() { expect_splice_pending_event(&nodes[1], &node_id_0); // Step 11: Mine, lock, and verify DiscardFunding for the replaced splice candidate. - lock_rbf_splice_after_blocks( + let result = lock_rbf_splice_after_blocks( &nodes[0], &nodes[1], &rbf_tx, ANTI_REORG_DELAY - 1, &[first_splice_tx.compute_txid()], ); + + // The test wallet reuses the same UTXO across RBF rounds (the wallet doesn't track + // in-flight spends), so all contributed inputs are in the promoted tx. No unique + // contributions to discard. + assert!(result.node_a_discarded.is_empty()); + assert!(result.node_b_discarded.is_empty()); +} + +#[test] +fn test_splice_rbf_discard_unique_contribution() { + // Verify that DiscardFunding events contain the correct unique inputs and outputs when the + // RBF round uses different UTXOs than the initial splice. By clearing the wallet between + // rounds and providing fresh UTXOs, we force distinct inputs per round. Round 0 also + // includes a splice-out output with a unique script_pubkey not present in the RBF tx. + // When the RBF is promoted, round 0's inputs and splice-out output should appear in + // DiscardFunding. The change output is filtered because it shares a script_pubkey with the + // promoted tx's change output. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Round 0: Splice-in-and-out from node 0 with a splice-out output. + let splice_out_output = TxOut { + value: Amount::from_sat(5_000), + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), + }; + let funding_contribution = do_initiate_splice_in_and_out( + &nodes[0], + &nodes[1], + channel_id, + added_value, + vec![splice_out_output.clone()], + ); + let round_0_inputs: Vec<_> = funding_contribution.contributed_inputs().collect(); + assert!(!round_0_inputs.is_empty()); + + let (first_splice_tx, new_funding_script) = + splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + + // Clear node 0's wallet so round 1 must use different UTXOs. + nodes[0].wallet_source.clear_utxos(); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Round 1: RBF with fresh UTXOs, splice-in only (no splice-out output). + let rbf_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let funding_contribution = funding_template + .without_prior_contribution(rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(added_value) + .build() + .unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, funding_contribution.clone(), None) + .unwrap(); + let round_1_inputs: Vec<_> = funding_contribution.contributed_inputs().collect(); + assert_ne!(round_0_inputs, round_1_inputs, "Rounds must use different UTXOs"); + + complete_rbf_handshake(&nodes[0], &nodes[1]); + + complete_interactive_funding_negotiation( + &nodes[0], + &nodes[1], + channel_id, + funding_contribution, + new_funding_script.clone(), + ); + + let (rbf_tx, splice_locked) = sign_interactive_funding_tx( + &nodes[0], + &nodes[1], + false, + Some(first_splice_tx.compute_txid()), + ); + assert!(splice_locked.is_none()); + + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + let result = lock_rbf_splice_after_blocks( + &nodes[0], + &nodes[1], + &rbf_tx, + ANTI_REORG_DELAY - 1, + &[first_splice_tx.compute_txid()], + ); + + // Node 0's round 0 inputs are NOT in the promoted tx (which uses round 1's fresh UTXOs), + // so they appear as unique contributions to discard. The splice-out output also survives + // because its script_pubkey is not in the promoted tx. The change output is filtered + // because it shares a script_pubkey with the promoted tx's change output. + assert_eq!(result.node_a_discarded.len(), 1); + let (ref inputs, ref outputs) = result.node_a_discarded[0]; + assert_eq!(*inputs, round_0_inputs); + assert_eq!(*outputs, vec![splice_out_output]); + + // Node 1 (non-contributing acceptor) has no contributions to discard. + assert!(result.node_b_discarded.is_empty()); } #[test] @@ -5663,13 +5774,18 @@ pub fn do_test_splice_rbf_tiebreak( expect_splice_pending_event(&nodes[1], &node_id_0); // Mine, lock, and verify DiscardFunding for the replaced splice candidate. - lock_rbf_splice_after_blocks( + let result = lock_rbf_splice_after_blocks( &nodes[0], &nodes[1], &rbf_tx, ANTI_REORG_DELAY - 1, &[first_splice_tx.compute_txid()], ); + + // The test wallet reuses the same UTXOs across RBF rounds, so all contributed inputs + // are in the promoted tx and nothing is unique to discard. + assert!(result.node_a_discarded.is_empty()); + assert!(result.node_b_discarded.is_empty()); } else { // Acceptor does not contribute — complete with only node 0's inputs/outputs. complete_interactive_funding_negotiation_for_both( @@ -5698,14 +5814,14 @@ pub fn do_test_splice_rbf_tiebreak( // Mine, lock, and verify DiscardFunding for the replaced splice candidate. // Node 1's QuiescentAction was preserved, so after splice_locked it re-initiates // quiescence to retry its contribution in a future splice. - let node_b_stfu = lock_rbf_splice_after_blocks( + let result = lock_rbf_splice_after_blocks( &nodes[0], &nodes[1], &rbf_tx, ANTI_REORG_DELAY - 1, &[first_splice_tx.compute_txid()], ); - let stfu_1 = if let Some(MessageSendEvent::SendStfu { msg, .. }) = node_b_stfu { + let stfu_1 = if let Some(MessageSendEvent::SendStfu { msg, .. }) = result.stfu { msg } else { panic!("Expected SendStfu from node 1"); @@ -5985,13 +6101,18 @@ fn test_splice_rbf_acceptor_recontributes() { expect_splice_pending_event(&nodes[1], &node_id_0); // Step 12: Mine, lock, and verify DiscardFunding for the replaced splice candidate. - lock_rbf_splice_after_blocks( + let result = lock_rbf_splice_after_blocks( &nodes[0], &nodes[1], &rbf_tx, ANTI_REORG_DELAY - 1, &[first_splice_tx.compute_txid()], ); + + // The test wallet reuses the same UTXOs across RBF rounds, so all contributed inputs + // are in the promoted tx and nothing is unique to discard. + assert!(result.node_a_discarded.is_empty()); + assert!(result.node_b_discarded.is_empty()); } #[test] @@ -6314,13 +6435,18 @@ fn test_splice_rbf_sequential() { // --- Mine and lock the final RBF, verifying DiscardFunding for both replaced candidates. --- let splice_tx_0_txid = splice_tx_0.compute_txid(); let splice_tx_1_txid = splice_tx_1.compute_txid(); - lock_rbf_splice_after_blocks( + let result = lock_rbf_splice_after_blocks( &nodes[0], &nodes[1], &rbf_tx_final, ANTI_REORG_DELAY - 1, &[splice_tx_0_txid, splice_tx_1_txid], ); + + // The test wallet reuses the same UTXOs across RBF rounds, so all contributed inputs + // are in the promoted tx and nothing is unique to discard. + assert!(result.node_a_discarded.is_empty()); + assert!(result.node_b_discarded.is_empty()); } #[test] @@ -6913,7 +7039,7 @@ fn test_funding_contributed_rbf_adjustment_exceeds_max_feerate() { // Mine and lock the pending splice → pending_splice is cleared. mine_transaction(&nodes[0], &_splice_tx); mine_transaction(&nodes[1], &_splice_tx); - let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1).stfu; // STFU is sent during lock — the splice proceeds as a fresh splice (not RBF). let stfu = match stfu { @@ -6990,7 +7116,7 @@ fn test_funding_contributed_rbf_adjustment_insufficient_budget() { // Mine and lock the pending splice → pending_splice is cleared. mine_transaction(&nodes[0], &_splice_tx); mine_transaction(&nodes[1], &_splice_tx); - let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1).stfu; // STFU is sent during lock — the splice proceeds as a fresh splice (not RBF). let stfu = match stfu {