From e82e4e5866d5dcab022d63909503680311d37bee 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 | 109 +++++++++++++++++ lightning/src/ln/channel.rs | 45 +++++-- lightning/src/ln/channelmanager.rs | 35 ++++-- lightning/src/ln/functional_test_utils.rs | 9 +- lightning/src/ln/splicing_tests.rs | 137 ++++++++++++++++++---- 5 files changed, 294 insertions(+), 41 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 73c4a39c76f..18ae1813438 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -99,6 +99,109 @@ 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::CounterpartyAborted { .. } + | Self::NegotiationError { .. } + | Self::ContributionInvalid + | Self::FeeRateTooLow => true, + 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, + (0, Unknown) => {}, + (2, PeerDisconnected) => {}, + (4, CounterpartyAborted) => { + (1, msg, required), + }, + (6, NegotiationError) => { + (1, msg, required), + }, + (8, ContributionInvalid) => {}, + (10, LocallyAbandoned) => {}, + (12, ChannelClosing) => {}, + (14, 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 +1689,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 +2484,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 +2493,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 +3138,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 +3147,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 32c0e94bdc8..46ce0082a84 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -36,7 +36,7 @@ use crate::chain::channelmonitor::{ }; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::chain::BestBlock; -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, @@ -3174,7 +3174,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 { @@ -7133,9 +7143,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, } @@ -7144,7 +7155,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, _ => { @@ -12540,7 +12551,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() { @@ -12556,6 +12570,7 @@ where ); return Err(QuiescentError::FailSplice( self.splice_funding_failed_for(contribution), + NegotiationFailureReason::FeeRateTooLow, )); } } @@ -13996,9 +14011,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!( @@ -14115,7 +14139,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 2e782701e47..41f1e805e29 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4115,6 +4115,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, )); @@ -4421,6 +4422,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, )); @@ -4927,6 +4929,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, )); @@ -6609,12 +6612,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 { @@ -6623,6 +6629,7 @@ impl< user_channel_id, abandoned_funding_txo: funding_txo, channel_type, + reason, }, None, )); @@ -6764,7 +6771,7 @@ impl< "Channel {} already has a pending funding contribution", channel_id, ), - QuiescentError::FailSplice(_) => format!( + QuiescentError::FailSplice(..) => format!( "Channel {} cannot accept funding contribution", channel_id, ), @@ -11858,6 +11865,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ user_channel_id: channel.context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type.clone(), + reason: events::NegotiationFailureReason::NegotiationError { + msg: format!("{:?}", err), + }, }, None, )); @@ -12017,6 +12027,9 @@ 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.clone(), + reason: events::NegotiationFailureReason::NegotiationError { + msg: format!("{:?}", err), + }, }, None, )); @@ -12187,6 +12200,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, )); @@ -12335,6 +12353,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, )); @@ -15451,6 +15470,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(), @@ -18123,6 +18143,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 84cdf785da5..6a8336ccb88 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -17,8 +17,8 @@ use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Listen, Watch 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, @@ -3243,13 +3243,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 9adccd17627..c1ac2b2eed0 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -13,7 +13,9 @@ use crate::chain::chaininterface::{TransactionType, FEERATE_FLOOR_SATS_PER_KW}; 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::{ CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, @@ -26,6 +28,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; @@ -282,7 +285,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) }, } @@ -888,7 +896,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); @@ -939,7 +952,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); @@ -1021,7 +1039,14 @@ 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("No active signing session. The associated funding transaction may have already been broadcast.".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. @@ -1105,7 +1130,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); @@ -2560,7 +2590,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. @@ -2631,7 +2668,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. @@ -2727,7 +2769,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); @@ -3017,8 +3068,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), } @@ -3037,7 +3089,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); } @@ -4014,7 +4071,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] @@ -4195,7 +4257,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. @@ -4230,7 +4297,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; @@ -4381,7 +4453,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, @@ -4389,7 +4466,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] { @@ -5841,7 +5921,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] { @@ -5920,8 +6003,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), } @@ -5961,7 +6045,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] { @@ -6498,7 +6585,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] @@ -6643,7 +6735,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), } } From e11cbe04de2b629276baa26dd13d75abd39b9aaa 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 | 53 +++--- lightning/src/ln/channelmanager.rs | 212 ++++++++++------------ lightning/src/ln/functional_test_utils.rs | 3 +- lightning/src/ln/funding.rs | 8 +- lightning/src/ln/splicing_tests.rs | 35 ++-- 6 files changed, 168 insertions(+), 182 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 18ae1813438..36f9e8f9a1d 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; @@ -1663,19 +1664,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. @@ -1685,12 +1687,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. @@ -2482,18 +2489,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 @@ -3134,20 +3139,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 46ce0082a84..af5805c76df 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7029,17 +7029,31 @@ 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, - - /// The features that this channel will operate with, if available. - pub channel_type: Option, - /// UTXOs spent as inputs contributed to the splice transaction. - pub contributed_inputs: Vec, + contributed_inputs: Vec, /// Outputs contributed to the splice transaction. - pub contributed_outputs: Vec, + contributed_outputs: Vec, + + /// The funding contribution from the failed round, if available. + contribution: Option, +} + +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 { @@ -7049,15 +7063,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() @@ -7088,12 +7093,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 }) }) }}; } @@ -7124,6 +7127,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() { @@ -7134,10 +7138,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 41f1e805e29..84a5f281138 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}; @@ -4107,28 +4107,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` @@ -4415,27 +4414,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 { @@ -4921,28 +4919,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(()) @@ -6612,36 +6609,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, )); } @@ -11857,30 +11840,26 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ exited_quiescence, }) => { 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, counterparty_node_id: *counterparty_node_id, user_channel_id: channel.context().get_user_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), }, }, 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, + )); + } } debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_))); @@ -12019,30 +11998,29 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ exited_quiescence, }) => { 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: msg.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.clone(), + contribution, reason: events::NegotiationFailureReason::NegotiationError { msg: format!("{:?}", err), }, }, 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, + )); + } } debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_))); @@ -12192,14 +12170,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(), @@ -12208,16 +12186,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 exited_quiescence { @@ -12345,28 +12322,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 { @@ -15464,21 +15440,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 { @@ -18136,27 +18111,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 6a8336ccb88..8c4a97946bf 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3248,9 +3248,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/funding.rs b/lightning/src/ln/funding.rs index c08a0a9f471..8e382bd3059 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -710,7 +710,8 @@ impl_writeable_tlv_based!(FundingContribution, { }); impl FundingContribution { - pub(super) fn feerate(&self) -> FeeRate { + /// Returns the feerate of this contribution. + pub fn feerate(&self) -> FeeRate { self.feerate } @@ -731,6 +732,11 @@ impl FundingContribution { self.value_added } + /// Returns the inputs included in this contribution. + pub fn inputs(&self) -> &[FundingTxInput] { + &self.inputs + } + /// Returns the outputs (e.g., withdrawal destinations) included in this contribution. /// /// This does not include the change output; see [`FundingContribution::change_output`]. diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index c1ac2b2eed0..e98e0824311 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3068,9 +3068,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), } @@ -4466,9 +4467,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), } @@ -5917,24 +5919,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:?}"); @@ -6003,9 +6003,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), } @@ -6043,18 +6044,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); @@ -6735,9 +6733,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 40d1b265c52e94d678aa25f9e5d771d47e645d3f 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 | 178 ++++++++++++---------- lightning/src/ln/functional_test_utils.rs | 18 +-- lightning/src/ln/splicing_tests.rs | 50 +++--- 3 files changed, 131 insertions(+), 115 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 84a5f281138..75290349d2f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4109,6 +4109,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, @@ -4119,15 +4128,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` @@ -4415,6 +4415,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, @@ -4425,15 +4434,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 { @@ -4921,6 +4921,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, @@ -4931,15 +4940,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(()) @@ -6612,6 +6612,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, @@ -6622,12 +6628,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { channel_id, funding_info }, - None, - )); - } }, } } @@ -11842,6 +11842,12 @@ 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 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, @@ -11854,12 +11860,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, - )); - } } debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_))); @@ -12000,6 +12000,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 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, @@ -12012,15 +12021,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, - )); - } } debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_))); @@ -12172,6 +12172,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, @@ -12186,15 +12195,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 exited_quiescence { @@ -12324,6 +12324,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, @@ -12334,15 +12343,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 { @@ -15167,6 +15167,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 { @@ -15441,6 +15457,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, @@ -15448,12 +15470,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 { @@ -18112,6 +18128,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(), @@ -18122,15 +18147,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 8c4a97946bf..05ccfa4709d 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3248,18 +3248,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 { @@ -3268,6 +3260,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 e98e0824311..7279b8b9342 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3068,14 +3068,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 }, .. @@ -3089,6 +3081,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], @@ -4467,14 +4467,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 }, .. @@ -4484,6 +4476,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]); @@ -5999,18 +5999,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 }, .. @@ -6023,6 +6015,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 5c3575f017b62398b6928b1c341401f669511cc3 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 | 55 ++++++++++--------- lightning/src/ln/functional_test_utils.rs | 6 +- lightning/src/ln/funding.rs | 4 +- lightning/src/ln/splicing_tests.rs | 46 ++++++++-------- .../4388-splice-failed-discard-funding.txt | 8 +-- 10 files changed, 81 insertions(+), 80 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index f20f93c789c..527670e1317 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -2015,7 +2015,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, @@ -2027,7 +2027,7 @@ pub fn do_test(data: &[u8], out: Out) { assert_eq!(new_funding_txo.txid, splice_tx.compute_txid()); chain_state.confirm_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 c1d7982e5e4..6de7308e27e 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1141,10 +1141,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 36f9e8f9a1d..065c2dbb5ab 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1645,8 +1645,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. @@ -1666,7 +1666,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. /// @@ -1676,7 +1676,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 @@ -2467,7 +2467,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, @@ -2485,7 +2485,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, @@ -3124,7 +3124,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(), @@ -3145,7 +3145,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 af5805c76df..32bf5d8a75a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1170,7 +1170,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, /// Whether we were quiescent when we received the message, and are no longer due to aborting /// the session. @@ -1258,7 +1258,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, } @@ -1266,7 +1266,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, } @@ -7041,7 +7041,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() { @@ -12323,7 +12323,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 75290349d2f..61de33ca66d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4119,7 +4119,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(), @@ -4425,7 +4425,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, @@ -4931,7 +4931,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(), @@ -6619,7 +6619,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id, counterparty_node_id, user_channel_id, @@ -6665,14 +6665,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. /// @@ -6891,7 +6891,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(), @@ -10984,7 +10984,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(), @@ -11849,7 +11849,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, counterparty_node_id: *counterparty_node_id, user_channel_id: channel.context().get_user_id(), @@ -12010,7 +12010,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, user_channel_id: chan.context().get_user_id(), @@ -12099,7 +12099,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let exited_quiescence = 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(), @@ -12182,7 +12182,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(), @@ -12334,7 +12334,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(), @@ -15167,16 +15167,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, ); } @@ -15463,7 +15463,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(), @@ -18119,8 +18119,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(); @@ -18138,7 +18139,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(), @@ -18267,7 +18268,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 05ccfa4709d..3444c82b47a 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2389,7 +2389,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 ); } @@ -3232,7 +3232,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 }, @@ -3261,7 +3261,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 8e382bd3059..8eb805f5ba5 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -120,10 +120,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 7279b8b9342..fb077a98e6c 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3052,7 +3052,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]) }; @@ -3082,12 +3082,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( @@ -3894,7 +3894,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) @@ -4030,7 +4030,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); @@ -4059,7 +4059,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, @@ -4414,7 +4414,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); @@ -4453,7 +4453,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, @@ -4461,7 +4461,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(); @@ -4477,12 +4477,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. @@ -5844,7 +5844,7 @@ fn test_splice_rbf_sequential() { 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]); @@ -5924,12 +5924,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 @@ -5999,7 +5999,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] { @@ -6016,12 +6016,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. @@ -6046,12 +6046,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]); @@ -6453,7 +6453,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 @@ -6728,16 +6728,16 @@ 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 1549b74dda7f9d8edf724887a73d5cd864a18960 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 32bf5d8a75a..e81e5b447fe 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7293,20 +7293,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 bc7490d97df15cb79bb9338049d08f59c953910d 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 | 52 ++++++++++++++++++------------ 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 8eb805f5ba5..decf8f2b62e 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -777,7 +777,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 fb077a98e6c..dd8ae3bfe58 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3766,11 +3766,11 @@ fn test_funding_contributed_splice_already_pending() { ) .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 @@ -3803,8 +3803,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!( @@ -3824,11 +3823,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"); } @@ -3921,14 +3919,26 @@ 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 + .splice_in_and_out_sync( + splice_in_amount, + vec![splice_out_output.clone()], + feerate, + FeeRate::MAX, + &wallet, + ) + .unwrap(); // First funding_contributed - sets up the quiescent action and queues STFU nodes[0] @@ -3973,10 +3983,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 { @@ -3984,15 +3994,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 dc0609d94a825905105c90beae73b747fc47211d 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 e81e5b447fe..33645b2c9cf 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6861,13 +6861,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) } @@ -6875,10 +6868,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. @@ -7056,48 +7045,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), + }), + } }}; } @@ -7127,21 +7097,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 { @@ -7285,21 +7250,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 @@ -7309,6 +7276,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(); } @@ -7326,12 +7305,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 2d41c3f8e882725074550263c23ad2a723b50287 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 9957205716c..d5da65f8b3a 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 f43c8920159611d00c5d82bbc76d20c29da58b51 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 7de125c18500b8d3d65709b06c3862839a487aec 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 | 10 ++- lightning/src/ln/channel.rs | 10 +++ lightning/src/ln/splicing_tests.rs | 98 ++++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 1 deletion(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 527670e1317..fa69b433093 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -860,6 +860,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") )); } diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 065c2dbb5ab..5315eeece52 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 { @@ -165,7 +171,7 @@ impl NegotiationFailureReason { | Self::NegotiationError { .. } | Self::ContributionInvalid | Self::FeeRateTooLow => true, - Self::LocallyAbandoned | Self::ChannelClosing => false, + Self::LocallyAbandoned | Self::ChannelClosing | Self::CannotInitiateRbf => false, } } } @@ -184,6 +190,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"), } } } @@ -201,6 +208,7 @@ impl_writeable_tlv_based_enum_upgradable!(NegotiationFailureReason, (10, LocallyAbandoned) => {}, (12, ChannelClosing) => {}, (14, FeeRateTooLow) => {}, + (16, 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 33645b2c9cf..8fa588992ba 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -14155,6 +14155,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 dd8ae3bfe58..576d26096ae 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -4871,6 +4871,104 @@ 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. + let rbf_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); + let _funding_contribution = + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + + // 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 62847a1107a65644f043750b68e433af3f8ac738 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 | 183 +++++++++++++++++++++++------ 2 files changed, 161 insertions(+), 52 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8fa588992ba..6ec9e2aab7d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -11569,7 +11569,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); @@ -11577,22 +11576,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 576d26096ae..49e1427ee38 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -675,9 +675,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); @@ -690,7 +696,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 @@ -727,29 +733,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); } @@ -787,18 +787,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); @@ -1468,7 +1468,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()); @@ -1496,7 +1496,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()); } @@ -1732,7 +1732,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 @@ -4561,13 +4561,109 @@ 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_contribution = + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + 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); + 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] @@ -5321,13 +5417,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( @@ -5352,14 +5453,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"); @@ -5618,13 +5719,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] @@ -5941,13 +6047,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] @@ -6309,7 +6420,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 { @@ -6386,7 +6497,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 {