From 135605fa2953b23fddce609b6b07cbfb9376ae7f Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 2 Dec 2025 11:27:36 -0800 Subject: [PATCH] Rework ChannelManager::funding_transaction_signed Previously, we'd emit a `FundingTransactionReadyForSigning` event once the initial `commitment_signed` is exchanged for a splicing/dual-funding attempt and require users to call back with their signed inputs using `ChannelManager::funding_transaction_signed`. While this approach worked in practice, it prevents us from abandoning a splice if we cannot or no longer wish to sign as the splice has already been committed to by this point. This commit reworks the API such that this is now possible. After exchanging `tx_complete`, we will no longer immediately send our initial `commitment_signed`. We will now emit the `FundingTransactionReadyForSigning` event and wait for the user to call back before releasing both our initial `commitment_signed` and our `tx_signatures`. As a result, the event is now persisted, as there is only one possible path in which it is generated. Note that we continue to only emit the event if a local contribution to negotiated transaction was made. Future work will expose a cancellation API such that we can abandon splice attempts safely (we can just force close the channel with dual-funding). --- lightning/src/events/mod.rs | 39 +++- lightning/src/ln/channel.rs | 357 +++++++++++++++++------------ lightning/src/ln/channelmanager.rs | 281 ++++++++++------------- lightning/src/ln/interactivetxs.rs | 4 +- lightning/src/ln/splicing_tests.rs | 252 ++++++++------------ 5 files changed, 459 insertions(+), 474 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index b9c4b1ca1ef..4542894ffb3 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1835,7 +1835,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 ())`), but will only be regenerated as needed after restarts. + /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager /// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed @@ -2304,10 +2304,19 @@ impl Writeable for Event { 47u8.write(writer)?; // Never write StaticInvoiceRequested events as buffered onion messages aren't serialized. }, - &Event::FundingTransactionReadyForSigning { .. } => { - 49u8.write(writer)?; - // We never write out FundingTransactionReadyForSigning events as they will be regenerated when - // necessary. + &Event::FundingTransactionReadyForSigning { + ref channel_id, + ref counterparty_node_id, + ref user_channel_id, + ref unsigned_transaction, + } => { + 48u8.write(writer)?; + write_tlv_fields!(writer, { + (1, channel_id, required), + (3, counterparty_node_id, required), + (5, user_channel_id, required), + (7, unsigned_transaction, required), + }); }, &Event::SplicePending { ref channel_id, @@ -2930,8 +2939,24 @@ impl MaybeReadable for Event { 45u8 => Ok(None), // Note that we do not write a length-prefixed TLV for StaticInvoiceRequested events. 47u8 => Ok(None), - // Note that we do not write a length-prefixed TLV for FundingTransactionReadyForSigning events. - 49u8 => Ok(None), + 48u8 => { + let mut f = || { + _init_and_read_len_prefixed_tlv_fields!(reader, { + (1, channel_id, required), + (3, counterparty_node_id, required), + (5, user_channel_id, required), + (7, unsigned_transaction, required), + }); + + Ok(Some(Event::FundingTransactionReadyForSigning { + channel_id: channel_id.0.unwrap(), + user_channel_id: user_channel_id.0.unwrap(), + counterparty_node_id: counterparty_node_id.0.unwrap(), + unsigned_transaction: unsigned_transaction.0.unwrap(), + })) + }; + f() + }, 50u8 => { let mut f = || { _init_and_read_len_prefixed_tlv_fields!(reader, { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 5b4ac4c0aa5..b688b21fcc2 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1894,9 +1894,9 @@ where } pub fn tx_complete( - &mut self, msg: &msgs::TxComplete, logger: &L, + &mut self, msg: &msgs::TxComplete, best_block_height: u32, logger: &L, ) -> Result< - (Option, Option), + (Option, Option), (ChannelError, Option), > where @@ -1933,10 +1933,11 @@ where return Ok((interactive_tx_msg_send, None)); }; - let commitment_signed = self - .funding_tx_constructed(funding_outpoint, logger) + let res = self + .funding_tx_constructed(funding_outpoint, best_block_height, logger) .map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))?; - Ok((interactive_tx_msg_send, Some(commitment_signed))) + + Ok((interactive_tx_msg_send, Some(res))) } pub fn tx_abort( @@ -2042,14 +2043,147 @@ where result.map(|monitor| (self.as_funded_mut().expect("Channel should be funded"), monitor)) } + fn generate_tx_signatures( + &mut self, funding_txid_signed: Txid, witnesses: Vec, + ) -> Result<(Option, Option), String> { + let (funding, context, pending_splice) = match &mut self.phase { + ChannelPhase::Undefined => unreachable!(), + ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => { + return Err(format!( + "Channel {} not expecting funding signatures", + self.context().channel_id + )); + }, + ChannelPhase::UnfundedV2(channel) => (&channel.funding, &mut channel.context, None), + ChannelPhase::Funded(channel) => { + (&channel.funding, &mut channel.context, channel.pending_splice.as_ref()) + }, + }; + let signing_session = + context.interactive_tx_signing_session.as_mut().ok_or("Missing signing session")?; + + let tx = signing_session.unsigned_tx().tx(); + if funding_txid_signed != tx.compute_txid() { + return Err("Transaction was malleated prior to signing".to_owned()); + } + + let shared_input_signature = + if let Some(splice_input_index) = signing_session.unsigned_tx().shared_input_index() { + let sig = match &context.holder_signer { + ChannelSignerType::Ecdsa(signer) => signer.sign_splice_shared_input( + &funding.channel_transaction_parameters, + tx, + splice_input_index as usize, + &context.secp_ctx, + ), + #[cfg(taproot)] + ChannelSignerType::Taproot(_) => todo!(), + }; + Some(sig) + } else { + None + }; + debug_assert_eq!(pending_splice.is_some(), shared_input_signature.is_some()); + + let tx_signatures = msgs::TxSignatures { + channel_id: context.channel_id, + tx_hash: funding_txid_signed, + witnesses, + shared_input_signature, + }; + signing_session.provide_holder_witnesses(tx_signatures, &context.secp_ctx) + } + + pub fn funding_transaction_signed( + &mut self, funding_txid_signed: Txid, witnesses: Vec, best_block_height: u32, + logger: &WithChannelContext, + ) -> Result + where + L::Target: Logger, + { + if let Some(signing_session) = self.context().interactive_tx_signing_session.as_ref() { + if signing_session.holder_tx_signatures().is_some() { + // Our `tx_signatures` either should've been the first time we processed them, + // or we're waiting for our counterparty to send theirs first. + return Ok(FundingTxSigned { + commit_sig: None, + tx_signatures: None, + funding_tx: None, + splice_negotiated: None, + splice_locked: None, + }); + } + } else { + if Some(funding_txid_signed) == self.funding().get_funding_txid() { + // We may be handling a duplicate call and the funding was already locked so we + // no longer have the signing session present. + return Ok(FundingTxSigned { + commit_sig: None, + tx_signatures: None, + funding_tx: None, + splice_negotiated: None, + splice_locked: None, + }); + } + let err = + format!("Channel {} not expecting funding signatures", self.context().channel_id); + return Err(err); + }; + + let (tx_signatures, funding_tx) = + self.generate_tx_signatures(funding_txid_signed, witnesses)?; + + let (splice_negotiated, splice_locked) = if let Some(funding_tx) = funding_tx.clone() { + debug_assert!(tx_signatures.is_some()); + self.as_funded_mut() + .expect("Channel with signed funding transaction must be funded") + .on_tx_signatures_exchange(funding_tx, best_block_height, &&logger) + } else { + (None, None) + }; + + let pending_splice = match &self.phase { + ChannelPhase::Undefined => unreachable!(), + ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => { + debug_assert!(false); + None + }, + ChannelPhase::UnfundedV2(_) => None, + ChannelPhase::Funded(channel) => channel.pending_splice.as_ref(), + }; + let funding = pending_splice + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + .and_then(|funding_negotiation| { + debug_assert!(matches!( + funding_negotiation, + FundingNegotiation::AwaitingSignatures { .. } + )); + funding_negotiation.as_funding() + }) + .unwrap_or(self.funding()); + let commit_sig = self + .context() + .get_initial_commitment_signed_v2(funding, &logger) + // TODO(splicing): Support async signing + .ok_or("Failed to compute commitment_signed signatures".to_owned())?; + + Ok(FundingTxSigned { + commit_sig: Some(commit_sig), + tx_signatures, + funding_tx, + splice_negotiated, + splice_locked, + }) + } + fn funding_tx_constructed( - &mut self, funding_outpoint: OutPoint, logger: &L, - ) -> Result + &mut self, funding_outpoint: OutPoint, best_block_height: u32, logger: &L, + ) -> Result where L::Target: Logger, { let logger = WithChannelContext::from(logger, self.context(), None); - let (interactive_tx_constructor, commitment_signed) = match &mut self.phase { + let interactive_tx_constructor = match &mut self.phase { ChannelPhase::UnfundedV2(chan) => { debug_assert_eq!( chan.context.channel_state, @@ -2073,19 +2207,7 @@ where .take() .expect("PendingV2Channel::interactive_tx_constructor should be set"); - let commitment_signed = - chan.context.get_initial_commitment_signed_v2(&chan.funding, &&logger); - let commitment_signed = match commitment_signed { - Some(commitment_signed) => commitment_signed, - // TODO(dual_funding): Support async signing - None => { - return Err(AbortReason::InternalError( - "Failed to compute commitment_signed signatures", - )); - }, - }; - - (interactive_tx_constructor, commitment_signed) + interactive_tx_constructor }, ChannelPhase::Funded(chan) => { if let Some(pending_splice) = chan.pending_splice.as_mut() { @@ -2114,30 +2236,12 @@ where .and_then(|(is_initiator, mut funding, interactive_tx_constructor)| { funding.channel_transaction_parameters.funding_outpoint = Some(funding_outpoint); - match chan.context.get_initial_commitment_signed_v2(&funding, &&logger) - { - Some(commitment_signed) => { - // Advance the state - pending_splice.funding_negotiation = - Some(FundingNegotiation::AwaitingSignatures { - is_initiator, - funding, - }); - Ok((interactive_tx_constructor, commitment_signed)) - }, - // TODO(splicing): Support async signing - None => { - // Restore the taken state for later error handling - pending_splice.funding_negotiation = - Some(FundingNegotiation::ConstructingTransaction { - funding, - interactive_tx_constructor, - }); - Err(AbortReason::InternalError( - "Failed to compute commitment_signed signatures", - )) - }, - } + pending_splice.funding_negotiation = + Some(FundingNegotiation::AwaitingSignatures { + is_initiator, + funding, + }); + Ok(interactive_tx_constructor) })? } else { return Err(AbortReason::InternalError( @@ -2154,8 +2258,29 @@ where }; let signing_session = interactive_tx_constructor.into_signing_session(); + let has_local_contribution = signing_session.has_local_contribution(); + let unsigned_funding_tx = signing_session.unsigned_tx().tx().clone(); self.context_mut().interactive_tx_signing_session = Some(signing_session); - Ok(commitment_signed) + + if has_local_contribution { + Ok(FundingTxConstructed::EmitSigningEvent(unsigned_funding_tx)) + } else { + let txid = unsigned_funding_tx.compute_txid(); + self.funding_transaction_signed(txid, vec![], best_block_height, &logger) + .map(|funding_tx_signed| { + debug_assert!(funding_tx_signed.tx_signatures.is_none()); + debug_assert!(funding_tx_signed.funding_tx.is_none()); + debug_assert!(funding_tx_signed.splice_locked.is_none()); + debug_assert!(funding_tx_signed.splice_negotiated.is_none()); + FundingTxConstructed::FundingSigned( + funding_tx_signed.commit_sig.expect("Async signing not yet supported"), + ) + }) + .map_err(|err| { + log_warn!(logger, "Failed signing interactive funding transaction: {err:?}"); + AbortReason::InternalError("Failed signing interactive funding transcation") + }) + } } pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult { @@ -2910,6 +3035,7 @@ where monitor_pending_channel_ready: bool, monitor_pending_revoke_and_ack: bool, monitor_pending_commitment_signed: bool, + monitor_pending_tx_signatures: bool, // TODO: If a channel is drop'd, we don't know whether the `ChannelMonitor` is ultimately // responsible for some of the HTLCs here or not - we don't know whether the update in question @@ -3641,6 +3767,7 @@ where monitor_pending_channel_ready: false, monitor_pending_revoke_and_ack: false, monitor_pending_commitment_signed: false, + monitor_pending_tx_signatures: false, monitor_pending_forwards: Vec::new(), monitor_pending_failures: Vec::new(), monitor_pending_finalized_fulfills: Vec::new(), @@ -3880,6 +4007,7 @@ where monitor_pending_channel_ready: false, monitor_pending_revoke_and_ack: false, monitor_pending_commitment_signed: false, + monitor_pending_tx_signatures: false, monitor_pending_forwards: Vec::new(), monitor_pending_failures: Vec::new(), monitor_pending_finalized_fulfills: Vec::new(), @@ -6854,8 +6982,22 @@ type BestBlockUpdatedRes = ( Option, ); +/// Action to take after exchanging `tx_complete` with our counterparty. +pub enum FundingTxConstructed { + /// Emit a [`Event::FundingTransactionReadyForSigning`] for the negotiated transaction due to a + /// local contribution made. + /// + /// [`Event::FundingTransactionReadyForSigning`]: crate::events::Event::FundingTransactionReadyForSigning + EmitSigningEvent(Transaction), + + /// Immediately commit to the negotiated transaction as no local contribution was made. + FundingSigned(msgs::CommitmentSigned), +} + /// The result of signing a funding transaction negotiated using the interactive-tx protocol. pub struct FundingTxSigned { + pub commit_sig: Option, + /// Signatures that should be sent to the counterparty, if necessary. pub tx_signatures: Option, @@ -7996,6 +8138,7 @@ where .expect("Signing session must exist for negotiated pending splice") .received_commitment_signed(); self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); + self.context.monitor_pending_tx_signatures = true; Ok(self.push_ret_blockable_mon_update(monitor_update)) } @@ -9006,107 +9149,6 @@ where } } - pub fn funding_transaction_signed( - &mut self, funding_txid_signed: Txid, witnesses: Vec, best_block_height: u32, - logger: &L, - ) -> Result - where - L::Target: Logger, - { - let signing_session = - if let Some(signing_session) = self.context.interactive_tx_signing_session.as_mut() { - if let Some(pending_splice) = self.pending_splice.as_ref() { - debug_assert!(pending_splice - .funding_negotiation - .as_ref() - .map(|funding_negotiation| matches!( - funding_negotiation, - FundingNegotiation::AwaitingSignatures { .. } - )) - .unwrap_or(false)); - } - - if signing_session.holder_tx_signatures().is_some() { - // Our `tx_signatures` either should've been the first time we processed them, - // or we're waiting for our counterparty to send theirs first. - return Ok(FundingTxSigned { - tx_signatures: None, - funding_tx: None, - splice_negotiated: None, - splice_locked: None, - }); - } - - signing_session - } else { - if Some(funding_txid_signed) == self.funding.get_funding_txid() { - // We may be handling a duplicate call and the funding was already locked so we - // no longer have the signing session present. - return Ok(FundingTxSigned { - tx_signatures: None, - funding_tx: None, - splice_negotiated: None, - splice_locked: None, - }); - } - let err = - format!("Channel {} not expecting funding signatures", self.context.channel_id); - return Err(APIError::APIMisuseError { err }); - }; - - let tx = signing_session.unsigned_tx().tx(); - if funding_txid_signed != tx.compute_txid() { - return Err(APIError::APIMisuseError { - err: "Transaction was malleated prior to signing".to_owned(), - }); - } - - let shared_input_signature = - if let Some(splice_input_index) = signing_session.unsigned_tx().shared_input_index() { - let sig = match &self.context.holder_signer { - ChannelSignerType::Ecdsa(signer) => signer.sign_splice_shared_input( - &self.funding.channel_transaction_parameters, - tx, - splice_input_index as usize, - &self.context.secp_ctx, - ), - #[cfg(taproot)] - ChannelSignerType::Taproot(_) => todo!(), - }; - Some(sig) - } else { - None - }; - debug_assert_eq!(self.pending_splice.is_some(), shared_input_signature.is_some()); - - let tx_signatures = msgs::TxSignatures { - channel_id: self.context.channel_id, - tx_hash: funding_txid_signed, - witnesses, - shared_input_signature, - }; - let (tx_signatures, funding_tx) = signing_session - .provide_holder_witnesses(tx_signatures, &self.context.secp_ctx) - .map_err(|err| APIError::APIMisuseError { err })?; - - let logger = WithChannelContext::from(logger, &self.context, None); - if tx_signatures.is_some() { - log_info!( - logger, - "Sending tx_signatures for interactive funding transaction {funding_txid_signed}" - ); - } - - let (splice_negotiated, splice_locked) = if let Some(funding_tx) = funding_tx.clone() { - debug_assert!(tx_signatures.is_some()); - self.on_tx_signatures_exchange(funding_tx, best_block_height, &logger) - } else { - (None, None) - }; - - Ok(FundingTxSigned { tx_signatures, funding_tx, splice_negotiated, splice_locked }) - } - pub fn tx_signatures( &mut self, msg: &msgs::TxSignatures, best_block_height: u32, logger: &L, ) -> Result @@ -9172,6 +9214,7 @@ where }; Ok(FundingTxSigned { + commit_sig: None, tx_signatures: holder_tx_signatures, funding_tx, splice_negotiated, @@ -9375,6 +9418,14 @@ where self.context.channel_state.clear_monitor_update_in_progress(); assert_eq!(self.blocked_monitor_updates_pending(), 0); + let tx_signatures = self + .context + .monitor_pending_tx_signatures + .then(|| ()) + .and_then(|_| self.context.interactive_tx_signing_session.as_ref()) + .and_then(|signing_session| signing_session.holder_tx_signatures().clone()); + self.context.monitor_pending_tx_signatures = false; + // If we're past (or at) the AwaitingChannelReady stage on an outbound (or V2-established) channel, // try to (re-)broadcast the funding transaction as we may have declined to broadcast it when we // first received the funding_signed. @@ -9463,7 +9514,7 @@ where match commitment_order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"}); MonitorRestoreUpdates { raa, commitment_update, commitment_order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, - pending_update_adds, funding_broadcastable, channel_ready, announcement_sigs, tx_signatures: None, + pending_update_adds, funding_broadcastable, channel_ready, announcement_sigs, tx_signatures, channel_ready_order, } } @@ -9954,6 +10005,10 @@ where // - if the `commitment_signed` bit is set in `retransmit_flags`: if !session.has_received_tx_signatures() && next_funding.should_retransmit(msgs::NextFundingFlag::CommitmentSigned) + // We intentionally hold back our commitment signed until the user decides to + // approve the interactively funded transaction via + // [`Channel::funding_transaction_signed`]. + && session.holder_tx_signatures().is_some() { // - MUST retransmit its `commitment_signed` for that funding transaction. let funding = self @@ -14845,6 +14900,8 @@ where if !self.context.monitor_pending_update_adds.is_empty() { monitor_pending_update_adds = Some(&self.context.monitor_pending_update_adds); } + let monitor_pending_tx_signatures = + self.context.monitor_pending_tx_signatures.then_some(()); let is_manual_broadcast = Some(self.context.is_manual_broadcast); let holder_commitment_point_previous_revoked = @@ -14879,6 +14936,7 @@ where (9, self.context.target_closing_feerate_sats_per_kw, option), (10, monitor_pending_update_adds, option), // Added in 0.0.122 (11, self.context.monitor_pending_finalized_fulfills, required_vec), + (12, monitor_pending_tx_signatures, option), (13, self.context.channel_creation_height, required), (15, preimages, required_vec), (17, self.context.announcement_sigs_state, required), @@ -15282,6 +15340,7 @@ where let mut malformed_htlcs: Option> = None; let mut monitor_pending_update_adds: Option> = None; + let mut monitor_pending_tx_signatures: Option<()> = None; let mut holder_commitment_point_previous_revoked_opt: Option = None; let mut holder_commitment_point_last_revoked_opt: Option = None; @@ -15315,6 +15374,7 @@ where (9, target_closing_feerate_sats_per_kw, option), (10, monitor_pending_update_adds, option), // Added in 0.0.122 (11, monitor_pending_finalized_fulfills, optional_vec), + (12, monitor_pending_tx_signatures, option), (13, channel_creation_height, required), (15, preimages, required_vec), // The preimages transitioned from optional to required in 0.2 (17, announcement_sigs_state, required), @@ -15694,6 +15754,7 @@ where monitor_pending_channel_ready, monitor_pending_revoke_and_ack, monitor_pending_commitment_signed, + monitor_pending_tx_signatures: monitor_pending_tx_signatures.is_some(), monitor_pending_forwards, monitor_pending_failures, monitor_pending_finalized_fulfills: monitor_pending_finalized_fulfills.unwrap(), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 72585d69f80..a81cad8b44b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -58,9 +58,9 @@ use crate::ln::chan_utils::selected_commitment_sat_per_1000_weight; use crate::ln::channel::QuiescentAction; use crate::ln::channel::{ self, hold_time_since, Channel, ChannelError, ChannelUpdateStatus, DisconnectResult, - FundedChannel, FundingTxSigned, InboundV1Channel, OutboundV1Channel, PendingV2Channel, - ReconnectionMsg, ShutdownResult, SpliceFundingFailed, StfuResponse, UpdateFulfillCommitFetch, - WithChannelContext, + FundedChannel, FundingTxConstructed, FundingTxSigned, InboundV1Channel, OutboundV1Channel, + PendingV2Channel, ReconnectionMsg, ShutdownResult, SpliceFundingFailed, StfuResponse, + UpdateFulfillCommitFetch, WithChannelContext, }; use crate::ln::channel_state::ChannelDetails; use crate::ln::funding::SpliceContribution; @@ -6472,55 +6472,79 @@ where let mut peer_state = peer_state_mutex_opt.unwrap().lock().unwrap(); match peer_state.channel_by_id.get_mut(channel_id) { - Some(channel) => match channel.as_funded_mut() { - Some(chan) => { - let txid = transaction.compute_txid(); - let witnesses: Vec<_> = transaction - .input - .into_iter() - .map(|input| input.witness) - .filter(|witness| !witness.is_empty()) - .collect(); - let best_block_height = self.best_block.read().unwrap().height; - match chan.funding_transaction_signed( - txid, - witnesses, - best_block_height, - &self.logger, - ) { - Ok(FundingTxSigned { - tx_signatures: Some(tx_signatures), - funding_tx, - splice_negotiated, - splice_locked, - }) => { - if let Some(funding_tx) = funding_tx { - self.broadcast_interactive_funding( - chan, - &funding_tx, - &self.logger, + Some(channel) => { + let txid = transaction.compute_txid(); + let witnesses: Vec<_> = transaction + .input + .into_iter() + .map(|input| input.witness) + .filter(|witness| !witness.is_empty()) + .collect(); + let best_block_height = self.best_block.read().unwrap().height; + let logger = WithChannelContext::from(&self.logger, channel.context(), None); + match channel.funding_transaction_signed( + txid, + witnesses, + best_block_height, + &logger, + ) { + Ok(FundingTxSigned { + commit_sig, + tx_signatures, + funding_tx, + splice_negotiated, + splice_locked, + }) => { + if let Some(funding_tx) = funding_tx { + self.broadcast_interactive_funding( + channel.as_funded_mut().expect( + "Channel with signed funding transaction must be funded", + ), + &funding_tx, + &self.logger, + ); + } + if let Some(splice_negotiated) = splice_negotiated { + self.pending_events.lock().unwrap().push_back(( + events::Event::SplicePending { + channel_id: *channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id: channel.context().get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated + .funding_redeem_script, + }, + None, + )); + } + if channel.context().is_connected() { + if let Some(commit_sig) = commit_sig { + peer_state.pending_msg_events.push( + MessageSendEvent::UpdateHTLCs { + node_id: *counterparty_node_id, + channel_id: *channel_id, + updates: CommitmentUpdate { + commitment_signed: vec![commit_sig], + update_add_htlcs: vec![], + update_fulfill_htlcs: vec![], + update_fail_htlcs: vec![], + update_fail_malformed_htlcs: vec![], + update_fee: None, + }, + }, ); + } else { + debug_assert!(false); } - if let Some(splice_negotiated) = splice_negotiated { - self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { - channel_id: *channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: chan.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated - .funding_redeem_script, + if let Some(tx_signatures) = tx_signatures { + peer_state.pending_msg_events.push( + MessageSendEvent::SendTxSignatures { + node_id: *counterparty_node_id, + msg: tx_signatures, }, - None, - )); + ); } - peer_state.pending_msg_events.push( - MessageSendEvent::SendTxSignatures { - node_id: *counterparty_node_id, - msg: tx_signatures, - }, - ); if let Some(splice_locked) = splice_locked { peer_state.pending_msg_events.push( MessageSendEvent::SendSpliceLocked { @@ -6529,34 +6553,14 @@ where }, ); } - return NotifyOption::DoPersist; - }, - Err(err) => { - result = Err(err); - return NotifyOption::SkipPersistNoEvents; - }, - Ok(FundingTxSigned { - tx_signatures: None, - funding_tx, - splice_negotiated, - splice_locked, - }) => { - debug_assert!(funding_tx.is_none()); - debug_assert!(splice_negotiated.is_none()); - debug_assert!(splice_locked.is_none()); - return NotifyOption::SkipPersistNoEvents; - }, - } - }, - None => { - result = Err(APIError::APIMisuseError { - err: format!( - "Channel with id {} not expecting funding signatures", - channel_id - ), - }); - return NotifyOption::SkipPersistNoEvents; - }, + } + return NotifyOption::DoPersist; + }, + Err(err) => { + result = Err(APIError::APIMisuseError { err }); + return NotifyOption::SkipPersistNoEvents; + }, + } }, None => { result = Err(APIError::ChannelUnavailable { @@ -9718,79 +9722,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } - if let Some(signing_session) = (!channel.is_awaiting_monitor_update()) - .then(|| ()) - .and_then(|_| channel.context.interactive_tx_signing_session.as_mut()) - .filter(|signing_session| signing_session.has_received_commitment_signed()) - .filter(|signing_session| signing_session.holder_tx_signatures().is_none()) - { - if signing_session.has_local_contribution() { - let mut pending_events = self.pending_events.lock().unwrap(); - let unsigned_transaction = signing_session.unsigned_tx().tx().clone(); - let event_action = ( - Event::FundingTransactionReadyForSigning { - unsigned_transaction, - counterparty_node_id, - channel_id: channel.context.channel_id(), - user_channel_id: channel.context.get_user_id(), - }, - None, - ); - - if !pending_events.contains(&event_action) { - pending_events.push_back(event_action); - } - } else { - let txid = signing_session.unsigned_tx().compute_txid(); - let best_block_height = self.best_block.read().unwrap().height; - match channel.funding_transaction_signed(txid, vec![], best_block_height, &self.logger) { - Ok(FundingTxSigned { - tx_signatures: Some(tx_signatures), - funding_tx, - splice_negotiated, - splice_locked, - }) => { - if let Some(funding_tx) = funding_tx { - self.broadcast_interactive_funding(channel, &funding_tx, &self.logger); - } - - if let Some(splice_negotiated) = splice_negotiated { - self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { - channel_id: channel.context.channel_id(), - counterparty_node_id, - user_channel_id: channel.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated.funding_redeem_script, - }, - None, - )); - } - - if channel.context.is_connected() { - pending_msg_events.push(MessageSendEvent::SendTxSignatures { - node_id: counterparty_node_id, - msg: tx_signatures, - }); - if let Some(splice_locked) = splice_locked { - pending_msg_events.push(MessageSendEvent::SendSpliceLocked { - node_id: counterparty_node_id, - msg: splice_locked, - }); - } - } - }, - Ok(FundingTxSigned { tx_signatures: None, .. }) => { - debug_assert!(false, "If our tx_signatures is empty, then we should send it first!"); - }, - Err(err) => { - log_warn!(logger, "Failed signing interactive funding transaction: {err:?}"); - }, - } - } - } - { let mut pending_events = self.pending_events.lock().unwrap(); emit_channel_pending_event!(pending_events, channel); @@ -10725,9 +10656,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ match peer_state.channel_by_id.entry(msg.channel_id) { hash_map::Entry::Occupied(mut chan_entry) => { let chan = chan_entry.get_mut(); - match chan.tx_complete(msg, &self.logger) { - Ok((interactive_tx_msg_send, commitment_signed)) => { - let persist = if interactive_tx_msg_send.is_some() || commitment_signed.is_some() { + let best_block_height = self.best_block.read().unwrap().height; + match chan.tx_complete(msg, best_block_height, &self.logger) { + Ok((interactive_tx_msg_send, funding_tx_constructed)) => { + let persist = if interactive_tx_msg_send.is_some() { NotifyOption::SkipPersistHandleEvents } else { NotifyOption::SkipPersistNoEvents @@ -10736,19 +10668,38 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let msg_send_event = interactive_tx_msg_send.into_msg_send_event(counterparty_node_id); peer_state.pending_msg_events.push(msg_send_event); }; - if let Some(commitment_signed) = commitment_signed { - peer_state.pending_msg_events.push(MessageSendEvent::UpdateHTLCs { - node_id: counterparty_node_id, - channel_id: msg.channel_id, - updates: CommitmentUpdate { - commitment_signed: vec![commitment_signed], - update_add_htlcs: vec![], - update_fulfill_htlcs: vec![], - update_fail_htlcs: vec![], - update_fail_malformed_htlcs: vec![], - update_fee: None, + if let Some(funding_tx_constructed) = funding_tx_constructed { + match funding_tx_constructed { + FundingTxConstructed::EmitSigningEvent(unsigned_transaction) => { + let mut pending_events = self.pending_events.lock().unwrap(); + let event_action = ( + Event::FundingTransactionReadyForSigning { + unsigned_transaction, + counterparty_node_id, + channel_id: chan.context().channel_id(), + user_channel_id: chan.context().get_user_id(), + }, + None, + ); + if !pending_events.contains(&event_action) { + pending_events.push_back(event_action); + } }, - }); + FundingTxConstructed::FundingSigned(commit_sig) => { + peer_state.pending_msg_events.push(MessageSendEvent::UpdateHTLCs { + node_id: counterparty_node_id, + channel_id: msg.channel_id, + updates: CommitmentUpdate { + commitment_signed: vec![commit_sig], + update_add_htlcs: vec![], + update_fulfill_htlcs: vec![], + update_fail_htlcs: vec![], + update_fail_malformed_htlcs: vec![], + update_fee: None, + }, + }); + }, + } } Ok(persist) }, @@ -10794,6 +10745,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Some(chan) => { let best_block_height = self.best_block.read().unwrap().height; let FundingTxSigned { + commit_sig, tx_signatures, funding_tx, splice_negotiated, @@ -10804,6 +10756,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ chan.tx_signatures(msg, best_block_height, &self.logger), chan_entry ); + debug_assert!(commit_sig.is_none()); if let Some(tx_signatures) = tx_signatures { peer_state.pending_msg_events.push(MessageSendEvent::SendTxSignatures { node_id: *counterparty_node_id, diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 4340aad420a..b19f2ff13d1 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -668,10 +668,10 @@ impl InteractiveTxSigningSession { self.holder_tx_signatures = Some(tx_signatures); let funding_tx_opt = self.maybe_finalize_funding_tx(); - let holder_tx_signatures = (self.holder_sends_tx_signatures_first + let holder_tx_signatures = ((self.has_received_commitment_signed + && self.holder_sends_tx_signatures_first) || self.has_received_tx_signatures()) .then(|| { - debug_assert!(self.has_received_commitment_signed); self.holder_tx_signatures.clone().expect("Holder tx_signatures were just provided") }); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index a96af7bbc5d..8512346b3d1 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -25,7 +25,6 @@ use crate::ln::types::ChannelId; use crate::routing::router::{PaymentParameters, RouteParameters}; use crate::util::errors::APIError; use crate::util::ser::Writeable; -use crate::util::test_channel_signer::SignerOp; use bitcoin::secp256k1::PublicKey; use bitcoin::{Amount, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut}; @@ -73,7 +72,7 @@ fn test_v1_splice_in_negative_insufficient_inputs() { pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, initiator_contribution: SpliceContribution, -) -> msgs::CommitmentSigned { +) { let new_funding_script = complete_splice_handshake(initiator, acceptor, channel_id, initiator_contribution.clone()); complete_interactive_funding_negotiation( @@ -125,7 +124,7 @@ pub fn complete_splice_handshake<'a, 'b, 'c, 'd>( pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, initiator_contribution: SpliceContribution, new_funding_script: ScriptBuf, -) -> msgs::CommitmentSigned { +) { let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); @@ -182,21 +181,16 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( acceptor.node.handle_tx_add_output(node_id_initiator, &tx_add_output); } else { let mut msg_events = initiator.node.get_and_clear_pending_msg_events(); - assert_eq!( - msg_events.len(), - if acceptor_sent_tx_complete { 2 } else { 1 }, - "{msg_events:?}" - ); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); if let MessageSendEvent::SendTxComplete { ref msg, .. } = msg_events.remove(0) { acceptor.node.handle_tx_complete(node_id_initiator, msg); } else { panic!(); } if acceptor_sent_tx_complete { - if let MessageSendEvent::UpdateHTLCs { mut updates, .. } = msg_events.remove(0) { - return updates.commitment_signed.remove(0); - } - panic!(); + assert!(expected_initiator_inputs.is_empty()); + assert!(expected_initiator_scripts.is_empty()); + return; } } @@ -212,28 +206,21 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( } pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>( - initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, - initial_commit_sig_for_acceptor: msgs::CommitmentSigned, is_0conf: bool, + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, is_0conf: bool, ) -> (Transaction, Option<(msgs::SpliceLocked, PublicKey)>) { let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); assert!(initiator.node.get_and_clear_pending_msg_events().is_empty()); - acceptor.node.handle_commitment_signed(node_id_initiator, &initial_commit_sig_for_acceptor); let msg_events = acceptor.node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 2, "{msg_events:?}"); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); if let MessageSendEvent::UpdateHTLCs { ref updates, .. } = &msg_events[0] { let commitment_signed = &updates.commitment_signed[0]; initiator.node.handle_commitment_signed(node_id_acceptor, commitment_signed); } else { panic!(); } - if let MessageSendEvent::SendTxSignatures { ref msg, .. } = &msg_events[1] { - initiator.node.handle_tx_signatures(node_id_acceptor, msg); - } else { - panic!(); - } let event = get_event!(initiator, Event::FundingTransactionReadyForSigning); if let Event::FundingTransactionReadyForSigning { @@ -249,6 +236,23 @@ pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>( .funding_transaction_signed(&channel_id, &counterparty_node_id, partially_signed_tx) .unwrap(); } + + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + if let MessageSendEvent::UpdateHTLCs { ref updates, .. } = &msg_events[0] { + acceptor.node.handle_commitment_signed(node_id_initiator, &updates.commitment_signed[0]); + } else { + panic!(); + } + + let msg_events = acceptor.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + if let MessageSendEvent::SendTxSignatures { ref msg, .. } = &msg_events[0] { + initiator.node.handle_tx_signatures(node_id_acceptor, msg); + } else { + panic!(); + } + let mut msg_events = initiator.node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), if is_0conf { 2 } else { 1 }, "{msg_events:?}"); if let MessageSendEvent::SendTxSignatures { ref msg, .. } = &msg_events[0] { @@ -273,7 +277,7 @@ pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>( let mut initiator_txn = initiator.tx_broadcaster.txn_broadcast(); assert_eq!(initiator_txn.len(), 1); let acceptor_txn = acceptor.tx_broadcaster.txn_broadcast(); - assert_eq!(initiator_txn, acceptor_txn,); + assert_eq!(initiator_txn, acceptor_txn); initiator_txn.remove(0) }; (tx, splice_locked) @@ -289,15 +293,14 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( let new_funding_script = complete_splice_handshake(initiator, acceptor, channel_id, initiator_contribution.clone()); - let initial_commit_sig_for_acceptor = complete_interactive_funding_negotiation( + complete_interactive_funding_negotiation( initiator, acceptor, channel_id, initiator_contribution, new_funding_script, ); - let (splice_tx, splice_locked) = - sign_interactive_funding_tx(initiator, acceptor, initial_commit_sig_for_acceptor, false); + let (splice_tx, splice_locked) = sign_interactive_funding_tx(initiator, acceptor, false); assert!(splice_locked.is_none()); expect_splice_pending_event(initiator, &node_id_acceptor); @@ -585,15 +588,12 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { nodes[0].node.handle_tx_complete(node_id_1, &tx_complete); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 2); + assert_eq!(msg_events.len(), 1); if let MessageSendEvent::SendTxComplete { .. } = &msg_events[0] { } else { panic!("Unexpected event"); } - if let MessageSendEvent::UpdateHTLCs { .. } = &msg_events[1] { - } else { - panic!("Unexpected event"); - } + let _ = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); if reload { let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); @@ -1064,9 +1064,14 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { }, ], }; - let initial_commit_sig_for_acceptor = - negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); - assert_eq!(initial_commit_sig_for_acceptor.htlc_signatures.len(), 1); + negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); + + // Node 0 should have a signing event to handle since they had a contribution in the splice. + // Node 1 won't and will immediately send their `commitment_signed`. + let signing_event = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); let initial_commit_sig_for_initiator = get_htlc_update_msgs(&nodes[1], &node_id_0); assert_eq!(initial_commit_sig_for_initiator.commitment_signed.len(), 1); assert_eq!(initial_commit_sig_for_initiator.commitment_signed[0].htlc_signatures.len(), 1); @@ -1081,8 +1086,8 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { }; } - // Reestablishing now should force both nodes to retransmit their initial `commitment_signed` - // message as they were never delivered. + // Reestablishing now should force node 1 to retransmit their initial `commitment_signed` + // message as it was never delivered. if reload { let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); reload_node!( @@ -1116,12 +1121,27 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { } let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); - reconnect_args.send_interactive_tx_commit_sig = (true, true); + reconnect_args.send_interactive_tx_commit_sig = (true, false); reconnect_nodes(reconnect_args); - // The `commitment_signed` messages were delivered in the reestablishment, so we should expect - // to see a `RenegotiatedFunding` monitor update on both nodes. + // The `commitment_signed` message was only delivered to nodes[0] in the reestablishment, so we + // should expect to see a `RenegotiatedFunding` monitor update. check_added_monitors(&nodes[0], 1); + check_added_monitors(&nodes[1], 0); + + // Have node 0 sign, we should see its `commitment_signed` go out. + if let Event::FundingTransactionReadyForSigning { unsigned_transaction, .. } = signing_event { + let tx = nodes[0].wallet_source.sign_tx(unsigned_transaction).unwrap(); + nodes[0].node.funding_transaction_signed(&channel_id, &node_id_1, tx).unwrap(); + } + let _ = get_htlc_update_msgs(&nodes[0], &node_id_1); + + // Reconnect to make sure node 0 retransmits its `commitment_signed` as it was never delivered. + reconnect_nodes!(|reconnect_args: &mut ReconnectArgs| { + reconnect_args.send_interactive_tx_commit_sig = (false, true); + }); + + check_added_monitors(&nodes[0], 0); check_added_monitors(&nodes[1], 1); if async_monitor_update { @@ -1138,38 +1158,22 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); } - // Node 0 should have a signing event to handle since they had a contribution in the splice. - // Node 1 won't and will immediately send `tx_signatures`. - let _ = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); - assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); - let _ = get_event_msg!(nodes[1], MessageSendEvent::SendTxSignatures, node_id_0); + // Node 1 should now send its `tx_signatures` as it goes first. Node 0 will wait to send theirs + // until they receive it. + assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); + let _ = get_event_msg!(&nodes[1], MessageSendEvent::SendTxSignatures, node_id_0); - // Reconnecting now should force node 1 to retransmit their `tx_signatures` since it was never - // delivered. Node 0 still hasn't called back with `funding_transaction_signed`, so its - // `tx_signatures` is not ready yet. + // Reconnect to make sure node 1 retransmits its `tx_signatures` as it was never delivered. reconnect_nodes!(|reconnect_args: &mut ReconnectArgs| { reconnect_args.send_interactive_tx_sigs = (true, false); }); - let _ = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); - - // Reconnect again to make sure node 1 doesn't retransmit `tx_signatures` unnecessarily as it - // was delivered in the previous reestablishment. - reconnect_nodes!(|_| {}); - - // Have node 0 sign, we should see its `tx_signatures` go out. - let event = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); - if let Event::FundingTransactionReadyForSigning { unsigned_transaction, .. } = event { - let tx = nodes[0].wallet_source.sign_tx(unsigned_transaction).unwrap(); - nodes[0].node.funding_transaction_signed(&channel_id, &node_id_1, tx).unwrap(); - } - let _ = get_event_msg!(nodes[0], MessageSendEvent::SendTxSignatures, node_id_1); - expect_splice_pending_event(&nodes[0], &node_id_1); + let _ = get_event_msg!(&nodes[0], MessageSendEvent::SendTxSignatures, node_id_1); // Reconnect to make sure node 0 retransmits its `tx_signatures` as it was never delivered. reconnect_nodes!(|reconnect_args: &mut ReconnectArgs| { reconnect_args.send_interactive_tx_sigs = (false, true); }); + expect_splice_pending_event(&nodes[0], &node_id_1); expect_splice_pending_event(&nodes[1], &node_id_0); // Reestablish the channel again to make sure node 0 doesn't retransmit `tx_signatures` @@ -1417,25 +1421,22 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { .unwrap(); // Negotiate the first splice to completion. - let initial_commit_sig = { - nodes[1].node.handle_splice_init(node_id_0, &splice_init); - let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); - nodes[0].node.handle_splice_ack(node_id_1, &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( - &nodes[0], - &nodes[1], - channel_id, - node_0_contribution, - new_funding_script, - ) - }; - let (splice_tx, splice_locked) = - sign_interactive_funding_tx(&nodes[0], &nodes[1], initial_commit_sig, use_0conf); + nodes[1].node.handle_splice_init(node_id_0, &splice_init); + let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); + nodes[0].node.handle_splice_ack(node_id_1, &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( + &nodes[0], + &nodes[1], + channel_id, + node_0_contribution, + new_funding_script, + ); + let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], use_0conf); expect_splice_pending_event(&nodes[0], &node_id_1); expect_splice_pending_event(&nodes[1], &node_id_0); @@ -1559,25 +1560,22 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { } let splice_init = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceInit, node_id_0); - let initial_commit_sig = { - nodes[0].node.handle_splice_init(node_id_1, &splice_init); - let splice_ack = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceAck, node_id_1); - nodes[1].node.handle_splice_ack(node_id_0, &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( - &nodes[1], - &nodes[0], - channel_id, - node_1_contribution, - new_funding_script, - ) - }; - let (splice_tx, splice_locked) = - sign_interactive_funding_tx(&nodes[1], &nodes[0], initial_commit_sig, use_0conf); + nodes[0].node.handle_splice_init(node_id_1, &splice_init); + let splice_ack = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceAck, node_id_1); + nodes[1].node.handle_splice_ack(node_id_0, &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( + &nodes[1], + &nodes[0], + channel_id, + node_1_contribution, + new_funding_script, + ); + let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[1], &nodes[0], use_0conf); expect_splice_pending_event(&nodes[0], &node_id_1); expect_splice_pending_event(&nodes[1], &node_id_0); @@ -1624,8 +1622,8 @@ fn disconnect_on_unexpected_interactive_tx_message() { // Complete interactive-tx construction, but fail by having the acceptor send a duplicate // tx_complete instead of commitment_signed. - let _ = negotiate_splice_tx(initiator, acceptor, channel_id, contribution.clone()); - + negotiate_splice_tx(initiator, acceptor, channel_id, contribution.clone()); + let _ = get_event!(initiator, Event::FundingTransactionReadyForSigning); let mut msg_events = acceptor.node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 1); assert!(matches!(msg_events.remove(0), MessageSendEvent::UpdateHTLCs { .. })); @@ -1688,58 +1686,6 @@ fn fail_splice_on_interactive_tx_error() { let tx_abort = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); initiator.node.handle_tx_abort(node_id_acceptor, &tx_abort); - - // Fail signing the commitment transaction, which prevents the initiator from sending - // tx_complete. - initiator.disable_channel_signer_op( - &node_id_acceptor, - &channel_id, - SignerOp::SignCounterpartyCommitment, - ); - let _ = complete_splice_handshake(initiator, acceptor, channel_id, contribution.clone()); - - let tx_add_input = - get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor); - acceptor.node.handle_tx_add_input(node_id_initiator, &tx_add_input); - - let tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - initiator.node.handle_tx_complete(node_id_acceptor, &tx_complete); - - let tx_add_input = - get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor); - acceptor.node.handle_tx_add_input(node_id_initiator, &tx_add_input); - - let tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - initiator.node.handle_tx_complete(node_id_acceptor, &tx_complete); - - let tx_add_output = - get_event_msg!(initiator, MessageSendEvent::SendTxAddOutput, node_id_acceptor); - acceptor.node.handle_tx_add_output(node_id_initiator, &tx_add_output); - - let tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - initiator.node.handle_tx_complete(node_id_acceptor, &tx_complete); - - let tx_add_output = - get_event_msg!(initiator, MessageSendEvent::SendTxAddOutput, node_id_acceptor); - acceptor.node.handle_tx_add_output(node_id_initiator, &tx_add_output); - - let tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - initiator.node.handle_tx_complete(node_id_acceptor, &tx_complete); - - let event = get_event!(initiator, Event::SpliceFailed); - match event { - Event::SpliceFailed { contributed_inputs, .. } => { - assert_eq!(contributed_inputs.len(), 1); - assert_eq!(contributed_inputs[0], contribution.inputs()[0].outpoint()); - }, - _ => panic!("Expected Event::SpliceFailed"), - } - - let tx_abort = get_event_msg!(initiator, MessageSendEvent::SendTxAbort, node_id_acceptor); - 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); } #[test]