From 554d833c2709f9da3c1c6852ee471385194a085c Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Mar 2026 10:50:26 -0700 Subject: [PATCH 1/2] Allow cancellation of pending splice funding negotiations A user may wish to cancel an in-flight funding negotiation for whatever reason (e.g., mempool feerates have gone down, inability to sign, etc.), so we should make it possible for them to do so. Note that this can only be done for splice funding negotiations for which the user has made a contribution to. --- lightning/src/events/mod.rs | 20 +- lightning/src/ln/channel.rs | 101 ++++++-- lightning/src/ln/channelmanager.rs | 164 ++++++------ lightning/src/ln/interactivetxs.rs | 6 + lightning/src/ln/splicing_tests.rs | 403 ++++++++++++++++++++++++++++- 5 files changed, 575 insertions(+), 119 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 0d5b8f757ab..2be6ef13965 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -137,8 +137,10 @@ pub enum NegotiationFailureReason { /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel /// [`FundingTemplate`]: crate::ln::funding::FundingTemplate ContributionInvalid, - /// The negotiation was locally abandoned via `ChannelManager::abandon_splice`. - LocallyAbandoned, + /// The negotiation was locally canceled via [`ChannelManager::cancel_funding_contributed`]. + /// + /// [`ChannelManager::cancel_funding_contributed`]: crate::ln::channelmanager::ChannelManager::cancel_funding_contributed + LocallyCanceled, /// The channel is closing, so the negotiation cannot continue. See [`Event::ChannelClosed`] /// for the closure reason. ChannelClosing, @@ -171,7 +173,7 @@ impl NegotiationFailureReason { | Self::FeeRateTooLow => true, Self::CounterpartyAborted { .. } | Self::NegotiationError { .. } - | Self::LocallyAbandoned + | Self::LocallyCanceled | Self::ChannelClosing | Self::CannotInitiateRbf => false, } @@ -188,7 +190,7 @@ impl core::fmt::Display for NegotiationFailureReason { }, Self::NegotiationError { msg } => write!(f, "negotiation error: {}", msg), Self::ContributionInvalid => f.write_str("funding contribution was invalid"), - Self::LocallyAbandoned => f.write_str("splice locally abandoned"), + Self::LocallyCanceled => f.write_str("splice locally canceled"), Self::ChannelClosing => f.write_str("channel is closing"), Self::FeeRateTooLow => f.write_str("feerate too low for RBF"), @@ -207,7 +209,7 @@ impl_writeable_tlv_based_enum_upgradable!(NegotiationFailureReason, (1, msg, required), }, (9, ContributionInvalid) => {}, - (11, LocallyAbandoned) => {}, + (11, LocallyCanceled) => {}, (13, ChannelClosing) => {}, (15, FeeRateTooLow) => {}, (17, CannotInitiateRbf) => {}, @@ -1955,7 +1957,7 @@ pub enum Event { invoice_request: InvoiceRequest, }, /// Indicates that a channel funding transaction constructed interactively is ready to be - /// signed. This event will only be triggered if at least one input was contributed. + /// signed. This event will only be triggered if a contribution was made to the transaction. /// /// The transaction contains all inputs and outputs provided by both parties including the /// channel's funding output and a change output if applicable. @@ -1966,8 +1968,9 @@ pub enum Event { /// Each signature MUST use the `SIGHASH_ALL` flag to avoid invalidation of the initial commitment and /// hence possible loss of funds. /// - /// After signing, call [`ChannelManager::funding_transaction_signed`] with the (partially) signed - /// funding transaction. + /// After signing, call [`ChannelManager::funding_transaction_signed`] with the (partially) + /// signed funding transaction. For splices where you contributed inputs or outputs, call + /// [`ChannelManager::cancel_funding_contributed`] instead if you no longer wish to proceed. /// /// Generated in [`ChannelManager`] message handling. /// @@ -1976,6 +1979,7 @@ pub enum Event { /// returning `Err(ReplayEvent ())`), but will only be regenerated as needed after restarts. /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager + /// [`ChannelManager::cancel_funding_contributed`]: crate::ln::channelmanager::ChannelManager::cancel_funding_contributed /// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed FundingTransactionReadyForSigning { /// The `channel_id` of the channel which you'll need to pass back into diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 18236d761a0..d37ab2be400 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12779,30 +12779,93 @@ where } } - #[cfg(test)] - pub fn abandon_splice( - &mut self, - ) -> Result<(msgs::TxAbort, Option), APIError> { - if self.should_reset_pending_splice_state(false) { - let tx_abort = - msgs::TxAbort { channel_id: self.context.channel_id(), data: Vec::new() }; - let splice_funding_failed = self.reset_pending_splice_state(); - Ok((tx_abort, splice_funding_failed)) - } else if self.has_pending_splice_awaiting_signatures() { - Err(APIError::APIMisuseError { + pub fn cancel_funding_contributed(&mut self) -> Result { + if matches!(self.quiescent_action, Some(QuiescentAction::Splice { .. })) { + let splice_funding_failed = self.abandon_quiescent_action(); + debug_assert!(splice_funding_failed.is_some()); + let str = "Manually canceled funding contribution"; + let err = if self.context.channel_state.is_local_stfu_sent() + && !self.context.channel_state.is_remote_stfu_sent() + { + // If we've already sent `stfu` and haven't received the counterparty's yet, we know + // it corresponds to our action. + ChannelError::WarnAndDisconnect(str.into()) + } else { + // We don't need to send `tx_abort` because our action still pending means we're not + // quiescent for it. + ChannelError::Ignore(str.into()) + }; + return Ok(InteractiveTxMsgError { err, splice_funding_failed }); + } + + let funding_negotiation = self + .pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()); + let Some(funding_negotiation) = funding_negotiation else { + return Err(APIError::APIMisuseError { err: format!( - "Channel {} splice cannot be abandoned; already awaiting signatures", - self.context.channel_id(), + "Channel {} does not have a pending splice negotiation", + self.context.channel_id() ), - }) - } else { - Err(APIError::APIMisuseError { + }); + }; + + let made_contribution = match funding_negotiation { + FundingNegotiation::AwaitingAck { context, .. } => { + context.contributed_inputs().next().is_some() + || context.contributed_outputs().next().is_some() + }, + FundingNegotiation::ConstructingTransaction { interactive_tx_constructor, .. } => { + interactive_tx_constructor.contributed_inputs().next().is_some() + || interactive_tx_constructor.contributed_outputs().next().is_some() + }, + FundingNegotiation::AwaitingSignatures { .. } => self + .context + .interactive_tx_signing_session + .as_ref() + .expect("We have a pending splice awaiting signatures") + .has_local_contribution(), + }; + if !made_contribution { + return Err(APIError::APIMisuseError { err: format!( - "Channel {} splice cannot be abandoned; no pending splice", - self.context.channel_id(), + "Channel {} has a pending splice negotiation with no contribution made", + self.context.channel_id() ), - }) + }); } + + // We typically don't reset the pending funding negotiation when we're in + // [`FundingNegotiation::AwaitingSignatures`] since we're able to resume it on + // re-establishment, so we still need to handle this case separately if the user wishes to + // cancel. If they've yet to call [`Channel::funding_transaction_signed`], then we can + // guarantee to never have sent any signatures to the counterparty, or have processed any + // signatures from them. + if matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) { + let already_signed = self + .context + .interactive_tx_signing_session + .as_ref() + .expect("We have a pending splice awaiting signatures") + .has_holder_tx_signatures(); + if already_signed { + return Err(APIError::APIMisuseError { + err: format!( + "Channel {} has pending splice negotiation that was already signed", + self.context.channel_id(), + ), + }); + } + } + + debug_assert!(self.context.channel_state.is_quiescent()); + let splice_funding_failed = self.reset_pending_splice_state(); + debug_assert!(splice_funding_failed.is_some()); + Ok(InteractiveTxMsgError { + err: ChannelError::Abort(AbortReason::ManualIntervention), + splice_funding_failed, + }) } /// Checks during handling splice_init diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7a3a5bd1ee4..9bcc5414eae 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4920,96 +4920,80 @@ impl< } } - #[cfg(test)] - pub(crate) fn abandon_splice( - &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, - ) -> Result<(), APIError> { - let mut res = Ok(()); - PersistenceNotifierGuard::optionally_notify(self, || { - let result = self.internal_abandon_splice(channel_id, counterparty_node_id); - res = result; - match res { - Ok(_) => NotifyOption::SkipPersistHandleEvents, - Err(_) => NotifyOption::SkipPersistNoEvents, - } - }); - res - } - - #[cfg(test)] - fn internal_abandon_splice( + /// Cancels an in-flight [`FundingContribution`]. + /// + /// This is primarily useful after receiving an [`Event::FundingTransactionReadyForSigning`] for + /// a [`FundingContribution`] you no longer wish to proceed with. This may be called for any + /// pending [`FundingContribution`] after its corresponding + /// [`ChannelManager::funding_contributed`] call up until + /// [`ChannelManager::funding_transaction_signed`]. + /// + /// Returns [`ChannelUnavailable`] when a channel is not found or an incorrect + /// `counterparty_node_id` is provided, or [`APIMisuseError`] otherwise with the error details. + /// + /// [`Event::FundingTransactionReadyForSigning`]: events::Event::FundingTransactionReadyForSigning + /// [`ChannelUnavailable`]: APIError::ChannelUnavailable + /// [`APIMisuseError`]: APIError::APIMisuseError + pub fn cancel_funding_contributed( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, ) -> Result<(), APIError> { - let per_peer_state = self.per_peer_state.read().unwrap(); - - let peer_state_mutex = match per_peer_state - .get(counterparty_node_id) - .ok_or_else(|| APIError::no_such_peer(counterparty_node_id)) - { - Ok(p) => p, - Err(e) => return Err(e), - }; - - let mut peer_state_lock = peer_state_mutex.lock().unwrap(); - let peer_state = &mut *peer_state_lock; - - // Look for the channel - match peer_state.channel_by_id.entry(*channel_id) { - hash_map::Entry::Occupied(mut chan_phase_entry) => { - if !chan_phase_entry.get().context().is_connected() { - // TODO: We should probably support this, but right now `splice_channel` refuses when - // the peer is disconnected, so we just check it here. - return Err(APIError::ChannelUnavailable { - err: "Cannot abandon splice while peer is disconnected".to_owned(), - }); - } - - if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { - let (tx_abort, splice_funding_failed) = chan.abandon_splice()?; - - peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort { - node_id: *counterparty_node_id, - msg: tx_abort, - }); + let mut result = Ok(()); + PersistenceNotifierGuard::manually_notify(self, || { + let per_peer_state = self.per_peer_state.read().unwrap(); + let peer_state_mutex = match per_peer_state + .get(counterparty_node_id) + .ok_or_else(|| APIError::no_such_peer(counterparty_node_id)) + { + Ok(p) => p, + Err(e) => { + result = Err(e); + return; + }, + }; + let mut peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &mut *peer_state_lock; - if let Some(splice_funding_failed) = splice_funding_failed { - let (funding_info, contribution) = splice_funding_failed.into_parts(); - let pending_events = &mut self.pending_events.lock().unwrap(); - if let Some(funding_info) = funding_info { - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *channel_id, - funding_info, - }, - None, - )); - } - pending_events.push_back(( - events::Event::SpliceNegotiationFailed { - channel_id: *channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: chan.context.get_user_id(), - contribution, - reason: events::NegotiationFailureReason::LocallyAbandoned, + match peer_state.channel_by_id.entry(*channel_id) { + hash_map::Entry::Occupied(mut chan_entry) => { + if let Some(channel) = chan_entry.get_mut().as_funded_mut() { + let err = match channel.cancel_funding_contributed() { + Ok(v) => v, + Err(e) => { + result = Err(e); + return; }, - None, - )); - } + }; + let user_channel_id = channel.context().get_user_id(); + mem::drop(peer_state_lock); + mem::drop(per_peer_state); - Ok(()) - } else { - Err(APIError::ChannelUnavailable { - err: format!( - "Channel with id {} is not funded, cannot abandon splice", - channel_id - ), - }) - } - }, - hash_map::Entry::Vacant(_) => { - Err(APIError::no_such_channel_for_peer(channel_id, counterparty_node_id)) - }, - } + let err = self.handle_interactive_tx_msg_err( + err, + *channel_id, + counterparty_node_id, + user_channel_id, + Some(events::NegotiationFailureReason::LocallyCanceled), + ); + let _ = self.handle_error(Err::<(), _>(err), *counterparty_node_id); + self.event_persist_notifier.notify(); + } else { + result = Err(APIError::ChannelUnavailable { + err: format!( + "Channel with id {} is not funded, cannot cancel splice", + channel_id + ), + }); + return; + } + }, + hash_map::Entry::Vacant(_) => { + result = + Err(APIError::no_such_channel_for_peer(channel_id, counterparty_node_id)); + return; + }, + } + }); + result } fn forward_needs_intercept_to_known_chan( @@ -11962,7 +11946,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ fn handle_interactive_tx_msg_err( &self, err: InteractiveTxMsgError, channel_id: ChannelId, counterparty_node_id: &PublicKey, - user_channel_id: u128, + user_channel_id: u128, reason: Option, ) -> MsgHandleErrInternal { if let Some(splice_funding_failed) = err.splice_funding_failed { let (funding_info, contribution) = splice_funding_failed.into_parts(); @@ -11977,9 +11961,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ counterparty_node_id: *counterparty_node_id, user_channel_id, contribution, - reason: events::NegotiationFailureReason::NegotiationError { + reason: reason.unwrap_or(events::NegotiationFailureReason::NegotiationError { msg: format!("{:?}", err.err), - }, + }), }, None, )); @@ -12015,6 +11999,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ channel_id, counterparty_node_id, user_channel_id, + None, )) }, } @@ -12150,6 +12135,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ msg.channel_id, &counterparty_node_id, user_channel_id, + None, )) }, } @@ -13395,6 +13381,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ msg.channel_id, counterparty_node_id, user_channel_id, + None, )) }, } @@ -13452,6 +13439,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ msg.channel_id, counterparty_node_id, user_channel_id, + None, )) }, } diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 10dae95cefa..43589137c0b 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -136,6 +136,11 @@ pub(crate) enum AbortReason { NegotiationInProgress, /// The initiator's feerate exceeds our maximum. FeeRateTooHigh, + /// The user manually intervened to abort the funding negotiation via + /// [`ChannelManager::cancel_funding_contributed`]. + /// + /// [`ChannelManager::cancel_funding_contributed`]: crate::ln::channelmanager::ChannelManager::cancel_funding_contributed + ManualIntervention, /// Internal error InternalError(&'static str), } @@ -202,6 +207,7 @@ impl Display for AbortReason { AbortReason::FeeRateTooHigh => { f.write_str("The initiator's feerate exceeds our maximum") }, + AbortReason::ManualIntervention => f.write_str("Manually aborted funding negotiation"), AbortReason::InternalError(text) => { f.write_fmt(format_args!("Internal error: {}", text)) }, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 1b6879e69f0..b0ca6e494c5 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -2815,8 +2815,8 @@ fn fail_splice_on_tx_abort() { let _tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - acceptor.node.abandon_splice(&channel_id, &node_id_initiator).unwrap(); - let tx_abort = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + // Inject a fake `tx_abort` to the initiator to trigger the splice to be aborted. + let tx_abort = msgs::TxAbort { channel_id, data: Vec::new() }; initiator.node.handle_tx_abort(node_id_acceptor, &tx_abort); expect_splice_failed_events( @@ -2833,6 +2833,9 @@ fn fail_splice_on_tx_abort() { check_added_monitors(initiator, 1); if let MessageSendEvent::SendTxAbort { msg, .. } = &msg_events[0] { acceptor.node.handle_tx_abort(node_id_initiator, msg); + // The acceptor still tries to ack the abort by sending its own back to the initiator since + // a fake one was originally sent to it. + let _ = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); } else { panic!("Unexpected event {:?}", msg_events[0]); }; @@ -2844,6 +2847,398 @@ fn fail_splice_on_tx_abort() { }; } +#[test] +fn acceptor_with_local_contribution_can_cancel_funding_contributed_before_funding_transaction_signed( +) { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let initiator = &nodes[0]; + let acceptor = &nodes[1]; + + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); + + let initial_channel_capacity = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); + + provide_utxo_reserves(&nodes, 2, Amount::ONE_BTC); + + let outputs = vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: initiator.wallet_source.get_change_script().unwrap(), + }]; + let initiator_contribution = + initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap(); + let acceptor_contribution = initiate_splice_in( + acceptor, + initiator, + channel_id, + Amount::from_sat(initial_channel_capacity / 2), + ); + + let stfu_initiator = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); + let stfu_acceptor = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator); + + acceptor.node.handle_stfu(node_id_initiator, &stfu_initiator); + assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + + initiator.node.handle_stfu(node_id_acceptor, &stfu_acceptor); + + let splice_init = get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor); + acceptor.node.handle_splice_init(node_id_initiator, &splice_init); + let splice_ack = get_event_msg!(acceptor, MessageSendEvent::SendSpliceAck, node_id_initiator); + assert_ne!(splice_ack.funding_contribution_satoshis, 0); + initiator.node.handle_splice_ack(node_id_acceptor, &splice_ack); + + let new_funding_script = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + complete_interactive_funding_negotiation_for_both( + initiator, + acceptor, + channel_id, + initiator_contribution.clone(), + Some(acceptor_contribution.clone()), + splice_ack.funding_contribution_satoshis, + new_funding_script, + ); + + let event = get_event!(initiator, Event::FundingTransactionReadyForSigning); + if let Event::FundingTransactionReadyForSigning { + channel_id, + counterparty_node_id, + unsigned_transaction, + .. + } = event + { + let partially_signed_tx = initiator.wallet_source.sign_tx(unsigned_transaction).unwrap(); + initiator + .node + .funding_transaction_signed(&channel_id, &counterparty_node_id, partially_signed_tx) + .unwrap(); + } else { + unreachable!(); + } + + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + let initial_commit_sig = if let MessageSendEvent::UpdateHTLCs { updates, .. } = &msg_events[0] { + updates.commitment_signed[0].clone() + } else { + panic!("Unexpected event {:?}", msg_events[0]); + }; + acceptor.node.handle_commitment_signed(node_id_initiator, &initial_commit_sig); + assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + + let _signing_event = get_event!(acceptor, Event::FundingTransactionReadyForSigning); + + acceptor.node.cancel_funding_contributed(&channel_id, &node_id_initiator).unwrap(); + let events = acceptor.node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + assert!(matches!(events[0], Event::DiscardFunding { .. })); + assert!(matches!(events[1], Event::SpliceNegotiationFailed { .. })); + let tx_abort = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + + initiator.node.handle_tx_abort(node_id_acceptor, &tx_abort); + let reason = NegotiationFailureReason::CounterpartyAborted { + msg: UntrustedString("Manually aborted funding negotiation".into()), + }; + expect_splice_failed_events(initiator, &channel_id, initiator_contribution, reason); + let tx_abort = get_event_msg!(initiator, MessageSendEvent::SendTxAbort, node_id_acceptor); + acceptor.node.handle_tx_abort(node_id_initiator, &tx_abort); +} + +#[test] +fn acceptor_can_cancel_queued_funding_contributed_during_counterparty_splice() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let acceptor = &nodes[0]; + let initiator = &nodes[1]; + + let node_id_acceptor = acceptor.node.get_our_node_id(); + let node_id_initiator = initiator.node.get_our_node_id(); + + let initial_channel_capacity = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + let initiator_contribution = + do_initiate_splice_in(initiator, acceptor, channel_id, added_value); + + let stfu_initiator = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); + acceptor.node.handle_stfu(node_id_initiator, &stfu_initiator); + let stfu_acceptor = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator); + initiator.node.handle_stfu(node_id_acceptor, &stfu_acceptor); + + let splice_init = get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor); + acceptor.node.handle_splice_init(node_id_initiator, &splice_init); + let splice_ack = get_event_msg!(acceptor, MessageSendEvent::SendSpliceAck, node_id_initiator); + assert_eq!(splice_ack.funding_contribution_satoshis, 0); + + let funding_template = acceptor.node.splice_channel(&channel_id, &node_id_initiator).unwrap(); + let feerate = funding_template.min_rbf_feerate().unwrap(); + let wallet = WalletSync::new(Arc::clone(&acceptor.wallet_source), acceptor.logger); + let queued_contribution = funding_template + .splice_in_sync(Amount::from_sat(25_000), feerate, FeeRate::MAX, &wallet) + .unwrap(); + acceptor + .node + .funding_contributed(&channel_id, &node_id_initiator, queued_contribution.clone(), None) + .unwrap(); + assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + + acceptor.node.cancel_funding_contributed(&channel_id, &node_id_initiator).unwrap(); + let reason = NegotiationFailureReason::LocallyCanceled; + expect_splice_failed_events(acceptor, &channel_id, queued_contribution, reason); + assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + + initiator.node.handle_splice_ack(node_id_acceptor, &splice_ack); + let new_funding_script = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + complete_interactive_funding_negotiation( + initiator, + acceptor, + channel_id, + initiator_contribution, + new_funding_script, + ); + + let (splice_tx, splice_locked) = sign_interactive_funding_tx(initiator, acceptor, false, None); + assert!(splice_locked.is_none()); + expect_splice_pending_event(initiator, &node_id_acceptor); + expect_splice_pending_event(acceptor, &node_id_initiator); + + mine_transaction(initiator, &splice_tx); + mine_transaction(acceptor, &splice_tx); + assert!(lock_splice_after_blocks(initiator, acceptor, ANTI_REORG_DELAY - 1).stfu.is_none()); +} + +#[test] +fn cancel_funding_contributed_before_funding_transaction_signed() { + do_cancel_funding_contributed_before_funding_transaction_signed(0); // AwaitingQuiescence + do_cancel_funding_contributed_before_funding_transaction_signed(1); // AwaitingAck + do_cancel_funding_contributed_before_funding_transaction_signed(2); // ConstructingTransaction + do_cancel_funding_contributed_before_funding_transaction_signed(3); // AwaitingSignatures +} + +#[cfg(test)] +fn do_cancel_funding_contributed_before_funding_transaction_signed(state: u8) { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let initiator = &nodes[0]; + let acceptor = &nodes[1]; + + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); + + let initial_channel_capacity = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); + + let outputs = vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: initiator.wallet_source.get_change_script().unwrap(), + }]; + let funding_contribution = + initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap(); + + match state { + 0 => { + // Cancel after funding_contributed queues `stfu`, but before the quiescence attempt is + // delivered to the peer. + }, + 1 => { + // Deliver splice_init, but keep splice_ack queued so the initiator remains in + // FundingNegotiation::AwaitingAck while the acceptor tracks the pending splice. + let stfu_init = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); + acceptor.node.handle_stfu(node_id_initiator, &stfu_init); + let stfu_ack = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator); + initiator.node.handle_stfu(node_id_acceptor, &stfu_ack); + + let splice_init = + get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor); + acceptor.node.handle_splice_init(node_id_initiator, &splice_init); + assert!(initiator.node.get_and_clear_pending_msg_events().is_empty()); + + let msg_events = acceptor.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + assert!(matches!(msg_events[0], MessageSendEvent::SendSpliceAck { .. })); + }, + 2 => { + // Complete the splice handshake so the initiator is constructing the interactive tx. + let _new_funding_script = complete_splice_handshake(initiator, acceptor); + + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + assert!(matches!(msg_events[0], MessageSendEvent::SendTxAddInput { .. })); + assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + }, + 3 => { + // Complete interactive tx negotiation so the initiator is awaiting funding signatures. + let new_funding_script = complete_splice_handshake(initiator, acceptor); + complete_interactive_funding_negotiation( + initiator, + acceptor, + channel_id, + funding_contribution.clone(), + new_funding_script, + ); + + // The initiator should have a signing event to handle, while the acceptor immediately + // sends their initial commitment_signed. Deliver it before canceling to ensure it gets + // discarded with the splice. + let _signing_event = get_event!(initiator, Event::FundingTransactionReadyForSigning); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); + let acceptor_commit_sig = get_htlc_update_msgs(acceptor, &node_id_initiator); + initiator.node.handle_commitment_signed( + node_id_acceptor, + &acceptor_commit_sig.commitment_signed[0], + ); + check_added_monitors(initiator, 0); + assert!(initiator.node.get_and_clear_pending_msg_events().is_empty()); + }, + _ => panic!("unexpected state {state}"), + } + assert!(initiator.node.get_and_clear_pending_events().is_empty()); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); + + // Queue an outgoing HTLC to the holding cell. It should be freed once we cancel the splice and + // exit quiescence. + if state != 0 { + let (route, payment_hash, _payment_preimage, payment_secret) = + get_route_and_payment_hash!(initiator, acceptor, 1_000_000); + let onion = RecipientOnionFields::secret_only(payment_secret, 1_000_000); + let payment_id = PaymentId(payment_hash.0); + initiator.node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap(); + assert!(initiator.node.get_and_clear_pending_msg_events().is_empty()); + } + + initiator.node.cancel_funding_contributed(&channel_id, &node_id_acceptor).unwrap(); + let reason = NegotiationFailureReason::LocallyCanceled; + expect_splice_failed_events(initiator, &channel_id, funding_contribution, reason); + + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2, "{msg_events:?}"); + if state == 0 { + // We didn't reach quiescence prior to canceling, so we should see our `stfu` followed by a + // disconnect. + if let MessageSendEvent::SendStfu { .. } = &msg_events[0] { + } else { + panic!("Unexpected event {:?}", msg_events[0]); + } + if let MessageSendEvent::HandleError { action, .. } = &msg_events[1] { + assert!(matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })); + } else { + panic!("Unexpected event {:?}", msg_events[1]); + } + return; + } + + // We exit or terminate the quiescence attempt upon canceling the splice, so we should see a + // tx_abort followed by the holding cell HTLC being released immediately. + let tx_abort = if let MessageSendEvent::SendTxAbort { msg, .. } = &msg_events[0] { + msg + } else { + panic!("Unexpected event {:?}", msg_events[0]); + }; + let update = if let MessageSendEvent::UpdateHTLCs { updates, .. } = &msg_events[1] { + updates + } else { + panic!("Unexpected event {:?}", msg_events[1]); + }; + check_added_monitors(initiator, 1); + + acceptor.node.handle_tx_abort(node_id_initiator, tx_abort); + let tx_abort = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + initiator.node.handle_tx_abort(node_id_acceptor, &tx_abort); + + acceptor.node.handle_update_add_htlc(node_id_initiator, &update.update_add_htlcs[0]); + do_commitment_signed_dance(acceptor, initiator, &update.commitment_signed, false, false); +} + +#[test] +fn cannot_cancel_funding_contributed_after_funding_transaction_signed() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let initiator = &nodes[0]; + let acceptor = &nodes[1]; + + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); + + let initial_channel_capacity = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); + + let outputs = vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: initiator.wallet_source.get_change_script().unwrap(), + }]; + let funding_contribution = + initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap(); + let new_funding_script = complete_splice_handshake(initiator, acceptor); + complete_interactive_funding_negotiation( + initiator, + acceptor, + channel_id, + funding_contribution, + new_funding_script, + ); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); + let _acceptor_commit_sig = get_htlc_update_msgs(acceptor, &node_id_initiator); + + let event = get_event!(initiator, Event::FundingTransactionReadyForSigning); + if let Event::FundingTransactionReadyForSigning { + channel_id, + counterparty_node_id, + unsigned_transaction, + .. + } = event + { + let partially_signed_tx = initiator.wallet_source.sign_tx(unsigned_transaction).unwrap(); + initiator + .node + .funding_transaction_signed(&channel_id, &counterparty_node_id, partially_signed_tx) + .unwrap(); + } else { + unreachable!(); + } + + let res = initiator.node.cancel_funding_contributed(&channel_id, &node_id_acceptor); + match res { + Err(APIError::APIMisuseError { err }) => assert!(err.contains("already signed")), + _ => panic!("Unexpected result {res:?}"), + } + + assert!(initiator.node.get_and_clear_pending_events().is_empty()); + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert!( + msg_events.iter().all(|event| !matches!(event, MessageSendEvent::SendTxAbort { .. })), + "{msg_events:?}" + ); +} + #[test] fn fail_splice_on_tx_complete_error() { let chanmon_cfgs = create_chanmon_cfgs(2); @@ -7773,13 +8168,13 @@ fn test_no_disconnect_after_splice_aborted() { nodes[1].node.timer_tick_occurred(); // Abort the splice, which should clear the timer when exiting quiescence. - nodes[0].node.abandon_splice(&channel_id, &node_id_1).unwrap(); + nodes[0].node.cancel_funding_contributed(&channel_id, &node_id_1).unwrap(); expect_splice_failed_events( &nodes[0], &channel_id, funding_contribution, - NegotiationFailureReason::LocallyAbandoned, + NegotiationFailureReason::LocallyCanceled, ); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); From 637cc413aad41e37c5d5975b493d452c46da0279 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 19 Mar 2026 11:57:20 -0700 Subject: [PATCH 2/2] Rename should_reset_pending_splice_state argument There's a case in `should_reset_pending_splice_state` where we are awaiting signatures, but still want to preserve the pending negotiation upon a disconnection. We previously used `counterparty_aborted` as a way to toggle this behavior. Now that we support the user manually canceling an ongoing negotiation, we interpret the argument a bit more generically in terms of whether we wish to resume the negotiation or not when we are found in such a state. --- lightning/src/ln/channel.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index d37ab2be400..3f3a6feb414 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1724,7 +1724,7 @@ where if matches!(chan.context.channel_state, ChannelState::ChannelReady(_)) { chan.context.channel_state.clear_local_stfu_sent(); chan.context.channel_state.clear_remote_stfu_sent(); - if chan.should_reset_pending_splice_state(false) { + if chan.should_reset_pending_splice_state(true) { // If there was a pending splice negotiation that failed due to disconnecting, we // also take the opportunity to clean up our state. let splice_funding_failed = chan.reset_pending_splice_state(); @@ -1841,7 +1841,7 @@ where None }, ChannelPhase::Funded(funded_channel) => { - if funded_channel.should_reset_pending_splice_state(false) { + if funded_channel.should_reset_pending_splice_state(true) { funded_channel.reset_pending_splice_state() } else { debug_assert!(false, "We should never fail an interactive funding negotiation once we're exchanging tx_signatures"); @@ -2024,7 +2024,7 @@ where "Received tx_abort while awaiting tx_signatures exchange".to_owned(), )); } - if funded_channel.should_reset_pending_splice_state(true) { + if funded_channel.should_reset_pending_splice_state(false) { let has_funding_negotiation = funded_channel .pending_splice .as_ref() @@ -7159,7 +7159,7 @@ where fn maybe_fail_splice_negotiation(&mut self) -> Option { if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { - if self.should_reset_pending_splice_state(false) { + if self.should_reset_pending_splice_state(true) { self.reset_pending_splice_state() } else { self.abandon_quiescent_action() @@ -7216,7 +7216,7 @@ where /// Returns a boolean indicating whether we should reset the splice's /// [`PendingFunding::funding_negotiation`]. - fn should_reset_pending_splice_state(&self, counterparty_aborted: bool) -> bool { + fn should_reset_pending_splice_state(&self, allow_resumption: bool) -> bool { self.pending_splice .as_ref() .map(|pending_splice| { @@ -7228,7 +7228,11 @@ where funding_negotiation, FundingNegotiation::AwaitingSignatures { .. } ); - if counterparty_aborted { + if allow_resumption { + // If we want to resume the negotiation after reconnecting, we must be + // in [`FundingNegotiation::AwaitingSignatures`] to not reset our state. + !is_awaiting_signatures + } else { !is_awaiting_signatures || !self .context() @@ -7236,8 +7240,6 @@ where .as_ref() .expect("We have a pending splice awaiting signatures") .has_received_commitment_signed() - } else { - !is_awaiting_signatures } }) .unwrap_or_else(|| { @@ -7251,7 +7253,7 @@ where } fn reset_pending_splice_state(&mut self) -> Option { - debug_assert!(self.should_reset_pending_splice_state(true)); + debug_assert!(self.should_reset_pending_splice_state(false)); // Only clear the signing session if the current round is mid-signing. When an earlier // round completed signing and a later RBF round is in AwaitingAck or @@ -7325,7 +7327,7 @@ where } pub(super) fn maybe_splice_funding_failed(&self) -> Option { - if !self.should_reset_pending_splice_state(false) { + if !self.should_reset_pending_splice_state(true) { return None; } @@ -15647,7 +15649,7 @@ impl Writeable for FundedChannel { ChannelState::ChannelReady(_) => { channel_state.clear_local_stfu_sent(); channel_state.clear_remote_stfu_sent(); - if self.should_reset_pending_splice_state(false) + if self.should_reset_pending_splice_state(true) || !self.has_pending_splice_awaiting_signatures() { // We shouldn't be quiescent anymore upon reconnecting if: @@ -16037,7 +16039,7 @@ impl Writeable for FundedChannel { // We don't have to worry about resetting the pending `FundingNegotiation` because we // can only read `FundingNegotiation::AwaitingSignatures` variants anyway. let pending_splice = - self.pending_splice.as_ref().filter(|_| !self.should_reset_pending_splice_state(false)); + self.pending_splice.as_ref().filter(|_| !self.should_reset_pending_splice_state(true)); let monitor_pending_tx_signatures = self.context.monitor_pending_tx_signatures.then_some(());