From 2f8125b579ef6cc8e3660cd6131c58f8273ca0b8 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 17:56:42 -0500 Subject: [PATCH 1/9] 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 | 67 +++++++++++ lightning/src/ln/channel.rs | 37 +++++-- lightning/src/ln/channelmanager.rs | 27 +++-- lightning/src/ln/functional_test_utils.rs | 9 +- lightning/src/ln/splicing_tests.rs | 128 ++++++++++++++++++---- 5 files changed, 227 insertions(+), 41 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 73c4a39c76f..ebeec0e092e 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -99,6 +99,67 @@ 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. +#[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. Retry by calling + /// [`ChannelManager::splice_channel`] after the peer reconnects. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + PeerDisconnected, + /// The counterparty aborted the negotiation by sending `tx_abort`. Retry by calling + /// [`ChannelManager::splice_channel`], or wait for the counterparty to initiate. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + CounterpartyAborted, + /// An error occurred while negotiating the interactive transaction (e.g., the counterparty + /// sent an invalid message). Retry by calling [`ChannelManager::splice_channel`]. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + NegotiationError, + /// The funding contribution was invalid (e.g., insufficient balance for the splice amount). + /// Adjust the contribution and retry via [`ChannelManager::splice_channel`]. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + ContributionInvalid, + /// The negotiation was locally abandoned via [`ChannelManager::abandon_splice`]. + /// + /// [`ChannelManager::abandon_splice`]: crate::ln::channelmanager::ChannelManager::abandon_splice + LocallyAbandoned, + /// The channel was not in a state to accept the funding contribution. Retry by calling + /// [`ChannelManager::splice_channel`] once [`ChannelDetails::is_usable`] returns `true`. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + /// [`ChannelDetails::is_usable`]: crate::ln::channelmanager::ChannelDetails::is_usable + ChannelNotReady, + /// The channel is closing, so the negotiation cannot continue. See [`Event::ChannelClosed`] + /// for the closure reason. + ChannelClosing, + /// The contribution's feerate was too low. Retry with a higher feerate by calling + /// [`ChannelManager::splice_channel`] to obtain a new [`FundingTemplate`]. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + /// [`FundingTemplate`]: crate::ln::channelmanager::FundingTemplate + FeeRateTooLow, +} + +impl_writeable_tlv_based_enum!(NegotiationFailureReason, + (0, Unknown) => {}, + (2, PeerDisconnected) => {}, + (4, CounterpartyAborted) => {}, + (6, NegotiationError) => {}, + (8, ContributionInvalid) => {}, + (10, LocallyAbandoned) => {}, + (12, ChannelNotReady) => {}, + (14, ChannelClosing) => {}, + (16, 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 +1647,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 +2442,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 +2451,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 +3096,7 @@ impl MaybeReadable for Event { (5, user_channel_id, required), (7, counterparty_node_id, required), (9, abandoned_funding_txo, option), + (11, reason, (default_value, NegotiationFailureReason::Unknown)), }); Ok(Some(Event::SpliceFailed { @@ -3039,6 +3105,7 @@ impl MaybeReadable for Event { counterparty_node_id: counterparty_node_id.0.unwrap(), abandoned_funding_txo, channel_type, + reason: reason.0.unwrap(), })) }; f() diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 32c0e94bdc8..3b63850c5b9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -36,7 +36,7 @@ use crate::chain::channelmonitor::{ }; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::chain::BestBlock; -use crate::events::{ClosureReason, FundingInfo}; +use crate::events::{ClosureReason, FundingInfo, NegotiationFailureReason}; use crate::ln::chan_utils; use crate::ln::chan_utils::{ get_commitment_transaction_number_obscure_factor, max_htlcs, second_stage_tx_fees_sat, @@ -3174,7 +3174,17 @@ pub(crate) enum QuiescentAction { pub(super) enum QuiescentError { DoNothing, DiscardFunding { inputs: Vec, outputs: Vec }, - FailSplice(SpliceFundingFailed), + FailSplice(SpliceFundingFailed, NegotiationFailureReason), +} + +impl QuiescentError { + fn with_negotiation_failure_reason(mut self, reason: NegotiationFailureReason) -> Self { + match self { + QuiescentError::FailSplice(_, ref mut r) => *r = reason, + _ => debug_assert!(false, "Expected FailSplice variant"), + } + self + } } pub(crate) enum StfuResponse { @@ -7133,9 +7143,10 @@ where fn quiescent_action_into_error(&self, action: QuiescentAction) -> QuiescentError { match action { - QuiescentAction::Splice { contribution, .. } => { - QuiescentError::FailSplice(self.splice_funding_failed_for(contribution)) - }, + QuiescentAction::Splice { contribution, .. } => QuiescentError::FailSplice( + self.splice_funding_failed_for(contribution), + NegotiationFailureReason::Unknown, + ), #[cfg(any(test, fuzzing, feature = "_test_utils"))] QuiescentAction::DoNothing => QuiescentError::DoNothing, } @@ -7144,7 +7155,7 @@ where fn abandon_quiescent_action(&mut self) -> Option { let action = self.quiescent_action.take()?; match self.quiescent_action_into_error(action) { - QuiescentError::FailSplice(failed) => Some(failed), + QuiescentError::FailSplice(failed, _) => Some(failed), #[cfg(any(test, fuzzing, feature = "_test_utils"))] QuiescentError::DoNothing => None, _ => { @@ -12540,7 +12551,10 @@ where }) { log_error!(logger, "Channel {} cannot be funded: {}", self.context.channel_id(), e); - return Err(QuiescentError::FailSplice(self.splice_funding_failed_for(contribution))); + return Err(QuiescentError::FailSplice( + self.splice_funding_failed_for(contribution), + NegotiationFailureReason::ContributionInvalid, + )); } if let Some(pending_splice) = self.pending_splice.as_ref() { @@ -12556,6 +12570,7 @@ where ); return Err(QuiescentError::FailSplice( self.splice_funding_failed_for(contribution), + NegotiationFailureReason::FeeRateTooLow, )); } } @@ -13998,7 +14013,8 @@ where if !self.context.is_usable() { 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::ChannelNotReady)); } if self.quiescent_action.is_some() { log_debug!( @@ -14115,7 +14131,10 @@ where self.context.channel_id(), e, )), - QuiescentError::FailSplice(failed), + QuiescentError::FailSplice( + failed, + NegotiationFailureReason::ContributionInvalid, + ), )); } let prior_contribution = contribution.clone(); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2e782701e47..d0a9ebcffff 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4115,6 +4115,7 @@ impl< user_channel_id: chan.context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); @@ -4421,6 +4422,7 @@ impl< user_channel_id: shutdown_res.user_channel_id, abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); @@ -4927,6 +4929,7 @@ impl< user_channel_id: chan.context.get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::LocallyAbandoned, }, None, )); @@ -6609,12 +6612,15 @@ impl< )); } }, - QuiescentError::FailSplice(SpliceFundingFailed { - funding_txo, - channel_type, - contributed_inputs, - contributed_outputs, - }) => { + QuiescentError::FailSplice( + SpliceFundingFailed { + funding_txo, + channel_type, + contributed_inputs, + contributed_outputs, + }, + reason, + ) => { let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::SpliceFailed { @@ -6623,6 +6629,7 @@ impl< user_channel_id, abandoned_funding_txo: funding_txo, channel_type, + reason, }, None, )); @@ -6764,7 +6771,7 @@ impl< "Channel {} already has a pending funding contribution", channel_id, ), - QuiescentError::FailSplice(_) => format!( + QuiescentError::FailSplice(..) => format!( "Channel {} cannot accept funding contribution", channel_id, ), @@ -11858,6 +11865,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ user_channel_id: channel.context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type.clone(), + reason: events::NegotiationFailureReason::NegotiationError, }, None, )); @@ -12017,6 +12025,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.clone(), + reason: events::NegotiationFailureReason::NegotiationError, }, None, )); @@ -12187,6 +12196,7 @@ 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, }, None, )); @@ -12335,6 +12345,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ user_channel_id: chan.context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::ChannelClosing, }, None, )); @@ -15451,6 +15462,7 @@ impl< user_channel_id: chan.context().get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::PeerDisconnected, }); splice_failed_events.push(events::Event::DiscardFunding { channel_id: chan.context().channel_id(), @@ -18123,6 +18135,7 @@ impl< user_channel_id: chan.context.get_user_id(), abandoned_funding_txo: splice_funding_failed.funding_txo, channel_type: splice_funding_failed.channel_type, + reason: events::NegotiationFailureReason::PeerDisconnected, }, None, )); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 84cdf785da5..435bba28fea 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -17,8 +17,8 @@ use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Listen, Watch use crate::events::bump_transaction::sync::BumpTransactionEventHandlerSync; use crate::events::bump_transaction::BumpTransactionEvent; use crate::events::{ - ClaimedHTLC, ClosureReason, Event, FundingInfo, HTLCHandlingFailureType, PaidBolt12Invoice, - PathFailure, PaymentFailureReason, PaymentPurpose, + ClaimedHTLC, ClosureReason, Event, FundingInfo, HTLCHandlingFailureType, + NegotiationFailureReason, PaidBolt12Invoice, PathFailure, PaymentFailureReason, PaymentPurpose, }; use crate::ln::chan_utils::{ commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, TRUC_MAX_WEIGHT, @@ -3243,13 +3243,14 @@ pub fn expect_splice_pending_event<'a, 'b, 'c, 'd>( #[cfg(any(test, ldk_bench, feature = "_test_utils"))] pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( node: &'a Node<'b, 'c, 'd>, expected_channel_id: &ChannelId, - funding_contribution: FundingContribution, + funding_contribution: FundingContribution, expected_reason: NegotiationFailureReason, ) { let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match &events[0] { - Event::SpliceFailed { channel_id, .. } => { + Event::SpliceFailed { channel_id, reason, .. } => { assert_eq!(*expected_channel_id, *channel_id); + assert_eq!(*reason, expected_reason); }, _ => panic!("Unexpected event"), } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 9adccd17627..c04a1494a66 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -13,7 +13,9 @@ use crate::chain::chaininterface::{TransactionType, FEERATE_FLOOR_SATS_PER_KW}; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::OutPoint; use crate::chain::ChannelMonitorUpdateStatus; -use crate::events::{ClosureReason, Event, FundingInfo, HTLCHandlingFailureType}; +use crate::events::{ + ClosureReason, Event, FundingInfo, HTLCHandlingFailureType, NegotiationFailureReason, +}; use crate::ln::chan_utils; use crate::ln::channel::{ CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, @@ -282,7 +284,12 @@ pub fn initiate_splice_out<'a, 'b, 'c, 'd>( ) { Ok(()) => Ok(funding_contribution), Err(e) => { - expect_splice_failed_events(initiator, &channel_id, funding_contribution); + expect_splice_failed_events( + initiator, + &channel_id, + funding_contribution, + NegotiationFailureReason::ContributionInvalid, + ); Err(e) }, } @@ -888,7 +895,12 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { nodes[1].node.peer_disconnected(node_id_0); } - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_channel_ready = (true, true); @@ -939,7 +951,12 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { nodes[1].node.peer_disconnected(node_id_0); } - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_channel_ready = (true, true); @@ -1021,7 +1038,12 @@ 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, + ); // Attempt a splice negotiation that completes, (i.e. `tx_signatures` are exchanged). Reconnecting // should not abort the negotiation or reset the splice state. @@ -1105,7 +1127,12 @@ fn test_config_reject_inbound_splices() { nodes[0].node.peer_disconnected(node_id_1); nodes[1].node.peer_disconnected(node_id_0); - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_channel_ready = (true, true); @@ -2560,7 +2587,12 @@ 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, + ); // We exit quiescence upon sending `tx_abort`, so we should see the holding cell be immediately // freed. @@ -2631,7 +2663,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, + ); // We exit quiescence upon receiving `tx_abort`, so we should see our `tx_abort` echo and the // holding cell be immediately freed. @@ -2727,7 +2764,12 @@ 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, + ); let tx_abort = get_event_msg!(initiator, MessageSendEvent::SendTxAbort, node_id_acceptor); acceptor.node.handle_tx_abort(node_id_initiator, &tx_abort); @@ -3017,8 +3059,9 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => { + Event::SpliceFailed { channel_id: cid, reason, .. } => { assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::ChannelClosing); }, other => panic!("Expected SpliceFailed, got {:?}", other), } @@ -3037,7 +3080,12 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), } } else { - expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + funding_contribution, + NegotiationFailureReason::ChannelClosing, + ); } let _ = get_event_msg!(closee_node, MessageSendEvent::SendShutdown, closer_node_id); } @@ -4014,7 +4062,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::ChannelNotReady, + ); } #[test] @@ -4195,7 +4248,12 @@ fn do_test_splice_pending_htlcs(config: UserConfig) { let reconnect_args = ReconnectArgs::new(initiator, acceptor); reconnect_nodes(reconnect_args); - expect_splice_failed_events(initiator, &channel_id, contribution); + expect_splice_failed_events( + initiator, + &channel_id, + contribution, + NegotiationFailureReason::PeerDisconnected, + ); // 4) Try again with the additional satoshi removed from the splice-out message, and check that it passes // validation on the receiver's side. @@ -4230,7 +4288,12 @@ fn do_test_splice_pending_htlcs(config: UserConfig) { nodes[1].node.peer_disconnected(node_id_0); let reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_nodes(reconnect_args); - expect_splice_failed_events(&nodes[1], &channel_id, contribution); + expect_splice_failed_events( + &nodes[1], + &channel_id, + contribution, + NegotiationFailureReason::PeerDisconnected, + ); let details = &nodes[1].node.list_channels()[0]; let expected_outbound_htlc_max = (pre_splice_balance.to_sat() - details.unspendable_punishment_reserve.unwrap()) * 1000; @@ -4381,7 +4444,12 @@ fn test_splice_acceptor_disconnect_emits_events() { nodes[1].node.peer_disconnected(node_id_0); // The initiator should get SpliceFailed + DiscardFunding. - expect_splice_failed_events(&nodes[0], &channel_id, node_0_funding_contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + node_0_funding_contribution, + NegotiationFailureReason::PeerDisconnected, + ); // The acceptor should also get SpliceFailed + DiscardFunding with its contributions // so it can reclaim its UTXOs. The contribution is feerate-adjusted by handle_splice_init, @@ -4389,7 +4457,10 @@ fn test_splice_acceptor_disconnect_emits_events() { let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => assert_eq!(*cid, channel_id), + Event::SpliceFailed { channel_id: cid, reason, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + }, other => panic!("Expected SpliceFailed, got {:?}", other), } match &events[1] { @@ -5841,7 +5912,10 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => assert_eq!(*cid, channel_id), + Event::SpliceFailed { channel_id: cid, reason, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + }, other => panic!("Expected SpliceFailed, got {:?}", other), } match &events[1] { @@ -5920,8 +5994,9 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => { + Event::SpliceFailed { channel_id: cid, reason, .. } => { assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); }, other => panic!("Expected SpliceFailed, got {:?}", other), } @@ -5961,7 +6036,10 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => assert_eq!(*cid, channel_id), + Event::SpliceFailed { channel_id: cid, reason, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); + }, other => panic!("Expected SpliceFailed, got {:?}", other), } match &events[1] { @@ -6498,7 +6576,12 @@ fn test_splice_revalidation_at_quiescence() { assert_eq!(msg_events.len(), 1, "{msg_events:?}"); assert!(matches!(msg_events[0], MessageSendEvent::HandleError { .. })); - expect_splice_failed_events(&nodes[0], &channel_id, contribution); + expect_splice_failed_events( + &nodes[0], + &channel_id, + contribution, + NegotiationFailureReason::ContributionInvalid, + ); } #[test] @@ -6643,7 +6726,10 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, .. } => assert_eq!(*cid, channel_id), + Event::SpliceFailed { channel_id: cid, reason, .. } => { + assert_eq!(*cid, channel_id); + assert_eq!(*reason, NegotiationFailureReason::FeeRateTooLow); + }, other => panic!("Expected SpliceFailed, got {:?}", other), } } From 761b48afe3d39933e969a0c1a087aab475a458d1 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 18:20:35 -0500 Subject: [PATCH 2/9] 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 | 9 ++++++ lightning/src/ln/channelmanager.rs | 35 +++++++------------- lightning/src/ln/functional_test_utils.rs | 3 +- lightning/src/ln/funding.rs | 8 ++++- lightning/src/ln/splicing_tests.rs | 18 +++++++---- 6 files changed, 62 insertions(+), 50 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index ebeec0e092e..3478011d3a4 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; @@ -1621,19 +1622,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. @@ -1643,12 +1645,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::channelmanager::FundingTemplate + contribution: Option, }, /// Used to indicate to the user that they can abandon the funding transaction and recycle the /// inputs for another purpose. @@ -2440,18 +2447,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 @@ -3092,20 +3097,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, (default_value, NegotiationFailureReason::Unknown)), + (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.0.unwrap(), + contribution, })) }; f() diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 3b63850c5b9..cd97fde4dcc 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7040,6 +7040,9 @@ pub struct SpliceFundingFailed { /// Outputs contributed to the splice transaction. pub contributed_outputs: Vec, + + /// The funding contribution from the failed round, if available. + pub contribution: Option, } macro_rules! maybe_create_splice_funding_failed { @@ -7088,11 +7091,15 @@ macro_rules! maybe_create_splice_funding_failed { return None; } + let contribution = + $pending_splice_ref.and_then(|ps| ps.contributions.last().cloned()); + Some(SpliceFundingFailed { funding_txo, channel_type, contributed_inputs, contributed_outputs, + contribution, }) }) }}; @@ -7124,6 +7131,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() { @@ -7138,6 +7146,7 @@ where 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 d0a9ebcffff..d056b654243 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4113,8 +4113,7 @@ impl< 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: splice_funding_failed.contribution, reason: events::NegotiationFailureReason::ChannelClosing, }, None, @@ -4420,8 +4419,7 @@ impl< 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: splice_funding_failed.contribution, reason: events::NegotiationFailureReason::ChannelClosing, }, None, @@ -4927,8 +4925,7 @@ impl< 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: splice_funding_failed.contribution, reason: events::NegotiationFailureReason::LocallyAbandoned, }, None, @@ -6614,10 +6611,7 @@ impl< }, QuiescentError::FailSplice( SpliceFundingFailed { - funding_txo, - channel_type, - contributed_inputs, - contributed_outputs, + contributed_inputs, contributed_outputs, contribution, .. }, reason, ) => { @@ -6627,9 +6621,8 @@ impl< channel_id, counterparty_node_id, user_channel_id, - abandoned_funding_txo: funding_txo, - channel_type, reason, + contribution, }, None, )); @@ -11863,8 +11856,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: channel.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type.clone(), + contribution: splice_funding_failed.contribution.clone(), reason: events::NegotiationFailureReason::NegotiationError, }, None, @@ -12023,8 +12015,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ channel_id: msg.channel_id, counterparty_node_id, user_channel_id: chan.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type.clone(), + contribution: splice_funding_failed.contribution.clone(), reason: events::NegotiationFailureReason::NegotiationError, }, None, @@ -12194,8 +12185,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ 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: splice_funding_failed.contribution, reason: events::NegotiationFailureReason::CounterpartyAborted, }, None, @@ -12343,8 +12333,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ 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: splice_funding_failed.contribution, reason: events::NegotiationFailureReason::ChannelClosing, }, None, @@ -15460,8 +15449,7 @@ impl< 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: splice_funding_failed.contribution, reason: events::NegotiationFailureReason::PeerDisconnected, }); splice_failed_events.push(events::Event::DiscardFunding { @@ -18133,9 +18121,8 @@ impl< 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: splice_funding_failed.contribution, }, None, )); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 435bba28fea..7c3ca33c2c0 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3248,9 +3248,10 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match &events[0] { - Event::SpliceFailed { channel_id, reason, .. } => { + Event::SpliceFailed { channel_id, reason, contribution, .. } => { assert_eq!(*expected_channel_id, *channel_id); assert_eq!(*reason, expected_reason); + assert_eq!(contribution.as_ref(), Some(&funding_contribution)); }, _ => panic!("Unexpected event"), } diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index c08a0a9f471..8e382bd3059 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -710,7 +710,8 @@ impl_writeable_tlv_based!(FundingContribution, { }); impl FundingContribution { - pub(super) fn feerate(&self) -> FeeRate { + /// Returns the feerate of this contribution. + pub fn feerate(&self) -> FeeRate { self.feerate } @@ -731,6 +732,11 @@ impl FundingContribution { self.value_added } + /// Returns the inputs included in this contribution. + pub fn inputs(&self) -> &[FundingTxInput] { + &self.inputs + } + /// Returns the outputs (e.g., withdrawal destinations) included in this contribution. /// /// This does not include the change output; see [`FundingContribution::change_output`]. diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index c04a1494a66..9682d869629 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3059,9 +3059,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), } @@ -4457,9 +4458,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), } @@ -5912,9 +5914,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, 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), } @@ -5994,9 +5997,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), } @@ -6036,9 +6040,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), } @@ -6726,9 +6731,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 165f8a9a60d3e8fa73284d6e75c44103b8c3734a Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 11:48:05 -0500 Subject: [PATCH 3/9] f - Remove dead funding_txo and channel_type from SpliceFundingFailed Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index cd97fde4dcc..8f16dd305e0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7029,12 +7029,6 @@ 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, @@ -7052,15 +7046,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() @@ -7094,13 +7079,7 @@ macro_rules! maybe_create_splice_funding_failed { let contribution = $pending_splice_ref.and_then(|ps| ps.contributions.last().cloned()); - Some(SpliceFundingFailed { - funding_txo, - channel_type, - contributed_inputs, - contributed_outputs, - contribution, - }) + Some(SpliceFundingFailed { contributed_inputs, contributed_outputs, contribution }) }) }}; } @@ -7142,8 +7121,6 @@ where } } SpliceFundingFailed { - funding_txo: None, - channel_type: None, contributed_inputs: inputs, contributed_outputs: outputs, contribution: Some(cloned_contribution), From 8dda7a1e1f17037d4a5060b1aa83a79d8f730c71 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 18:44:24 -0500 Subject: [PATCH 4/9] 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 | 194 +++++++++++----------- lightning/src/ln/functional_test_utils.rs | 18 +- lightning/src/ln/splicing_tests.rs | 68 ++++---- 3 files changed, 140 insertions(+), 140 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d056b654243..62f39cb1897 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4108,16 +4108,6 @@ impl< if let Some(splice_funding_failed) = splice_funding_failed { 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(), - contribution: splice_funding_failed.contribution, - reason: events::NegotiationFailureReason::ChannelClosing, - }, - None, - )); pending_events.push_back(( events::Event::DiscardFunding { channel_id: *chan_id, @@ -4128,6 +4118,16 @@ impl< }, None, )); + 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(), + contribution: splice_funding_failed.contribution, + reason: events::NegotiationFailureReason::ChannelClosing, + }, + None, + )); } // We can send the `shutdown` message before updating the `ChannelMonitor` @@ -4414,16 +4414,6 @@ impl< )); if let Some(splice_funding_failed) = shutdown_res.splice_funding_failed.take() { - 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, - contribution: splice_funding_failed.contribution, - reason: events::NegotiationFailureReason::ChannelClosing, - }, - None, - )); pending_events.push_back(( events::Event::DiscardFunding { channel_id: shutdown_res.channel_id, @@ -4434,6 +4424,16 @@ impl< }, None, )); + 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, + contribution: splice_funding_failed.contribution, + reason: events::NegotiationFailureReason::ChannelClosing, + }, + None, + )); } if let Some(transaction) = shutdown_res.unbroadcasted_funding_tx { @@ -4920,16 +4920,6 @@ impl< if let Some(splice_funding_failed) = splice_funding_failed { 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(), - contribution: splice_funding_failed.contribution, - reason: events::NegotiationFailureReason::LocallyAbandoned, - }, - None, - )); pending_events.push_back(( events::Event::DiscardFunding { channel_id: *channel_id, @@ -4940,6 +4930,16 @@ impl< }, None, )); + 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(), + contribution: splice_funding_failed.contribution, + reason: events::NegotiationFailureReason::LocallyAbandoned, + }, + None, + )); } Ok(()) @@ -6616,16 +6616,6 @@ impl< reason, ) => { let pending_events = &mut self.pending_events.lock().unwrap(); - pending_events.push_back(( - events::Event::SpliceFailed { - channel_id, - counterparty_node_id, - user_channel_id, - reason, - contribution, - }, - None, - )); if !contributed_inputs.is_empty() || !contributed_outputs.is_empty() { pending_events.push_back(( events::Event::DiscardFunding { @@ -6638,6 +6628,16 @@ impl< None, )); } + pending_events.push_back(( + events::Event::SpliceFailed { + channel_id, + counterparty_node_id, + user_channel_id, + reason, + contribution, + }, + None, + )); }, } } @@ -11851,16 +11851,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }) => { if let Some(splice_funding_failed) = splice_funding_failed { let pending_events = &mut self.pending_events.lock().unwrap(); - pending_events.push_back(( - events::Event::SpliceFailed { - channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: channel.context().get_user_id(), - contribution: splice_funding_failed.contribution.clone(), - reason: events::NegotiationFailureReason::NegotiationError, - }, - None, - )); pending_events.push_back(( events::Event::DiscardFunding { channel_id, @@ -11871,6 +11861,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); + pending_events.push_back(( + events::Event::SpliceFailed { + channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id: channel.context().get_user_id(), + contribution: splice_funding_failed.contribution.clone(), + reason: events::NegotiationFailureReason::NegotiationError, + }, + None, + )); } debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_))); @@ -12010,16 +12010,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }) => { if let Some(splice_funding_failed) = splice_funding_failed { let pending_events = &mut self.pending_events.lock().unwrap(); - pending_events.push_back(( - events::Event::SpliceFailed { - channel_id: msg.channel_id, - counterparty_node_id, - user_channel_id: chan.context().get_user_id(), - contribution: splice_funding_failed.contribution.clone(), - reason: events::NegotiationFailureReason::NegotiationError, - }, - None, - )); pending_events.push_back(( events::Event::DiscardFunding { channel_id: msg.channel_id, @@ -12030,6 +12020,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); + pending_events.push_back(( + events::Event::SpliceFailed { + channel_id: msg.channel_id, + counterparty_node_id, + user_channel_id: chan.context().get_user_id(), + contribution: splice_funding_failed.contribution.clone(), + reason: events::NegotiationFailureReason::NegotiationError, + }, + None, + )); } debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_))); @@ -12180,16 +12180,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(splice_funding_failed) = splice_failed { 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(), - contribution: splice_funding_failed.contribution, - reason: events::NegotiationFailureReason::CounterpartyAborted, - }, - None, - )); pending_events.push_back(( events::Event::DiscardFunding { channel_id: msg.channel_id, @@ -12200,6 +12190,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); + 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(), + contribution: splice_funding_failed.contribution, + reason: events::NegotiationFailureReason::CounterpartyAborted, + }, + None, + )); } let holding_cell_res = if exited_quiescence { @@ -12328,16 +12328,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(splice_funding_failed) = splice_funding_failed { 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(), - contribution: splice_funding_failed.contribution, - reason: events::NegotiationFailureReason::ChannelClosing, - }, - None, - )); pending_events.push_back(( events::Event::DiscardFunding { channel_id: msg.channel_id, @@ -12348,6 +12338,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, None, )); + 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(), + contribution: splice_funding_failed.contribution, + reason: events::NegotiationFailureReason::ChannelClosing, + }, + None, + )); } if let Some(msg) = shutdown { @@ -15445,13 +15445,6 @@ impl< chan.peer_disconnected_is_resumable(&&logger); if let Some(splice_funding_failed) = splice_funding_failed { - splice_failed_events.push(events::Event::SpliceFailed { - channel_id: chan.context().channel_id(), - counterparty_node_id, - user_channel_id: chan.context().get_user_id(), - contribution: splice_funding_failed.contribution, - reason: events::NegotiationFailureReason::PeerDisconnected, - }); splice_failed_events.push(events::Event::DiscardFunding { channel_id: chan.context().channel_id(), funding_info: FundingInfo::Contribution { @@ -15459,6 +15452,13 @@ impl< outputs: splice_funding_failed.contributed_outputs, }, }); + splice_failed_events.push(events::Event::SpliceFailed { + channel_id: chan.context().channel_id(), + counterparty_node_id, + user_channel_id: chan.context().get_user_id(), + contribution: splice_funding_failed.contribution, + reason: events::NegotiationFailureReason::PeerDisconnected, + }); } if is_resumable { @@ -18116,16 +18116,6 @@ 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() { - 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(), - reason: events::NegotiationFailureReason::PeerDisconnected, - contribution: splice_funding_failed.contribution, - }, - None, - )); events.push_back(( events::Event::DiscardFunding { channel_id: chan.context().channel_id(), @@ -18136,6 +18126,16 @@ impl< }, None, )); + 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(), + reason: events::NegotiationFailureReason::PeerDisconnected, + contribution: splice_funding_failed.contribution, + }, + None, + )); } } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 7c3ca33c2c0..f093094b65b 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3248,18 +3248,10 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match &events[0] { - Event::SpliceFailed { channel_id, reason, contribution, .. } => { - assert_eq!(*expected_channel_id, *channel_id); - assert_eq!(*reason, expected_reason); - assert_eq!(contribution.as_ref(), Some(&funding_contribution)); - }, - _ => panic!("Unexpected event"), - } - match &events[1] { Event::DiscardFunding { funding_info, .. } => { if let FundingInfo::Contribution { inputs, outputs } = &funding_info { let (expected_inputs, expected_outputs) = - funding_contribution.into_contributed_inputs_and_outputs(); + funding_contribution.clone().into_contributed_inputs_and_outputs(); assert_eq!(*inputs, expected_inputs); assert_eq!(*outputs, expected_outputs); } else { @@ -3268,6 +3260,14 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( }, _ => panic!("Unexpected event"), } + match &events[1] { + Event::SpliceFailed { channel_id, reason, contribution, .. } => { + assert_eq!(*expected_channel_id, *channel_id); + assert_eq!(*reason, expected_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 9682d869629..ec6877b93cc 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3059,14 +3059,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 }, .. @@ -3080,6 +3072,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], @@ -4458,14 +4458,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 }, .. @@ -4475,6 +4467,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]); @@ -5910,10 +5910,14 @@ 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 should get DiscardFunding + SpliceFailed. 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 { .. }, .. } => {}, + 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); @@ -5921,10 +5925,6 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { }, 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 @@ -5993,18 +5993,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 }, .. @@ -6017,6 +6009,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]); @@ -6040,6 +6040,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::DiscardFunding { .. } => {}, + other => panic!("Expected DiscardFunding, got {:?}", other), + } + match &events[1] { Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); @@ -6047,10 +6051,6 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { }, 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); From e9510adaf82ca5409d2baa3563a53675841ffb62 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Mar 2026 18:58:50 -0500 Subject: [PATCH 5/9] 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 | 8 ++-- lightning/src/ln/channelmanager.rs | 43 ++++++++++---------- lightning/src/ln/functional_test_utils.rs | 6 +-- lightning/src/ln/funding.rs | 4 +- lightning/src/ln/splicing_tests.rs | 48 +++++++++++------------ 9 files changed, 71 insertions(+), 70 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index f20f93c789c..527670e1317 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -2015,7 +2015,7 @@ pub fn do_test(data: &[u8], out: Out) { ) .unwrap(); }, - events::Event::SplicePending { new_funding_txo, .. } => { + events::Event::SpliceNegotiated { new_funding_txo, .. } => { let broadcaster = match $node { 0 => &broadcast_a, 1 => &broadcast_b, @@ -2027,7 +2027,7 @@ pub fn do_test(data: &[u8], out: Out) { assert_eq!(new_funding_txo.txid, splice_tx.compute_txid()); chain_state.confirm_tx(splice_tx); }, - events::Event::SpliceFailed { .. } => {}, + events::Event::SpliceNegotiationFailed { .. } => {}, events::Event::DiscardFunding { funding_info: events::FundingInfo::Contribution { .. }, .. diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index c1d7982e5e4..6de7308e27e 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1141,10 +1141,10 @@ pub fn do_test(mut data: &[u8], logger: &Arc signed_tx, ); }, - Event::SplicePending { .. } => { + Event::SpliceNegotiated { .. } => { // Splice negotiation completed, waiting for confirmation }, - Event::SpliceFailed { .. } => { + Event::SpliceNegotiationFailed { .. } => { // Splice failed, inputs can be re-spent }, Event::OpenChannelRequest { diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 3478011d3a4..16dc817e099 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1603,8 +1603,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. @@ -1624,7 +1624,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. /// @@ -1634,7 +1634,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 @@ -2425,7 +2425,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, @@ -2443,7 +2443,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, @@ -3082,7 +3082,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(), @@ -3103,7 +3103,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 8f16dd305e0..528a197f33d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1170,7 +1170,7 @@ pub(super) struct InteractiveTxMsgError { /// The underlying error. pub(super) err: ChannelError, /// If a splice was in progress when processing the message, this contains the splice funding - /// information for emitting a `SpliceFailed` event. + /// information for emitting a `SpliceNegotiationFailed` event. pub(super) splice_funding_failed: Option, /// Whether we were quiescent when we received the message, and are no longer due to aborting /// the session. @@ -1258,7 +1258,7 @@ pub(crate) struct ShutdownResult { pub(crate) channel_funding_txo: Option, pub(crate) last_local_balance_msat: u64, /// If a splice was in progress when the channel was shut down, this contains - /// the splice funding information for emitting a SpliceFailed event. + /// the splice funding information for emitting a SpliceNegotiationFailed event. pub(crate) splice_funding_failed: Option, } @@ -1266,7 +1266,7 @@ pub(crate) struct ShutdownResult { pub(crate) struct DisconnectResult { pub(crate) is_resumable: bool, /// If a splice was in progress when the channel was shut down, this contains - /// the splice funding information for emitting a SpliceFailed event. + /// the splice funding information for emitting a SpliceNegotiationFailed event. pub(crate) splice_funding_failed: Option, } @@ -12306,7 +12306,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 62f39cb1897..82682c317ce 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4119,7 +4119,7 @@ impl< None, )); pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: *chan_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context().get_user_id(), @@ -4425,7 +4425,7 @@ impl< None, )); pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: shutdown_res.channel_id, counterparty_node_id: shutdown_res.counterparty_node_id, user_channel_id: shutdown_res.user_channel_id, @@ -4931,7 +4931,7 @@ impl< None, )); 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(), @@ -6629,7 +6629,7 @@ impl< )); } pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id, counterparty_node_id, user_channel_id, @@ -6675,14 +6675,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. /// @@ -6901,7 +6901,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(), @@ -10994,7 +10994,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(), @@ -11862,7 +11862,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ None, )); pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: channel.context().get_user_id(), @@ -12021,7 +12021,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ None, )); pending_events.push_back(( - events::Event::SpliceFailed { + events::Event::SpliceNegotiationFailed { channel_id: msg.channel_id, counterparty_node_id, user_channel_id: chan.context().get_user_id(), @@ -12108,7 +12108,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let exited_quiescence = splice_negotiated.is_some(); if let Some(splice_negotiated) = splice_negotiated { self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { + events::Event::SpliceNegotiated { channel_id: msg.channel_id, counterparty_node_id: *counterparty_node_id, user_channel_id: chan.context.get_user_id(), @@ -12191,7 +12191,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ None, )); 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(), @@ -12339,7 +12339,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ None, )); 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(), @@ -15452,7 +15452,7 @@ impl< outputs: splice_funding_failed.contributed_outputs, }, }); - 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(), @@ -18108,8 +18108,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(); @@ -18127,7 +18128,7 @@ impl< None, )); 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(), @@ -18256,7 +18257,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 f093094b65b..38ac5a018af 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2389,7 +2389,7 @@ pub fn check_closed_events(node: &Node, expected_close_events: &[ExpectedCloseEv discard_events_count ); assert_eq!( - events.iter().filter(|e| matches!(e, Event::SpliceFailed { .. },)).count(), + events.iter().filter(|e| matches!(e, Event::SpliceNegotiationFailed { .. },)).count(), splice_events_count ); } @@ -3232,7 +3232,7 @@ pub fn expect_splice_pending_event<'a, 'b, 'c, 'd>( let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match &events[0] { - crate::events::Event::SplicePending { channel_id, counterparty_node_id, .. } => { + crate::events::Event::SpliceNegotiated { channel_id, counterparty_node_id, .. } => { assert_eq!(*expected_counterparty_node_id, *counterparty_node_id); *channel_id }, @@ -3261,7 +3261,7 @@ pub fn expect_splice_failed_events<'a, 'b, 'c, 'd>( _ => panic!("Unexpected event"), } match &events[1] { - Event::SpliceFailed { channel_id, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id, reason, contribution, .. } => { assert_eq!(*expected_channel_id, *channel_id); assert_eq!(*reason, expected_reason); assert_eq!(contribution.as_ref(), Some(&funding_contribution)); diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 8e382bd3059..8eb805f5ba5 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -120,10 +120,10 @@ pub enum FundingContributionError { /// /// Note: [`FundingTemplate::min_rbf_feerate`] may be derived from an in-progress /// negotiation that later aborts, leaving a stale (higher than necessary) minimum. If - /// this error occurs after receiving [`Event::SpliceFailed`], call + /// this error occurs after receiving [`Event::SpliceNegotiationFailed`], call /// [`ChannelManager::splice_channel`] again to get a fresh template. /// - /// [`Event::SpliceFailed`]: crate::events::Event::SpliceFailed + /// [`Event::SpliceNegotiationFailed`]: crate::events::Event::SpliceNegotiationFailed /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel FeeRateBelowRbfMinimum { /// The requested feerate. diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index ec6877b93cc..5057b9b6a96 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3043,7 +3043,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]) }; @@ -3073,12 +3073,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( @@ -3885,7 +3885,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) @@ -4021,7 +4021,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); @@ -4050,7 +4050,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, @@ -4405,7 +4405,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); @@ -4444,7 +4444,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, @@ -4452,7 +4452,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(); @@ -4468,12 +4468,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. @@ -5835,7 +5835,7 @@ fn test_splice_rbf_sequential() { fn test_splice_rbf_acceptor_contributes_then_disconnects() { // When both nodes contribute to a splice and the initiator RBFs (with the acceptor // re-contributing via prior contribution), disconnecting mid-interactive-TX should emit - // SpliceFailed + DiscardFunding for both nodes so each can reclaim their UTXOs. + // SpliceNegotiationFailed + DiscardFunding for both nodes so each can reclaim their UTXOs. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -5910,7 +5910,7 @@ 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 DiscardFunding + SpliceFailed. + // The initiator should get DiscardFunding + SpliceNegotiationFailed. let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2, "{events:?}"); match &events[0] { @@ -5918,12 +5918,12 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { 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), } // The acceptor re-contributed the same UTXOs as round 0 (via prior contribution @@ -5993,7 +5993,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] { @@ -6010,12 +6010,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. @@ -6044,12 +6044,12 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { other => panic!("Expected DiscardFunding, 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), } let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); @@ -6451,7 +6451,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 @@ -6726,16 +6726,16 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let result = nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution, None); assert!(result.is_err(), "Expected rejection for low feerate: {:?}", result); - // SpliceFailed is emitted. DiscardFunding is not emitted because all inputs/outputs + // SpliceNegotiationFailed is emitted. DiscardFunding is not emitted because all inputs/outputs // are filtered out (same UTXOs reused for RBF, still committed to the prior splice tx). let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1, "{events:?}"); match &events[0] { - Event::SpliceFailed { channel_id: cid, reason, contribution, .. } => { + Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { assert_eq!(*cid, channel_id); assert_eq!(*reason, NegotiationFailureReason::FeeRateTooLow); assert!(contribution.is_some()); }, - other => panic!("Expected SpliceFailed, got {:?}", other), + other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), } } From 7cbcf4f2f45dbaacc476483a189e1a3576c01fac Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 16:05:31 -0500 Subject: [PATCH 6/9] 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 528a197f33d..18172af280c 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7276,20 +7276,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 fa558da4eb9d2e60b959563b8bc64ab46be5ff5b Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 18:52:27 -0500 Subject: [PATCH 7/9] Fix output filtering in into_unique_contributions Filter outputs by script_pubkey rather than full TxOut equality. Outputs reusing the same address as a prior round are still considered committed even if the value differs (e.g., different change amounts across RBF rounds with different feerates). Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/funding.rs | 2 +- lightning/src/ln/splicing_tests.rs | 52 ++++++++++++++++++------------ 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 8eb805f5ba5..decf8f2b62e 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -777,7 +777,7 @@ impl FundingContribution { inputs.retain(|input| *input != existing); } for existing in existing_outputs { - outputs.retain(|output| *output != *existing); + outputs.retain(|output| output.script_pubkey != existing.script_pubkey); } if inputs.is_empty() && outputs.is_empty() { None diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 5057b9b6a96..c13fc489398 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3757,11 +3757,11 @@ fn test_funding_contributed_splice_already_pending() { ) .unwrap(); - // Initiate a second splice with a DIFFERENT output to test that different outputs - // are included in DiscardFunding (not filtered out) + // Initiate a second splice with a DIFFERENT output (different script_pubkey) to test that + // non-overlapping outputs are included in DiscardFunding (not filtered out). let second_splice_out = TxOut { - value: Amount::from_sat(6_000), // Different amount - script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())), + value: Amount::from_sat(6_000), + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }; // Clear UTXOs and add a LARGER one for the second contribution to ensure @@ -3794,8 +3794,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!( @@ -3815,11 +3814,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"); } @@ -3912,14 +3910,26 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { let first_contribution = funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); - // Build second contribution with different UTXOs so inputs/outputs don't overlap + // Build second contribution with different UTXOs and a splice-out output using a different + // script_pubkey (node 1's address) so it survives script_pubkey-based filtering. nodes[0].wallet_source.clear_utxos(); provide_utxo_reserves(&nodes, 1, splice_in_amount * 3); + let splice_out_output = TxOut { + value: Amount::from_sat(1_000), + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), + }; let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let second_contribution = - funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); + let second_contribution = funding_template + .splice_in_and_out_sync( + splice_in_amount, + vec![splice_out_output.clone()], + feerate, + FeeRate::MAX, + &wallet, + ) + .unwrap(); // First funding_contributed - sets up the quiescent action and queues STFU nodes[0] @@ -3964,10 +3974,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 { @@ -3975,15 +3985,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 410324c6f44f826b1d8ec2eb28f28e2ab239b641 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 16:42:20 -0500 Subject: [PATCH 8/9] 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 | 139 +++++++++++++++++------------------- 1 file changed, 66 insertions(+), 73 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 18172af280c..74e61b75581 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7039,48 +7039,29 @@ pub struct SpliceFundingFailed { pub contribution: Option, } -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), + }), + } }}; } @@ -7110,21 +7091,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 { @@ -7268,21 +7244,19 @@ 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() { + 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 @@ -7292,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(); } @@ -7309,12 +7295,19 @@ where return None; } - maybe_create_splice_funding_failed!( + let pending_splice = self.pending_splice.as_ref()?; + 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 c469423a8c5061d67490a5c78b38624d6a8611e6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 27 Mar 2026 17:01:30 -0500 Subject: [PATCH 9/9] Remove unused contributed_inputs_and_outputs methods Now that splice_funding_failed_for! derives inputs and outputs from FundingContribution directly, remove the into_contributed_inputs_and_outputs and to_contributed_inputs_and_outputs methods that were only used by the old macro. Also remove the unused NegotiationError struct and into_negotiation_error methods from the interactive tx types. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 11 ---- lightning/src/ln/interactivetxs.rs | 83 ------------------------------ 2 files changed, 94 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 74e61b75581..c2f676507d1 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6861,13 +6861,6 @@ impl FundingNegotiationContext { } } - fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - let contributed_inputs = - self.our_funding_inputs.into_iter().map(|input| input.utxo.outpoint).collect(); - let contributed_outputs = self.our_funding_outputs; - (contributed_inputs, contributed_outputs) - } - fn contributed_inputs(&self) -> impl Iterator + '_ { self.our_funding_inputs.iter().map(|input| input.utxo.outpoint) } @@ -6875,10 +6868,6 @@ impl FundingNegotiationContext { fn contributed_outputs(&self) -> impl Iterator + '_ { self.our_funding_outputs.iter() } - - fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } } // Holder designates channel data owned for the benefit of the user client. diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 9957205716c..d5da65f8b3a 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -90,13 +90,6 @@ impl SerialIdExt for SerialId { } } -#[derive(Clone, Debug)] -pub(crate) struct NegotiationError { - pub reason: AbortReason, - pub contributed_inputs: Vec, - pub contributed_outputs: Vec, -} - #[derive(Debug, Clone, Copy, PartialEq)] pub(crate) enum AbortReason { InvalidStateTransition, @@ -370,11 +363,6 @@ impl ConstructedTransaction { Ok(tx) } - fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - let (contributed_inputs, contributed_outputs) = self.into_contributed_inputs_and_outputs(); - NegotiationError { reason, contributed_inputs, contributed_outputs } - } - fn contributed_inputs(&self) -> impl Iterator + '_ { self.tx .input @@ -401,40 +389,6 @@ impl ConstructedTransaction { .map(|(_, (txout, _))| txout) } - fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } - - fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - let contributed_inputs = self - .tx - .input - .into_iter() - .zip(self.input_metadata.iter()) - .enumerate() - .filter(|(_, (_, input))| input.is_local(self.holder_is_initiator)) - .filter(|(index, _)| { - self.shared_input_index - .map(|shared_index| *index != shared_index as usize) - .unwrap_or(true) - }) - .map(|(_, (txin, _))| txin.previous_output) - .collect(); - - let contributed_outputs = self - .tx - .output - .into_iter() - .zip(self.output_metadata.iter()) - .enumerate() - .filter(|(_, (_, output))| output.is_local(self.holder_is_initiator)) - .filter(|(index, _)| *index != self.shared_output_index as usize) - .map(|(_, (txout, _))| txout) - .collect(); - - (contributed_inputs, contributed_outputs) - } - pub fn tx(&self) -> &Transaction { &self.tx } @@ -921,10 +875,6 @@ impl InteractiveTxSigningSession { Ok(()) } - pub(crate) fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - self.unsigned_tx.into_negotiation_error(reason) - } - pub(super) fn contributed_inputs(&self) -> impl Iterator + '_ { self.unsigned_tx.contributed_inputs() } @@ -932,14 +882,6 @@ impl InteractiveTxSigningSession { pub(super) fn contributed_outputs(&self) -> impl Iterator + '_ { self.unsigned_tx.contributed_outputs() } - - pub(super) fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } - - pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - self.unsigned_tx.into_contributed_inputs_and_outputs() - } } impl_writeable_tlv_based!(InteractiveTxSigningSession, { @@ -2172,27 +2114,6 @@ impl InteractiveTxConstructor { Self::new(args, false) } - fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - let (contributed_inputs, contributed_outputs) = self.into_contributed_inputs_and_outputs(); - NegotiationError { reason, contributed_inputs, contributed_outputs } - } - - pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - let contributed_inputs = self - .inputs_to_contribute - .into_iter() - .filter(|(_, input)| !input.is_shared()) - .map(|(_, input)| input.into_tx_in().previous_output) - .collect(); - let contributed_outputs = self - .outputs_to_contribute - .into_iter() - .filter(|(_, output)| !output.is_shared()) - .map(|(_, output)| output.into_tx_out()) - .collect(); - (contributed_inputs, contributed_outputs) - } - pub(super) fn contributed_inputs(&self) -> impl Iterator + '_ { self.inputs_to_contribute .iter() @@ -2207,10 +2128,6 @@ impl InteractiveTxConstructor { .map(|(_, output)| output.tx_out()) } - pub(super) fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) - } - pub fn is_initiator(&self) -> bool { self.is_initiator }