From fd21ff19368342b31a3beb657048b239d4d83533 Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Thu, 28 Nov 2024 12:06:32 +0200 Subject: [PATCH 1/7] Get `next_funding_txid` from the funding_outpoint based on state Instead of having an explicit `ChannelContext::next_funding_txid` to set and read, we can get this value on the fly when it is appropriate to do so. --- lightning/src/ln/channel.rs | 55 ++++++++++++------------------ lightning/src/ln/interactivetxs.rs | 2 +- lightning/src/ln/msgs.rs | 12 +++++++ 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4b60b83aaf1..5a869b36cc0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -30,7 +30,7 @@ use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentHash}; use crate::types::features::{ChannelTypeFeatures, InitFeatures}; use crate::ln::interactivetxs::{ - calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteValue, HandleTxCompleteResult, InteractiveTxConstructor, + calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteResult, InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend, InteractiveTxSigningSession, InteractiveTxMessageSendResult, OutputOwned, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT, }; @@ -2179,22 +2179,6 @@ pub(super) struct ChannelContext where SP::Target: SignerProvider { /// store it here and only release it to the `ChannelManager` once it asks for it. blocked_monitor_updates: Vec, - // The `next_funding_txid` field allows peers to finalize the signing steps of an interactive - // transaction construction, or safely abort that transaction if it was not signed by one of the - // peers, who has thus already removed it from its state. - // - // 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_txid` - // to the txid of that interactive transaction, else we MUST NOT set it. - // - // See the spec for further details on this: - // * `channel_reestablish`-sending node: https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L2466-L2470 - // * `channel_reestablish`-receiving node: https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L2520-L2531 - // - // TODO(dual_funding): Persist this when we actually contribute funding inputs. For now we always - // send an empty witnesses array in `tx_signatures` as a V2 channel acceptor - next_funding_txid: Option, - /// Only set when a counterparty `stfu` has been processed to track which node is allowed to /// propose "something fundamental" upon becoming quiescent. is_holder_quiescence_initiator: Option, @@ -2542,10 +2526,6 @@ impl PendingV2Channel where SP::Target: SignerProvider { } }; - if let HandleTxCompleteValue::SendTxComplete(_, ref signing_session) = tx_complete { - self.context.next_funding_txid = Some(signing_session.unsigned_tx.compute_txid()); - }; - HandleTxCompleteResult(Ok(tx_complete)) } @@ -2981,8 +2961,6 @@ impl ChannelContext where SP::Target: SignerProvider { is_manual_broadcast: false, - next_funding_txid: None, - is_holder_quiescence_initiator: None, }; @@ -3214,7 +3192,6 @@ impl ChannelContext where SP::Target: SignerProvider { blocked_monitor_updates: Vec::new(), local_initiated_shutdown: None, is_manual_broadcast: false, - next_funding_txid: None, is_holder_quiescence_initiator: None, }; @@ -6573,7 +6550,6 @@ impl FundedChannel where // We have a finalized funding transaction, so we can set the funding transaction and reset the // signing session fields. self.funding.funding_transaction = funding_tx_opt; - self.context.next_funding_txid = None; self.interactive_tx_signing_session = None; } @@ -8627,6 +8603,25 @@ impl FundedChannel where self.sign_channel_announcement(node_signer, announcement).ok() } + fn maybe_get_next_funding_txid(&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_txid` + // to the txid of that interactive transaction, else we MUST NOT set it. + if let Some(signing_session) = &self.interactive_tx_signing_session { + // Since we have a signing_session, this implies we've sent an initial `commitment_signed`... + if !signing_session.counterparty_sent_tx_signatures { + // ...but we didn't receive a `tx_signatures` from the counterparty yet. + Some(self.funding_outpoint().txid) + } else { + // ...and we received a `tx_signatures` from the counterparty. + None + } + } else { + // We don't have an active signing session. + None + } + } + /// May panic if called on a channel that wasn't immediately-previously /// self.remove_uncommitted_htlcs_and_mark_paused()'d fn get_channel_reestablish(&mut self, logger: &L) -> msgs::ChannelReestablish where L::Target: Logger { @@ -8676,7 +8671,7 @@ impl FundedChannel where next_remote_commitment_number: INITIAL_COMMITMENT_NUMBER - self.context.cur_counterparty_commitment_transaction_number - 1, your_last_per_commitment_secret: remote_last_secret, my_current_per_commitment_point: dummy_pubkey, - next_funding_txid: self.context.next_funding_txid, + next_funding_txid: self.maybe_get_next_funding_txid(), } } @@ -11537,14 +11532,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel blocked_monitor_updates: blocked_monitor_updates.unwrap(), is_manual_broadcast: is_manual_broadcast.unwrap_or(false), - // TODO(dual_funding): Instead of getting this from persisted value, figure it out based on the - // funding transaction and other channel state. - // - // 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_txid` - // to the txid of that interactive transaction, else we MUST NOT set it. - next_funding_txid: None, - is_holder_quiescence_initiator: None, }, interactive_tx_signing_session: None, diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 28dbd3d1245..a2f89092535 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -310,10 +310,10 @@ impl ConstructedTransaction { #[derive(Debug, Clone, PartialEq)] pub(crate) struct InteractiveTxSigningSession { pub unsigned_tx: ConstructedTransaction, + pub counterparty_sent_tx_signatures: bool, holder_sends_tx_signatures_first: bool, received_commitment_signed: bool, holder_tx_signatures: Option, - counterparty_sent_tx_signatures: bool, } impl InteractiveTxSigningSession { diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index c27db4a55b4..667501efca6 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -857,6 +857,18 @@ pub struct ChannelReestablish { /// The sender's per-commitment point for their current commitment transaction pub my_current_per_commitment_point: PublicKey, /// The next funding transaction ID + /// + /// Allows peers to finalize the signing steps of an interactive transaction construction, or + /// safely abort that transaction if it was not signed by one of the peers, who has thus already + /// removed it from its state. + /// + /// 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_txid` + /// to the txid of that interactive transaction, else we MUST NOT set it. + /// + /// See the spec for further details on this: + /// * `channel_reestablish`-sending node: https:///github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L2466-L2470 + /// * `channel_reestablish`-receiving node: https:///github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L2520-L2531 pub next_funding_txid: Option, } From e9dfc8d97f90f8070aaac76726d3b40c2e54b68e Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Thu, 28 Nov 2024 15:35:33 +0200 Subject: [PATCH 2/7] Handle receiving channel_reestablish with `next_funding_txid` This follows the the specification closely in branching without being too verbose, so that it should be easy to follow the logic. See: https://github.com/lightning/bolts/blob/aa5207a/02-peer-protocol.md?plain=1#L2520-L2531 --- lightning/src/ln/channel.rs | 100 +++++++++++++++++++++++++++-- lightning/src/ln/channelmanager.rs | 16 +++-- lightning/src/ln/interactivetxs.rs | 36 ++++++++--- 3 files changed, 133 insertions(+), 19 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 5a869b36cc0..57fe8153b12 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1074,6 +1074,8 @@ pub(super) struct ReestablishResponses { pub order: RAACommitmentOrder, pub announcement_sigs: Option, pub shutdown_msg: Option, + pub tx_signatures: Option, + pub tx_abort: Option, } /// The first message we send to our peer after connection @@ -2540,7 +2542,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { let mut output_index = None; let expected_spk = self.funding.get_funding_redeemscript().to_p2wsh(); - for (idx, outp) in signing_session.unsigned_tx.outputs().enumerate() { + for (idx, outp) in signing_session.unsigned_tx().outputs().enumerate() { if outp.script_pubkey() == &expected_spk && outp.value() == self.funding.get_value_satoshis() { if output_index.is_some() { return Err(ChannelError::Close( @@ -2553,7 +2555,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { } } let outpoint = if let Some(output_index) = output_index { - OutPoint { txid: signing_session.unsigned_tx.compute_txid(), index: output_index } + OutPoint { txid: signing_session.unsigned_tx().compute_txid(), index: output_index } } else { return Err(ChannelError::Close( ( @@ -2567,7 +2569,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger); let commitment_signed = match commitment_signed { Ok(commitment_signed) => { - self.funding.funding_transaction = Some(signing_session.unsigned_tx.build_unsigned_tx()); + self.funding.funding_transaction = Some(signing_session.unsigned_tx().build_unsigned_tx()); commitment_signed }, Err(err) => { @@ -6513,7 +6515,7 @@ impl FundedChannel where } if let Some(ref mut signing_session) = self.interactive_tx_signing_session { - if msg.tx_hash != signing_session.unsigned_tx.compute_txid() { + if msg.tx_hash != signing_session.unsigned_tx().compute_txid() { return Err(ChannelError::Close( ( "The txid for the transaction does not match".to_string(), @@ -7163,7 +7165,10 @@ impl FundedChannel where } if msg.next_local_commitment_number >= INITIAL_COMMITMENT_NUMBER || msg.next_remote_commitment_number >= INITIAL_COMMITMENT_NUMBER || - msg.next_local_commitment_number == 0 { + (msg.next_local_commitment_number == 0 && msg.next_funding_txid.is_none()) { + // Note: This also covers the following case in the V2 channel establishment specification: + // if `next_funding_txid` is not set, and `next_commitment_number` is zero: + // MUST immediately fail the channel and broadcast any relevant latest commitment transaction. return Err(ChannelError::close("Peer sent an invalid channel_reestablish to force close in a non-standard way".to_owned())); } @@ -7227,6 +7232,8 @@ impl FundedChannel where raa: None, commitment_update: None, order: RAACommitmentOrder::CommitmentFirst, shutdown_msg, announcement_sigs, + tx_signatures: None, + tx_abort: None, }); } @@ -7236,6 +7243,8 @@ impl FundedChannel where raa: None, commitment_update: None, order: RAACommitmentOrder::CommitmentFirst, shutdown_msg, announcement_sigs, + tx_signatures: None, + tx_abort: None, }); } @@ -7278,11 +7287,84 @@ impl FundedChannel where log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id()); } + // if next_funding_txid is set: + let (commitment_update, tx_signatures, tx_abort) = if let Some(next_funding_txid) = msg.next_funding_txid { + if let Some(session) = &self.interactive_tx_signing_session { + // if next_funding_txid matches the latest interactive funding transaction: + let our_next_funding_txid = session.unsigned_tx().compute_txid(); + if our_next_funding_txid == next_funding_txid { + debug_assert_eq!(session.unsigned_tx().compute_txid(), self.maybe_get_next_funding_txid().unwrap()); + + let commitment_update = if !session.has_received_tx_signatures() && msg.next_local_commitment_number == 0 { + // if it has not received tx_signatures for that funding transaction AND + // if next_commitment_number is zero: + // MUST retransmit its commitment_signed for that funding transaction. + let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger)?; + Some(msgs::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, + }) + } else { None }; + // TODO(dual_funding): For async signing support we need to hold back `tx_signatures` until the `commitment_signed` is ready. + let tx_signatures = if ( + // if it has not received tx_signatures for that funding transaction AND + // if it has already received commitment_signed AND it should sign first, as specified in the tx_signatures requirements: + // MUST send its tx_signatures for that funding transaction. + !session.has_received_tx_signatures() && session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first() + // else if it has already received tx_signatures for that funding transaction: + // MUST send its tx_signatures for that funding transaction. + ) || session.has_received_tx_signatures() { + if self.context.channel_state.is_monitor_update_in_progress() { + // The `monitor_pending_tx_signatures` field should have already been set in `commitment_signed_initial_v2` + // if we were up first for signing and had a monitor update in progress, but check again just in case. + debug_assert!(self.context.monitor_pending_tx_signatures.is_some(), "monitor_pending_tx_signatures should already be set"); + log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures."); + if self.context.monitor_pending_tx_signatures.is_none() { + self.context.monitor_pending_tx_signatures = session.holder_tx_signatures().clone(); + } + None + } else { + // 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 + // holder must send one. + session.holder_tx_signatures().clone() + } + } else { + None + }; + if !session.has_received_commitment_signed() { + self.context.expecting_peer_commitment_signed = true; + } + (commitment_update, tx_signatures, None) + } else { + // The `next_funding_txid` does not match the latest interactive funding transaction so we + // MUST send tx_abort to let the remote know that they can forget this funding transaction. + (None, None, Some(msgs::TxAbort { + channel_id: self.context.channel_id(), + data: format!( + "next_funding_txid {} does match our latest interactive funding txid {}", + next_funding_txid, our_next_funding_txid, + ).into_bytes() })) + } + } else { + return Err(ChannelError::Warn("No active signing session. The associated funding transaction may have already been broadcast.".into())); + } + } else { + // Don't send anything related to interactive signing if `next_funding_txid` is not set. + (None, None, None) + }; + Ok(ReestablishResponses { channel_ready, shutdown_msg, announcement_sigs, raa: required_revoke, - commitment_update: None, + commitment_update, order: self.context.resend_order.clone(), + tx_signatures, + tx_abort, }) } else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 { if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack { @@ -7297,6 +7379,8 @@ impl FundedChannel where channel_ready, shutdown_msg, announcement_sigs, commitment_update: None, raa: None, order: self.context.resend_order.clone(), + tx_signatures: None, + tx_abort: None, }) } else { let commitment_update = if self.context.resend_order == RAACommitmentOrder::RevokeAndACKFirst @@ -7319,6 +7403,8 @@ impl FundedChannel where channel_ready, shutdown_msg, announcement_sigs, raa, commitment_update, order: self.context.resend_order.clone(), + tx_signatures: None, + tx_abort: None, }) } } else if msg.next_local_commitment_number < next_counterparty_commitment_number { @@ -8609,7 +8695,7 @@ impl FundedChannel where // to the txid of that interactive transaction, else we MUST NOT set it. if let Some(signing_session) = &self.interactive_tx_signing_session { // Since we have a signing_session, this implies we've sent an initial `commitment_signed`... - if !signing_session.counterparty_sent_tx_signatures { + if !signing_session.has_received_tx_signatures() { // ...but we didn't receive a `tx_signatures` from the counterparty yet. Some(self.funding_outpoint().txid) } else { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d0a73e89992..c3a4cdb610b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3229,7 +3229,7 @@ macro_rules! handle_monitor_update_completion { &mut $peer_state.pending_msg_events, $chan, updates.raa, updates.commitment_update, updates.order, updates.accepted_htlcs, updates.pending_update_adds, updates.funding_broadcastable, updates.channel_ready, - updates.announcement_sigs, updates.tx_signatures); + updates.announcement_sigs, updates.tx_signatures, None); if let Some(upd) = channel_update { $peer_state.pending_msg_events.push(upd); } @@ -7648,10 +7648,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ pending_forwards: Vec<(PendingHTLCInfo, u64)>, pending_update_adds: Vec, funding_broadcastable: Option, channel_ready: Option, announcement_sigs: Option, - tx_signatures: Option + tx_signatures: Option, tx_abort: Option, ) -> (Option<(u64, Option, OutPoint, ChannelId, u128, Vec<(PendingHTLCInfo, u64)>)>, Option<(u64, Vec)>) { let logger = WithChannelContext::from(&self.logger, &channel.context, None); - log_trace!(logger, "Handling channel resumption for channel {} with {} RAA, {} commitment update, {} pending forwards, {} pending update_add_htlcs, {}broadcasting funding, {} channel ready, {} announcement, {} tx_signatures", + log_trace!(logger, "Handling channel resumption for channel {} with {} RAA, {} commitment update, {} pending forwards, {} pending update_add_htlcs, {}broadcasting funding, {} channel ready, {} announcement, {} tx_signatures, {} tx_abort", &channel.context.channel_id(), if raa.is_some() { "an" } else { "no" }, if commitment_update.is_some() { "a" } else { "no" }, @@ -7660,6 +7660,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if channel_ready.is_some() { "sending" } else { "without" }, if announcement_sigs.is_some() { "sending" } else { "without" }, if tx_signatures.is_some() { "sending" } else { "without" }, + if tx_abort.is_some() { "sending" } else { "without" }, ); let counterparty_node_id = channel.context.get_counterparty_node_id(); @@ -7693,6 +7694,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ msg, }); } + if let Some(msg) = tx_abort { + pending_msg_events.push(MessageSendEvent::SendTxAbort { + node_id: counterparty_node_id, + msg, + }); + } macro_rules! handle_cs { () => { if let Some(update) = commitment_update { @@ -9474,7 +9481,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let need_lnd_workaround = chan.context.workaround_lnd_bug_4006.take(); let (htlc_forwards, decode_update_add_htlcs) = self.handle_channel_resumption( &mut peer_state.pending_msg_events, chan, responses.raa, responses.commitment_update, responses.order, - Vec::new(), Vec::new(), None, responses.channel_ready, responses.announcement_sigs, None); + Vec::new(), Vec::new(), None, responses.channel_ready, responses.announcement_sigs, + responses.tx_signatures, responses.tx_abort); debug_assert!(htlc_forwards.is_none()); debug_assert!(decode_update_add_htlcs.is_none()); if let Some(upd) = channel_update { diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index a2f89092535..5cc935540d9 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -309,16 +309,36 @@ impl ConstructedTransaction { /// https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#sharing-funding-signatures-tx_signatures #[derive(Debug, Clone, PartialEq)] pub(crate) struct InteractiveTxSigningSession { - pub unsigned_tx: ConstructedTransaction, - pub counterparty_sent_tx_signatures: bool, + unsigned_tx: ConstructedTransaction, + has_received_tx_signatures: bool, holder_sends_tx_signatures_first: bool, - received_commitment_signed: bool, + has_received_commitment_signed: bool, holder_tx_signatures: Option, } impl InteractiveTxSigningSession { + pub fn unsigned_tx(&self) -> &ConstructedTransaction { + &self.unsigned_tx + } + + pub fn has_received_tx_signatures(&self) -> bool { + self.has_received_tx_signatures + } + + pub fn holder_sends_tx_signatures_first(&self) -> bool { + self.holder_sends_tx_signatures_first + } + + pub fn has_received_commitment_signed(&self) -> bool { + self.has_received_commitment_signed + } + + pub fn holder_tx_signatures(&self) -> &Option { + &self.holder_tx_signatures + } + pub fn received_commitment_signed(&mut self) -> Option { - self.received_commitment_signed = true; + self.has_received_commitment_signed = true; if self.holder_sends_tx_signatures_first { self.holder_tx_signatures.clone() } else { @@ -327,7 +347,7 @@ impl InteractiveTxSigningSession { } pub fn get_tx_signatures(&self) -> Option { - if self.received_commitment_signed { + if self.has_received_commitment_signed { self.holder_tx_signatures.clone() } else { None @@ -352,7 +372,7 @@ impl InteractiveTxSigningSession { return Err(()); } self.unsigned_tx.add_remote_witnesses(tx_signatures.witnesses.clone()); - self.counterparty_sent_tx_signatures = true; + self.has_received_tx_signatures = true; let holder_tx_signatures = if !self.holder_sends_tx_signatures_first { self.holder_tx_signatures.clone() @@ -1008,9 +1028,9 @@ macro_rules! define_state_transitions { let signing_session = InteractiveTxSigningSession { holder_sends_tx_signatures_first: tx.holder_sends_tx_signatures_first, unsigned_tx: tx, - received_commitment_signed: false, + has_received_commitment_signed: false, holder_tx_signatures: None, - counterparty_sent_tx_signatures: false, + has_received_tx_signatures: false, }; Ok(NegotiationComplete(signing_session)) } From b7157217e643e0e3704ed8d3cd13deeab84afb6b Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Wed, 19 Mar 2025 10:02:41 +0200 Subject: [PATCH 3/7] Introduce interactive signing state flags This intoduces the INTERACTIVE_SIGNING, THEIR_TX_SIGNATURES_SENT, and OUR_TX_SIGNATURES_SENT funded state flags. A top-level state flag for INTERACTIVE_SIGNING was avoided so that this work is compatible with splicing as well as V2 channel establishment (dual-funding). This commit also ensures that `ChannelPending` is only emitted after peers exchange `tx_signatures`. --- lightning/src/ln/channel.rs | 162 ++++++++++++++++--------- lightning/src/ln/channelmanager.rs | 4 +- lightning/src/ln/dual_funding_tests.rs | 14 +-- lightning/src/ln/interactivetxs.rs | 15 --- 4 files changed, 116 insertions(+), 79 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 57fe8153b12..a135cd816df 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -578,6 +578,9 @@ 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!( @@ -610,6 +613,21 @@ 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) + ] +); + define_state_flags!( "Flags that only apply to [`ChannelState::AwaitingChannelReady`].", FUNDED_STATE, AwaitingChannelReadyFlags, [ @@ -668,7 +686,11 @@ enum ChannelState { /// We have sent `funding_created` and are awaiting a `funding_signed` to advance to /// `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. - FundingNegotiated, + /// + /// 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. + FundingNegotiated(FundingNegotiatedFlags), /// We've received/sent `funding_created` and `funding_signed` and are thus now waiting on the /// funding transaction to confirm. AwaitingChannelReady(AwaitingChannelReadyFlags), @@ -711,7 +733,7 @@ macro_rules! impl_state_flag { } }; ($get: ident, $set: ident, $clear: ident, FUNDED_STATES) => { - impl_state_flag!($get, $set, $clear, [AwaitingChannelReady, ChannelReady]); + impl_state_flag!($get, $set, $clear, [FundingNegotiated, AwaitingChannelReady, ChannelReady]); }; ($get: ident, $set: ident, $clear: ident, $state: ident) => { impl_state_flag!($get, $set, $clear, [$state]); @@ -721,10 +743,12 @@ macro_rules! impl_state_flag { impl ChannelState { fn from_u32(state: u32) -> Result { match state { - state_flags::FUNDING_NEGOTIATED => Ok(ChannelState::FundingNegotiated), state_flags::SHUTDOWN_COMPLETE => Ok(ChannelState::ShutdownComplete), val => { - if val & state_flags::AWAITING_CHANNEL_READY == state_flags::AWAITING_CHANNEL_READY { + if val & state_flags::FUNDING_NEGOTIATED == state_flags::FUNDING_NEGOTIATED { + FundingNegotiatedFlags::from_u32(val & !state_flags::FUNDING_NEGOTIATED) + .map(|flags| ChannelState::FundingNegotiated(flags)) + } else if val & state_flags::AWAITING_CHANNEL_READY == state_flags::AWAITING_CHANNEL_READY { AwaitingChannelReadyFlags::from_u32(val & !state_flags::AWAITING_CHANNEL_READY) .map(|flags| ChannelState::AwaitingChannelReady(flags)) } else if val & state_flags::CHANNEL_READY == state_flags::CHANNEL_READY { @@ -742,7 +766,7 @@ impl ChannelState { fn to_u32(self) -> u32 { match self { ChannelState::NegotiatingFunding(flags) => flags.0, - ChannelState::FundingNegotiated => state_flags::FUNDING_NEGOTIATED, + ChannelState::FundingNegotiated(flags) => state_flags::FUNDING_NEGOTIATED | flags.0, ChannelState::AwaitingChannelReady(flags) => state_flags::AWAITING_CHANNEL_READY | flags.0, ChannelState::ChannelReady(flags) => state_flags::CHANNEL_READY | flags.0, ChannelState::ShutdownComplete => state_flags::SHUTDOWN_COMPLETE, @@ -750,7 +774,11 @@ impl ChannelState { } fn is_pre_funded_state(&self) -> bool { - matches!(self, ChannelState::NegotiatingFunding(_)|ChannelState::FundingNegotiated) + match self { + ChannelState::NegotiatingFunding(_) => true, + ChannelState::FundingNegotiated(flags) => !flags.is_interactive_signing(), + _ => false, + } } fn is_both_sides_shutdown(&self) -> bool { @@ -784,6 +812,9 @@ impl ChannelState { impl_state_flag!(is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress, FUNDED_STATES); impl_state_flag!(is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent, FUNDED_STATES); impl_state_flag!(is_remote_shutdown_sent, set_remote_shutdown_sent, 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, clear_our_channel_ready, AwaitingChannelReady); impl_state_flag!(is_their_channel_ready, set_their_channel_ready, clear_their_channel_ready, AwaitingChannelReady); impl_state_flag!(is_waiting_for_batch, set_waiting_for_batch, clear_waiting_for_batch, AwaitingChannelReady); @@ -1624,7 +1655,6 @@ impl Channel where context: chan.context, interactive_tx_signing_session: chan.interactive_tx_signing_session, holder_commitment_point, - is_v2_established: true, #[cfg(splicing)] pending_splice: None, }; @@ -2268,14 +2298,17 @@ trait InitialRemoteCommitmentReceiver where SP::Target: SignerProvide // Now that we're past error-generating stuff, update our local state: + let is_v2_established = self.is_v2_established(); let context = self.context_mut(); context.channel_id = channel_id; assert!(!context.channel_state.is_monitor_update_in_progress()); // We have not had any monitor(s) yet to fail update! - if context.is_batch_funding() { - context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::WAITING_FOR_BATCH); - } else { - context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); + if !is_v2_established { + if context.is_batch_funding() { + context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::WAITING_FOR_BATCH); + } else { + context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); + } } if holder_commitment_point.advance(&context.holder_signer, &context.secp_ctx, logger).is_err() { // We only fail to advance our commitment point/number if we're currently @@ -2307,6 +2340,8 @@ trait InitialRemoteCommitmentReceiver where SP::Target: SignerProvide Ok((channel_monitor, counterparty_initial_commitment_tx)) } + + fn is_v2_established(&self) -> bool; } impl InitialRemoteCommitmentReceiver for OutboundV1Channel where SP::Target: SignerProvider { @@ -2329,6 +2364,10 @@ impl InitialRemoteCommitmentReceiver for OutboundV1Channel wh fn received_msg(&self) -> &'static str { "funding_signed" } + + fn is_v2_established(&self) -> bool { + false + } } impl InitialRemoteCommitmentReceiver for InboundV1Channel where SP::Target: SignerProvider { @@ -2351,6 +2390,10 @@ impl InitialRemoteCommitmentReceiver for InboundV1Channel whe fn received_msg(&self) -> &'static str { "funding_created" } + + fn is_v2_established(&self) -> bool { + false + } } impl InitialRemoteCommitmentReceiver for FundedChannel where SP::Target: SignerProvider { @@ -2373,6 +2416,18 @@ impl InitialRemoteCommitmentReceiver for FundedChannel where fn received_msg(&self) -> &'static str { "commitment_signed" } + + fn is_v2_established(&self) -> bool { + let channel_parameters = &self.funding().channel_transaction_parameters; + // This will return false if `counterparty_parameters` is `None`, but for a `FundedChannel`, it + // should never be `None`. + debug_assert!(channel_parameters.counterparty_parameters.is_some()); + channel_parameters.counterparty_parameters.as_ref().map_or(false, |counterparty_parameters| { + self.context().channel_id().is_v2_channel_id( + &channel_parameters.holder_pubkeys.revocation_basepoint, + &counterparty_parameters.pubkeys.revocation_basepoint) + }) + } } impl PendingV2Channel where SP::Target: SignerProvider { @@ -2568,10 +2623,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { self.context.assert_no_commitment_advancement(transaction_number, "initial commitment_signed"); let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger); let commitment_signed = match commitment_signed { - Ok(commitment_signed) => { - self.funding.funding_transaction = Some(signing_session.unsigned_tx().build_unsigned_tx()); - commitment_signed - }, + Ok(commitment_signed) => commitment_signed, Err(err) => { self.funding.channel_transaction_parameters.funding_outpoint = None; return Err(ChannelError::Close((err.to_string(), ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }))); @@ -2616,7 +2668,9 @@ impl PendingV2Channel where SP::Target: SignerProvider { ))); }; - self.context.channel_state = ChannelState::FundingNegotiated; + let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); + channel_state.set_interactive_signing(); + self.context.channel_state = channel_state; // Clear the interactive transaction constructor self.interactive_tx_constructor.take(); @@ -3699,7 +3753,7 @@ impl ChannelContext where SP::Target: SignerProvider { fn unset_funding_info(&mut self, funding: &mut FundingScope) { debug_assert!( - matches!(self.channel_state, ChannelState::FundingNegotiated) + 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; @@ -4630,7 +4684,7 @@ impl ChannelContext where SP::Target: SignerProvider { fn if_unbroadcasted_funding(&self, f: F) -> Option where F: Fn() -> Option { match self.channel_state { - ChannelState::FundingNegotiated => f(), + ChannelState::FundingNegotiated(_) => f(), ChannelState::AwaitingChannelReady(flags) => if flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH) || flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into()) @@ -5074,9 +5128,6 @@ pub(super) struct FundedChannel where SP::Target: SignerProvider { pub context: ChannelContext, pub interactive_tx_signing_session: Option, holder_commitment_point: HolderCommitmentPoint, - /// Indicates whether this funded channel had been established with V2 channel - /// establishment. - is_v2_established: bool, /// Info about an in-progress, pending splice (if any), on the pre-splice channel #[cfg(splicing)] pending_splice: Option, @@ -5643,6 +5694,8 @@ impl FundedChannel where self.context.counterparty_prev_commitment_point = self.context.counterparty_cur_commitment_point; self.context.counterparty_cur_commitment_point = Some(msg.next_per_commitment_point); + // Clear any interactive signing session. + self.interactive_tx_signing_session = None; log_info!(logger, "Received channel_ready from peer for channel {}", &self.context.channel_id()); @@ -5842,7 +5895,7 @@ impl FundedChannel where ) -> Result::EcdsaSigner>, ChannelError> where L::Target: Logger { - if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) { + if !matches!(self.context.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing()) { return Err(ChannelError::Close( ( "Received initial commitment_signed before funding transaction constructed!".to_owned(), @@ -5863,9 +5916,7 @@ impl FundedChannel where log_info!(logger, "Received initial commitment_signed from peer for channel {}", &self.context.channel_id()); - let need_channel_ready = self.check_get_channel_ready(0, logger).is_some(); - self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); - self.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new()); + self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); if let Some(tx_signatures) = self.interactive_tx_signing_session.as_mut().and_then( |session| session.received_commitment_signed() @@ -6507,11 +6558,11 @@ impl FundedChannel where } } - pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result, ChannelError> + pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<(Option, Option), ChannelError> where L::Target: Logger { - if !matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(_)) { - return Err(ChannelError::close("Received tx_signatures in strange state!".to_owned())); + if !matches!(self.context.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing()) { + 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 { @@ -6547,21 +6598,29 @@ impl FundedChannel where let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg.clone()) .map_err(|_| ChannelError::Warn("Witness count did not match contributed input count".to_string()))?; + // Set `THEIR_TX_SIGNATURES_SENT` flag after all potential errors. + self.context.channel_state.set_their_tx_signatures_sent(); if funding_tx_opt.is_some() { - // We have a finalized funding transaction, so we can set the funding transaction and reset the - // signing session fields. - self.funding.funding_transaction = funding_tx_opt; - self.interactive_tx_signing_session = None; + // We have a finalized funding transaction, so we can set the funding transaction. + self.funding.funding_transaction = funding_tx_opt.clone(); } + // Note that `holder_tx_signatures_opt` will be `None` if we sent `tx_signatures` first, so this + // case checks if there is a monitor persist in progress when we need to respond with our `tx_signatures` + // and sets it as pending. if holder_tx_signatures_opt.is_some() && self.is_awaiting_initial_mon_persist() { log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures."); self.context.monitor_pending_tx_signatures = holder_tx_signatures_opt; - return Ok(None); + return Ok((None, None)); + } + + if holder_tx_signatures_opt.is_some() { + self.context.channel_state.set_our_tx_signatures_ready(); } - Ok(holder_tx_signatures_opt) + self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); + Ok((funding_tx_opt, holder_tx_signatures_opt)) } else { Err(ChannelError::Close(( "Unexpected tx_signatures. No funding transaction awaiting signatures".to_string(), @@ -6823,6 +6882,9 @@ impl FundedChannel where // MonitorUpdateInProgress (and we assume the user will never directly broadcast the funding // transaction and waits for us to do it). let tx_signatures = self.context.monitor_pending_tx_signatures.take(); + if tx_signatures.is_some() { + self.context.channel_state.set_our_tx_signatures_ready(); + } if self.context.channel_state.is_peer_disconnected() { self.context.monitor_pending_revoke_and_ack = false; @@ -7295,7 +7357,7 @@ impl FundedChannel where if our_next_funding_txid == next_funding_txid { debug_assert_eq!(session.unsigned_tx().compute_txid(), self.maybe_get_next_funding_txid().unwrap()); - let commitment_update = if !session.has_received_tx_signatures() && msg.next_local_commitment_number == 0 { + let commitment_update = if !self.context.channel_state.is_their_tx_signatures_sent() && msg.next_local_commitment_number == 0 { // if it has not received tx_signatures for that funding transaction AND // if next_commitment_number is zero: // MUST retransmit its commitment_signed for that funding transaction. @@ -7314,10 +7376,10 @@ impl FundedChannel where // if it has not received tx_signatures for that funding transaction AND // if it has already received commitment_signed AND it should sign first, as specified in the tx_signatures requirements: // MUST send its tx_signatures for that funding transaction. - !session.has_received_tx_signatures() && session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first() + !self.context.channel_state.is_their_tx_signatures_sent() && session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first() // else if it has already received tx_signatures for that funding transaction: // MUST send its tx_signatures for that funding transaction. - ) || session.has_received_tx_signatures() { + ) || self.context.channel_state.is_their_tx_signatures_sent() { if self.context.channel_state.is_monitor_update_in_progress() { // The `monitor_pending_tx_signatures` field should have already been set in `commitment_signed_initial_v2` // if we were up first for signing and had a monitor update in progress, but check again just in case. @@ -7351,7 +7413,7 @@ impl FundedChannel where ).into_bytes() })) } } else { - return Err(ChannelError::Warn("No active signing session. The associated funding transaction may have already been broadcast.".into())); + return Err(ChannelError::close("Counterparty set `next_funding_txid` at incorrect state".into())); } } else { // Don't send anything related to interactive signing if `next_funding_txid` is not set. @@ -8693,9 +8755,9 @@ impl FundedChannel where // 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_txid` // to the txid of that interactive transaction, else we MUST NOT set it. - if let Some(signing_session) = &self.interactive_tx_signing_session { + if self.context.channel_state.is_interactive_signing() { // Since we have a signing_session, this implies we've sent an initial `commitment_signed`... - if !signing_session.has_received_tx_signatures() { + if !self.context.channel_state.is_their_tx_signatures_sent() { // ...but we didn't receive a `tx_signatures` from the counterparty yet. Some(self.funding_outpoint().txid) } else { @@ -9612,10 +9674,6 @@ impl FundedChannel where false } } - - pub fn is_v2_established(&self) -> bool { - self.is_v2_established - } } /// A not-yet-funded outbound (from holder) channel using V1 channel establishment. @@ -9746,7 +9804,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { // Now that we're past error-generating stuff, update our local state: - self.context.channel_state = ChannelState::FundingNegotiated; + self.context.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); self.context.channel_id = ChannelId::v1_from_funding_outpoint(funding_txo); // If the funding transaction is a coinbase transaction, we need to set the minimum depth to 100. @@ -9863,7 +9921,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { if !self.funding.is_outbound() { return Err((self, ChannelError::close("Received funding_signed for an inbound channel?".to_owned()))); } - if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) { + if !matches!(self.context.channel_state, ChannelState::FundingNegotiated(_)) { return Err((self, ChannelError::close("Received funding_signed in strange state!".to_owned()))); } let mut holder_commitment_point = match self.unfunded_context.holder_commitment_point { @@ -9887,7 +9945,6 @@ impl OutboundV1Channel where SP::Target: SignerProvider { pending_funding: vec![], context: self.context, interactive_tx_signing_session: None, - is_v2_established: false, holder_commitment_point, #[cfg(splicing)] pending_splice: None, @@ -10164,7 +10221,6 @@ impl InboundV1Channel where SP::Target: SignerProvider { pending_funding: vec![], context: self.context, interactive_tx_signing_session: None, - is_v2_established: false, holder_commitment_point, #[cfg(splicing)] pending_splice: None, @@ -11483,10 +11539,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel } }, }; - let is_v2_established = channel_id.is_v2_channel_id( - &channel_parameters.holder_pubkeys.revocation_basepoint, - &channel_parameters.counterparty_parameters.as_ref() - .expect("Persisted channel must have counterparty parameters").pubkeys.revocation_basepoint); Ok(FundedChannel { funding: FundingScope { @@ -11621,7 +11673,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel is_holder_quiescence_initiator: None, }, interactive_tx_signing_session: None, - is_v2_established, holder_commitment_point, #[cfg(splicing)] pending_splice: None, @@ -11689,11 +11740,12 @@ mod tests { #[test] fn test_channel_state_order() { use crate::ln::channel::NegotiatingFundingFlags; + use crate::ln::channel::FundingNegotiatedFlags; use crate::ln::channel::AwaitingChannelReadyFlags; use crate::ln::channel::ChannelReadyFlags; - assert!(ChannelState::NegotiatingFunding(NegotiatingFundingFlags::new()) < ChannelState::FundingNegotiated); - assert!(ChannelState::FundingNegotiated < ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new())); + assert!(ChannelState::NegotiatingFunding(NegotiatingFundingFlags::new()) < ChannelState::FundingNegotiated(FundingNegotiatedFlags::new())); + assert!(ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()) < ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new())); assert!(ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()) < ChannelState::ChannelReady(ChannelReadyFlags::new())); assert!(ChannelState::ChannelReady(ChannelReadyFlags::new()) < ChannelState::ShutdownComplete); } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c3a4cdb610b..5fe8a1597cf 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8573,14 +8573,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ match chan_entry.get_mut().as_funded_mut() { Some(chan) => { let logger = WithChannelContext::from(&self.logger, &chan.context, None); - let tx_signatures_opt = try_channel_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_entry); + let (funding_tx_opt, tx_signatures_opt) = try_channel_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_entry); if let Some(tx_signatures) = tx_signatures_opt { peer_state.pending_msg_events.push(MessageSendEvent::SendTxSignatures { node_id: *counterparty_node_id, msg: tx_signatures, }); } - if let Some(ref funding_tx) = chan.context.unbroadcasted_funding(&chan.funding) { + if let Some(ref funding_tx) = funding_tx_opt { self.tx_broadcaster.broadcast_transactions(&[funding_tx]); { let mut pending_events = self.pending_events.lock().unwrap(); diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index 1a7bab39dc4..1784e9c2ed5 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -206,13 +206,6 @@ fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession) assert!(events.is_empty()); nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id); - 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"), - }; - let tx_signatures_msg = get_event_msg!( nodes[1], MessageSendEvent::SendTxSignatures, @@ -234,6 +227,13 @@ fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession) }, ); + 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(); diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 5cc935540d9..a2a6133d114 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -310,7 +310,6 @@ impl ConstructedTransaction { #[derive(Debug, Clone, PartialEq)] pub(crate) struct InteractiveTxSigningSession { unsigned_tx: ConstructedTransaction, - has_received_tx_signatures: bool, holder_sends_tx_signatures_first: bool, has_received_commitment_signed: bool, holder_tx_signatures: Option, @@ -321,10 +320,6 @@ impl InteractiveTxSigningSession { &self.unsigned_tx } - pub fn has_received_tx_signatures(&self) -> bool { - self.has_received_tx_signatures - } - pub fn holder_sends_tx_signatures_first(&self) -> bool { self.holder_sends_tx_signatures_first } @@ -346,14 +341,6 @@ impl InteractiveTxSigningSession { } } - pub fn get_tx_signatures(&self) -> Option { - if self.has_received_commitment_signed { - self.holder_tx_signatures.clone() - } else { - None - } - } - /// Handles a `tx_signatures` message received from the counterparty. /// /// If the holder is required to send their `tx_signatures` message and these signatures have @@ -372,7 +359,6 @@ impl InteractiveTxSigningSession { return Err(()); } self.unsigned_tx.add_remote_witnesses(tx_signatures.witnesses.clone()); - self.has_received_tx_signatures = true; let holder_tx_signatures = if !self.holder_sends_tx_signatures_first { self.holder_tx_signatures.clone() @@ -1030,7 +1016,6 @@ macro_rules! define_state_transitions { unsigned_tx: tx, has_received_commitment_signed: false, holder_tx_signatures: None, - has_received_tx_signatures: false, }; Ok(NegotiationComplete(signing_session)) } From 8bbe77c1173a07e6cd4fe5de5c7a34e9de6f0ddb Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Wed, 19 Mar 2025 10:03:37 +0200 Subject: [PATCH 4/7] Persist `InteractiveTxSigningSession` to resume signing across restarts We fully persist `InteractiveTxSigningSession` as it provides the full context of the constructed transaction which is still needed for signing. --- lightning/src/ln/channel.rs | 13 +++++++- lightning/src/ln/interactivetxs.rs | 51 ++++++++++++++++++++++++++++++ lightning/src/util/ser.rs | 23 ++++++++++++-- 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a135cd816df..1db4fac8504 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5126,6 +5126,13 @@ pub(super) struct FundedChannel where SP::Target: SignerProvider { pub funding: FundingScope, pending_funding: Vec, 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, /// Info about an in-progress, pending splice (if any), on the pre-splice channel @@ -11032,6 +11039,7 @@ impl Writeable for FundedChannel where SP::Target: SignerProvider (54, self.pending_funding, optional_vec), // Added in 0.2 (55, removed_htlc_failure_attribution_data, optional_vec), // Added in 0.2 (57, holding_cell_failure_attribution_data, optional_vec), // Added in 0.2 + (58, self.interactive_tx_signing_session, option) // Added in 0.2 }); Ok(()) @@ -11348,6 +11356,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel let mut pending_funding = Some(Vec::new()); + let mut interactive_tx_signing_session: Option = None; + read_tlv_fields!(reader, { (0, announcement_sigs, option), (1, minimum_depth, option), @@ -11386,6 +11396,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel (54, pending_funding, optional_vec), // Added in 0.2 (55, removed_htlc_failure_attribution_data, optional_vec), (57, holding_cell_failure_attribution_data, optional_vec), + (58, interactive_tx_signing_session, option), // Added in 0.2 }); let holder_signer = signer_provider.derive_channel_signer(channel_keys_id); @@ -11672,7 +11683,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel is_holder_quiescence_initiator: None, }, - interactive_tx_signing_session: None, + interactive_tx_signing_session, holder_commitment_point, #[cfg(splicing)] pending_splice: None, diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index a2a6133d114..ee991a0ae8c 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -189,6 +189,18 @@ pub(crate) struct ConstructedTransaction { holder_sends_tx_signatures_first: bool, } +impl_writeable_tlv_based!(ConstructedTransaction, { + (1, holder_is_initiator, required), + (3, inputs, required), + (5, outputs, required), + (7, local_inputs_value_satoshis, required), + (9, local_outputs_value_satoshis, required), + (11, remote_inputs_value_satoshis, required), + (13, remote_outputs_value_satoshis, required), + (15, lock_time, required), + (17, holder_sends_tx_signatures_first, required), +}); + impl ConstructedTransaction { fn new(context: NegotiationContext) -> Self { let local_inputs_value_satoshis = context @@ -439,6 +451,13 @@ impl InteractiveTxSigningSession { } } +impl_writeable_tlv_based!(InteractiveTxSigningSession, { + (1, unsigned_tx, required), + (3, holder_sends_tx_signatures_first, required), + (5, has_received_commitment_signed, required), + (7, holder_tx_signatures, required), +}); + #[derive(Debug)] struct NegotiationContext { holder_node_id: PublicKey, @@ -1162,6 +1181,11 @@ enum AddingRole { Remote, } +impl_writeable_tlv_based_enum!(AddingRole, + (1, Local) => {}, + (3, Remote) => {}, +); + /// Represents an input -- local or remote (both have the same fields) #[derive(Clone, Debug, Eq, PartialEq)] pub struct LocalOrRemoteInput { @@ -1170,6 +1194,12 @@ pub struct LocalOrRemoteInput { prev_output: TxOut, } +impl_writeable_tlv_based!(LocalOrRemoteInput, { + (1, serial_id, required), + (3, input, required), + (5, prev_output, required), +}); + #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) enum InteractiveTxInput { Local(LocalOrRemoteInput), @@ -1177,12 +1207,22 @@ pub(crate) enum InteractiveTxInput { // TODO(splicing) SharedInput should be added } +impl_writeable_tlv_based_enum!(InteractiveTxInput, + {1, Local} => (), + {3, Remote} => (), +); + #[derive(Clone, Debug, Eq, PartialEq)] pub(super) struct SharedOwnedOutput { tx_out: TxOut, local_owned: u64, } +impl_writeable_tlv_based!(SharedOwnedOutput, { + (1, tx_out, required), + (3, local_owned, required), +}); + impl SharedOwnedOutput { pub fn new(tx_out: TxOut, local_owned: u64) -> SharedOwnedOutput { debug_assert!( @@ -1210,6 +1250,11 @@ pub(super) enum OutputOwned { Shared(SharedOwnedOutput), } +impl_writeable_tlv_based_enum!(OutputOwned, + {1, Single} => (), + {3, Shared} => (), +); + impl OutputOwned { pub fn tx_out(&self) -> &TxOut { match self { @@ -1264,6 +1309,12 @@ pub(crate) struct InteractiveTxOutput { output: OutputOwned, } +impl_writeable_tlv_based!(InteractiveTxOutput, { + (1, serial_id, required), + (3, added_by, required), + (5, output, required), +}); + impl InteractiveTxOutput { pub fn tx_out(&self) -> &TxOut { self.output.tx_out() diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 2560c3af1b9..737a558946e 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -15,6 +15,7 @@ use crate::io::{self, BufRead, Read, Write}; use crate::io_extras::{copy, sink}; +use crate::ln::interactivetxs::{InteractiveTxInput, InteractiveTxOutput}; use crate::ln::onion_utils::{HMAC_COUNT, HMAC_LEN, HOLD_TIME_LEN, MAX_HOPS}; use crate::prelude::*; use crate::sync::{Mutex, RwLock}; @@ -24,6 +25,7 @@ use core::ops::Deref; use alloc::collections::BTreeMap; +use bitcoin::absolute::LockTime as AbsoluteLockTime; use bitcoin::amount::Amount; use bitcoin::consensus::Encodable; use bitcoin::constants::ChainHash; @@ -39,14 +41,14 @@ use bitcoin::secp256k1::ecdsa; use bitcoin::secp256k1::schnorr; use bitcoin::secp256k1::{PublicKey, SecretKey}; use bitcoin::transaction::{OutPoint, Transaction, TxOut}; -use bitcoin::{consensus, Witness}; +use bitcoin::{consensus, TxIn, Witness}; use dnssec_prover::rr::Name; use crate::chain::ClaimId; -use crate::ln::msgs::DecodeError; #[cfg(taproot)] use crate::ln::msgs::PartialSignatureWithNonce; +use crate::ln::msgs::{DecodeError, SerialId}; use crate::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; use core::time::Duration; @@ -1079,6 +1081,9 @@ impl_for_vec!(crate::ln::channelmanager::MonitorUpdateCompletionAction); impl_for_vec!(crate::ln::channelmanager::PaymentClaimDetails); impl_for_vec!(crate::ln::msgs::SocketAddress); impl_for_vec!((A, B), A, B); +impl_for_vec!(SerialId); +impl_for_vec!(InteractiveTxInput); +impl_for_vec!(InteractiveTxOutput); impl_writeable_for_vec!(&crate::routing::router::BlindedTail); impl_readable_for_vec!(crate::routing::router::BlindedTail); impl_for_vec!(crate::routing::router::TrampolineHop); @@ -1350,6 +1355,19 @@ impl Readable for Option { } } +impl Writeable for AbsoluteLockTime { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.to_consensus_u32().write(w) + } +} + +impl Readable for AbsoluteLockTime { + fn read(r: &mut R) -> Result { + let lock_time: u32 = Readable::read(r)?; + Ok(AbsoluteLockTime::from_consensus(lock_time)) + } +} + impl Writeable for Amount { fn write(&self, w: &mut W) -> Result<(), io::Error> { self.to_sat().write(w) @@ -1451,6 +1469,7 @@ macro_rules! impl_consensus_ser { }; } impl_consensus_ser!(Transaction); +impl_consensus_ser!(TxIn); impl_consensus_ser!(TxOut); impl_consensus_ser!(Witness); From 21d3c784418f65ea1b0559398281e0d54cc6fa1e Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Wed, 5 Mar 2025 10:53:32 +0200 Subject: [PATCH 5/7] Add `enable_dual_funded_channels` to `UserConfig` When this config field is enabled, the dual_fund feature bit will be set which determines support when receiving `open_channel2` messages. --- fuzz/src/full_stack.rs | 4 ++-- lightning/src/ln/channelmanager.rs | 10 +++++++++- lightning/src/ln/dual_funding_tests.rs | 4 +++- lightning/src/util/config.rs | 6 ++++++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 9ff3b852f9f..1007a26b338 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1082,7 +1082,7 @@ fn two_peer_forwarding_seed() -> Vec { // our network key ext_from_hex("0100000000000000000000000000000000000000000000000000000000000000", &mut test); // config - ext_from_hex("0000000000900000000000000000640001000000000001ffff0000000000000000ffffffffffffffffffffffffffffffff0000000000000000ffffffffffffffff000000ffffffff00ffff1a000400010000020400000000040200000a08ffffffffffffffff0001000000", &mut test); + ext_from_hex("0000000000900000000000000000640001000000000001ffff0000000000000000ffffffffffffffffffffffffffffffff0000000000000000ffffffffffffffff000000ffffffff00ffff1a000400010000020400000000040200000a08ffffffffffffffff000100000000", &mut test); // new outbound connection with id 0 ext_from_hex("00", &mut test); @@ -1598,7 +1598,7 @@ fn gossip_exchange_seed() -> Vec { // our network key ext_from_hex("0100000000000000000000000000000000000000000000000000000000000000", &mut test); // config - ext_from_hex("0000000000900000000000000000640001000000000001ffff0000000000000000ffffffffffffffffffffffffffffffff0000000000000000ffffffffffffffff000000ffffffff00ffff1a000400010000020400000000040200000a08ffffffffffffffff0001000000", &mut test); + ext_from_hex("0000000000900000000000000000640001000000000001ffff0000000000000000ffffffffffffffffffffffffffffffff0000000000000000ffffffffffffffff000000ffffffff00ffff1a000400010000020400000000040200000a08ffffffffffffffff000100000000", &mut test); // new outbound connection with id 0 ext_from_hex("00", &mut test); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5fe8a1597cf..93dd16e7df4 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -11987,6 +11987,12 @@ where } fn handle_open_channel_v2(&self, counterparty_node_id: PublicKey, msg: &msgs::OpenChannelV2) { + if !self.init_features().supports_dual_fund() { + let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close( + "Dual-funded channels not supported".to_owned(), + msg.common_fields.temporary_channel_id.clone())), counterparty_node_id); + return; + } // Note that we never need to persist the updated ChannelManager for an inbound // open_channel message - pre-funded channels are never written so there should be no // change to the contents. @@ -12902,7 +12908,9 @@ pub fn provided_init_features(config: &UserConfig) -> InitFeatures { if config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx { features.set_anchors_zero_fee_htlc_tx_optional(); } - features.set_dual_fund_optional(); + if config.enable_dual_funded_channels { + features.set_dual_fund_optional(); + } // Only signal quiescence support in tests for now, as we don't yet support any // quiescent-dependent protocols (e.g., splicing). #[cfg(any(test, fuzzing))] diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index 1784e9c2ed5..6a7ef317ba8 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -39,7 +39,9 @@ struct V2ChannelEstablishmentTestSession { 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 node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + 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()); diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 152a6a62eb6..7fcebcf1e73 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -875,6 +875,10 @@ pub struct UserConfig { /// [`ChannelManager::send_payment_for_bolt12_invoice`]: crate::ln::channelmanager::ChannelManager::send_payment_for_bolt12_invoice /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment pub manually_handle_bolt12_invoices: bool, + /// If this is set to `true`, dual-funded channels will be enabled. + /// + /// Default value: `false` + pub enable_dual_funded_channels: bool, } impl Default for UserConfig { @@ -888,6 +892,7 @@ impl Default for UserConfig { manually_accept_inbound_channels: false, accept_intercept_htlcs: false, manually_handle_bolt12_invoices: false, + enable_dual_funded_channels: false, } } } @@ -907,6 +912,7 @@ impl Readable for UserConfig { manually_accept_inbound_channels: Readable::read(reader)?, accept_intercept_htlcs: Readable::read(reader)?, manually_handle_bolt12_invoices: Readable::read(reader)?, + enable_dual_funded_channels: Readable::read(reader)?, }) } } From d6bd9fe5f86e81516b703e79df20c32da3d3c5b3 Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Mon, 14 Apr 2025 14:54:34 +0200 Subject: [PATCH 6/7] Introduce `FundingTransactionReadyForSignatures` event The `FundingTransactionReadyForSignatures` event requests witnesses from the client for their contributed inputs to an interactively constructed transaction. The client calls `ChannelManager::funding_transaction_signed` to provide the witnesses to LDK. --- lightning/src/events/mod.rs | 58 ++++++++++++++++++++++++++- lightning/src/ln/channel.rs | 64 ++++++++++++++++++------------ lightning/src/ln/channelmanager.rs | 53 +++++++++++++++++++++++++ lightning/src/ln/interactivetxs.rs | 16 ++++++-- 4 files changed, 161 insertions(+), 30 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 8d28c9b4191..7fc31bb4c5f 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1507,7 +1507,56 @@ pub enum Event { /// The node id of the peer we just connected to, who advertises support for /// onion messages. peer_node_id: PublicKey, - } + }, + /// Indicates that a funding transaction constructed via interactive transaction construction for a + /// channel is ready to be signed by the client. This event will only be triggered + /// if at least one input was contributed by the holder and needs to be signed. + /// + /// The transaction contains all inputs provided by both parties along with the channel's funding + /// output and a change output if applicable. + /// + /// No part of the transaction should be changed before signing as the content of the transaction + /// has already been negotiated with the counterparty. + /// + /// Each signature MUST use the SIGHASH_ALL flag to avoid invalidation of the initial commitment and + /// hence possible loss of funds. + /// + /// After signing, call [`ChannelManager::funding_transaction_signed`] with the (partially) signed + /// funding transaction. + /// + /// Generated in [`ChannelManager`] message handling. + /// + /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager + /// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed + FundingTransactionReadyForSigning { + /// The channel_id of the channel which you'll need to pass back into + /// [`ChannelManager::funding_transaction_signed`]. + /// + /// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed + channel_id: ChannelId, + /// The counterparty's node_id, which you'll need to pass back into + /// [`ChannelManager::funding_transaction_signed`]. + /// + /// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed + counterparty_node_id: PublicKey, + // TODO(dual_funding): Enable links when methods are implemented + /// The `user_channel_id` value passed in to `ChannelManager::create_dual_funded_channel` for outbound + /// channels, or to [`ChannelManager::accept_inbound_channel`] or `ChannelManager::accept_inbound_channel_with_contribution` + /// for inbound channels if [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. + /// Otherwise `user_channel_id` will be randomized for an inbound channel. + /// This may be zero for objects serialized with LDK versions prior to 0.0.113. + /// + /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel + /// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels + // [`ChannelManager::create_dual_funded_channel`]: crate::ln::channelmanager::ChannelManager::create_dual_funded_channel + // [`ChannelManager::accept_inbound_channel_with_contribution`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel_with_contribution + user_channel_id: u128, + /// The unsigned transaction to be signed and passed back to + /// [`ChannelManager::funding_transaction_signed`]. + /// + /// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed + unsigned_transaction: Transaction, + }, } impl Writeable for Event { @@ -1842,6 +1891,13 @@ impl Writeable for Event { (8, former_temporary_channel_id, required), }); }, + &Event::FundingTransactionReadyForSigning { .. } => { + 45u8.write(writer)?; + // We never write out FundingTransactionReadyForSigning events as, upon disconnection, peers + // drop any V2-established/spliced channels which have not yet exchanged the initial `commitment_signed`. + // We only exhange the initial `commitment_signed` after the client calls + // `ChannelManager::funding_transaction_signed` and ALWAYS before we send a `tx_signatures` + }, // Note that, going forward, all new events must only write data inside of // `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write // data via `write_tlv_fields`. diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1db4fac8504..5b54a1782c1 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -14,7 +14,7 @@ use bitcoin::transaction::{Transaction, TxIn, TxOut}; use bitcoin::sighash::EcdsaSighashType; use bitcoin::consensus::encode; use bitcoin::absolute::LockTime; -use bitcoin::Weight; +use bitcoin::{Weight, Witness}; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; @@ -2630,7 +2630,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { }, }; - let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 { + let funding_ready_for_sig_event_opt = if signing_session.local_inputs_count() == 0 { debug_assert_eq!(our_funding_satoshis, 0); if signing_session.provide_holder_witnesses(self.context.channel_id, Vec::new()).is_err() { debug_assert!( @@ -2644,28 +2644,12 @@ impl PendingV2Channel where SP::Target: SignerProvider { } None } else { - // TODO(dual_funding): Send event for signing if we've contributed funds. - // Inform the user that SIGHASH_ALL must be used for all signatures when contributing - // inputs/signatures. - // Also warn the user that we don't do anything to prevent the counterparty from - // providing non-standard witnesses which will prevent the funding transaction from - // confirming. This warning must appear in doc comments wherever the user is contributing - // funds, whether they are initiator or acceptor. - // - // The following warning can be used when the APIs allowing contributing inputs become available: - //
- // WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which - // will prevent the funding transaction from being relayed on the bitcoin network and hence being - // confirmed. - //
- debug_assert!( - false, - "We don't support users providing inputs but somehow we had more than zero inputs", - ); - return Err(ChannelError::Close(( - "V2 channel rejected due to sender error".into(), - ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) } - ))); + Some(Event::FundingTransactionReadyForSigning { + channel_id: self.context.channel_id, + counterparty_node_id: self.context.counterparty_node_id, + user_channel_id: self.context.user_id, + unsigned_transaction: signing_session.unsigned_tx().build_unsigned_tx(), + }) }; let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); @@ -2676,7 +2660,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { self.interactive_tx_constructor.take(); self.interactive_tx_signing_session = Some(signing_session); - Ok((commitment_signed, funding_ready_for_sig_event)) + Ok((commitment_signed, funding_ready_for_sig_event_opt)) } } @@ -6565,6 +6549,36 @@ impl FundedChannel where } } + fn verify_interactive_tx_signatures(&mut self, _witnesses: &Vec) { + if let Some(ref mut _signing_session) = self.interactive_tx_signing_session { + // Check that sighash_all was used: + // TODO(dual_funding): Check sig for sighash + } + } + + pub fn funding_transaction_signed(&mut self, witnesses: Vec, logger: &L) -> Result, APIError> + where L::Target: Logger + { + self.verify_interactive_tx_signatures(&witnesses); + if let Some(ref mut signing_session) = self.interactive_tx_signing_session { + let logger = WithChannelContext::from(logger, &self.context, None); + if let Some(holder_tx_signatures) = signing_session + .provide_holder_witnesses(self.context.channel_id, witnesses) + .map_err(|err| APIError::APIMisuseError { err })? + { + if self.is_awaiting_initial_mon_persist() { + log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures."); + self.context.monitor_pending_tx_signatures = Some(holder_tx_signatures); + return Ok(None); + } + return Ok(Some(holder_tx_signatures)); + } else { return Ok(None) } + } else { + return Err(APIError::APIMisuseError { + err: format!("Channel with id {} not expecting funding signatures", self.context.channel_id)}); + } + } + pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<(Option, Option), ChannelError> where L::Target: Logger { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 93dd16e7df4..92cf6d9826f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5511,6 +5511,59 @@ where result } + /// Handles a signed funding transaction generated by interactive transaction construction and + /// provided by the client. + /// + /// Do NOT broadcast the funding transaction yourself. When we have safely received our + /// counterparty's signature(s) the funding transaction will automatically be broadcast via the + /// [`BroadcasterInterface`] provided when this `ChannelManager` was constructed. + /// + /// SIGHASH_ALL MUST be used for all signatures when providing signatures. + /// + ///
+ /// WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which + /// will prevent the funding transaction from being relayed on the bitcoin network and hence being + /// confirmed. + ///
+ pub fn funding_transaction_signed( + &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, transaction: Transaction + ) -> Result<(), APIError> { + let witnesses: Vec<_> = transaction.input.into_iter().filter_map(|input| { + if input.witness.is_empty() { None } else { Some(input.witness) } + }).collect(); + + let per_peer_state = self.per_peer_state.read().unwrap(); + let peer_state_mutex = per_peer_state.get(counterparty_node_id) + .ok_or_else(|| APIError::ChannelUnavailable { + err: format!("Can't find a peer matching the passed counterparty node_id {}", + counterparty_node_id) })?; + + let mut peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &mut *peer_state_lock; + + match peer_state.channel_by_id.get_mut(channel_id) { + Some(channel) => { + match channel.as_funded_mut() { + Some(chan) => { + if let Some(tx_signatures) = chan.funding_transaction_signed(witnesses, &self.logger)? { + peer_state.pending_msg_events.push(MessageSendEvent::SendTxSignatures { + node_id: *counterparty_node_id, + msg: tx_signatures, + }); + } + }, + None => return Err(APIError::APIMisuseError { + err: format!("Channel with id {} not expecting funding signatures", channel_id)}), + } + }, + None => return Err(APIError::ChannelUnavailable{ + err: format!("Channel with id {} not found for the passed counterparty node_id {}", channel_id, + counterparty_node_id) }), + } + + Ok(()) + } + /// Atomically applies partial updates to the [`ChannelConfig`] of the given channels. /// /// Once the updates are applied, each eligible channel (advertised with a known short channel diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index ee991a0ae8c..1736d911a0b 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -396,9 +396,13 @@ impl InteractiveTxSigningSession { /// unsigned transaction. pub fn provide_holder_witnesses( &mut self, channel_id: ChannelId, witnesses: Vec, - ) -> Result<(), ()> { - if self.local_inputs_count() != witnesses.len() { - return Err(()); + ) -> Result, String> { + let local_inputs_count = self.local_inputs_count(); + if local_inputs_count != witnesses.len() { + return Err(format!( + "Provided witness count of {} does not match required count for {} inputs", + witnesses.len(), local_inputs_count + )); } self.unsigned_tx.add_local_witnesses(witnesses.clone()); @@ -409,7 +413,11 @@ impl InteractiveTxSigningSession { shared_input_signature: None, }); - Ok(()) + if self.holder_sends_tx_signatures_first && self.has_received_commitment_signed { + Ok(self.holder_tx_signatures.clone()) + } else { + Ok(None) + } } pub fn remote_inputs_count(&self) -> usize { From fabbf86c703b13d0e8607d61617b772d5cafbdf1 Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Fri, 25 Apr 2025 11:52:06 +0200 Subject: [PATCH 7/7] wip pass funding inputs --- lightning/src/ln/channel.rs | 55 ++++++++++++-- lightning/src/ln/channelmanager.rs | 64 +++++++++++++--- lightning/src/ln/dual_funding_tests.rs | 2 +- lightning/src/ln/interactivetxs.rs | 100 ++++++++++++++++++------- 4 files changed, 176 insertions(+), 45 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 5b54a1782c1..1521b872400 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -30,7 +30,7 @@ use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentHash}; use crate::types::features::{ChannelTypeFeatures, InitFeatures}; use crate::ln::interactivetxs::{ - calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteResult, InteractiveTxConstructor, + calculate_change_output_value, estimate_input_weight, get_output_weight, AbortReason, HandleTxCompleteResult, InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend, InteractiveTxSigningSession, InteractiveTxMessageSendResult, OutputOwned, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT, }; @@ -4997,6 +4997,41 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis)) } +fn calculate_our_funding_satoshis( + is_initiator: bool, funding_inputs: &[(TxIn, TransactionU16LenLimited, Weight)], funding_outputs: &[TxOut], + funding_feerate_sat_per_1000_weight: u32, holder_dust_limit_satoshis: u64, +) -> Result { + let mut total_input_satoshis = 0u64; + let mut our_contributed_weight = 0u64; + + for (idx, input) in funding_inputs.iter().enumerate() { + if let Some(output) = input.1.as_transaction().output.get(input.0.previous_output.vout as usize) { + total_input_satoshis = total_input_satoshis.saturating_add(output.value.to_sat()); + our_contributed_weight = our_contributed_weight.saturating_add(estimate_input_weight(output).to_wu()); + } else { + return Err(APIError::APIMisuseError { + err: format!("Transaction with txid {} does not have an output with vout of {} corresponding to TxIn at funding_inputs[{}]", + input.1.as_transaction().compute_txid(), input.0.previous_output.vout, idx) }); + } + } + our_contributed_weight = our_contributed_weight.saturating_add(funding_outputs.iter().fold(0u64, |weight, txout| { + weight.saturating_add(get_output_weight(&txout.script_pubkey).to_wu()) + })); + + // If we are the initiator, we must pay for weight of all common fields in the funding transaction. + if is_initiator { + our_contributed_weight = our_contributed_weight.saturating_add(TX_COMMON_FIELDS_WEIGHT); + } + + let funding_satoshis = total_input_satoshis + .saturating_sub(fee_for_weight(funding_feerate_sat_per_1000_weight, our_contributed_weight)); + if funding_satoshis < holder_dust_limit_satoshis { + Ok(0) + } else { + Ok(funding_satoshis) + } +} + /// Estimate our part of the fee of the new funding transaction. /// input_count: Number of contributed inputs. /// witness_weight: The witness weight for contributed inputs. @@ -5101,7 +5136,7 @@ pub(super) struct DualFundingChannelContext { /// /// Note that this field may be emptied once the interactive negotiation has been started. #[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled. - pub our_funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>, + pub our_funding_inputs: Vec<(TxIn, TransactionU16LenLimited, Weight)>, } // Holder designates channel data owned for the benefit of the user client. @@ -10290,7 +10325,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { pub fn new_outbound( fee_estimator: &LowerBoundedFeeEstimator, entropy_source: &ES, signer_provider: &SP, counterparty_node_id: PublicKey, their_features: &InitFeatures, funding_satoshis: u64, - funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>, user_id: u128, config: &UserConfig, + funding_inputs: Vec<(TxIn, TransactionU16LenLimited, Weight)>, user_id: u128, config: &UserConfig, current_chain_height: u32, outbound_scid_alias: u64, funding_confirmation_target: ConfirmationTarget, logger: L, ) -> Result @@ -10430,16 +10465,20 @@ impl PendingV2Channel where SP::Target: SignerProvider { pub fn new_inbound( fee_estimator: &LowerBoundedFeeEstimator, entropy_source: &ES, signer_provider: &SP, holder_node_id: PublicKey, counterparty_node_id: PublicKey, our_supported_features: &ChannelTypeFeatures, - their_features: &InitFeatures, msg: &msgs::OpenChannelV2, - user_id: u128, config: &UserConfig, current_chain_height: u32, logger: &L, + their_features: &InitFeatures, msg: &msgs::OpenChannelV2, user_id: u128, config: &UserConfig, + current_chain_height: u32, logger: &L, our_funding_inputs: Vec<(TxIn, TransactionU16LenLimited, Weight)>, ) -> Result where ES::Target: EntropySource, F::Target: FeeEstimator, L::Target: Logger, { - // TODO(dual_funding): Take these as input once supported - let our_funding_satoshis = 0u64; - let our_funding_inputs = Vec::new(); + let our_funding_satoshis = calculate_our_funding_satoshis( + false, &our_funding_inputs, &[], msg.funding_feerate_sat_per_1000_weight, + msg.common_fields.dust_limit_satoshis, + ).map_err(|_| ChannelError::Close(( + "Failed to accept channel".to_string(), + ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) } + )))?; let channel_value_satoshis = our_funding_satoshis.saturating_add(msg.common_fields.funding_satoshis); let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 92cf6d9826f..4e65b198746 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -30,9 +30,7 @@ use bitcoin::hash_types::{BlockHash, Txid}; use bitcoin::secp256k1::{SecretKey,PublicKey}; use bitcoin::secp256k1::Secp256k1; -use bitcoin::{secp256k1, Sequence}; -#[cfg(splicing)] -use bitcoin::{TxIn, Weight}; +use bitcoin::{secp256k1, Sequence, TxIn, Weight}; use crate::events::FundingInfo; use crate::blinded_path::message::{AsyncPaymentsContext, MessageContext, OffersContext}; @@ -84,7 +82,7 @@ use crate::util::config::{ChannelConfig, ChannelConfigUpdate, ChannelConfigOverr use crate::util::wakers::{Future, Notifier}; use crate::util::scid_utils::fake_scid; use crate::util::string::UntrustedString; -use crate::util::ser::{BigSize, FixedLengthReader, LengthReadable, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter}; +use crate::util::ser::{BigSize, FixedLengthReader, LengthReadable, MaybeReadable, Readable, ReadableArgs, TransactionU16LenLimited, VecWriter, Writeable, Writer}; use crate::util::logger::{Level, Logger, WithContext}; use crate::util::errors::APIError; #[cfg(async_payments)] use { @@ -7875,7 +7873,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ /// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest /// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id pub fn accept_inbound_channel(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, user_channel_id: u128, config_overrides: Option) -> Result<(), APIError> { - self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id, config_overrides) + self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id, config_overrides, vec![]) } /// Accepts a request to open a channel after a [`events::Event::OpenChannelRequest`], treating @@ -7897,13 +7895,61 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ /// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest /// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id pub fn accept_inbound_channel_from_trusted_peer_0conf(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, user_channel_id: u128, config_overrides: Option) -> Result<(), APIError> { - self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, true, user_channel_id, config_overrides) + self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, true, user_channel_id, config_overrides, vec![]) + } + + /// Accepts a request to open a dual-funded channel with a contribution provided by us after an + /// [`Event::OpenChannelRequest`]. + /// + /// The `temporary_channel_id` parameter indicates which inbound channel should be accepted, + /// and the `counterparty_node_id` parameter is the id of the peer which has requested to open + /// the channel. + /// + /// The `user_channel_id` parameter will be provided back in + /// [`Event::ChannelClosed::user_channel_id`] to allow tracking of which events correspond + /// with which `accept_inbound_channel_*` call. + /// + /// The `funding_inputs` parameter provides the `txin`s along with their previous transactions, and + /// a corresponding witness weight for each input that will be used to contribute towards our + /// portion of the channel value. Our contribution will be calculated as the total value of these + /// inputs minus the fees we need to cover for the interactive funding transaction. The witness + /// weights must correspond to the witnesses you will provide through [`ChannelManager::funding_transaction_signed`] + /// after receiving [`Event::FundingTransactionReadyForSigning`]. + /// + /// Note that this method will return an error and reject the channel if it requires support for + /// zero confirmations. + // TODO(dual_funding): Discussion on complications with 0conf dual-funded channels where "locking" + // of UTXOs used for funding would be required and other issues. + // See https://diyhpl.us/~bryan/irc/bitcoin/bitcoin-dev/linuxfoundation-pipermail/lightning-dev/2023-May/003922.txt + /// + /// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest + /// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id + /// [`Event::FundingTransactionReadyForSigning`]: events::Event::FundingTransactionReadyForSigning + /// [`ChannelManager::funding_transaction_signed`]: ChannelManager::funding_transaction_signed + pub fn accept_inbound_channel_with_contribution( + &self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, user_channel_id: u128, + config_overrides: Option, funding_inputs: Vec<(TxIn, Transaction, Weight)> + ) -> Result<(), APIError> { + let funding_inputs = Self::length_limit_holder_input_prev_txs(funding_inputs)?; + self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id, + config_overrides, funding_inputs) + } + + fn length_limit_holder_input_prev_txs(funding_inputs: Vec<(TxIn, Transaction, Weight)>) -> Result, APIError> { + funding_inputs.into_iter().map(|(txin, tx, witness_weight)| { + match TransactionU16LenLimited::new(tx) { + Ok(tx) => Ok((txin, tx, witness_weight)), + Err(err) => Err(err) + } + }).collect::, ()>>() + .map_err(|_| APIError::APIMisuseError { err: "One or more transactions had a serialized length exceeding 65535 bytes".into() }) } /// TODO(dual_funding): Allow contributions, pass intended amount and inputs fn do_accept_inbound_channel( &self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, accept_0conf: bool, - user_channel_id: u128, config_overrides: Option + user_channel_id: u128, config_overrides: Option, + funding_inputs: Vec<(TxIn, TransactionU16LenLimited, Weight)> ) -> Result<(), APIError> { let mut config = self.default_configuration.clone(); @@ -7962,7 +8008,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.channel_type_features(), &peer_state.latest_features, &open_channel_msg, user_channel_id, &config, best_block_height, - &self.logger, + &self.logger, funding_inputs, ).map_err(|_| MsgHandleErrInternal::from_chan_no_close( ChannelError::Close( ( @@ -8243,7 +8289,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.fee_estimator, &self.entropy_source, &self.signer_provider, self.get_our_node_id(), *counterparty_node_id, &self.channel_type_features(), &peer_state.latest_features, msg, user_channel_id, - &self.default_configuration, best_block_height, &self.logger, + &self.default_configuration, best_block_height, &self.logger, vec![], ).map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.common_fields.temporary_channel_id))?; let message_send_event = MessageSendEvent::SendAcceptChannelV2 { node_id: *counterparty_node_id, diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index 6a7ef317ba8..1242e40ba9e 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -51,7 +51,7 @@ fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession) &[session.initiator_input_value_satoshis], ) .into_iter() - .map(|(txin, tx, _)| (txin, TransactionU16LenLimited::new(tx).unwrap())) + .map(|(txin, tx, weight)| (txin, TransactionU16LenLimited::new(tx).unwrap(), weight)) .collect(); // Alice creates a dual-funded channel as initiator. diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 1736d911a0b..cc2818850a1 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -401,7 +401,8 @@ impl InteractiveTxSigningSession { if local_inputs_count != witnesses.len() { return Err(format!( "Provided witness count of {} does not match required count for {} inputs", - witnesses.len(), local_inputs_count + witnesses.len(), + local_inputs_count )); } @@ -508,6 +509,12 @@ pub(crate) fn estimate_input_weight(prev_output: &TxOut) -> Weight { }) } +pub(crate) fn get_input_weight(witness_weight: Weight) -> Weight { + Weight::from_wu( + (BASE_INPUT_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT).saturating_add(witness_weight.to_wu()), + ) +} + pub(crate) fn get_output_weight(script_pubkey: &ScriptBuf) -> Weight { Weight::from_wu( (8 /* value */ + script_pubkey.consensus_encode(&mut sink()).unwrap() as u64) @@ -1534,7 +1541,7 @@ where pub feerate_sat_per_kw: u32, pub is_initiator: bool, pub funding_tx_locktime: AbsoluteLockTime, - pub inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited)>, + pub inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited, Weight)>, pub outputs_to_contribute: Vec, pub expected_remote_shared_funding_output: Option<(ScriptBuf, u64)>, } @@ -1607,7 +1614,7 @@ impl InteractiveTxConstructor { let mut inputs_to_contribute: Vec<(SerialId, TxIn, TransactionU16LenLimited)> = inputs_to_contribute .into_iter() - .map(|(input, tx)| { + .map(|(input, tx, _weight)| { let serial_id = generate_holder_serial_id(entropy_source, is_initiator); (serial_id, input, tx) }) @@ -1751,14 +1758,15 @@ impl InteractiveTxConstructor { #[allow(dead_code)] // TODO(dual_funding): Remove once begin_interactive_funding_tx_construction() is used pub(super) fn calculate_change_output_value( is_initiator: bool, our_contribution: u64, - funding_inputs: &Vec<(TxIn, TransactionU16LenLimited)>, funding_outputs: &Vec, - funding_feerate_sat_per_1000_weight: u32, change_output_dust_limit: u64, + funding_inputs: &Vec<(TxIn, TransactionU16LenLimited, Weight)>, + funding_outputs: &Vec, funding_feerate_sat_per_1000_weight: u32, + change_output_dust_limit: u64, ) -> Result, AbortReason> { // Process inputs and their prev txs: // calculate value sum and weight sum of inputs, also perform checks let mut total_input_satoshis = 0u64; let mut our_funding_inputs_weight = 0u64; - for (txin, tx) in funding_inputs.iter() { + for (txin, tx, witness_weight) in funding_inputs.iter() { let txid = tx.as_transaction().compute_txid(); if txin.previous_output.txid != txid { return Err(AbortReason::PrevTxOutInvalid); @@ -1766,7 +1774,7 @@ pub(super) fn calculate_change_output_value( if let Some(output) = tx.as_transaction().output.get(txin.previous_output.vout as usize) { total_input_satoshis = total_input_satoshis.saturating_add(output.value.to_sat()); our_funding_inputs_weight = - our_funding_inputs_weight.saturating_add(estimate_input_weight(output).to_wu()); + our_funding_inputs_weight.saturating_add(get_input_weight(*witness_weight).to_wu()); } else { return Err(AbortReason::PrevTxOutInvalid); } @@ -1816,12 +1824,14 @@ mod tests { use crate::util::ser::TransactionU16LenLimited; use bitcoin::absolute::LockTime as AbsoluteLockTime; use bitcoin::amount::Amount; + use bitcoin::ecdsa::Signature; use bitcoin::hashes::Hash; + use bitcoin::hex::FromHex as _; use bitcoin::key::UntweakedPublicKey; - use bitcoin::opcodes; use bitcoin::script::Builder; use bitcoin::secp256k1::{Keypair, PublicKey, Secp256k1, SecretKey}; use bitcoin::transaction::Version; + use bitcoin::{opcodes, Weight}; use bitcoin::{ OutPoint, PubkeyHash, ScriptBuf, Sequence, Transaction, TxIn, TxOut, WPubkeyHash, Witness, }; @@ -1877,9 +1887,9 @@ mod tests { struct TestSession { description: &'static str, - inputs_a: Vec<(TxIn, TransactionU16LenLimited)>, + inputs_a: Vec<(TxIn, TransactionU16LenLimited, Weight)>, outputs_a: Vec, - inputs_b: Vec<(TxIn, TransactionU16LenLimited)>, + inputs_b: Vec<(TxIn, TransactionU16LenLimited, Weight)>, outputs_b: Vec, expect_error: Option<(AbortReason, ErrorCulprit)>, /// A node adds no shared output, but expects the peer to add one, with the specific script pubkey, and local contribution @@ -2152,7 +2162,7 @@ mod tests { } } - fn generate_inputs(outputs: &[TestOutput]) -> Vec<(TxIn, TransactionU16LenLimited)> { + fn generate_inputs(outputs: &[TestOutput]) -> Vec<(TxIn, TransactionU16LenLimited, Weight)> { let tx = generate_tx(outputs); let txid = tx.compute_txid(); tx.output @@ -2163,9 +2173,16 @@ mod tests { previous_output: OutPoint { txid, vout: idx as u32 }, script_sig: Default::default(), sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Default::default(), + witness: Witness::p2wpkh( + &Signature::sighash_all( + bitcoin::secp256k1::ecdsa::Signature::from_der(&>::from_hex("3044022008f4f37e2d8f74e18c1b8fde2374d5f28402fb8ab7fd1cc5b786aa40851a70cb022032b1374d1a0f125eae4f69d1bc0b7f896c964cfdba329f38a952426cf427484c").unwrap()[..]).unwrap() + ) + .into(), + &PublicKey::from_slice(&[2; 33]).unwrap(), + ), }; - (input, TransactionU16LenLimited::new(tx.clone()).unwrap()) + let witness_weight = Weight::from_wu_usize(input.witness.size()); + (input, TransactionU16LenLimited::new(tx.clone()).unwrap(), witness_weight) }) .collect() } @@ -2215,12 +2232,15 @@ mod tests { vec![generate_shared_funding_output_one(value, local_value)] } - fn generate_fixed_number_of_inputs(count: u16) -> Vec<(TxIn, TransactionU16LenLimited)> { + fn generate_fixed_number_of_inputs( + count: u16, + ) -> Vec<(TxIn, TransactionU16LenLimited, Weight)> { // Generate transactions with a total `count` number of outputs such that no transaction has a // serialized length greater than u16::MAX. let max_outputs_per_prevtx = 1_500; let mut remaining = count; - let mut inputs: Vec<(TxIn, TransactionU16LenLimited)> = Vec::with_capacity(count as usize); + let mut inputs: Vec<(TxIn, TransactionU16LenLimited, Weight)> = + Vec::with_capacity(count as usize); while remaining > 0 { let tx_output_count = remaining.min(max_outputs_per_prevtx); @@ -2233,7 +2253,7 @@ mod tests { ); let txid = tx.compute_txid(); - let mut temp: Vec<(TxIn, TransactionU16LenLimited)> = tx + let mut temp: Vec<(TxIn, TransactionU16LenLimited, Weight)> = tx .output .iter() .enumerate() @@ -2242,9 +2262,16 @@ mod tests { previous_output: OutPoint { txid, vout: idx as u32 }, script_sig: Default::default(), sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Default::default(), + witness: Witness::p2wpkh( + &Signature::sighash_all( + bitcoin::secp256k1::ecdsa::Signature::from_der(&>::from_hex("3044022008f4f37e2d8f74e18c1b8fde2374d5f28402fb8ab7fd1cc5b786aa40851a70cb022032b1374d1a0f125eae4f69d1bc0b7f896c964cfdba329f38a952426cf427484c").unwrap()[..]).unwrap() + ) + .into(), + &PublicKey::from_slice(&[2; 33]).unwrap(), + ), }; - (input, TransactionU16LenLimited::new(tx.clone()).unwrap()) + let witness_weight = Weight::from_wu_usize(input.witness.size()); + (input, TransactionU16LenLimited::new(tx.clone()).unwrap(), witness_weight) }) .collect(); @@ -2438,9 +2465,15 @@ mod tests { previous_output: OutPoint { txid: tx.as_transaction().compute_txid(), vout: 0 }, ..Default::default() }; + let invalid_sequence_input_witness_weight = + Weight::from_wu_usize(invalid_sequence_input.witness.size()); do_test_interactive_tx_constructor(TestSession { description: "Invalid input sequence from initiator", - inputs_a: vec![(invalid_sequence_input, tx.clone())], + inputs_a: vec![( + invalid_sequence_input, + tx.clone(), + invalid_sequence_input_witness_weight, + )], outputs_a: generate_output(&TestOutput::P2WPKH(1_000_000)), inputs_b: vec![], outputs_b: vec![], @@ -2453,9 +2486,13 @@ mod tests { sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, ..Default::default() }; + let duplicate_input_witness_weight = Weight::from_wu_usize(duplicate_input.witness.size()); do_test_interactive_tx_constructor(TestSession { description: "Duplicate prevout from initiator", - inputs_a: vec![(duplicate_input.clone(), tx.clone()), (duplicate_input, tx.clone())], + inputs_a: vec![ + (duplicate_input.clone(), tx.clone(), duplicate_input_witness_weight), + (duplicate_input, tx.clone(), duplicate_input_witness_weight), + ], outputs_a: generate_output(&TestOutput::P2WPKH(1_000_000)), inputs_b: vec![], outputs_b: vec![], @@ -2469,11 +2506,12 @@ mod tests { sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, ..Default::default() }; + let duplicate_input_witness_weight = Weight::from_wu_usize(duplicate_input.witness.size()); do_test_interactive_tx_constructor(TestSession { description: "Non-initiator uses same prevout as initiator", - inputs_a: vec![(duplicate_input.clone(), tx.clone())], + inputs_a: vec![(duplicate_input.clone(), tx.clone(), duplicate_input_witness_weight)], outputs_a: generate_shared_funding_output(1_000_000, 905_000), - inputs_b: vec![(duplicate_input.clone(), tx.clone())], + inputs_b: vec![(duplicate_input.clone(), tx.clone(), duplicate_input_witness_weight)], outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)), a_expected_remote_shared_output: None, @@ -2484,11 +2522,12 @@ mod tests { sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, ..Default::default() }; + let duplicate_input_witness_weight = Weight::from_wu_usize(duplicate_input.witness.size()); do_test_interactive_tx_constructor(TestSession { description: "Non-initiator uses same prevout as initiator", - inputs_a: vec![(duplicate_input.clone(), tx.clone())], + inputs_a: vec![(duplicate_input.clone(), tx.clone(), duplicate_input_witness_weight)], outputs_a: generate_output(&TestOutput::P2WPKH(1_000_000)), - inputs_b: vec![(duplicate_input.clone(), tx.clone())], + inputs_b: vec![(duplicate_input.clone(), tx.clone(), duplicate_input_witness_weight)], outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)), a_expected_remote_shared_output: None, @@ -2756,11 +2795,18 @@ mod tests { previous_output: OutPoint { txid, vout: 0 }, script_sig: ScriptBuf::new(), sequence: Sequence::ZERO, - witness: Witness::new(), + witness: Witness::p2wpkh( + &Signature::sighash_all( + bitcoin::secp256k1::ecdsa::Signature::from_der(&>::from_hex("3044022008f4f37e2d8f74e18c1b8fde2374d5f28402fb8ab7fd1cc5b786aa40851a70cb022032b1374d1a0f125eae4f69d1bc0b7f896c964cfdba329f38a952426cf427484c").unwrap()[..]).unwrap() + ) + .into(), + &PublicKey::from_slice(&[2; 33]).unwrap(), + ), }; - (txin, TransactionU16LenLimited::new(tx).unwrap()) + let witness_weight = Weight::from_wu_usize(txin.witness.size()); + (txin, TransactionU16LenLimited::new(tx).unwrap(), witness_weight) }) - .collect::>(); + .collect::>(); let our_contributed = 110_000; let txout = TxOut { value: Amount::from_sat(128_000), script_pubkey: ScriptBuf::new() }; let value = txout.value.to_sat();