From 5745fc25acbed341e96bee756098429d9ec5c976 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 17:56:42 -0500 Subject: [PATCH 01/18] 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 | 32 ++++- lightning/src/ln/functional_test_utils.rs | 9 +- lightning/src/ln/splicing_tests.rs | 151 ++++++++++++++++++---- 5 files changed, 303 insertions(+), 43 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 e6397aefbcb..e8fccc44c9c 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::BlockLocator; -use crate::events::{ClosureReason, FundingInfo}; +use crate::events::{ClosureReason, FundingInfo, NegotiationFailureReason}; use crate::ln::chan_utils; use crate::ln::chan_utils::{ get_commitment_transaction_number_obscure_factor, max_htlcs, second_stage_tx_fees_sat, @@ -3163,7 +3163,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 { @@ -7121,9 +7131,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, } @@ -7132,7 +7143,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, _ => { @@ -12594,7 +12605,10 @@ where self.validate_splice_contributions(our_funding_contribution, SignedAmount::ZERO) { 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() { @@ -12610,6 +12624,7 @@ where ); return Err(QuiescentError::FailSplice( self.splice_funding_failed_for(contribution), + NegotiationFailureReason::FeeRateTooLow, )); } } @@ -14051,9 +14066,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!( @@ -14168,7 +14192,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 1f32423507f..e1475b9f6f2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4169,6 +4169,7 @@ impl< user_channel_id: chan.context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); @@ -4475,6 +4476,7 @@ impl< user_channel_id: shutdown_res.user_channel_id, abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); @@ -4981,6 +4983,7 @@ impl< user_channel_id: chan.context.get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::LocallyAbandoned, }, None, )); @@ -6673,12 +6676,15 @@ impl< )); } }, - QuiescentError::FailSplice(SpliceFundingFailed { - funding_txo, - channel_type, - contributed_inputs, - contributed_outputs, - }) => { + QuiescentError::FailSplice( + SpliceFundingFailed { + funding_txo, + channel_type, + contributed_inputs, + contributed_outputs, + }, + reason, + ) => { let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { @@ -6687,6 +6693,7 @@ impl< user_channel_id, abandoned_funding_txo: funding_txo, channel_type, + reason, }, None, )); @@ -6840,7 +6847,7 @@ impl< "Channel {} already has a pending funding contribution", channel_id, ), - QuiescentError::FailSplice(_) => format!( + QuiescentError::FailSplice(..) => format!( "Channel {} cannot accept funding contribution", channel_id, ), @@ -11981,6 +11988,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ user_channel_id, abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type.clone(), + reason: events::NegotiationFailureReason::NegotiationError { + msg: format!("{:?}", err.err), + }, }, None, )); @@ -12317,6 +12327,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, )); @@ -12465,6 +12480,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, )); @@ -15541,6 +15557,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(), @@ -18169,6 +18186,7 @@ impl< user_channel_id: chan.context.get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::PeerDisconnected, }, None, )); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index b48d76d646d..b8ef5890899 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -19,8 +19,8 @@ use crate::chain::{BlockLocator, ChannelMonitorUpdateStatus, Confirm, Listen, Wa use crate::events::bump_transaction::sync::BumpTransactionEventHandlerSync; use crate::events::bump_transaction::BumpTransactionEvent; use crate::events::{ - ClaimedHTLC, ClosureReason, Event, FundingInfo, HTLCHandlingFailureType, PaidBolt12Invoice, - PathFailure, PaymentFailureReason, PaymentPurpose, + ClaimedHTLC, ClosureReason, Event, FundingInfo, HTLCHandlingFailureType, + NegotiationFailureReason, PaidBolt12Invoice, PathFailure, PaymentFailureReason, PaymentPurpose, }; use crate::ln::chan_utils::{ commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, TRUC_MAX_WEIGHT, @@ -3232,13 +3232,14 @@ pub fn expect_splice_pending_event<'a, 'b, 'c, 'd>( #[cfg(any(test, ldk_bench, feature = "_test_utils"))] pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( node: &'a Node<'b, 'c, 'd>, expected_channel_id: &ChannelId, - funding_contribution: FundingContribution, + funding_contribution: FundingContribution, expected_reason: NegotiationFailureReason, ) { let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match &events[0] { - Event::SpliceFailed { channel_id, .. } => { + Event::SpliceFailed { channel_id, reason, .. } => { assert_eq!(*expected_channel_id, *channel_id); + assert_eq!(expected_reason, *reason); }, _ => panic!("Unexpected event"), } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index b2cb1eda375..c2ce67d92e7 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, DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, @@ -27,6 +29,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; @@ -251,7 +254,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) }, } @@ -861,7 +869,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); @@ -912,7 +925,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); @@ -994,7 +1012,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. @@ -1078,7 +1103,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); @@ -2673,7 +2703,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. @@ -2744,7 +2781,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. @@ -2840,7 +2882,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); @@ -3130,8 +3181,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), } @@ -3150,7 +3202,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); } @@ -4123,7 +4180,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] @@ -4304,7 +4366,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. @@ -4339,7 +4406,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; @@ -4490,7 +4562,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, @@ -4498,7 +4575,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] { @@ -6387,7 +6467,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] { @@ -6466,8 +6549,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), } @@ -6507,7 +6591,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] { @@ -7044,7 +7131,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] @@ -7282,7 +7374,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), } } @@ -7378,7 +7473,12 @@ fn test_no_disconnect_after_splice_aborted() { // Abort the splice, which should clear the timer when exiting quiescence. nodes[0].node.abandon_splice(&channel_id, &node_id_1).unwrap(); - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::LocallyAbandoned, + ); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); let tx_abort = msg_events @@ -7450,7 +7550,12 @@ fn test_no_disconnect_after_quiescence_on_reconnect() { nodes[0].node.peer_disconnected(node_id_1); nodes[1].node.peer_disconnected(node_id_0); - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_channel_ready = (true, true); From ba5d59259e48ff4d2b49345c83c0306e6134a7f6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 28 Apr 2026 17:12:02 -0500 Subject: [PATCH 02/18] f - mark NegotiationError and CounterpartyAborted as not retriable --- lightning/src/events/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 18ae1813438..89f4c1dc27f 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -160,11 +160,12 @@ impl NegotiationFailureReason { match self { Self::Unknown | Self::PeerDisconnected - | Self::CounterpartyAborted { .. } - | Self::NegotiationError { .. } | Self::ContributionInvalid | Self::FeeRateTooLow => true, - Self::LocallyAbandoned | Self::ChannelClosing => false, + Self::CounterpartyAborted { .. } + | Self::NegotiationError { .. } + | Self::LocallyAbandoned + | Self::ChannelClosing => false, } } } From a32fcb757df128ece4b98d105ea8156983280f79 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 5 May 2026 13:09:45 -0500 Subject: [PATCH 03/18] f - use odd TLV variant IDs for forward compatibility Since `reason` is read as `upgradable_option`, odd variant IDs let older readers skip unknown future variants (falling back to `Unknown`) instead of failing with `UnknownRequiredFeature`. --- lightning/src/events/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 89f4c1dc27f..0c99ee02c79 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -189,18 +189,18 @@ impl core::fmt::Display for NegotiationFailureReason { } impl_writeable_tlv_based_enum_upgradable!(NegotiationFailureReason, - (0, Unknown) => {}, - (2, PeerDisconnected) => {}, - (4, CounterpartyAborted) => { + (1, Unknown) => {}, + (3, PeerDisconnected) => {}, + (5, CounterpartyAborted) => { (1, msg, required), }, - (6, NegotiationError) => { + (7, NegotiationError) => { (1, msg, required), }, - (8, ContributionInvalid) => {}, - (10, LocallyAbandoned) => {}, - (12, ChannelClosing) => {}, - (14, FeeRateTooLow) => {}, + (9, ContributionInvalid) => {}, + (11, LocallyAbandoned) => {}, + (13, ChannelClosing) => {}, + (15, FeeRateTooLow) => {}, ); /// Some information provided on receipt of payment depends on whether the payment received is a From 2aa10ff8d4e44424e9de161ad9e7a5e7b800633a Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 5 May 2026 13:23:27 -0500 Subject: [PATCH 04/18] f - rephrase tx_abort message per spec rationale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per BOLT 2's channel_reestablish rationale, next_funding's tx_abort branch is reached when the receiver did not sign and has discarded the funding transaction. The prior "may have been broadcast" wording was misleading — broadcast requires both signing steps to complete, which is incompatible with this path. --- lightning/src/ln/channel.rs | 2 +- lightning/src/ln/splicing_tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e8fccc44c9c..f810124dd9a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10399,7 +10399,7 @@ where tx_abort = Some(msgs::TxAbort { channel_id: self.context.channel_id(), data: - "No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() }); + "Signing was not completed for this funding transaction; it may be forgotten.".as_bytes().to_vec() }); } } if let Some(funding_txid) = retransmit_funding_commit_sig { diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index c2ce67d92e7..27abedb5f05 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -1017,7 +1017,7 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { &channel_id, funding_contribution, NegotiationFailureReason::CounterpartyAborted { - msg: UntrustedString("No active signing session. The associated funding transaction may have already been broadcast.".to_string()), + msg: UntrustedString("Signing was not completed for this funding transaction; it may be forgotten.".to_string()), }, ); From a5acce682e44ebb2d98605cb148a48515b813aff Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 18:20:35 -0500 Subject: [PATCH 05/18] 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 | 191 ++++++++++------------ lightning/src/ln/functional_test_utils.rs | 3 +- lightning/src/ln/funding.rs | 8 +- lightning/src/ln/splicing_tests.rs | 80 ++++----- 6 files changed, 173 insertions(+), 201 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 0c99ee02c79..5a52be026fb 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -25,6 +25,7 @@ use crate::blinded_path::payment::{ use crate::chain::transaction; use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS; use crate::ln::channelmanager::{InterceptId, PaymentId}; +use crate::ln::funding::FundingContribution; use crate::ln::msgs; use crate::ln::onion_utils::LocalHTLCFailureReason; use crate::ln::outbound_payment::RecipientOnionFields; @@ -1664,19 +1665,20 @@ pub enum Event { /// The witness script that is used to lock the channel's funding output to commitment transactions. new_funding_redeem_script: ScriptBuf, }, - /// Used to indicate that a splice for the given `channel_id` has failed. + /// Used to indicate that a splice negotiation round for the given `channel_id` has failed. /// - /// This event may be emitted if a splice fails after it has been initiated but prior to signing - /// any negotiated funding transaction. + /// Each splice attempt (initial or RBF) resolves to either [`Event::SplicePending`] on + /// success or this event on failure. Prior successfully negotiated splice transactions are + /// unaffected. /// - /// Any UTXOs contributed to be spent by the funding transaction may be reused and will be - /// given in `contributed_inputs`. + /// Any UTXOs contributed to the failed round that are not committed to a prior negotiated + /// splice transaction will be returned via a preceding [`Event::DiscardFunding`]. /// /// # Failure Behavior and Persistence /// This event will eventually be replayed after failures-to-handle (i.e., the event handler /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. SpliceFailed { - /// The `channel_id` of the channel for which the splice failed. + /// The `channel_id` of the channel for which the splice negotiation round failed. channel_id: ChannelId, /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound /// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels. @@ -1686,12 +1688,17 @@ pub enum Event { user_channel_id: u128, /// The `node_id` of the channel counterparty. counterparty_node_id: PublicKey, - /// The outpoint of the channel's splice funding transaction, if one was created. - abandoned_funding_txo: Option, - /// The features that this channel will operate with, if available. - channel_type: Option, /// The reason the splice negotiation failed. reason: NegotiationFailureReason, + /// The funding contribution from the failed negotiation round, if available. This can be + /// fed back to [`ChannelManager::funding_contributed`] to retry with the same parameters. + /// Alternatively, call [`ChannelManager::splice_channel`] to obtain a fresh + /// [`FundingTemplate`] and build a new contribution. + /// + /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + /// [`FundingTemplate`]: crate::ln::funding::FundingTemplate + contribution: Option, }, /// Used to indicate to the user that they can abandon the funding transaction and recycle the /// inputs for another purpose. @@ -2483,18 +2490,16 @@ impl Writeable for Event { ref channel_id, ref user_channel_id, ref counterparty_node_id, - ref abandoned_funding_txo, - ref channel_type, ref reason, + ref contribution, } => { 52u8.write(writer)?; write_tlv_fields!(writer, { (1, channel_id, required), - (3, channel_type, option), (5, user_channel_id, required), (7, counterparty_node_id, required), - (9, abandoned_funding_txo, option), (11, reason, required), + (13, contribution, option), }); }, // Note that, going forward, all new events must only write data inside of @@ -3135,20 +3140,18 @@ impl MaybeReadable for Event { let mut f = || { _init_and_read_len_prefixed_tlv_fields!(reader, { (1, channel_id, required), - (3, channel_type, option), (5, user_channel_id, required), (7, counterparty_node_id, required), - (9, abandoned_funding_txo, option), (11, reason, upgradable_option), + (13, contribution, option), }); Ok(Some(Event::SpliceFailed { channel_id: channel_id.0.unwrap(), user_channel_id: user_channel_id.0.unwrap(), counterparty_node_id: counterparty_node_id.0.unwrap(), - abandoned_funding_txo, - channel_type, reason: reason.unwrap_or(NegotiationFailureReason::Unknown), + contribution, })) }; f() diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f810124dd9a..17aacbaf0e5 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7017,17 +7017,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 { @@ -7037,15 +7051,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() @@ -7076,12 +7081,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 }) }) }}; } @@ -7112,6 +7115,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() { @@ -7122,10 +7126,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 e1475b9f6f2..46e84ad2df3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -61,8 +61,8 @@ use crate::ln::channel::QuiescentError; use crate::ln::channel::{ self, hold_time_since, Channel, ChannelError, ChannelUpdateStatus, DisconnectResult, FundedChannel, FundingTxSigned, InboundV1Channel, InteractiveTxMsgError, OutboundHop, - OutboundV1Channel, PendingV2Channel, ReconnectionMsg, ShutdownResult, SpliceFundingFailed, - StfuResponse, UpdateFulfillCommitFetch, WithChannelContext, + OutboundV1Channel, PendingV2Channel, ReconnectionMsg, ShutdownResult, StfuResponse, + UpdateFulfillCommitFetch, WithChannelContext, }; use crate::ln::channel_state::ChannelDetails; use crate::ln::funding::{FundingContribution, FundingTemplate}; @@ -4161,28 +4161,27 @@ impl< failed_htlcs = htlcs; if let Some(splice_funding_failed) = splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: *chan_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *chan_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: *chan_id, + funding_info, }, - }, - None, - )); + None, + )); + } } // We can send the `shutdown` message before updating the `ChannelMonitor` @@ -4469,27 +4468,26 @@ impl< )); if let Some(splice_funding_failed) = shutdown_res.splice_funding_failed.take() { + let (funding_info, contribution) = splice_funding_failed.into_parts(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: shutdown_res.channel_id, counterparty_node_id: shutdown_res.counterparty_node_id, user_channel_id: shutdown_res.user_channel_id, - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: shutdown_res.channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: shutdown_res.channel_id, + funding_info, }, - }, - None, - )); + None, + )); + } } if let Some(transaction) = shutdown_res.unbroadcasted_funding_tx { @@ -4975,28 +4973,27 @@ impl< }); if let Some(splice_funding_failed) = splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: *channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context.get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::LocallyAbandoned, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: *channel_id, + funding_info, }, - }, - None, - )); + None, + )); + } } Ok(()) @@ -6676,36 +6673,22 @@ impl< )); } }, - QuiescentError::FailSplice( - SpliceFundingFailed { - funding_txo, - channel_type, - contributed_inputs, - contributed_outputs, - }, - reason, - ) => { + QuiescentError::FailSplice(splice_funding_failed, reason) => { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id, counterparty_node_id, user_channel_id, - abandoned_funding_txo: funding_txo, - channel_type, reason, + contribution, }, None, )); - if !contributed_inputs.is_empty() || !contributed_outputs.is_empty() { + if let Some(funding_info) = funding_info { pending_events.push_back(( - events::Event::DiscardFunding { - channel_id, - funding_info: FundingInfo::Contribution { - inputs: contributed_inputs, - outputs: contributed_outputs, - }, - }, + events::Event::DiscardFunding { channel_id, funding_info }, None, )); } @@ -11980,30 +11963,26 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ user_channel_id: u128, ) -> MsgHandleErrInternal { if let Some(splice_funding_failed) = err.splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id, - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type.clone(), + contribution, reason: events::NegotiationFailureReason::NegotiationError { msg: format!("{:?}", err.err), }, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, - }, - }, - None, - )); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { channel_id, funding_info }, + None, + )); + } } MsgHandleErrInternal::from_chan_no_close(err.err, channel_id) } @@ -12319,14 +12298,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } if let Some(splice_funding_failed) = splice_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: msg.channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan_entry.get().context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::CounterpartyAborted { msg: UntrustedString( String::from_utf8_lossy(&msg.data).to_string(), @@ -12335,16 +12314,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: msg.channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: msg.channel_id, + funding_info, }, - }, - None, - )); + None, + )); + } } let holding_cell_res = if needs_holding_cell_release { @@ -12472,28 +12450,27 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ dropped_htlcs = htlcs; if let Some(splice_funding_failed) = splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { channel_id: msg.channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: msg.channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: msg.channel_id, + funding_info, }, - }, - None, - )); + None, + )); + } } if let Some(msg) = shutdown { @@ -15551,21 +15528,20 @@ impl< chan.peer_disconnected_is_resumable(&&logger); if let Some(splice_funding_failed) = splice_funding_failed { + let (funding_info, contribution) = splice_funding_failed.into_parts(); splice_failed_events.push(events::Event::SpliceFailed { channel_id: chan.context().channel_id(), counterparty_node_id, user_channel_id: chan.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, + contribution, reason: events::NegotiationFailureReason::PeerDisconnected, }); - splice_failed_events.push(events::Event::DiscardFunding { - channel_id: chan.context().channel_id(), - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, - }, - }); + if let Some(funding_info) = funding_info { + splice_failed_events.push(events::Event::DiscardFunding { + channel_id: chan.context().channel_id(), + funding_info, + }); + } } if is_resumable { @@ -18179,27 +18155,26 @@ impl< for peer_state in peer_states.iter() { for chan in peer_state.channel_by_id.values().filter_map(Channel::as_funded) { if let Some(splice_funding_failed) = chan.maybe_splice_funding_failed() { + let (funding_info, contribution) = splice_funding_failed.into_parts(); events.push_back(( events::Event::SpliceFailed { channel_id: chan.context.channel_id(), counterparty_node_id: chan.context.get_counterparty_node_id(), user_channel_id: chan.context.get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, reason: events::NegotiationFailureReason::PeerDisconnected, + contribution, }, None, )); - events.push_back(( - events::Event::DiscardFunding { - channel_id: chan.context().channel_id(), - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, + if let Some(funding_info) = funding_info { + events.push_back(( + events::Event::DiscardFunding { + channel_id: chan.context().channel_id(), + funding_info, }, - }, - None, - )); + None, + )); + } } } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index b8ef5890899..df161715152 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3237,9 +3237,10 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match &events[0] { - Event::SpliceFailed { channel_id, reason, .. } => { + Event::SpliceFailed { channel_id, reason, contribution, .. } => { assert_eq!(*expected_channel_id, *channel_id); assert_eq!(expected_reason, *reason); + assert_eq!(contribution.as_ref(), Some(&funding_contribution)); }, _ => panic!("Unexpected event"), } diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 20366fe772a..d0c24424e63 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -578,7 +578,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 } @@ -610,6 +611,11 @@ impl FundingContribution { .unwrap_or(Amount::ZERO) } + /// 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 27abedb5f05..f6d0ca126f3 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -209,8 +209,12 @@ pub fn do_initiate_rbf_splice_in<'a, 'b, 'c, 'd>( let node_id_counterparty = counterparty.node.get_our_node_id(); let funding_template = node.node.splice_channel(&channel_id, &node_id_counterparty).unwrap(); let wallet = WalletSync::new(Arc::clone(&node.wallet_source), node.logger); - let funding_contribution = - funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap(); + let funding_contribution = funding_template + .without_prior_contribution(feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(value_added) + .build() + .unwrap(); node.node .funding_contributed(&channel_id, &node_id_counterparty, funding_contribution.clone(), None) .unwrap(); @@ -3181,9 +3185,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), } @@ -4575,9 +4580,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), } @@ -4804,17 +4810,14 @@ fn test_splice_rbf_insufficient_feerate() { // Node 0 echoes tx_abort and exits quiescence, freeing the holding cell. nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); - // TODO: the RBF round's inputs are partially filtered against the prior round's committed - // UTXOs, so the DiscardFunding carries coin-selection-dependent residue. Revisit once - // #4514 lands to see if its semantics change what DiscardFunding contains here. + // The RBF round contributed the same inputs and outputs as the prior round, so after + // filtering against the prior round's committed UTXOs nothing remains to discard and + // `DiscardFunding` is suppressed; only `SpliceFailed` is emitted. let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2, "{events:?}"); + assert_eq!(events.len(), 1, "{events:?}"); assert!( matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) ); - assert!( - matches!(&events[1], Event::DiscardFunding { channel_id: cid, .. } if *cid == channel_id) - ); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 2, "{msg_events:?}"); @@ -4865,15 +4868,12 @@ fn test_splice_rbf_insufficient_feerate() { nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); let tx_abort_echo = get_event_msg!(nodes[0], MessageSendEvent::SendTxAbort, node_id_1); - // TODO: same as above — revisit once #4514 lands. + // As above: nothing remains after filtering, so `DiscardFunding` is suppressed. let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2); + assert_eq!(events.len(), 1); assert!( matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) ); - assert!( - matches!(&events[1], Event::DiscardFunding { channel_id: cid, .. } if *cid == channel_id) - ); nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); @@ -4958,26 +4958,13 @@ fn test_splice_rbf_insufficient_feerate_high() { nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); let tx_abort_echo = get_event_msg!(nodes[0], MessageSendEvent::SendTxAbort, node_id_1); - // TODO: the RBF round's inputs are fully filtered against the prior round's committed - // UTXOs, so this DiscardFunding is emitted with empty inputs and outputs. Once #4514 - // lands, a fully-drained DiscardFunding should be suppressed entirely — expect - // `events.len() == 1`. + // The RBF round's inputs and outputs are fully filtered against the prior round's + // committed UTXOs, so `DiscardFunding` is suppressed. let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2); + assert_eq!(events.len(), 1); assert!( matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) ); - match &events[1] { - Event::DiscardFunding { - channel_id: cid, - funding_info: FundingInfo::Contribution { inputs, outputs }, - } => { - assert_eq!(*cid, channel_id); - assert!(inputs.is_empty(), "Expected inputs filtered, got {inputs:?}"); - assert!(outputs.is_empty(), "Expected outputs filtered, got {outputs:?}"); - }, - other => panic!("Expected DiscardFunding with Contribution, got {other:?}"), - } nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); @@ -6463,24 +6450,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:?}"); @@ -6549,9 +6534,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), } @@ -6589,18 +6575,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); @@ -7374,9 +7357,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 97e04c6c6c3802004e1a80e59a8f52263556e383 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 5 May 2026 16:41:53 -0500 Subject: [PATCH 06/18] f - note that contributed inputs/outputs exclude prior rounds These fields hold only the unique-to-this-round contributions; the full set is reachable via the `contribution` field's helpers, so duplicating prior-round entries here would be redundant. --- lightning/src/ln/channel.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 17aacbaf0e5..21dc96b1e26 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7017,10 +7017,12 @@ pub struct SpliceFundingNegotiated { /// Information about a splice funding negotiation that has failed. pub struct SpliceFundingFailed { - /// UTXOs spent as inputs contributed to the splice transaction. + /// UTXOs spent as inputs contributed to the splice transaction. Excludes inputs already + /// contributed in prior rounds, which may be included in `contribution`. contributed_inputs: Vec, - /// Outputs contributed to the splice transaction. + /// Outputs contributed to the splice transaction. Excludes outputs already contributed + /// in prior rounds, which may be included in `contribution`. contributed_outputs: Vec, /// The funding contribution from the failed round, if available. From 98efbba10b53753cc4d97b718f8c827f495711b6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 5 May 2026 17:35:01 -0500 Subject: [PATCH 07/18] f - split RBF helper into amend default + at-feerate variant Make do_initiate_rbf_splice_in (and do_initiate_rbf_splice_in_and_out) amend the prior contribution with a pure feerate bump. Every caller was passing the same `added_value` as the initial splice; with amend semantics the parameter is unused, so drop it. Add do_initiate_splice_in_at_feerate for first-contribution cases that need an explicit feerate (used by the tiebreak tests' node[1]). --- lightning/src/ln/splicing_tests.rs | 77 ++++++++++++++++-------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index f6d0ca126f3..d437d22a3e1 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -204,17 +204,12 @@ pub fn do_initiate_splice_in<'a, 'b, 'c, 'd>( pub fn do_initiate_rbf_splice_in<'a, 'b, 'c, 'd>( node: &'a Node<'b, 'c, 'd>, counterparty: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - value_added: Amount, feerate: FeeRate, + feerate: FeeRate, ) -> FundingContribution { let node_id_counterparty = counterparty.node.get_our_node_id(); let funding_template = node.node.splice_channel(&channel_id, &node_id_counterparty).unwrap(); - let wallet = WalletSync::new(Arc::clone(&node.wallet_source), node.logger); - let funding_contribution = funding_template - .without_prior_contribution(feerate, FeeRate::MAX) - .with_coin_selection_source_sync(&wallet) - .add_value(value_added) - .build() - .unwrap(); + let funding_contribution = + funding_template.with_prior_contribution(feerate, FeeRate::MAX).build().unwrap(); node.node .funding_contributed(&channel_id, &node_id_counterparty, funding_contribution.clone(), None) .unwrap(); @@ -223,15 +218,12 @@ pub fn do_initiate_rbf_splice_in<'a, 'b, 'c, 'd>( pub fn do_initiate_rbf_splice_in_and_out<'a, 'b, 'c, 'd>( node: &'a Node<'b, 'c, 'd>, counterparty: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - value_added: Amount, outputs: Vec, feerate: FeeRate, + outputs: Vec, feerate: FeeRate, ) -> FundingContribution { let node_id_counterparty = counterparty.node.get_our_node_id(); let funding_template = node.node.splice_channel(&channel_id, &node_id_counterparty).unwrap(); - let wallet = WalletSync::new(Arc::clone(&node.wallet_source), node.logger); let funding_contribution = funding_template - .without_prior_contribution(feerate, FeeRate::MAX) - .with_coin_selection_source_sync(&wallet) - .add_value(value_added) + .with_prior_contribution(feerate, FeeRate::MAX) .add_outputs(outputs) .build() .unwrap(); @@ -241,6 +233,22 @@ pub fn do_initiate_rbf_splice_in_and_out<'a, 'b, 'c, 'd>( funding_contribution } +pub fn do_initiate_splice_in_at_feerate<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, + value_added: Amount, feerate: FeeRate, +) -> FundingContribution { + let node_id_acceptor = acceptor.node.get_our_node_id(); + let funding_template = initiator.node.splice_channel(&channel_id, &node_id_acceptor).unwrap(); + let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); + let funding_contribution = + funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap(); + initiator + .node + .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) + .unwrap(); + funding_contribution +} + pub fn initiate_splice_out<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, outputs: Vec, @@ -4640,7 +4648,7 @@ fn test_splice_rbf_acceptor_basic() { let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); // Steps 4-8: STFU exchange → tx_init_rbf → tx_ack_rbf. complete_rbf_handshake(&nodes[0], &nodes[1]); @@ -4699,7 +4707,7 @@ fn test_splice_rbf_at_high_feerate() { provide_utxo_reserves(&nodes, 2, added_value * 2); let high_feerate = FeeRate::from_sat_per_kwu(1000); let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, high_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, high_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], @@ -4720,7 +4728,7 @@ fn test_splice_rbf_at_high_feerate() { funding_template.min_rbf_feerate().unwrap() }; let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], @@ -4784,7 +4792,7 @@ fn test_splice_rbf_insufficient_feerate() { // Node 0 initiates a proper RBF but we tamper the feerate to be insufficient. provide_utxo_reserves(&nodes, 2, added_value * 2); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -4850,7 +4858,7 @@ fn test_splice_rbf_insufficient_feerate() { // Node 0 initiates another proper RBF but we tamper the feerate to the 25/24 value. provide_utxo_reserves(&nodes, 2, added_value * 2); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -4880,7 +4888,7 @@ fn test_splice_rbf_insufficient_feerate() { // Acceptor-side: prev + 25 = 278 satisfies the combined BIP125 rule and is accepted. provide_utxo_reserves(&nodes, 2, added_value * 2); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -4921,7 +4929,7 @@ fn test_splice_rbf_insufficient_feerate_high() { provide_utxo_reserves(&nodes, 2, added_value * 2); let high_feerate = FeeRate::from_sat_per_kwu(1000); let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, high_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, high_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], @@ -4941,7 +4949,7 @@ fn test_splice_rbf_insufficient_feerate_high() { provide_utxo_reserves(&nodes, 2, added_value * 2); let min_rbf_feerate = FeeRate::from_sat_per_kwu(1041); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -4971,7 +4979,7 @@ fn test_splice_rbf_insufficient_feerate_high() { // Feerate 1041 satisfies both rules — accepted. provide_utxo_reserves(&nodes, 2, added_value * 2); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, min_rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); @@ -5263,7 +5271,7 @@ fn test_splice_rbf_not_quiescence_initiator() { let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); // STFU exchange: node 0 initiates quiescence. let stfu_init = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); @@ -5373,10 +5381,10 @@ pub fn do_test_splice_rbf_tiebreak( // Node 0 calls splice_channel + funding_contributed. let node_0_funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate_0); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate_0); // Node 1 calls splice_channel + funding_contributed. - let node_1_funding_contribution = do_initiate_rbf_splice_in( + let node_1_funding_contribution = do_initiate_splice_in_at_feerate( &nodes[1], &nodes[0], channel_id, @@ -5777,7 +5785,7 @@ fn test_splice_rbf_acceptor_recontributes() { let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let rbf_funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); // Steps 6-9: STFU exchange → tx_init_rbf → tx_ack_rbf. // Node 1 should re-contribute via our_prior_contribution. @@ -5900,7 +5908,7 @@ fn test_splice_rbf_after_counterparty_rbf_aborted() { let rbf_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); let _rbf_funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); let tx_ack_rbf = complete_rbf_handshake(&nodes[0], &nodes[1]); assert!(tx_ack_rbf.funding_output_contribution.is_some()); @@ -6095,7 +6103,7 @@ fn test_splice_rbf_sequential() { let rbf_feerate_1 = FeeRate::from_sat_per_kwu(feerate_1_sat_per_kwu); let funding_contribution_1 = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate_1); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate_1); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( @@ -6115,7 +6123,7 @@ fn test_splice_rbf_sequential() { let rbf_feerate_2 = FeeRate::from_sat_per_kwu(feerate_2_sat_per_kwu); let funding_contribution_2 = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate_2); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate_2); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( @@ -6438,7 +6446,7 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let _rbf_funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); let tx_ack_rbf = complete_rbf_handshake(&nodes[0], &nodes[1]); assert!( @@ -6518,7 +6526,6 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { &nodes[0], &nodes[1], channel_id, - added_value, vec![splice_out_output.clone()], rbf_feerate, ); @@ -6567,7 +6574,7 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { let rbf_feerate_2 = FeeRate::from_sat_per_kwu(feerate_1_sat_per_kwu); let _funding_contribution_2 = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate_2); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate_2); complete_rbf_handshake(&nodes[0], &nodes[1]); // Disconnect again to clean up the in-progress interactive TX negotiation. @@ -7250,7 +7257,7 @@ fn test_splice_rbf_rejects_low_feerate_after_several_attempts() { provide_utxo_reserves(&nodes, 2, added_value * 2); let rbf_feerate = FeeRate::from_sat_per_kwu(feerate); let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], @@ -7271,7 +7278,7 @@ fn test_splice_rbf_rejects_low_feerate_after_several_attempts() { provide_utxo_reserves(&nodes, 2, added_value * 2); let rbf_feerate = FeeRate::from_sat_per_kwu(next_feerate); let _contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu_0); let stfu_1 = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); @@ -7321,7 +7328,7 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { provide_utxo_reserves(&nodes, 2, added_value * 2); let rbf_feerate = FeeRate::from_sat_per_kwu(feerate); let contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, rbf_feerate); complete_rbf_handshake(&nodes[0], &nodes[1]); complete_interactive_funding_negotiation( &nodes[0], From 37351c27e7427d97e7030c3a2e003520b377089e Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 18:44:24 -0500 Subject: [PATCH 08/18] 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 | 158 ++++++++++++---------- lightning/src/ln/functional_test_utils.rs | 18 +-- lightning/src/ln/splicing_tests.rs | 50 +++---- 3 files changed, 120 insertions(+), 106 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 46e84ad2df3..48fa7e64c8b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4163,6 +4163,15 @@ impl< if let Some(splice_funding_failed) = splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let mut pending_events = self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: *chan_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: *chan_id, @@ -4173,15 +4182,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *chan_id, - funding_info, - }, - None, - )); - } } // We can send the `shutdown` message before updating the `ChannelMonitor` @@ -4469,6 +4469,15 @@ impl< if let Some(splice_funding_failed) = shutdown_res.splice_funding_failed.take() { let (funding_info, contribution) = splice_funding_failed.into_parts(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: shutdown_res.channel_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: shutdown_res.channel_id, @@ -4479,15 +4488,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: shutdown_res.channel_id, - funding_info, - }, - None, - )); - } } if let Some(transaction) = shutdown_res.unbroadcasted_funding_tx { @@ -4975,6 +4975,15 @@ impl< if let Some(splice_funding_failed) = splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: *channel_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: *channel_id, @@ -4985,15 +4994,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *channel_id, - funding_info, - }, - None, - )); - } } Ok(()) @@ -6676,6 +6676,12 @@ impl< QuiescentError::FailSplice(splice_funding_failed, reason) => { let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { channel_id, funding_info }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id, @@ -6686,12 +6692,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { channel_id, funding_info }, - None, - )); - } }, } } @@ -11965,6 +11965,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(splice_funding_failed) = err.splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events + .push_back((events::Event::DiscardFunding { channel_id, funding_info }, None)); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id, @@ -11977,12 +11981,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { channel_id, funding_info }, - None, - )); - } } MsgHandleErrInternal::from_chan_no_close(err.err, channel_id) } @@ -12300,6 +12298,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(splice_funding_failed) = splice_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let pending_events = &mut self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: msg.channel_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: msg.channel_id, @@ -12314,15 +12321,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: msg.channel_id, - funding_info, - }, - None, - )); - } } let holding_cell_res = if needs_holding_cell_release { @@ -12452,6 +12450,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(splice_funding_failed) = splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); let mut pending_events = self.pending_events.lock().unwrap(); + if let Some(funding_info) = funding_info { + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: msg.channel_id, + funding_info, + }, + None, + )); + } pending_events.push_back(( events::Event::SpliceFailed { channel_id: msg.channel_id, @@ -12462,15 +12469,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: msg.channel_id, - funding_info, - }, - None, - )); - } } if let Some(msg) = shutdown { @@ -15253,6 +15251,22 @@ impl< self.process_pending_events(&event_handler); let collected_events = events.into_inner(); + // When both DiscardFunding and SpliceFailed are emitted for the same channel, + // DiscardFunding must come first so that inputs are unlocked before any retry. + // Each pair is emitted adjacently under a single lock, so checking adjacent + // events is sufficient. + for window in collected_events.windows(2) { + if let events::Event::SpliceFailed { channel_id, .. } = &window[0] { + if let events::Event::DiscardFunding { channel_id: cid, .. } = &window[1] { + assert!( + channel_id != cid, + "DiscardFunding must precede SpliceFailed for channel {}", + channel_id, + ); + } + } + } + // To expand the coverage and make sure all events are properly serialised and deserialised, // we test all generated events round-trip: for event in &collected_events { @@ -15529,6 +15543,12 @@ impl< if let Some(splice_funding_failed) = splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); + if let Some(funding_info) = funding_info { + splice_failed_events.push(events::Event::DiscardFunding { + channel_id: chan.context().channel_id(), + funding_info, + }); + } splice_failed_events.push(events::Event::SpliceFailed { channel_id: chan.context().channel_id(), counterparty_node_id, @@ -15536,12 +15556,6 @@ impl< contribution, reason: events::NegotiationFailureReason::PeerDisconnected, }); - if let Some(funding_info) = funding_info { - splice_failed_events.push(events::Event::DiscardFunding { - channel_id: chan.context().channel_id(), - funding_info, - }); - } } if is_resumable { @@ -18156,6 +18170,15 @@ impl< for chan in peer_state.channel_by_id.values().filter_map(Channel::as_funded) { if let Some(splice_funding_failed) = chan.maybe_splice_funding_failed() { let (funding_info, contribution) = splice_funding_failed.into_parts(); + if let Some(funding_info) = funding_info { + events.push_back(( + events::Event::DiscardFunding { + channel_id: chan.context().channel_id(), + funding_info, + }, + None, + )); + } events.push_back(( events::Event::SpliceFailed { channel_id: chan.context.channel_id(), @@ -18166,15 +18189,6 @@ impl< }, None, )); - if let Some(funding_info) = funding_info { - events.push_back(( - events::Event::DiscardFunding { - channel_id: chan.context().channel_id(), - funding_info, - }, - None, - )); - } } } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index df161715152..c5b1104cc64 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3237,18 +3237,10 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match &events[0] { - Event::SpliceFailed { channel_id, reason, contribution, .. } => { - assert_eq!(*expected_channel_id, *channel_id); - assert_eq!(expected_reason, *reason); - assert_eq!(contribution.as_ref(), Some(&funding_contribution)); - }, - _ => panic!("Unexpected event"), - } - match &events[1] { Event::DiscardFunding { funding_info, .. } => { if let FundingInfo::Contribution { inputs, outputs } = &funding_info { let (expected_inputs, expected_outputs) = - funding_contribution.into_contributed_inputs_and_outputs(); + funding_contribution.clone().into_contributed_inputs_and_outputs(); assert_eq!(*inputs, expected_inputs); assert_eq!(*outputs, expected_outputs); } else { @@ -3257,6 +3249,14 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( }, _ => panic!("Unexpected event"), } + match &events[1] { + Event::SpliceFailed { channel_id, reason, contribution, .. } => { + assert_eq!(*expected_channel_id, *channel_id); + assert_eq!(expected_reason, *reason); + assert_eq!(contribution.as_ref(), Some(&funding_contribution)); + }, + _ => panic!("Unexpected event"), + } } #[cfg(any(test, ldk_bench, feature = "_test_utils"))] diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index d437d22a3e1..f3264efc315 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3193,14 +3193,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 }, .. @@ -3214,6 +3206,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], @@ -4588,14 +4588,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 }, .. @@ -4605,6 +4597,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]); @@ -6537,18 +6537,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 }, .. @@ -6561,6 +6553,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 e28f74e414ece1a9118326f1957f778d9c192518 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 18:58:50 -0500 Subject: [PATCH 09/18] Rename SplicePending and SpliceFailed events Rename Event::SplicePending to Event::SpliceNegotiated and Event::SpliceFailed to Event::SpliceNegotiationFailed. These names better reflect the per-round semantics: each negotiation attempt resolves to one of these two outcomes, independent of the overall splice lifecycle. Co-Authored-By: Claude Opus 4.6 (1M context) --- fuzz/src/chanmon_consistency.rs | 4 +- fuzz/src/full_stack.rs | 4 +- lightning/src/events/mod.rs | 16 +++--- lightning/src/ln/async_signer_tests.rs | 8 +-- lightning/src/ln/channel.rs | 10 ++-- lightning/src/ln/channelmanager.rs | 53 +++++++++--------- lightning/src/ln/functional_test_utils.rs | 6 +-- lightning/src/ln/funding.rs | 4 +- lightning/src/ln/splicing_tests.rs | 54 +++++++++---------- .../4388-splice-failed-discard-funding.txt | 8 +-- 10 files changed, 84 insertions(+), 83 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 655fb76200b..24eaf6f0e1e 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -2092,7 +2092,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, @@ -2104,7 +2104,7 @@ pub fn do_test(data: &[u8], out: Out) { assert_eq!(new_funding_txo.txid, splice_tx.compute_txid()); chain_state.add_pending_tx(splice_tx); }, - events::Event::SpliceFailed { .. } => {}, + events::Event::SpliceNegotiationFailed { .. } => {}, events::Event::DiscardFunding { funding_info: events::FundingInfo::Contribution { .. } | events::FundingInfo::Tx { .. }, diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 405d615e6f0..e79bef7c5ec 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1137,10 +1137,10 @@ pub fn do_test(mut data: &[u8], logger: &Arc signed_tx, ); }, - Event::SplicePending { .. } => { + Event::SpliceNegotiated { .. } => { // Splice negotiation completed, waiting for confirmation }, - Event::SpliceFailed { .. } => { + Event::SpliceNegotiationFailed { .. } => { // Splice failed, inputs can be re-spent }, Event::OpenChannelRequest { diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 5a52be026fb..9d00273cb46 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1646,8 +1646,8 @@ pub enum Event { /// # Failure Behavior and Persistence /// This event will eventually be replayed after failures-to-handle (i.e., the event handler /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. - SplicePending { - /// The `channel_id` of the channel that has a pending splice funding transaction. + SpliceNegotiated { + /// The `channel_id` of the channel with the negotiated splice funding transaction. channel_id: ChannelId, /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound /// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels. @@ -1667,7 +1667,7 @@ pub enum Event { }, /// Used to indicate that a splice negotiation round for the given `channel_id` has failed. /// - /// Each splice attempt (initial or RBF) resolves to either [`Event::SplicePending`] on + /// Each splice attempt (initial or RBF) resolves to either [`Event::SpliceNegotiated`] on /// success or this event on failure. Prior successfully negotiated splice transactions are /// unaffected. /// @@ -1677,7 +1677,7 @@ pub enum Event { /// # Failure Behavior and Persistence /// This event will eventually be replayed after failures-to-handle (i.e., the event handler /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. - SpliceFailed { + SpliceNegotiationFailed { /// The `channel_id` of the channel for which the splice negotiation round failed. channel_id: ChannelId, /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound @@ -2468,7 +2468,7 @@ impl Writeable for Event { // We never write out FundingTransactionReadyForSigning events as they will be regenerated when // necessary. }, - &Event::SplicePending { + &Event::SpliceNegotiated { ref channel_id, ref user_channel_id, ref counterparty_node_id, @@ -2486,7 +2486,7 @@ impl Writeable for Event { (11, new_funding_redeem_script, required), }); }, - &Event::SpliceFailed { + &Event::SpliceNegotiationFailed { ref channel_id, ref user_channel_id, ref counterparty_node_id, @@ -3125,7 +3125,7 @@ impl MaybeReadable for Event { (11, new_funding_redeem_script, required), }); - Ok(Some(Event::SplicePending { + Ok(Some(Event::SpliceNegotiated { channel_id: channel_id.0.unwrap(), user_channel_id: user_channel_id.0.unwrap(), counterparty_node_id: counterparty_node_id.0.unwrap(), @@ -3146,7 +3146,7 @@ impl MaybeReadable for Event { (13, contribution, option), }); - Ok(Some(Event::SpliceFailed { + Ok(Some(Event::SpliceNegotiationFailed { channel_id: channel_id.0.unwrap(), user_channel_id: user_channel_id.0.unwrap(), counterparty_node_id: counterparty_node_id.0.unwrap(), diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index f238c1db060..ae73dd830ac 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -1647,8 +1647,8 @@ fn test_async_splice_initial_commit_sig() { get_event_msg!(initiator, MessageSendEvent::SendTxSignatures, acceptor_node_id); acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); - let _ = get_event!(initiator, Event::SplicePending); - let _ = get_event!(acceptor, Event::SplicePending); + let _ = get_event!(initiator, Event::SpliceNegotiated); + let _ = get_event!(acceptor, Event::SpliceNegotiated); } #[test] @@ -1739,6 +1739,6 @@ fn test_async_splice_initial_commit_sig_waits_for_monitor_before_tx_signatures() get_event_msg!(initiator, MessageSendEvent::SendTxSignatures, acceptor_node_id); acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); - let _ = get_event!(initiator, Event::SplicePending); - let _ = get_event!(acceptor, Event::SplicePending); + let _ = get_event!(initiator, Event::SpliceNegotiated); + let _ = get_event!(acceptor, Event::SpliceNegotiated); } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 21dc96b1e26..08972249e05 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, } @@ -1255,7 +1255,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, } @@ -1263,7 +1263,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, } @@ -7031,7 +7031,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() { @@ -12378,7 +12378,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 48fa7e64c8b..062047c31cc 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4173,7 +4173,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: *chan_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), @@ -4479,7 +4479,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: shutdown_res.channel_id, counterparty_node_id: shutdown_res.counterparty_node_id, user_channel_id: shutdown_res.user_channel_id, @@ -4985,7 +4985,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: *channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context.get_user_id(), @@ -6683,7 +6683,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id, counterparty_node_id, user_channel_id, @@ -6741,14 +6741,14 @@ impl< /// # Events /// /// Calling this method will commence the process of creating a new funding transaction for the - /// channel. Once the funding transaction has been constructed, an [`Event::SplicePending`] + /// channel. Once the funding transaction has been constructed, an [`Event::SpliceNegotiated`] /// will be emitted. At this point, any inputs contributed to the splice can only be re-spent /// if an [`Event::DiscardFunding`] is seen. /// - /// If any failures occur while negotiating the funding transaction, an [`Event::SpliceFailed`] - /// will be emitted. Any contributed inputs no longer used will be included in an - /// [`Event::DiscardFunding`] and thus can be re-spent. If a [`FundingTemplate`] was obtained - /// while a previous splice was still being negotiated, its + /// If any failures occur while negotiating the funding transaction, an + /// [`Event::SpliceNegotiationFailed`] will be emitted. Any contributed inputs no longer used + /// will be included in an [`Event::DiscardFunding`] and thus can be re-spent. If a + /// [`FundingTemplate`] was obtained while a previous splice was still being negotiated, its /// [`min_rbf_feerate`][FundingTemplate::min_rbf_feerate] may be stale after the failure. /// Call [`ChannelManager::splice_channel`] again to get a fresh template. /// @@ -6967,7 +6967,7 @@ impl< } if let Some(splice_negotiated) = splice_negotiated { self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { + events::Event::SpliceNegotiated { channel_id: *channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), @@ -11129,7 +11129,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(), @@ -11970,7 +11970,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ .push_back((events::Event::DiscardFunding { channel_id, funding_info }, None)); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id, @@ -12222,7 +12222,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let needs_holding_cell_release = splice_negotiated.is_some(); if let Some(splice_negotiated) = splice_negotiated { self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { + events::Event::SpliceNegotiated { channel_id: msg.channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context.get_user_id(), @@ -12308,7 +12308,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(), @@ -12460,7 +12460,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(), @@ -15251,16 +15251,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, ); } @@ -15549,7 +15549,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(), @@ -18161,8 +18161,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(); @@ -18180,7 +18181,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(), @@ -18309,7 +18310,7 @@ impl< (23, self.best_block.read().unwrap().previous_blocks, required), }); - // Remove the SpliceFailed and DiscardFunding events added earlier. + // Remove the SpliceNegotiationFailed and DiscardFunding events added earlier. events.truncate(event_count); Ok(()) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index c5b1104cc64..f89fdd0572b 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2378,7 +2378,7 @@ pub fn check_closed_events(node: &Node, expected_close_events: &[ExpectedCloseEv discard_events_count ); assert_eq!( - events.iter().filter(|e| matches!(e, Event::SpliceFailed { .. },)).count(), + events.iter().filter(|e| matches!(e, Event::SpliceNegotiationFailed { .. },)).count(), splice_events_count ); } @@ -3221,7 +3221,7 @@ pub fn expect_splice_pending_event<'a, 'b, 'c, 'd>( let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match &events[0] { - crate::events::Event::SplicePending { channel_id, counterparty_node_id, .. } => { + crate::events::Event::SpliceNegotiated { channel_id, counterparty_node_id, .. } => { assert_eq!(*expected_counterparty_node_id, *counterparty_node_id); *channel_id }, @@ -3250,7 +3250,7 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( _ => panic!("Unexpected event"), } match &events[1] { - Event::SpliceFailed { channel_id, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id, reason, contribution, .. } => { assert_eq!(*expected_channel_id, *channel_id); assert_eq!(expected_reason, *reason); assert_eq!(contribution.as_ref(), Some(&funding_contribution)); diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index d0c24424e63..6c3a9d81e79 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -121,10 +121,10 @@ pub enum FundingContributionError { /// /// Note: [`FundingTemplate::min_rbf_feerate`] may be derived from an in-progress /// negotiation that later aborts, leaving a stale (higher than necessary) minimum. If - /// this error occurs after receiving [`Event::SpliceFailed`], call + /// this error occurs after receiving [`Event::SpliceNegotiationFailed`], call /// [`ChannelManager::splice_channel`] again to get a fresh template. /// - /// [`Event::SpliceFailed`]: crate::events::Event::SpliceFailed + /// [`Event::SpliceNegotiationFailed`]: crate::events::Event::SpliceNegotiationFailed /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel FeeRateBelowRbfMinimum { /// The requested feerate. diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index f3264efc315..d242545d80b 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3177,7 +3177,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]) }; @@ -3207,12 +3207,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( @@ -4015,7 +4015,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) @@ -4151,7 +4151,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); @@ -4180,7 +4180,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, @@ -4535,7 +4535,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); @@ -4574,7 +4574,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, @@ -4582,7 +4582,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(); @@ -4598,12 +4598,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. @@ -4820,11 +4820,11 @@ fn test_splice_rbf_insufficient_feerate() { // The RBF round contributed the same inputs and outputs as the prior round, so after // filtering against the prior round's committed UTXOs nothing remains to discard and - // `DiscardFunding` is suppressed; only `SpliceFailed` is emitted. + // `DiscardFunding` is suppressed; only `SpliceNegotiationFailed` is emitted. let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); assert!( - matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) + matches!(&events[0], Event::SpliceNegotiationFailed { channel_id: cid, .. } if *cid == channel_id) ); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); @@ -4880,7 +4880,7 @@ fn test_splice_rbf_insufficient_feerate() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); assert!( - matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) + matches!(&events[0], Event::SpliceNegotiationFailed { channel_id: cid, .. } if *cid == channel_id) ); nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); @@ -4971,7 +4971,7 @@ fn test_splice_rbf_insufficient_feerate_high() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); assert!( - matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) + matches!(&events[0], Event::SpliceNegotiationFailed { channel_id: cid, .. } if *cid == channel_id) ); nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); @@ -6383,7 +6383,7 @@ fn test_splice_rbf_amends_prior_net_negative_contribution_request() { fn test_splice_rbf_acceptor_contributes_then_disconnects() { // When both nodes contribute to a splice and the initiator RBFs (with the acceptor // re-contributing via prior contribution), disconnecting mid-interactive-TX should emit - // SpliceFailed + DiscardFunding for both nodes so each can reclaim their UTXOs. + // SpliceNegotiationFailed + DiscardFunding for both nodes so each can reclaim their UTXOs. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -6463,12 +6463,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 @@ -6537,7 +6537,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] { @@ -6554,12 +6554,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. @@ -6584,12 +6584,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]); @@ -6993,7 +6993,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 @@ -7359,17 +7359,17 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let result = nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution, None); assert!(result.is_err(), "Expected rejection for low feerate: {:?}", result); - // SpliceFailed is emitted. DiscardFunding is not emitted because all inputs/outputs + // SpliceNegotiationFailed is emitted. DiscardFunding is not emitted because all inputs/outputs // are filtered out (same UTXOs reused for RBF, still committed to the prior splice tx). let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::FeeRateTooLow); assert!(contribution.is_some()); }, - other => panic!("Expected SpliceFailed, got {:?}", other), + other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), } } diff --git a/pending_changelog/4388-splice-failed-discard-funding.txt b/pending_changelog/4388-splice-failed-discard-funding.txt index 64fc4ab4e26..67680f49cb1 100644 --- a/pending_changelog/4388-splice-failed-discard-funding.txt +++ b/pending_changelog/4388-splice-failed-discard-funding.txt @@ -1,21 +1,21 @@ # API Updates - * `Event::SpliceFailed` no longer carries `contributed_inputs` or `contributed_outputs` fields. + * `Event::SpliceNegotiationFailed` no longer carries `contributed_inputs` or `contributed_outputs` fields. Instead, a separate `Event::DiscardFunding` event with `FundingInfo::Contribution` is emitted for UTXO cleanup. * `Event::DiscardFunding` with `FundingInfo::Contribution` is also emitted without a - corresponding `Event::SpliceFailed` when `ChannelManager::funding_contributed` returns an + corresponding `Event::SpliceNegotiationFailed` when `ChannelManager::funding_contributed` returns an error (e.g., channel or peer not found, wrong channel state, duplicate contribution). # Backwards Compatibility * Older serializations that included `contributed_inputs` and `contributed_outputs` in - `SpliceFailed` will have those fields silently ignored on deserialization (they were odd TLV + `SpliceNegotiationFailed` will have those fields silently ignored on deserialization (they were odd TLV fields). A `DiscardFunding` event will not be produced when reading these older serializations. # Forward Compatibility * Downgrading will not set the removed `contributed_inputs`/`contributed_outputs` fields on - `SpliceFailed`, so older code expecting those fields will see empty vectors for splice + `SpliceNegotiationFailed`, so older code expecting those fields will see empty vectors for splice failures. From 2a563d828598cd25637aa37e275d45d5c89d1523 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 16:05:31 -0500 Subject: [PATCH 10/18] 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 08972249e05..e572416117e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7283,20 +7283,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 b77b315906523d7a4d1c028c5b87d8967d62db2f Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 18:52:27 -0500 Subject: [PATCH 11/18] Fix output filtering in into_unique_contributions Filter outputs by script_pubkey rather than full TxOut equality. Outputs reusing the same address as a prior round are still considered committed even if the value differs (e.g., different change amounts across RBF rounds with different feerates). Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/funding.rs | 2 +- lightning/src/ln/splicing_tests.rs | 50 ++++++++++++++++++------------ 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 6c3a9d81e79..d673ce1b02f 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -755,7 +755,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 d242545d80b..c6e685f441f 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3889,11 +3889,11 @@ fn test_funding_contributed_splice_already_pending() { .build() .unwrap(); - // Initiate a second splice with a DIFFERENT output to test that different outputs - // are included in DiscardFunding (not filtered out) + // Initiate a second splice with a DIFFERENT output (different script_pubkey) to test that + // non-overlapping outputs are included in DiscardFunding (not filtered out). let second_splice_out = TxOut { - value: Amount::from_sat(6_000), // Different amount - script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())), + value: Amount::from_sat(6_000), + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }; // Clear UTXOs and add a LARGER one for the second contribution to ensure @@ -3924,8 +3924,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!( @@ -3945,11 +3944,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"); } @@ -4042,14 +4040,24 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { let first_contribution = funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); - // Build second contribution with different UTXOs so inputs/outputs don't overlap + // Build second contribution with different UTXOs and a splice-out output using a different + // script_pubkey (node 1's address) so it survives script_pubkey-based filtering. nodes[0].wallet_source.clear_utxos(); provide_utxo_reserves(&nodes, 1, splice_in_amount * 3); + let splice_out_output = TxOut { + value: Amount::from_sat(1_000), + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), + }; let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let second_contribution = - funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); + let second_contribution = funding_template + .without_prior_contribution(feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(splice_in_amount) + .add_outputs(vec![splice_out_output.clone()]) + .build() + .unwrap(); // First funding_contributed - sets up the quiescent action and queues STFU nodes[0] @@ -4094,10 +4102,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 { @@ -4105,15 +4113,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 53d295478ebb21fb4e870a91653305c954616f73 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 16:42:20 -0500 Subject: [PATCH 12/18] 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 e572416117e..48fb02023c1 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6849,13 +6849,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) } @@ -6863,10 +6856,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. @@ -7046,48 +7035,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), + }), + } }}; } @@ -7117,21 +7087,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 { @@ -7275,21 +7240,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 @@ -7299,6 +7266,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(); } @@ -7316,12 +7295,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 7a5ee0d0c739f95712fc19ebcfac273dc81ad562 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 17:01:30 -0500 Subject: [PATCH 13/18] Remove unused NegotiationError and contributed_inputs_and_outputs methods Now that splice_funding_failed_for! derives inputs and outputs from FundingContribution directly, remove the unused NegotiationError struct and into_negotiation_error methods from the interactive tx types, along with the into/to_contributed_inputs_and_outputs methods on ConstructedTransaction. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/interactivetxs.rs | 83 ------------------------------ 1 file changed, 83 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index ca8c4450012..10dae95cefa 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -90,13 +90,6 @@ impl SerialIdExt for SerialId { } } -#[derive(Clone, Debug)] -pub(crate) struct NegotiationError { - pub reason: AbortReason, - pub contributed_inputs: Vec, - pub contributed_outputs: Vec, -} - #[derive(Debug, Clone, Copy, PartialEq)] pub(crate) enum AbortReason { InvalidStateTransition, @@ -370,11 +363,6 @@ impl ConstructedTransaction { Ok(tx) } - fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - let (contributed_inputs, contributed_outputs) = self.into_contributed_inputs_and_outputs(); - NegotiationError { reason, contributed_inputs, contributed_outputs } - } - fn contributed_inputs(&self) -> impl Iterator + '_ { self.tx .input @@ -401,40 +389,6 @@ impl ConstructedTransaction { .map(|(_, (txout, _))| txout) } - fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } - - fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - let contributed_inputs = self - .tx - .input - .into_iter() - .zip(self.input_metadata.iter()) - .enumerate() - .filter(|(_, (_, input))| input.is_local(self.holder_is_initiator)) - .filter(|(index, _)| { - self.shared_input_index - .map(|shared_index| *index != shared_index as usize) - .unwrap_or(true) - }) - .map(|(_, (txin, _))| txin.previous_output) - .collect(); - - let contributed_outputs = self - .tx - .output - .into_iter() - .zip(self.output_metadata.iter()) - .enumerate() - .filter(|(_, (_, output))| output.is_local(self.holder_is_initiator)) - .filter(|(index, _)| *index != self.shared_output_index as usize) - .map(|(_, (txout, _))| txout) - .collect(); - - (contributed_inputs, contributed_outputs) - } - pub fn tx(&self) -> &Transaction { &self.tx } @@ -921,10 +875,6 @@ impl InteractiveTxSigningSession { Ok(()) } - pub(crate) fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - self.unsigned_tx.into_negotiation_error(reason) - } - pub(super) fn contributed_inputs(&self) -> impl Iterator + '_ { self.unsigned_tx.contributed_inputs() } @@ -932,14 +882,6 @@ impl InteractiveTxSigningSession { pub(super) fn contributed_outputs(&self) -> impl Iterator + '_ { self.unsigned_tx.contributed_outputs() } - - pub(super) fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } - - pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - self.unsigned_tx.into_contributed_inputs_and_outputs() - } } impl_writeable_tlv_based!(InteractiveTxSigningSession, { @@ -2172,27 +2114,6 @@ impl InteractiveTxConstructor { Self::new(args, false) } - fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - let (contributed_inputs, contributed_outputs) = self.into_contributed_inputs_and_outputs(); - NegotiationError { reason, contributed_inputs, contributed_outputs } - } - - pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - let contributed_inputs = self - .inputs_to_contribute - .into_iter() - .filter(|(_, input)| !input.is_shared()) - .map(|(_, input)| input.into_tx_in().previous_output) - .collect(); - let contributed_outputs = self - .outputs_to_contribute - .into_iter() - .filter(|(_, output)| !output.is_shared()) - .map(|(_, output)| output.into_tx_out()) - .collect(); - (contributed_inputs, contributed_outputs) - } - pub(super) fn contributed_inputs(&self) -> impl Iterator + '_ { self.inputs_to_contribute .iter() @@ -2207,10 +2128,6 @@ impl InteractiveTxConstructor { .map(|(_, output)| output.tx_out()) } - pub(super) fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } - pub fn is_initiator(&self) -> bool { self.is_initiator } From fd6c9e17f781e05258b48463119e9864b4cf3511 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 3 Apr 2026 10:28:45 -0500 Subject: [PATCH 14/18] 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 bc4cc8efca77928ae8ab393feb5e6a40c9aa8cbd Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 13 Apr 2026 17:48:52 -0500 Subject: [PATCH 15/18] Check can_initiate_rbf in stfu handler before sending tx_init_rbf If splice_locked is sent between our outgoing STFU and the counterparty's STFU response, the stfu() handler would proceed to send tx_init_rbf for an already-confirmed splice. Guard against this by re-checking can_initiate_rbf when entering quiescence. Disconnect because there is no way to cancel quiescence after both sides have exchanged STFU. Co-Authored-By: Claude Opus 4.6 (1M context) --- fuzz/src/chanmon_consistency.rs | 1 + lightning/src/events/mod.rs | 11 +++- lightning/src/ln/channel.rs | 10 +++ lightning/src/ln/splicing_tests.rs | 98 ++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 1 deletion(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 24eaf6f0e1e..d716f8548d3 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -932,6 +932,7 @@ fn assert_action_timeout_awaiting_response(action: &msgs::ErrorAction) { action, msgs::ErrorAction::DisconnectPeerWithWarning { msg } if msg.data.contains("Disconnecting due to timeout awaiting response") + || msg.data.contains("already sent splice_locked, cannot RBF") ), "Expected timeout disconnect, got: {:?}", action, diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 9d00273cb46..0d5b8f757ab 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -149,6 +149,12 @@ pub enum NegotiationFailureReason { /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel /// [`FundingTemplate`]: crate::ln::funding::FundingTemplate FeeRateTooLow, + /// An RBF attempt could not be initiated (e.g., a prior splice transaction already + /// confirmed). The channel remains operational — start a new splice with + /// [`ChannelManager::splice_channel`] if further changes are needed. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + CannotInitiateRbf, } impl NegotiationFailureReason { @@ -166,7 +172,8 @@ impl NegotiationFailureReason { Self::CounterpartyAborted { .. } | Self::NegotiationError { .. } | Self::LocallyAbandoned - | Self::ChannelClosing => false, + | Self::ChannelClosing + | Self::CannotInitiateRbf => false, } } } @@ -185,6 +192,7 @@ impl core::fmt::Display for NegotiationFailureReason { Self::ChannelClosing => f.write_str("channel is closing"), Self::FeeRateTooLow => f.write_str("feerate too low for RBF"), + Self::CannotInitiateRbf => f.write_str("cannot initiate RBF"), } } } @@ -202,6 +210,7 @@ impl_writeable_tlv_based_enum_upgradable!(NegotiationFailureReason, (11, LocallyAbandoned) => {}, (13, ChannelClosing) => {}, (15, FeeRateTooLow) => {}, + (17, CannotInitiateRbf) => {}, ); /// Some information provided on receipt of payment depends on whether the payment received is a diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 48fb02023c1..c3e5f601b52 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -14210,6 +14210,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 c6e685f441f..807dd2513ac 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -5162,6 +5162,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 0e1e6133c5c45d70709fa1028f588e60bb9eb69e Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 5 May 2026 17:38:41 -0500 Subject: [PATCH 16/18] f - inline fresh selection in stfu_after_splice_locked test The earlier fixup dropped do_initiate_rbf_splice_in's value_added parameter and switched it to amend semantics. This test specifically verifies that DiscardFunding fires on the can_initiate_rbf failure path with a unique input surviving filtering, which requires fresh input selection across rounds. Inline the splice_channel + funding_contributed flow with without_prior_contribution rather than using the helper to preserve the original assertions. --- lightning/src/ln/splicing_tests.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 807dd2513ac..c1352897af4 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -5196,10 +5196,21 @@ fn test_splice_rbf_stfu_after_splice_locked() { // Provide more UTXOs for the RBF attempt. provide_utxo_reserves(&nodes, 2, added_value * 2); - // Initiate RBF from node 0. + // Initiate RBF from node 0 with fresh inputs so the RBF round has a unique input that + // survives filtering when the failure cleanup runs. let rbf_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); - let _funding_contribution = - do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let funding_contribution = funding_template + .without_prior_contribution(rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(added_value) + .build() + .unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, funding_contribution.clone(), None) + .unwrap(); // Node 0 sends STFU (can_initiate_rbf passes since no splice_locked yet). let stfu_init = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); From 0653be661ad4c0e37ecbd948878fd6e7df4409e8 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 15 Apr 2026 21:04:46 -0500 Subject: [PATCH 17/18] 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 c3e5f601b52..a685c772bd0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -11613,7 +11613,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); @@ -11621,22 +11620,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 c1352897af4..2dabdb6d487 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -660,9 +660,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); @@ -675,7 +681,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 @@ -712,29 +718,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); } @@ -772,18 +772,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); @@ -1374,7 +1374,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()); @@ -1402,7 +1402,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()); } @@ -1638,7 +1638,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 @@ -4680,13 +4680,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] @@ -5623,13 +5719,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( @@ -5654,14 +5755,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"); @@ -5935,13 +6036,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] @@ -6260,13 +6366,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] @@ -6856,7 +6967,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 { @@ -6933,7 +7044,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 { From 65a257b987b3fc2736e8107513b0830c739cbc16 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 5 May 2026 17:39:31 -0500 Subject: [PATCH 18/18] f - inline fresh selection in discard_unique_contribution test This test specifically requires fresh inputs across RBF rounds (it clears the wallet between rounds and asserts the inputs differ). Inline the splice_channel + funding_contributed flow with without_prior_contribution rather than using a helper, since this is the only call site that needs that behavior. --- lightning/src/ln/splicing_tests.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 2dabdb6d487..46c398e937b 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -4743,8 +4743,18 @@ fn test_splice_rbf_discard_unique_contribution() { // 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 funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let funding_contribution = funding_template + .without_prior_contribution(rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(added_value) + .build() + .unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, funding_contribution.clone(), None) + .unwrap(); let round_1_inputs: Vec<_> = funding_contribution.contributed_inputs().collect(); assert_ne!(round_0_inputs, round_1_inputs, "Rounds must use different UTXOs");