From 1644a7820a1e20560f1ad735eadcd5a1d5fc9adc Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 11 Sep 2025 15:55:39 -0700 Subject: [PATCH 1/4] Remove incomplete dual funding tests The tests have become a bit painful to maintain, and they don't even fully test the production code paths, so we opt to just remove them for now. In the future, we plan to unify the dual funding code paths with splicing. --- lightning/src/ln/channel.rs | 13 -- lightning/src/ln/dual_funding_tests.rs | 240 ------------------------- 2 files changed, 253 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9dd0f734347..a2a7e76a22e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6257,19 +6257,6 @@ where } } - #[cfg(all(test))] - pub fn get_initial_counterparty_commitment_signature_for_test( - &mut self, funding: &mut FundingScope, logger: &L, - counterparty_next_commitment_point_override: PublicKey, - ) -> Option - where - SP::Target: SignerProvider, - L::Target: Logger, - { - self.counterparty_next_commitment_point = Some(counterparty_next_commitment_point_override); - self.get_initial_counterparty_commitment_signature(funding, logger) - } - fn check_funding_meets_minimum_depth(&self, funding: &FundingScope, height: u32) -> bool { let minimum_depth = self .minimum_depth(funding) diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index e49a0f566e0..fc66ec315ca 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -8,243 +8,3 @@ // licenses. //! Tests that test the creation of dual-funded channels in ChannelManager. - -use { - crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator}, - crate::events::Event, - crate::ln::chan_utils::{ - make_funding_redeemscript, ChannelPublicKeys, ChannelTransactionParameters, - CounterpartyChannelTransactionParameters, - }, - crate::ln::channel::PendingV2Channel, - crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint, RevocationBasepoint}, - crate::ln::functional_test_utils::*, - crate::ln::funding::FundingTxInput, - crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, MessageSendEvent}, - crate::ln::msgs::{CommitmentSigned, TxAddInput, TxAddOutput, TxComplete, TxSignatures}, - crate::ln::types::ChannelId, - crate::prelude::*, - crate::util::test_utils, - bitcoin::Witness, -}; - -// Dual-funding: V2 Channel Establishment Tests -struct V2ChannelEstablishmentTestSession { - funding_input_sats: u64, - initiator_input_value_satoshis: u64, -} - -// TODO(dual_funding): Use real node and API for creating V2 channels as initiator when available, -// instead of manually constructing messages. -fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession) { - let chanmon_cfgs = create_chanmon_cfgs(2); - let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let mut node_1_user_config = test_default_channel_config(); - node_1_user_config.enable_dual_funded_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(node_1_user_config)]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let logger_a = test_utils::TestLogger::with_id("node a".to_owned()); - - // Create a funding input for the new channel along with its previous transaction. - let initiator_funding_inputs: Vec<_> = create_dual_funding_utxos_with_prev_txs( - &nodes[0], - &[session.initiator_input_value_satoshis], - ); - - // Alice creates a dual-funded channel as initiator. - let funding_satoshis = session.funding_input_sats; - let mut channel = PendingV2Channel::new_outbound( - &LowerBoundedFeeEstimator(node_cfgs[0].fee_estimator), - &nodes[0].node.entropy_source, - &nodes[0].node.signer_provider, - nodes[1].node.get_our_node_id(), - &nodes[1].node.init_features(), - funding_satoshis, - initiator_funding_inputs.clone(), - 42, /* user_channel_id */ - &nodes[0].node.get_current_config(), - nodes[0].best_block_info().1, - nodes[0].node.create_and_insert_outbound_scid_alias_for_test(), - ConfirmationTarget::NonAnchorChannelFee, - &logger_a, - ) - .unwrap(); - let open_channel_v2_msg = channel.get_open_channel_v2(nodes[0].chain_source.chain_hash); - - nodes[1].node.handle_open_channel_v2(nodes[0].node.get_our_node_id(), &open_channel_v2_msg); - - let accept_channel_v2_msg = get_event_msg!( - nodes[1], - MessageSendEvent::SendAcceptChannelV2, - nodes[0].node.get_our_node_id() - ); - let channel_id = ChannelId::v2_from_revocation_basepoints( - &RevocationBasepoint::from(accept_channel_v2_msg.common_fields.revocation_basepoint), - &RevocationBasepoint::from(open_channel_v2_msg.common_fields.revocation_basepoint), - ); - - let FundingTxInput { sequence, prevtx, .. } = &initiator_funding_inputs[0]; - let tx_add_input_msg = TxAddInput { - channel_id, - serial_id: 2, // Even serial_id from initiator. - prevtx: Some(prevtx.clone()), - prevtx_out: 0, - sequence: sequence.0, - shared_input_txid: None, - }; - let input_value = tx_add_input_msg.prevtx.as_ref().unwrap().output - [tx_add_input_msg.prevtx_out as usize] - .value; - assert_eq!(input_value.to_sat(), session.initiator_input_value_satoshis); - - nodes[1].node.handle_tx_add_input(nodes[0].node.get_our_node_id(), &tx_add_input_msg); - - let _tx_complete_msg = - get_event_msg!(nodes[1], MessageSendEvent::SendTxComplete, nodes[0].node.get_our_node_id()); - - let tx_add_output_msg = TxAddOutput { - channel_id, - serial_id: 4, - sats: funding_satoshis, - script: make_funding_redeemscript( - &open_channel_v2_msg.common_fields.funding_pubkey, - &accept_channel_v2_msg.common_fields.funding_pubkey, - ) - .to_p2wsh(), - }; - nodes[1].node.handle_tx_add_output(nodes[0].node.get_our_node_id(), &tx_add_output_msg); - - let _tx_complete_msg = - get_event_msg!(nodes[1], MessageSendEvent::SendTxComplete, nodes[0].node.get_our_node_id()); - - let tx_complete_msg = TxComplete { channel_id }; - - nodes[1].node.handle_tx_complete(nodes[0].node.get_our_node_id(), &tx_complete_msg); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - let _msg_commitment_signed_from_1 = match msg_events[0] { - MessageSendEvent::UpdateHTLCs { ref node_id, channel_id: _, ref updates } => { - assert_eq!(*node_id, nodes[0].node.get_our_node_id()); - updates.commitment_signed.clone() - }, - _ => panic!("Unexpected event"), - }; - - let (funding_outpoint, channel_type_features) = { - let per_peer_state = nodes[1].node.per_peer_state.read().unwrap(); - let peer_state = - per_peer_state.get(&nodes[0].node.get_our_node_id()).unwrap().lock().unwrap(); - let channel_funding = - peer_state.channel_by_id.get(&tx_complete_msg.channel_id).unwrap().funding(); - (channel_funding.get_funding_txo(), channel_funding.get_channel_type().clone()) - }; - - channel.funding.channel_transaction_parameters = ChannelTransactionParameters { - counterparty_parameters: Some(CounterpartyChannelTransactionParameters { - pubkeys: ChannelPublicKeys { - funding_pubkey: accept_channel_v2_msg.common_fields.funding_pubkey, - revocation_basepoint: RevocationBasepoint( - accept_channel_v2_msg.common_fields.revocation_basepoint, - ), - payment_point: accept_channel_v2_msg.common_fields.payment_basepoint, - delayed_payment_basepoint: DelayedPaymentBasepoint( - accept_channel_v2_msg.common_fields.delayed_payment_basepoint, - ), - htlc_basepoint: HtlcBasepoint(accept_channel_v2_msg.common_fields.htlc_basepoint), - }, - selected_contest_delay: accept_channel_v2_msg.common_fields.to_self_delay, - }), - holder_pubkeys: ChannelPublicKeys { - funding_pubkey: open_channel_v2_msg.common_fields.funding_pubkey, - revocation_basepoint: RevocationBasepoint( - open_channel_v2_msg.common_fields.revocation_basepoint, - ), - payment_point: open_channel_v2_msg.common_fields.payment_basepoint, - delayed_payment_basepoint: DelayedPaymentBasepoint( - open_channel_v2_msg.common_fields.delayed_payment_basepoint, - ), - htlc_basepoint: HtlcBasepoint(open_channel_v2_msg.common_fields.htlc_basepoint), - }, - holder_selected_contest_delay: open_channel_v2_msg.common_fields.to_self_delay, - is_outbound_from_holder: true, - funding_outpoint, - splice_parent_funding_txid: None, - channel_type_features, - channel_value_satoshis: funding_satoshis, - }; - - let msg_commitment_signed_from_0 = CommitmentSigned { - channel_id, - signature: channel - .context - .get_initial_counterparty_commitment_signature_for_test( - &mut channel.funding, - &&logger_a, - accept_channel_v2_msg.common_fields.first_per_commitment_point, - ) - .unwrap(), - htlc_signatures: vec![], - funding_txid: None, - #[cfg(taproot)] - partial_signature_with_nonce: None, - }; - - chanmon_cfgs[1].persister.set_update_ret(crate::chain::ChannelMonitorUpdateStatus::InProgress); - - // Handle the initial commitment_signed exchange. Order is not important here. - nodes[1] - .node - .handle_commitment_signed(nodes[0].node.get_our_node_id(), &msg_commitment_signed_from_0); - check_added_monitors(&nodes[1], 1); - - // The funding transaction should not have been broadcast before persisting initial monitor has - // been completed. - assert_eq!(nodes[1].tx_broadcaster.txn_broadcast().len(), 0); - assert_eq!(nodes[1].node.get_and_clear_pending_events().len(), 0); - - // Complete the persistence of the monitor. - let events = nodes[1].node.get_and_clear_pending_events(); - assert!(events.is_empty()); - nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id); - - let tx_signatures_msg = get_event_msg!( - nodes[1], - MessageSendEvent::SendTxSignatures, - nodes[0].node.get_our_node_id() - ); - - assert_eq!(tx_signatures_msg.channel_id, channel_id); - - let mut witness = Witness::new(); - witness.push([0x0]); - // Receive tx_signatures from channel initiator. - nodes[1].node.handle_tx_signatures( - nodes[0].node.get_our_node_id(), - &TxSignatures { - channel_id, - tx_hash: funding_outpoint.unwrap().txid, - witnesses: vec![witness], - shared_input_signature: None, - }, - ); - - let events = nodes[1].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 1); - match events[0] { - Event::ChannelPending { channel_id: chan_id, .. } => assert_eq!(chan_id, channel_id), - _ => panic!("Unexpected event"), - }; - - // For an inbound channel V2 channel the transaction should be broadcast once receiving a - // tx_signature and applying local tx_signatures: - let broadcasted_txs = nodes[1].tx_broadcaster.txn_broadcast(); - assert_eq!(broadcasted_txs.len(), 1); -} - -#[test] -fn test_v2_channel_establishment() { - do_test_v2_channel_establishment(V2ChannelEstablishmentTestSession { - funding_input_sats: 100_00, - initiator_input_value_satoshis: 150_000, - }); -} From 470dd2c0d811cb0c00991e0027ec4c489235be76 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 11 Sep 2025 16:11:15 -0700 Subject: [PATCH 2/4] Move interactive signing session into ChannelContext Since the `InteractiveTxSigningSession` already tracks everything we need, we can remove the duplicate interactive signing state tracking within `ChannelState`. --- lightning/src/ln/channel.rs | 74 +++++++++++++++--------------- lightning/src/ln/channelmanager.rs | 2 +- 2 files changed, 37 insertions(+), 39 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a2a7e76a22e..a0a195f05d5 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1969,21 +1969,19 @@ where let logger = WithChannelContext::from(logger, self.context(), None); match &mut self.phase { ChannelPhase::UnfundedV2(chan) => { - let mut signing_session = chan + let signing_session = chan .interactive_tx_constructor .take() .expect("PendingV2Channel::interactive_tx_constructor should be set") .into_signing_session(); let commitment_signed = chan.context.funding_tx_constructed( &mut chan.funding, - &mut signing_session, + signing_session, false, chan.unfunded_context.transaction_number(), &&logger, )?; - chan.interactive_tx_signing_session = Some(signing_session); - return Ok(commitment_signed); }, ChannelPhase::Funded(chan) => { @@ -1994,17 +1992,15 @@ where interactive_tx_constructor, } = funding_negotiation { - let mut signing_session = - interactive_tx_constructor.into_signing_session(); + let signing_session = interactive_tx_constructor.into_signing_session(); let commitment_signed = chan.context.funding_tx_constructed( &mut funding, - &mut signing_session, + signing_session, true, chan.holder_commitment_point.next_transaction_number(), &&logger, )?; - chan.interactive_tx_signing_session = Some(signing_session); pending_splice.funding_negotiation = Some(FundingNegotiation::AwaitingSignatures { funding }); @@ -2057,7 +2053,6 @@ where let mut funded_channel = FundedChannel { funding: chan.funding, context: chan.context, - interactive_tx_signing_session: chan.interactive_tx_signing_session, holder_commitment_point, pending_splice: None, quiescent_action: None, @@ -2082,6 +2077,7 @@ where .map(|funding_negotiation| funding_negotiation.as_funding().is_some()) .unwrap_or(false); let session_received_commitment_signed = funded_channel + .context .interactive_tx_signing_session .as_ref() .map(|session| session.has_received_commitment_signed()) @@ -2993,6 +2989,16 @@ where /// If we can't release a [`ChannelMonitorUpdate`] until some external action completes, we /// store it here and only release it to the `ChannelManager` once it asks for it. blocked_monitor_updates: Vec, + + /// The signing session for the current interactive tx construction, if any. + /// + /// This is populated when the interactive tx construction phase completes + /// (i.e., upon receiving a consecutive `tx_complete`) and the channel enters + /// the signing phase (`FundingNegotiated` state with the `INTERACTIVE_SIGNING` flag set). + /// + /// This field is cleared once our counterparty sends a `channel_ready` or upon splice funding + /// promotion. + pub interactive_tx_signing_session: Option, } /// A channel struct implementing this trait can receive an initial counterparty commitment @@ -3567,6 +3573,8 @@ where blocked_monitor_updates: Vec::new(), is_manual_broadcast: false, + + interactive_tx_signing_session: None, }; Ok((funding, channel_context)) @@ -3803,6 +3811,8 @@ where blocked_monitor_updates: Vec::new(), local_initiated_shutdown: None, is_manual_broadcast: false, + + interactive_tx_signing_session: None, }; Ok((funding, channel_context)) @@ -6112,7 +6122,7 @@ where #[rustfmt::skip] fn funding_tx_constructed( - &mut self, funding: &mut FundingScope, signing_session: &mut InteractiveTxSigningSession, + &mut self, funding: &mut FundingScope, signing_session: InteractiveTxSigningSession, is_splice: bool, holder_commitment_transaction_number: u64, logger: &L ) -> Result where @@ -6135,7 +6145,7 @@ where }; funding .channel_transaction_parameters.funding_outpoint = Some(outpoint); - + self.interactive_tx_signing_session = Some(signing_session); self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); self.channel_state.set_interactive_signing(); @@ -6226,7 +6236,7 @@ where } fn get_initial_commitment_signed_v2( - &mut self, funding: &FundingScope, logger: &L, + &self, funding: &FundingScope, logger: &L, ) -> Option where SP::Target: SignerProvider, @@ -6668,14 +6678,6 @@ where { pub funding: FundingScope, pub context: ChannelContext, - /// The signing session for the current interactive tx construction, if any. - /// - /// This is populated when the interactive tx construction phase completes - /// (i.e., upon receiving a consecutive `tx_complete`) and the channel enters - /// the signing phase (`FundingNegotiated` state with the `INTERACTIVE_SIGNING` flag set). - /// - /// This field is cleared once our counterparty sends a `channel_ready`. - pub interactive_tx_signing_session: Option, holder_commitment_point: HolderCommitmentPoint, /// Information about any pending splice candidates, including RBF attempts. @@ -7408,8 +7410,7 @@ where self.context.counterparty_current_commitment_point = self.context.counterparty_next_commitment_point; self.context.counterparty_next_commitment_point = Some(msg.next_per_commitment_point); - // Clear any interactive signing session. - self.interactive_tx_signing_session = None; + self.context.interactive_tx_signing_session = None; log_info!(logger, "Received channel_ready from peer for channel {}", &self.context.channel_id()); @@ -7575,7 +7576,7 @@ where log_info!(logger, "Received initial commitment_signed from peer for channel {}", &self.context.channel_id()); self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); - self.interactive_tx_signing_session.as_mut().expect("signing session should be present").received_commitment_signed(); + self.context.interactive_tx_signing_session.as_mut().expect("signing session should be present").received_commitment_signed(); Ok(channel_monitor) } @@ -7668,7 +7669,8 @@ where channel_id: Some(self.context.channel_id()), }; - self.interactive_tx_signing_session + self.context + .interactive_tx_signing_session .as_mut() .expect("Signing session must exist for negotiated pending splice") .received_commitment_signed(); @@ -8615,6 +8617,7 @@ where } let (tx_signatures_opt, funding_tx_opt) = self + .context .interactive_tx_signing_session .as_mut() .ok_or_else(|| APIError::APIMisuseError { @@ -8682,7 +8685,7 @@ where return Err(ChannelError::Ignore("Ignoring tx_signatures received outside of interactive signing".to_owned())); } - if let Some(ref mut signing_session) = self.interactive_tx_signing_session { + if let Some(ref mut signing_session) = self.context.interactive_tx_signing_session { if msg.tx_hash != signing_session.unsigned_tx().compute_txid() { let msg = "The txid for the transaction does not match"; let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; @@ -8955,7 +8958,7 @@ where // An active interactive signing session or an awaiting channel_ready state implies that a // commitment_signed retransmission is an initial one for funding negotiation. Thus, the // signatures should be sent before channel_ready. - let channel_ready_order = if self.interactive_tx_signing_session.is_some() { + let channel_ready_order = if self.context.interactive_tx_signing_session.is_some() { ChannelReadyOrder::SignaturesFirst } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(_)) { ChannelReadyOrder::SignaturesFirst @@ -9463,7 +9466,7 @@ where if let Some(next_funding) = &msg.next_funding { // - if `next_funding` matches the latest interactive funding transaction // or the current channel funding transaction: - if let Some(session) = &self.interactive_tx_signing_session { + if let Some(session) = &self.context.interactive_tx_signing_session { let our_next_funding_txid = session.unsigned_tx().compute_txid(); if our_next_funding_txid != next_funding.txid { return Err(ChannelError::close(format!( @@ -10763,7 +10766,7 @@ where .collect::>() }; - self.interactive_tx_signing_session = None; + self.context.interactive_tx_signing_session = None; self.pending_splice = None; self.context.announcement_sigs = None; self.context.announcement_sigs_state = AnnouncementSigsState::NotSent; @@ -11320,7 +11323,7 @@ where // Since we have a signing_session, this implies we've sent an initial `commitment_signed`... if !self.context.channel_state.is_their_tx_signatures_sent() { // ...but we didn't receive a `tx_signatures` from the counterparty yet. - self.interactive_tx_signing_session + self.context.interactive_tx_signing_session .as_ref() .map(|signing_session| { let mut next_funding = msgs::NextFunding { @@ -11857,7 +11860,7 @@ where })?; let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message(); - debug_assert!(self.interactive_tx_signing_session.is_none()); + debug_assert!(self.context.interactive_tx_signing_session.is_none()); pending_splice.funding_negotiation = Some(FundingNegotiation::ConstructingTransaction { funding: splice_funding, @@ -13128,7 +13131,6 @@ where let mut channel = FundedChannel { funding: self.funding, context: self.context, - interactive_tx_signing_session: None, holder_commitment_point, pending_splice: None, quiescent_action: None, @@ -13413,7 +13415,6 @@ where let mut channel = FundedChannel { funding: self.funding, context: self.context, - interactive_tx_signing_session: None, holder_commitment_point, pending_splice: None, quiescent_action: None, @@ -13457,8 +13458,6 @@ where pub funding_negotiation_context: FundingNegotiationContext, /// The current interactive transaction construction session under negotiation. pub interactive_tx_constructor: Option, - /// The signing session created after `tx_complete` handling - pub interactive_tx_signing_session: Option, } impl PendingV2Channel @@ -13533,7 +13532,6 @@ where unfunded_context, funding_negotiation_context, interactive_tx_constructor: None, - interactive_tx_signing_session: None, }; Ok(chan) } @@ -13721,7 +13719,6 @@ where context, funding_negotiation_context, interactive_tx_constructor, - interactive_tx_signing_session: None, unfunded_context, }) } @@ -14328,7 +14325,7 @@ where (53, funding_tx_broadcast_safe_event_emitted, option), // Added in 0.0.124 (55, removed_htlc_attribution_data, optional_vec), // Added in 0.2 (57, holding_cell_attribution_data, optional_vec), // Added in 0.2 - (58, self.interactive_tx_signing_session, option), // Added in 0.2 + (58, self.context.interactive_tx_signing_session, option), // Added in 0.2 (59, self.funding.minimum_depth_override, option), // Added in 0.2 (60, self.context.historical_scids, optional_vec), // Added in 0.2 (61, fulfill_attribution_data, optional_vec), // Added in 0.2 @@ -15098,8 +15095,9 @@ where blocked_monitor_updates: blocked_monitor_updates.unwrap(), is_manual_broadcast: is_manual_broadcast.unwrap_or(false), + + interactive_tx_signing_session, }, - interactive_tx_signing_session, holder_commitment_point, pending_splice, quiescent_action, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f991d228967..ba02faf04f2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9132,7 +9132,7 @@ 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.interactive_tx_signing_session.as_mut()) + .and_then(|_| channel.context.interactive_tx_signing_session.as_mut()) .filter(|signing_session| signing_session.holder_tx_signatures().is_none()) { if signing_session.has_local_contribution() { From 67aef54479dbf4743ff05204445e36b4cdafe3ce Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 4 Sep 2025 10:09:23 -0700 Subject: [PATCH 3/4] Remove tx_signatures flags for interactive signing ChannelState This commit addresses an overlap of state between `InteractiveTxSigningSession` and `ChannelState::FundingNegotiated`. The signing session already tracks whether both holder and counterparty `tx_signatures` have been produced, so tracking the state duplicatively at the `ChannelState` level is unnecessary. --- lightning/src/ln/channel.rs | 375 ++++++++++++----------------- lightning/src/ln/interactivetxs.rs | 4 + 2 files changed, 160 insertions(+), 219 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a0a195f05d5..2aed908efd7 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -495,7 +495,7 @@ enum HTLCUpdateAwaitingACK { } macro_rules! define_state_flags { - ($flag_type_doc: expr, $flag_type: ident, [$(($flag_doc: expr, $flag: ident, $value: expr, $get: ident, $set: ident, $clear: ident)),+], $extra_flags: expr) => { + ($flag_type_doc: expr, $flag_type: ident, [$(($flag_doc: expr, $flag: ident, $value: expr, $get: ident, $set: ident, $clear: ident)),*], $extra_flags: expr) => { #[doc = $flag_type_doc] #[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq)] struct $flag_type(u32); @@ -620,9 +620,6 @@ mod state_flags { pub const LOCAL_STFU_SENT: u32 = 1 << 15; pub const REMOTE_STFU_SENT: u32 = 1 << 16; pub const QUIESCENT: u32 = 1 << 17; - pub const INTERACTIVE_SIGNING: u32 = 1 << 18; - pub const OUR_TX_SIGNATURES_READY: u32 = 1 << 19; - pub const THEIR_TX_SIGNATURES_SENT: u32 = 1 << 20; } define_state_flags!( @@ -657,17 +654,9 @@ define_state_flags!( define_state_flags!( "Flags that only apply to [`ChannelState::FundingNegotiated`].", - FUNDED_STATE, FundingNegotiatedFlags, [ - ("Indicates we have an active interactive signing session for an interactive transaction", - INTERACTIVE_SIGNING, state_flags::INTERACTIVE_SIGNING, - is_interactive_signing, set_interactive_signing, clear_interactive_signing), - ("Indicates they sent us a `tx_signatures` message.", - THEIR_TX_SIGNATURES_SENT, state_flags::THEIR_TX_SIGNATURES_SENT, - is_their_tx_signatures_sent, set_their_tx_signatures_sent, clear_their_tx_signatures_sent), - ("Indicates we are ready to send them a `tx_signatures` message and it has been queued to send.", - OUR_TX_SIGNATURES_READY, state_flags::OUR_TX_SIGNATURES_READY, - is_our_tx_signatures_ready, set_our_tx_signatures_ready, clear_our_tx_signatures_ready) - ] + FUNDED_STATE, + FundingNegotiatedFlags, + [] ); define_state_flags!( @@ -818,14 +807,6 @@ impl ChannelState { } } - fn can_resume_on_reconnect(&self) -> bool { - match self { - ChannelState::NegotiatingFunding(_) => false, - ChannelState::FundingNegotiated(flags) => flags.is_interactive_signing(), - _ => true, - } - } - fn is_both_sides_shutdown(&self) -> bool { self.is_local_shutdown_sent() && self.is_remote_shutdown_sent() } @@ -882,24 +863,6 @@ impl ChannelState { clear_remote_shutdown_sent, FUNDED_STATES ); - impl_state_flag!( - is_interactive_signing, - set_interactive_signing, - clear_interactive_signing, - FundingNegotiated - ); - impl_state_flag!( - is_our_tx_signatures_ready, - set_our_tx_signatures_ready, - clear_our_tx_signatures_ready, - FundingNegotiated - ); - impl_state_flag!( - is_their_tx_signatures_sent, - set_their_tx_signatures_sent, - clear_their_tx_signatures_sent, - FundingNegotiated - ); impl_state_flag!( is_our_channel_ready, set_our_channel_ready, @@ -2992,9 +2955,8 @@ where /// The signing session for the current interactive tx construction, if any. /// - /// This is populated when the interactive tx construction phase completes - /// (i.e., upon receiving a consecutive `tx_complete`) and the channel enters - /// the signing phase (`FundingNegotiated` state with the `INTERACTIVE_SIGNING` flag set). + /// This is populated when the interactive tx construction phase completes (i.e., upon receiving + /// a consecutive `tx_complete`) and the channel enters the signing phase. /// /// This field is cleared once our counterparty sends a `channel_ready` or upon splice funding /// promotion. @@ -4328,13 +4290,21 @@ where self.is_manual_broadcast = true; } + fn can_resume_on_reconnect(&self) -> bool { + match self.channel_state { + ChannelState::NegotiatingFunding(_) => false, + ChannelState::FundingNegotiated(_) => self.interactive_tx_signing_session.is_some(), + _ => true, + } + } + /// Returns true if this channel can be resume after a restart, implying its past the initial /// funding negotiation stages (and any assocated batch channels are similarly past initial /// funding negotiation). /// /// This is equivalent to saying the channel can be persisted to disk. pub fn can_resume_on_restart(&self) -> bool { - self.channel_state.can_resume_on_reconnect() + self.can_resume_on_reconnect() && match self.channel_state { ChannelState::AwaitingChannelReady(flags) => !flags.is_waiting_for_batch(), _ => true, @@ -4346,7 +4316,11 @@ where fn is_funding_broadcastable(&self) -> bool { match self.channel_state { ChannelState::NegotiatingFunding(_) => false, - ChannelState::FundingNegotiated(flags) => !flags.is_our_tx_signatures_ready(), + ChannelState::FundingNegotiated(_) => self + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| signing_session.holder_tx_signatures().is_some()) + .unwrap_or(false), ChannelState::AwaitingChannelReady(flags) => !flags.is_waiting_for_batch(), _ => true, } @@ -4354,10 +4328,6 @@ where #[rustfmt::skip] fn unset_funding_info(&mut self, funding: &mut FundingScope) { - debug_assert!( - matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if !flags.is_their_tx_signatures_sent() && !flags.is_our_tx_signatures_ready()) - || matches!(self.channel_state, ChannelState::AwaitingChannelReady(_)) - ); funding.channel_transaction_parameters.funding_outpoint = None; self.channel_id = self.temporary_channel_id.expect( "temporary_channel_id should be set since unset_funding_info is only called on funded \ @@ -6147,7 +6117,6 @@ where .channel_transaction_parameters.funding_outpoint = Some(outpoint); self.interactive_tx_signing_session = Some(signing_session); self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); - self.channel_state.set_interactive_signing(); if is_splice { debug_assert_eq!( @@ -6243,7 +6212,7 @@ where L::Target: Logger, { assert!( - matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing()) + matches!(self.channel_state, ChannelState::FundingNegotiated(_) if self.interactive_tx_signing_session.is_some()) ); let signature = self.get_initial_counterparty_commitment_signature(funding, logger); @@ -7322,13 +7291,28 @@ where self.context.channel_state.clear_waiting_for_batch(); } - /// Unsets the existing funding information. + /// Unsets the existing funding information for V1 funded channels. /// /// This must only be used if the channel has not yet completed funding and has not been used. /// /// Further, the channel must be immediately shut down after this with a call to /// [`ChannelContext::force_shutdown`]. pub fn unset_funding_info(&mut self) { + let sent_or_received_tx_signatures = self + .context + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| { + signing_session.holder_tx_signatures().is_some() + || signing_session.has_received_tx_signatures() + }) + .unwrap_or(false); + debug_assert!( + matches!( + self.context.channel_state, + ChannelState::FundingNegotiated(_) if !sent_or_received_tx_signatures + ) || matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(_)) + ); self.context.unset_funding_info(&mut self.funding); } @@ -7558,13 +7542,17 @@ where ) -> Result::EcdsaSigner>, ChannelError> where L::Target: Logger { - if !self.context.channel_state.is_interactive_signing() - || self.context.channel_state.is_their_tx_signatures_sent() - { - let msg = "Received initial commitment_signed before funding transaction constructed or after peer's tx_signatures received!"; + if let Some(signing_session) = self.context.interactive_tx_signing_session.as_ref() { + if signing_session.has_received_tx_signatures() { + let msg = "Received initial commitment_signed after peer's tx_signatures received!"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + } + } else { + let msg = "Received initial commitment_signed before funding transaction constructed!"; let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; return Err(ChannelError::Close((msg.to_owned(), reason))); - } + }; let holder_commitment_point = &mut self.holder_commitment_point.clone(); self.context.assert_no_commitment_advancement(holder_commitment_point.next_transaction_number(), "initial commitment_signed"); @@ -7592,19 +7580,18 @@ where /// Note that our `commitment_signed` send did not include a monitor update. This is due to: /// 1. Updates cannot be made since the state machine is paused until `tx_signatures`. /// 2. We're still able to abort negotiation until `tx_signatures`. - pub fn splice_initial_commitment_signed( + fn splice_initial_commitment_signed( &mut self, msg: &msgs::CommitmentSigned, logger: &L, ) -> Result, ChannelError> where L::Target: Logger, { - if !self.context.channel_state.is_interactive_signing() - || self.context.channel_state.is_their_tx_signatures_sent() - { - return Err(ChannelError::close( - "Received splice initial commitment_signed during invalid state".to_owned(), - )); - } + debug_assert!(self + .context + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| !signing_session.has_received_tx_signatures()) + .unwrap_or(false)); let pending_splice_funding = self .pending_splice @@ -7615,6 +7602,7 @@ where }) .and_then(|funding_negotiation| funding_negotiation.as_funding()) .expect("Funding must exist for negotiated pending splice"); + let transaction_number = self.holder_commitment_point.current_transaction_number(); let commitment_point = self.holder_commitment_point.current_point().ok_or_else(|| { debug_assert!(false); @@ -8588,85 +8576,49 @@ where pub fn funding_transaction_signed( &mut self, funding_txid_signed: Txid, witnesses: Vec, ) -> Result<(Option, Option), APIError> { - if !self.context.channel_state.is_interactive_signing() { - let err = - format!("Channel {} not expecting funding signatures", self.context.channel_id); - return Err(APIError::APIMisuseError { err }); - } - if self.context.channel_state.is_our_tx_signatures_ready() { - let err = - format!("Channel {} already received funding signatures", self.context.channel_id); - return Err(APIError::APIMisuseError { err }); - } - if let Some(pending_splice) = self.pending_splice.as_ref() { - if !pending_splice - .funding_negotiation - .as_ref() - .map(|funding_negotiation| { - matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) - }) - .unwrap_or(false) - { - debug_assert!(false); - let err = format!( - "Channel {} with pending splice is not expecting funding signatures yet", - self.context.channel_id - ); + let signing_session = + if let Some(signing_session) = self.context.interactive_tx_signing_session.as_mut() { + signing_session + } else { + let err = + format!("Channel {} not expecting funding signatures", self.context.channel_id); return Err(APIError::APIMisuseError { err }); - } - } - - let (tx_signatures_opt, funding_tx_opt) = self - .context - .interactive_tx_signing_session - .as_mut() - .ok_or_else(|| APIError::APIMisuseError { - err: format!( - "Channel {} not expecting funding signatures", - self.context.channel_id - ), - }) - .and_then(|signing_session| { - let tx = signing_session.unsigned_tx().build_unsigned_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 = signing_session.unsigned_tx().build_unsigned_tx(); + if funding_txid_signed != tx.compute_txid() { + return Err(APIError::APIMisuseError { + err: "Transaction was malleated prior to signing".to_owned(), + }); + } - let tx_signatures = msgs::TxSignatures { - channel_id: self.context.channel_id, - tx_hash: funding_txid_signed, - witnesses, - shared_input_signature, + 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!(), }; - signing_session - .provide_holder_witnesses(tx_signatures, &self.context.secp_ctx) - .map_err(|err| APIError::APIMisuseError { err }) - })?; + Some(sig) + } else { + None + }; + debug_assert_eq!(self.pending_splice.is_some(), shared_input_signature.is_some()); - if tx_signatures_opt.is_some() { - self.context.channel_state.set_our_tx_signatures_ready(); - } + let tx_signatures = msgs::TxSignatures { + channel_id: self.context.channel_id, + tx_hash: funding_txid_signed, + witnesses, + shared_input_signature, + }; + let (tx_signatures_opt, funding_tx_opt) = signing_session + .provide_holder_witnesses(tx_signatures, &self.context.secp_ctx) + .map_err(|err| APIError::APIMisuseError { err })?; if funding_tx_opt.is_some() { self.funding.funding_transaction = funding_tx_opt.clone(); @@ -8679,57 +8631,45 @@ where #[rustfmt::skip] pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures) -> Result<(Option, Option), ChannelError> { - if !self.context.channel_state.is_interactive_signing() - || self.context.channel_state.is_their_tx_signatures_sent() - { - return Err(ChannelError::Ignore("Ignoring tx_signatures received outside of interactive signing".to_owned())); - } - - if let Some(ref mut signing_session) = self.context.interactive_tx_signing_session { - if msg.tx_hash != signing_session.unsigned_tx().compute_txid() { - let msg = "The txid for the transaction does not match"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); + let signing_session = if let Some(signing_session) = self.context.interactive_tx_signing_session.as_mut() { + if signing_session.has_received_tx_signatures() { + return Err(ChannelError::Ignore("Ignoring duplicate tx_signatures".to_owned())); } - - // We need to close the channel if our peer hasn't sent their commitment signed already. - // Technically we'd wait on having an initial monitor persisted, so we shouldn't be broadcasting - // the transaction, but this may risk losing funds for a manual broadcast if we continue. if !signing_session.has_received_commitment_signed() { - let msg = "Received tx_signatures before initial commitment_signed"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); + return Err(ChannelError::close("Received tx_signatures before initial commitment_signed".to_owned())); } + signing_session + } else { + return Err(ChannelError::Ignore("Ignoring unexpected tx_signatures".to_owned())); + }; - for witness in &msg.witnesses { - if witness.is_empty() { - let msg = "Unexpected empty witness in tx_signatures received"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - } - } - - let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg) - .map_err(|msg| ChannelError::Warn(msg))?; - - // Set `THEIR_TX_SIGNATURES_SENT` flag after all potential errors. - self.context.channel_state.set_their_tx_signatures_sent(); + if msg.tx_hash != signing_session.unsigned_tx().compute_txid() { + let msg = "The txid for the transaction does not match"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + } - if funding_tx_opt.is_some() { - // TODO(splicing): Transition back to `ChannelReady` and not `AwaitingChannelReady` - // We will also need to use the pending `FundingScope` in the splicing case. - // - // We have a finalized funding transaction, so we can set the funding transaction. - self.funding.funding_transaction = funding_tx_opt.clone(); - self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); + for witness in &msg.witnesses { + if witness.is_empty() { + let msg = "Unexpected empty witness in tx_signatures received"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); } + } - Ok((holder_tx_signatures_opt, funding_tx_opt)) - } else { - let msg = "Unexpected tx_signatures. No funding transaction awaiting signatures"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); + let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg) + .map_err(|msg| ChannelError::Warn(msg))?; + + if funding_tx_opt.is_some() { + // TODO(splicing): Transition back to `ChannelReady` and not `AwaitingChannelReady` + // We will also need to use the pending `FundingScope` in the splicing case. + // + // We have a finalized funding transaction, so we can set the funding transaction. + self.funding.funding_transaction = funding_tx_opt.clone(); + self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); } + + Ok((holder_tx_signatures_opt, funding_tx_opt)) } /// Queues up an outbound update fee by placing it in the holding cell. You should call @@ -8810,7 +8750,7 @@ where #[rustfmt::skip] fn remove_uncommitted_htlcs_and_mark_paused(&mut self, logger: &L) -> Result<(), ()> where L::Target: Logger { assert!(!matches!(self.context.channel_state, ChannelState::ShutdownComplete)); - if !self.context.channel_state.can_resume_on_reconnect() { + if !self.context.can_resume_on_reconnect() { return Err(()) } @@ -9532,7 +9472,7 @@ where // - if it has already received `tx_signatures` for that funding transaction: // - MUST send its `tx_signatures` for that funding transaction. if (session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first()) - || self.context.channel_state.is_their_tx_signatures_sent() + || session.has_received_tx_signatures() { // If `holder_tx_signatures` is `None` here, the `tx_signatures` message will be sent // when the holder provides their witnesses as this will queue a `tx_signatures` if the @@ -9920,16 +9860,19 @@ where "Peer sent shutdown when we needed a channel_reestablish".to_owned(), )); } - let mut not_broadcasted = + let mut not_broadcasted_initial_funding = matches!(self.context.channel_state, ChannelState::NegotiatingFunding(_)); - if let ChannelState::FundingNegotiated(flags) = &self.context.channel_state { - if !flags.is_our_tx_signatures_ready() { - // If we're a V1 channel or we haven't yet sent our `tx_signatures`, the funding tx - // couldn't be broadcasted yet, so just short-circuit the shutdown logic. - not_broadcasted = true; + if matches!(self.context.channel_state, ChannelState::FundingNegotiated(_)) { + if let Some(signing_session) = self.context.interactive_tx_signing_session.as_ref() { + if signing_session.holder_tx_signatures().is_none() { + // If we're a V1 channel or we haven't yet sent our `tx_signatures` for a dual + // funded channel, the funding tx couldn't be broadcasted yet, so just short-circuit + // the shutdown logic. + not_broadcasted_initial_funding = true; + } } } - if not_broadcasted { + if not_broadcasted_initial_funding { // Spec says we should fail the connection, not the channel, but that's nonsense, there // are plenty of reasons you may want to fail a channel pre-funding, and spec says you // can do that via error message without getting a connection fail anyway... @@ -11314,38 +11257,27 @@ where self.sign_channel_announcement(node_signer, announcement).ok() } - #[rustfmt::skip] fn maybe_get_next_funding(&self) -> Option { // If we've sent `commtiment_signed` for an interactively constructed transaction // during a signing session, but have not received `tx_signatures` we MUST set `next_funding` // to the txid of that interactive transaction, else we MUST NOT set it. - if self.context.channel_state.is_interactive_signing() { - // Since we have a signing_session, this implies we've sent an initial `commitment_signed`... - if !self.context.channel_state.is_their_tx_signatures_sent() { - // ...but we didn't receive a `tx_signatures` from the counterparty yet. - self.context.interactive_tx_signing_session - .as_ref() - .map(|signing_session| { - let mut next_funding = msgs::NextFunding { - txid: signing_session.unsigned_tx().compute_txid(), - retransmit_flags: 0, - }; + self.context + .interactive_tx_signing_session + .as_ref() + .filter(|session| !session.has_received_tx_signatures()) + .map(|signing_session| { + let mut next_funding = msgs::NextFunding { + txid: signing_session.unsigned_tx().compute_txid(), + retransmit_flags: 0, + }; - // TODO(splicing): Add comment for spec requirements - if !signing_session.has_received_commitment_signed() { - next_funding.retransmit(msgs::NextFundingFlag::CommitmentSigned); - } + // TODO(splicing): Add comment for spec requirements + if !signing_session.has_received_commitment_signed() { + next_funding.retransmit(msgs::NextFundingFlag::CommitmentSigned); + } - next_funding - }) - } else { - // ...and we received a `tx_signatures` from the counterparty. - None - } - } else { - // We don't have an active signing session. - None - } + next_funding + }) } fn maybe_get_my_current_funding_locked(&self) -> Option { @@ -13174,6 +13106,10 @@ where /// The channel must be immediately shut down after this with a call to /// [`ChannelContext::force_shutdown`]. pub fn unset_funding_info(&mut self) { + debug_assert!(matches!( + self.context.channel_state, + ChannelState::FundingNegotiated(_) if self.context.interactive_tx_signing_session.is_none() + )); self.context.unset_funding_info(&mut self.funding); } } @@ -13928,7 +13864,8 @@ where channel_state.clear_remote_stfu_sent(); channel_state.clear_quiescent(); }, - ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing() => {}, + ChannelState::FundingNegotiated(_) + if self.context.interactive_tx_signing_session.is_some() => {}, _ => debug_assert!(false, "Pre-funded/shutdown channels should not be written"), } channel_state.set_peer_disconnected(); diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index d199b37edae..3d65996df88 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -439,6 +439,10 @@ impl InteractiveTxSigningSession { self.has_received_commitment_signed } + pub fn has_received_tx_signatures(&self) -> bool { + self.has_received_tx_signatures + } + pub fn holder_tx_signatures(&self) -> &Option { &self.holder_tx_signatures } From 7597140ccbd850dcc2b37581a6984de335a1a47a Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 4 Sep 2025 10:09:24 -0700 Subject: [PATCH 4/4] Address ChannelState inconsistency throughout splicing Once a channel open has become locked (i.e., we've entered `ChannelState::ChannelReady`), the channel is intended to remain within this state for the rest of its lifetime until shutdown. Previously, we had assumed a channel being spliced would go through the `ChannelState` lifecycle again starting from `NegotiatingFunding` but skipping `AwaitingChannelReady`. This inconsistency departs from what we strive to achieve with `ChannelState` and also makes the state of a channel harder to reason about. This commit ensures a channel undergoing a splice remains in `ChannelReady`, clearing the quiescent flag once the negotiation is complete. Dual funding is unaffected by this change as the channel is being opened and we want to maintain the same `ChannelState` lifecycle. --- lightning/src/ln/channel.rs | 97 ++++++++++++++++++++++++++++--------- 1 file changed, 73 insertions(+), 24 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 2aed908efd7..de894de8088 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -718,9 +718,9 @@ enum ChannelState { /// `AwaitingChannelReady`. Note that this is nonsense for an inbound channel as we immediately generate /// `funding_signed` upon receipt of `funding_created`, so simply skip this state. /// - /// For inbound and outbound interactively funded channels (dual-funding/splicing), this flag indicates - /// that interactive transaction construction has been completed and we are now interactively signing - /// the funding/splice transaction. + /// For inbound and outbound interactively funded channels (dual-funding), this state indicates + /// that interactive transaction construction has been completed and we are now interactively + /// signing the initial funding transaction. FundingNegotiated(FundingNegotiatedFlags), /// We've received/sent `funding_created` and `funding_signed` and are thus now waiting on the /// funding transaction to confirm. @@ -1932,6 +1932,14 @@ where let logger = WithChannelContext::from(logger, self.context(), None); match &mut self.phase { ChannelPhase::UnfundedV2(chan) => { + debug_assert_eq!( + chan.context.channel_state, + ChannelState::NegotiatingFunding( + NegotiatingFundingFlags::OUR_INIT_SENT + | NegotiatingFundingFlags::THEIR_INIT_SENT + ), + ); + let signing_session = chan .interactive_tx_constructor .take() @@ -6116,7 +6124,6 @@ where funding .channel_transaction_parameters.funding_outpoint = Some(outpoint); self.interactive_tx_signing_session = Some(signing_session); - self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); if is_splice { debug_assert_eq!( @@ -6127,6 +6134,7 @@ where return Err(AbortReason::InternalError("Splicing not yet supported")); } else { self.assert_no_commitment_advancement(holder_commitment_transaction_number, "initial commitment_signed"); + self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); } let commitment_signed = self.get_initial_commitment_signed_v2(&funding, logger); @@ -6211,9 +6219,7 @@ where SP::Target: SignerProvider, L::Target: Logger, { - assert!( - matches!(self.channel_state, ChannelState::FundingNegotiated(_) if self.interactive_tx_signing_session.is_some()) - ); + debug_assert!(self.interactive_tx_signing_session.is_some()); let signature = self.get_initial_counterparty_commitment_signature(funding, logger); if let Some(signature) = signature { @@ -8573,11 +8579,43 @@ where } } + fn on_tx_signatures_exchange(&mut self, funding_tx: Transaction) { + debug_assert!(!self.context.channel_state.is_monitor_update_in_progress()); + debug_assert!(!self.context.channel_state.is_awaiting_remote_revoke()); + + if let Some(pending_splice) = self.pending_splice.as_mut() { + if let Some(FundingNegotiation::AwaitingSignatures { mut funding }) = + pending_splice.funding_negotiation.take() + { + funding.funding_transaction = Some(funding_tx); + pending_splice.negotiated_candidates.push(funding); + } else { + debug_assert!(false); + } + self.context.channel_state.clear_quiescent(); + } else { + self.funding.funding_transaction = Some(funding_tx); + self.context.channel_state = + ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); + } + } + pub fn funding_transaction_signed( &mut self, funding_txid_signed: Txid, witnesses: Vec, ) -> Result<(Option, Option), APIError> { 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)); + } + signing_session } else { let err = @@ -8620,24 +8658,40 @@ where .provide_holder_witnesses(tx_signatures, &self.context.secp_ctx) .map_err(|err| APIError::APIMisuseError { err })?; - if funding_tx_opt.is_some() { - self.funding.funding_transaction = funding_tx_opt.clone(); - self.context.channel_state = - ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); + if let Some(funding_tx) = funding_tx_opt.clone() { + debug_assert!(tx_signatures_opt.is_some()); + self.on_tx_signatures_exchange(funding_tx); } Ok((tx_signatures_opt, funding_tx_opt)) } - #[rustfmt::skip] - pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures) -> Result<(Option, Option), ChannelError> { - let signing_session = if let Some(signing_session) = self.context.interactive_tx_signing_session.as_mut() { + pub fn tx_signatures( + &mut self, msg: &msgs::TxSignatures, + ) -> Result<(Option, Option), ChannelError> { + let signing_session = if let Some(signing_session) = + self.context.interactive_tx_signing_session.as_mut() + { if signing_session.has_received_tx_signatures() { return Err(ChannelError::Ignore("Ignoring duplicate tx_signatures".to_owned())); } if !signing_session.has_received_commitment_signed() { - return Err(ChannelError::close("Received tx_signatures before initial commitment_signed".to_owned())); + return Err(ChannelError::close( + "Received tx_signatures before initial commitment_signed".to_owned(), + )); + } + + 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)); } + signing_session } else { return Err(ChannelError::Ignore("Ignoring unexpected tx_signatures".to_owned())); @@ -8657,16 +8711,11 @@ where } } - let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg) - .map_err(|msg| ChannelError::Warn(msg))?; + let (holder_tx_signatures_opt, funding_tx_opt) = + signing_session.received_tx_signatures(msg).map_err(|msg| ChannelError::Warn(msg))?; - if funding_tx_opt.is_some() { - // TODO(splicing): Transition back to `ChannelReady` and not `AwaitingChannelReady` - // We will also need to use the pending `FundingScope` in the splicing case. - // - // We have a finalized funding transaction, so we can set the funding transaction. - self.funding.funding_transaction = funding_tx_opt.clone(); - self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); + if let Some(funding_tx) = funding_tx_opt.clone() { + self.on_tx_signatures_exchange(funding_tx); } Ok((holder_tx_signatures_opt, funding_tx_opt))