From 6bad3fd84711fdf45e33a498ebec442ab608dc15 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 | 8 +- lightning/src/ln/channel.rs | 84 ++++++-- lightning/src/ln/channelmanager.rs | 173 +++++++++-------- lightning/src/ln/interactivetxs.rs | 3 + lightning/src/ln/splicing_tests.rs | 299 ++++++++++++++++++++++++++++- 5 files changed, 460 insertions(+), 107 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 73c4a39c76f..0f873fab07b 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1833,7 +1833,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. @@ -1844,8 +1844,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. /// @@ -1854,6 +1855,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 32c0e94bdc8..3f743119d0d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12659,30 +12659,76 @@ 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 { + 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, + exited_quiescence: true, + }) } /// Checks during handling splice_init diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 758b1e30d8c..1d6e5343591 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4841,96 +4841,103 @@ 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. Canceling is only allowed up + /// until [`ChannelManager::funding_transaction_signed`] is called for the corresponding + /// [`FundingContribution`]. + /// + /// 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; + 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; - // 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(), - }); - } + 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 InteractiveTxMsgError { err, splice_funding_failed, exited_quiescence } = + match channel.cancel_funding_contributed() { + Ok(v) => v, + Err(e) => { + result = Err(e); + return; + }, + }; - if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { - let (tx_abort, splice_funding_failed) = chan.abandon_splice()?; + let splice_funding_failed = splice_funding_failed + .expect("Only splices with local contributions can be canceled"); + { + 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: channel.context().get_user_id(), + abandoned_funding_txo: splice_funding_failed.funding_txo, + channel_type: splice_funding_failed.channel_type.clone(), + }, + None, + )); + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id: *channel_id, + funding_info: FundingInfo::Contribution { + inputs: splice_funding_failed.contributed_inputs, + outputs: splice_funding_failed.contributed_outputs, + }, + }, + None, + )); + } - peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort { - node_id: *counterparty_node_id, - msg: tx_abort, - }); + mem::drop(peer_state_lock); + mem::drop(per_peer_state); - 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(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, - }, - None, - )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, - }, - }, - None, - )); + self.needs_persist_flag.store(true, Ordering::Release); + self.event_persist_notifier.notify(); + let err: Result<(), _> = + Err(MsgHandleErrInternal::from_chan_no_close(err, *channel_id) + .with_exited_quiescence(exited_quiescence)); + let _ = self.handle_error(err, *counterparty_node_id); + } else { + result = Err(APIError::ChannelUnavailable { + err: format!( + "Channel with id {} is not funded, cannot cancel splice", + channel_id + ), + }); + return; } - - 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)) - }, - } + }, + 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( diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 9957205716c..be844f73c2d 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -143,6 +143,8 @@ pub(crate) enum AbortReason { NegotiationInProgress, /// The initiator's feerate exceeds our maximum. FeeRateTooHigh, + /// The user manually intervened to abort the funding negotiation. + ManualIntervention, /// Internal error InternalError(&'static str), } @@ -209,6 +211,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 9adccd17627..9ddf5f22f87 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -2627,8 +2627,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(initiator, &channel_id, funding_contribution); @@ -2640,6 +2640,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]); }; @@ -2651,6 +2654,298 @@ 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::SpliceFailed { .. })); + assert!(matches!(events[1], Event::DiscardFunding { .. })); + 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, initiator_contribution); + let tx_abort = get_event_msg!(initiator, MessageSendEvent::SendTxAbort, node_id_acceptor); + acceptor.node.handle_tx_abort(node_id_initiator, &tx_abort); +} + +#[test] +fn cancel_funding_contributed_before_funding_transaction_signed() { + do_cancel_funding_contributed_before_funding_transaction_signed(0); // AwaitingAck + do_cancel_funding_contributed_before_funding_transaction_signed(1); // ConstructingTransaction + do_cancel_funding_contributed_before_funding_transaction_signed(2); // 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 => { + // 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 { .. })); + }, + 1 => { + // 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()); + }, + 2 => { + // 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. + 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(); + expect_splice_failed_events(initiator, &channel_id, funding_contribution); + + // We exit quiescence upon canceling the splice, so we should see a tx_abort followed by the + // holding cell HTLC being released immediately. + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2, "{msg_events:?}"); + 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); From 7ab9d6f69df81b24e6bf59b00c9dadb8dba4566d 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 3f743119d0d..6300228e72e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1712,7 +1712,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(); @@ -1828,7 +1828,7 @@ where (None, false) }, 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(), true) } else { debug_assert!(false, "We should never fail an interactive funding negotiation once we're exchanging tx_signatures"); @@ -2020,7 +2020,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() @@ -7156,7 +7156,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() @@ -7213,7 +7213,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| { @@ -7225,7 +7225,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() @@ -7233,8 +7237,6 @@ where .as_ref() .expect("We have a pending splice awaiting signatures") .has_received_commitment_signed() - } else { - !is_awaiting_signatures } }) .unwrap_or_else(|| { @@ -7248,7 +7250,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 @@ -7308,7 +7310,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; } @@ -15423,7 +15425,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: @@ -15813,7 +15815,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(());