From a07eb8546e6a9598b6d4b669b9fc7b54da3bb71b Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 21 Aug 2025 08:54:53 -0700 Subject: [PATCH 1/6] Only sign funding transaction on monitor update resumption The `handle_channel_resumption` path is reachable from both channel reestablish and monitor update completion. Since we only want to sign once we know the monitor update has completed, it's possible we could have unintentionally attempted to sign if we were still pending the monitor update but had a channel reestablish occur. --- lightning/src/ln/channelmanager.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e34567d7a10..2d7031cc2b2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9006,10 +9006,13 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } - if let Some(signing_session) = &mut channel.interactive_tx_signing_session { - if signing_session.local_inputs_count() > 0 - && signing_session.holder_tx_signatures().is_none() - { + if let Some(signing_session) = (!channel.is_awaiting_monitor_update()) + .then(|| ()) + .and_then(|_| channel.interactive_tx_signing_session.as_mut()) + .filter(|signing_session| signing_session.holder_tx_signatures().is_none()) + { + let local_inputs_count = signing_session.local_inputs_count(); + if local_inputs_count > 0 { let mut pending_events = self.pending_events.lock().unwrap(); let unsigned_transaction = signing_session.unsigned_tx().build_unsigned_tx(); let event_action = ( @@ -9027,7 +9030,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } else { pending_events.push_back(event_action); } - } else if signing_session.local_inputs_count() == 0 && signing_session.holder_tx_signatures().is_none() { + } else { + let txid = signing_session.unsigned_tx().compute_txid(); match channel.funding_transaction_signed(vec![]) { Ok((Some(tx_signatures), funding_tx_opt)) => { if let Some(funding_tx) = funding_tx_opt { From dfd7779b329a91d420224e06a1e4d7ff30937751 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 21 Aug 2025 08:55:28 -0700 Subject: [PATCH 2/6] Remove reachable FundingTransactionReadyForSigning assertion This is reachable if the event doesn't get handled and a channel reestablish occurs. --- lightning/src/ln/channelmanager.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2d7031cc2b2..68369659ee3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9025,9 +9025,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ None, ); - if pending_events.contains(&event_action) { - debug_assert!(false, "FundingTransactionReadyForSigning should not have been queued already"); - } else { + if !pending_events.contains(&event_action) { pending_events.push_back(event_action); } } else { From bbe590d3e8498914c6c61de25210e0d77fbe5dd4 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 21 Aug 2025 08:56:05 -0700 Subject: [PATCH 3/6] Fix empty witness filter in funding_transaction_signed --- lightning/src/ln/channelmanager.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 68369659ee3..6db55a58975 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5973,7 +5973,7 @@ where .input .into_iter() .map(|input| input.witness) - .filter(|witness| witness.is_empty()) + .filter(|witness| !witness.is_empty()) .collect(); match chan.funding_transaction_signed(witnesses) { Ok((Some(tx_signatures), funding_tx_opt)) => { From 712b3857bb40af71113923c50fa9296ff87e4b2f Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 21 Aug 2025 09:04:34 -0700 Subject: [PATCH 4/6] Assume splicing input value from channel parameters --- lightning/src/sign/ecdsa.rs | 8 +++++--- lightning/src/sign/mod.rs | 12 ++++++++++-- lightning/src/util/dyn_signer.rs | 3 +-- lightning/src/util/test_channel_signer.rs | 10 ++-------- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/lightning/src/sign/ecdsa.rs b/lightning/src/sign/ecdsa.rs index f9c330bbc4c..a25d5df2761 100644 --- a/lightning/src/sign/ecdsa.rs +++ b/lightning/src/sign/ecdsa.rs @@ -247,15 +247,17 @@ pub trait EcdsaChannelSigner: ChannelSigner { /// In splicing, the previous funding transaction output is spent as the input of /// the new funding transaction, and is a 2-of-2 multisig. /// + /// `channel_parameters`: The [`ChannelTransactionParameters`] for the channel's current funding + /// transaction that is being spent in the splice transaction to sign. A new set of + /// [`ChannelTransactionParameters`] will become available for the new funding transaction. + /// /// `input_index`: The index of the input within the new funding transaction `tx`, /// spending the previous funding transaction's output /// - /// `input_value`: The value of the previous funding transaction output. - /// /// This method is *not* asynchronous. If an `Err` is returned, the channel will be immediately /// closed. fn sign_splicing_funding_input( &self, channel_parameters: &ChannelTransactionParameters, tx: &Transaction, - input_index: usize, input_value: u64, secp_ctx: &Secp256k1, + input_index: usize, secp_ctx: &Secp256k1, ) -> Result; } diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 057a7406a5a..abdb03c0e8b 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -1755,9 +1755,17 @@ impl EcdsaChannelSigner for InMemorySigner { fn sign_splicing_funding_input( &self, channel_parameters: &ChannelTransactionParameters, tx: &Transaction, - input_index: usize, input_value: u64, secp_ctx: &Secp256k1, + input_index: usize, secp_ctx: &Secp256k1, ) -> Result { assert!(channel_parameters.is_populated(), "Channel parameters must be fully populated"); + assert_eq!( + tx.input[input_index].previous_output, + channel_parameters + .funding_outpoint + .as_ref() + .expect("Funding outpoint must be known prior to signing") + .into_bitcoin_outpoint() + ); let funding_key = self.funding_key(channel_parameters.splice_parent_funding_txid); let funding_pubkey = funding_key.public_key(secp_ctx); @@ -1769,7 +1777,7 @@ impl EcdsaChannelSigner for InMemorySigner { .p2wsh_signature_hash( input_index, &funding_redeemscript, - Amount::from_sat(input_value), + Amount::from_sat(channel_parameters.channel_value_satoshis), EcdsaSighashType::All, ) .unwrap()[..]; diff --git a/lightning/src/util/dyn_signer.rs b/lightning/src/util/dyn_signer.rs index fc2f6329aeb..baa4eeda7df 100644 --- a/lightning/src/util/dyn_signer.rs +++ b/lightning/src/util/dyn_signer.rs @@ -160,8 +160,7 @@ delegate!(DynSigner, EcdsaChannelSigner, inner, fn sign_holder_htlc_transaction(, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1) -> Result, fn sign_splicing_funding_input(, channel_parameters: &ChannelTransactionParameters, - tx: &Transaction, input_index: usize, input_value: u64, - secp_ctx: &Secp256k1) -> Result + tx: &Transaction, input_index: usize, secp_ctx: &Secp256k1) -> Result ); delegate!(DynSigner, ChannelSigner, diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index 04b24825bd4..7e89650a480 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -486,15 +486,9 @@ impl EcdsaChannelSigner for TestChannelSigner { fn sign_splicing_funding_input( &self, channel_parameters: &ChannelTransactionParameters, tx: &Transaction, - input_index: usize, input_value: u64, secp_ctx: &Secp256k1, + input_index: usize, secp_ctx: &Secp256k1, ) -> Result { - self.inner.sign_splicing_funding_input( - channel_parameters, - tx, - input_index, - input_value, - secp_ctx, - ) + self.inner.sign_splicing_funding_input(channel_parameters, tx, input_index, secp_ctx) } } From 1170e1ba5b01b6178973986ef49cc5f31dbd48ad Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 21 Aug 2025 09:39:29 -0700 Subject: [PATCH 5/6] Support splice shared input signing This commit tracks all data related to the shared input of a splice, such that a valid witness can be formed upon the splice transaction finalization. --- lightning/src/ln/channel.rs | 46 ++++-- lightning/src/ln/channelmanager.rs | 7 +- lightning/src/ln/interactivetxs.rs | 243 +++++++++++++++++++++-------- 3 files changed, 218 insertions(+), 78 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 924e101c30c..9c274057f6b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2192,6 +2192,11 @@ impl FundingScope { self.channel_transaction_parameters.make_funding_redeemscript() } + #[cfg(splicing)] + fn holder_funding_pubkey(&self) -> &PublicKey { + &self.get_holder_pubkeys().funding_pubkey + } + fn counterparty_funding_pubkey(&self) -> &PublicKey { &self.get_counterparty_pubkeys().funding_pubkey } @@ -2335,8 +2340,16 @@ impl FundingScope { }; let local_owned = self.value_to_self_msat / 1000; - - SharedOwnedInput::new(input, prev_output, local_owned) + let holder_sig_first = self.holder_funding_pubkey().serialize()[..] + < self.counterparty_funding_pubkey().serialize()[..]; + + SharedOwnedInput::new( + input, + prev_output, + local_owned, + holder_sig_first, + self.get_funding_redeemscript(), + ) } } @@ -7969,9 +7982,9 @@ where } pub fn funding_transaction_signed( - &mut self, witnesses: Vec, + &mut self, funding_txid_signed: Txid, witnesses: Vec, ) -> Result<(Option, Option), APIError> { - let (funding_tx_opt, tx_signatures_opt) = self + let (tx_signatures_opt, funding_tx_opt) = self .interactive_tx_signing_session .as_mut() .ok_or_else(|| APIError::APIMisuseError { @@ -7981,12 +7994,21 @@ where ), }) .and_then(|signing_session| { + let tx = signing_session.unsigned_tx().build_unsigned_tx(); + if funding_txid_signed != tx.compute_txid() { + return Err(APIError::APIMisuseError { + err: "Transaction was malleated prior to signing".to_owned(), + }); + } + + let tx_signatures = msgs::TxSignatures { + channel_id: self.context.channel_id, + tx_hash: funding_txid_signed, + witnesses, + shared_input_signature: None, + }; signing_session - .provide_holder_witnesses( - &self.context.secp_ctx, - self.context.channel_id, - witnesses, - ) + .provide_holder_witnesses(tx_signatures, &self.context.secp_ctx) .map_err(|err| APIError::APIMisuseError { err }) })?; @@ -8004,7 +8026,7 @@ where } #[rustfmt::skip] - pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures) -> Result<(Option, Option), ChannelError> { + pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures) -> Result<(Option, Option), ChannelError> { if !self.context.channel_state.is_interactive_signing() || self.context.channel_state.is_their_tx_signatures_sent() { @@ -8035,7 +8057,7 @@ where } } - let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg.clone()) + let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg) .map_err(|msg| ChannelError::Warn(msg))?; // Set `THEIR_TX_SIGNATURES_SENT` flag after all potential errors. @@ -8050,7 +8072,7 @@ where self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); } - Ok((funding_tx_opt, holder_tx_signatures_opt)) + Ok((holder_tx_signatures_opt, funding_tx_opt)) } else { let msg = "Unexpected tx_signatures. No funding transaction awaiting signatures"; let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6db55a58975..28f770b1386 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5969,13 +5969,14 @@ where match peer_state.channel_by_id.get_mut(channel_id) { Some(channel) => match channel.as_funded_mut() { Some(chan) => { + let txid = transaction.compute_txid(); let witnesses: Vec<_> = transaction .input .into_iter() .map(|input| input.witness) .filter(|witness| !witness.is_empty()) .collect(); - match chan.funding_transaction_signed(witnesses) { + match chan.funding_transaction_signed(txid, witnesses) { Ok((Some(tx_signatures), funding_tx_opt)) => { if let Some(funding_tx) = funding_tx_opt { self.broadcast_interactive_funding(chan, &funding_tx); @@ -9030,7 +9031,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } else { let txid = signing_session.unsigned_tx().compute_txid(); - match channel.funding_transaction_signed(vec![]) { + match channel.funding_transaction_signed(txid, vec![]) { Ok((Some(tx_signatures), funding_tx_opt)) => { if let Some(funding_tx) = funding_tx_opt { self.broadcast_interactive_funding(channel, &funding_tx); @@ -10030,7 +10031,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ hash_map::Entry::Occupied(mut chan_entry) => { match chan_entry.get_mut().as_funded_mut() { Some(chan) => { - let (funding_tx_opt, tx_signatures_opt) = try_channel_entry!(self, peer_state, chan.tx_signatures(msg), chan_entry); + let (tx_signatures_opt, funding_tx_opt) = try_channel_entry!(self, peer_state, chan.tx_signatures(msg), chan_entry); if let Some(tx_signatures) = tx_signatures_opt { peer_state.pending_msg_events.push(MessageSendEvent::SendTxSignatures { node_id: *counterparty_node_id, diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index b3b3d6df112..468b1fd7a5a 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -14,8 +14,10 @@ use bitcoin::absolute::LockTime as AbsoluteLockTime; use bitcoin::amount::Amount; use bitcoin::consensus::Encodable; use bitcoin::constants::WITNESS_SCALE_FACTOR; +use bitcoin::ecdsa::Signature as BitcoinSignature; use bitcoin::key::Secp256k1; use bitcoin::policy::MAX_STANDARD_TX_WEIGHT; +use bitcoin::secp256k1::ecdsa::Signature; use bitcoin::secp256k1::{Message, PublicKey}; use bitcoin::sighash::SighashCache; use bitcoin::transaction::Version; @@ -206,7 +208,7 @@ pub(crate) struct ConstructedTransaction { remote_outputs_value_satoshis: u64, lock_time: AbsoluteLockTime, - holder_sends_tx_signatures_first: bool, + shared_input_index: Option, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -244,7 +246,7 @@ impl_writeable_tlv_based!(ConstructedTransaction, { (11, remote_inputs_value_satoshis, required), (13, remote_outputs_value_satoshis, required), (15, lock_time, required), - (17, holder_sends_tx_signatures_first, required), + (17, shared_input_index, option), }); impl ConstructedTransaction { @@ -279,20 +281,18 @@ impl ConstructedTransaction { let mut inputs: Vec = context.inputs.into_values().map(|tx_input| tx_input.into_negotiated_input()).collect(); let mut outputs: Vec = context.outputs.into_values().collect(); - // Inputs and outputs must be sorted by serial_id inputs.sort_unstable_by_key(|input| input.serial_id); outputs.sort_unstable_by_key(|output| output.serial_id); - // There is a strict ordering for `tx_signatures` exchange to prevent deadlocks. - let holder_sends_tx_signatures_first = - if local_inputs_value_satoshis == remote_inputs_value_satoshis { - // If the amounts are the same then the peer with the lowest pubkey lexicographically sends its - // tx_signatures first - context.holder_node_id.serialize() < context.counterparty_node_id.serialize() - } else { - // Otherwise the peer with the lowest contributed input value sends its tx_signatures first. - local_inputs_value_satoshis < remote_inputs_value_satoshis - }; + let shared_input_index = + context.shared_funding_input.as_ref().and_then(|shared_funding_input| { + inputs + .iter() + .position(|input| { + input.txin.previous_output == shared_funding_input.input.previous_output + }) + .map(|position| position as u32) + }); let constructed_tx = Self { holder_is_initiator: context.holder_is_initiator, @@ -307,7 +307,7 @@ impl ConstructedTransaction { outputs, lock_time: context.tx_locktime, - holder_sends_tx_signatures_first, + shared_input_index, }; if constructed_tx.weight().to_wu() > MAX_STANDARD_TX_WEIGHT as u64 { @@ -357,10 +357,14 @@ impl ConstructedTransaction { fn add_local_witnesses(&mut self, witnesses: Vec) { self.inputs .iter_mut() - .filter(|input| { - !is_serial_id_valid_for_counterparty(self.holder_is_initiator, input.serial_id) + .enumerate() + .filter(|(_, input)| input.is_local(self.holder_is_initiator)) + .filter(|(index, _)| { + self.shared_input_index + .map(|shared_index| *index != shared_index as usize) + .unwrap_or(true) }) - .map(|input| &mut input.txin) + .map(|(_, input)| &mut input.txin) .zip(witnesses) .for_each(|(input, witness)| input.witness = witness); } @@ -371,10 +375,14 @@ impl ConstructedTransaction { fn add_remote_witnesses(&mut self, witnesses: Vec) { self.inputs .iter_mut() - .filter(|input| { - is_serial_id_valid_for_counterparty(self.holder_is_initiator, input.serial_id) + .enumerate() + .filter(|(_, input)| !input.is_local(self.holder_is_initiator)) + .filter(|(index, _)| { + self.shared_input_index + .map(|shared_index| *index != shared_index as usize) + .unwrap_or(true) }) - .map(|input| &mut input.txin) + .map(|(_, input)| &mut input.txin) .zip(witnesses) .for_each(|(input, witness)| input.witness = witness); } @@ -382,8 +390,25 @@ impl ConstructedTransaction { pub fn holder_is_initiator(&self) -> bool { self.holder_is_initiator } + + pub fn shared_input_index(&self) -> Option { + self.shared_input_index + } +} + +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct SharedInputSignature { + holder_signature_first: bool, + witness_script: ScriptBuf, + counterparty_signature: Option, } +impl_writeable_tlv_based!(SharedInputSignature, { + (1, holder_signature_first, required), + (3, witness_script, required), + (5, counterparty_signature, required), +}); + /// The InteractiveTxSigningSession coordinates the signing flow of interactively constructed /// transactions from exhange of `commitment_signed` to ensuring proper ordering of `tx_signature` /// message exchange. @@ -397,6 +422,7 @@ pub(crate) struct InteractiveTxSigningSession { holder_sends_tx_signatures_first: bool, has_received_commitment_signed: bool, has_received_tx_signatures: bool, + shared_input_signature: Option, holder_tx_signatures: Option, } @@ -433,7 +459,7 @@ impl InteractiveTxSigningSession { /// Returns an error if the witness count does not equal the counterparty's input count in the /// unsigned transaction or if the counterparty already provided their `tx_signatures`. pub fn received_tx_signatures( - &mut self, tx_signatures: TxSignatures, + &mut self, tx_signatures: &TxSignatures, ) -> Result<(Option, Option), String> { if self.has_received_tx_signatures { return Err("Already received a tx_signatures message".to_string()); @@ -441,7 +467,17 @@ impl InteractiveTxSigningSession { if self.remote_inputs_count() != tx_signatures.witnesses.len() { return Err("Witness count did not match contributed input count".to_string()); } + if self.shared_input().is_some() && tx_signatures.shared_input_signature.is_none() { + return Err("Missing shared input signature".to_string()); + } + if self.shared_input().is_none() && tx_signatures.shared_input_signature.is_some() { + return Err("Unexpected shared input signature".to_string()); + } + self.unsigned_tx.add_remote_witnesses(tx_signatures.witnesses.clone()); + if let Some(ref mut shared_input_sig) = self.shared_input_signature { + shared_input_sig.counterparty_signature = tx_signatures.shared_input_signature.clone(); + } self.has_received_tx_signatures = true; let holder_tx_signatures = if !self.holder_sends_tx_signatures_first { @@ -466,30 +502,26 @@ impl InteractiveTxSigningSession { /// /// Returns an error if the witness count does not equal the holder's input count in the /// unsigned transaction. - pub fn provide_holder_witnesses( - &mut self, secp_ctx: &Secp256k1, channel_id: ChannelId, - witnesses: Vec, - ) -> Result<(Option, Option), String> { + pub fn provide_holder_witnesses( + &mut self, tx_signatures: TxSignatures, secp_ctx: &Secp256k1, + ) -> Result<(Option, Option), String> { + if self.holder_tx_signatures.is_some() { + return Err("Holder witnesses were already provided".to_string()); + } + let local_inputs_count = self.local_inputs_count(); - if local_inputs_count != witnesses.len() { + if tx_signatures.witnesses.len() != local_inputs_count { return Err(format!( - "Provided witness count of {} does not match required count for {} inputs", - witnesses.len(), + "Provided witness count of {} does not match required count for {} non-shared inputs", + tx_signatures.witnesses.len(), local_inputs_count )); } - if self.holder_tx_signatures.is_some() { - return Err("Holder witnesses were already provided".to_string()); - } - self.verify_interactive_tx_signatures(secp_ctx, &witnesses)?; - self.unsigned_tx.add_local_witnesses(witnesses.clone()); - self.holder_tx_signatures = Some(TxSignatures { - channel_id, - witnesses, - tx_hash: self.unsigned_tx.compute_txid(), - shared_input_signature: None, - }); + self.verify_interactive_tx_signatures(secp_ctx, &tx_signatures.witnesses)?; + + self.unsigned_tx.add_local_witnesses(tx_signatures.witnesses.clone()); + self.holder_tx_signatures = Some(tx_signatures); let funding_tx_opt = self.has_received_tx_signatures.then(|| self.finalize_funding_tx()); let holder_tx_signatures = @@ -497,18 +529,19 @@ impl InteractiveTxSigningSession { debug_assert!(self.has_received_commitment_signed); self.holder_tx_signatures.clone().expect("Holder tx_signatures were just provided") }); - Ok((funding_tx_opt, holder_tx_signatures)) + + Ok((holder_tx_signatures, funding_tx_opt)) } pub fn remote_inputs_count(&self) -> usize { + let shared_index = self.unsigned_tx.shared_input_index.as_ref(); self.unsigned_tx .inputs .iter() - .filter(|input| { - is_serial_id_valid_for_counterparty( - self.unsigned_tx.holder_is_initiator, - input.serial_id, - ) + .enumerate() + .filter(|(_, input)| !input.is_local(self.unsigned_tx.holder_is_initiator)) + .filter(|(index, _)| { + shared_index.map(|shared_index| *index != *shared_index as usize).unwrap_or(true) }) .count() } @@ -517,29 +550,75 @@ impl InteractiveTxSigningSession { self.unsigned_tx .inputs .iter() - .filter(|input| { - !is_serial_id_valid_for_counterparty( - self.unsigned_tx.holder_is_initiator, - input.serial_id, - ) + .enumerate() + .filter(|(_, input)| input.is_local(self.unsigned_tx.holder_is_initiator)) + .filter(|(index, _)| { + self.unsigned_tx + .shared_input_index + .map(|shared_index| *index != shared_index as usize) + .unwrap_or(true) }) .count() } + pub fn shared_input(&self) -> Option<&NegotiatedTxInput> { + self.unsigned_tx + .shared_input_index + .and_then(|shared_input_index| self.unsigned_tx.inputs.get(shared_input_index as usize)) + } + fn finalize_funding_tx(&mut self) -> Transaction { let lock_time = self.unsigned_tx.lock_time; - let ConstructedTransaction { inputs, outputs, .. } = &mut self.unsigned_tx; + let ConstructedTransaction { inputs, outputs, shared_input_index, .. } = + &mut self.unsigned_tx; - Transaction { + let mut tx = Transaction { version: Version::TWO, lock_time, input: inputs.iter().cloned().map(|input| input.txin).collect(), output: outputs.iter().cloned().map(|output| output.into_tx_out()).collect(), + }; + + if let Some(shared_input_index) = shared_input_index { + if let Some(holder_shared_input_sig) = self + .holder_tx_signatures + .as_ref() + .and_then(|holder_tx_sigs| holder_tx_sigs.shared_input_signature) + { + if let Some(ref shared_input_sig) = self.shared_input_signature { + if let Some(counterparty_shared_input_sig) = + shared_input_sig.counterparty_signature + { + let mut witness = Witness::new(); + witness.push(Vec::new()); + let holder_sig = BitcoinSignature::sighash_all(holder_shared_input_sig); + let counterparty_sig = + BitcoinSignature::sighash_all(counterparty_shared_input_sig); + if shared_input_sig.holder_signature_first { + witness.push_ecdsa_signature(&holder_sig); + witness.push_ecdsa_signature(&counterparty_sig); + } else { + witness.push_ecdsa_signature(&counterparty_sig); + witness.push_ecdsa_signature(&holder_sig); + } + witness.push(&shared_input_sig.witness_script); + tx.input[*shared_input_index as usize].witness = witness; + } else { + debug_assert!(false); + } + } else { + debug_assert!(false); + } + } else { + debug_assert!(false); + } } + + tx } - pub fn verify_interactive_tx_signatures( - &self, secp_ctx: &Secp256k1, witnesses: &Vec, + fn verify_interactive_tx_signatures( + &self, secp_ctx: &Secp256k1, witnesses: &Vec, ) -> Result<(), String> { let unsigned_tx = self.unsigned_tx(); let built_tx = unsigned_tx.build_unsigned_tx(); @@ -552,7 +631,13 @@ impl InteractiveTxSigningSession { let script_pubkeys = unsigned_tx .inputs() .enumerate() - .filter(|(_, input)| input.is_local(unsigned_tx.holder_is_initiator())); + .filter(|(_, input)| input.is_local(unsigned_tx.holder_is_initiator())) + .filter(|(index, _)| { + unsigned_tx + .shared_input_index + .map(|shared_index| *index != shared_index as usize) + .unwrap_or(true) + }); for ((input_idx, input), witness) in script_pubkeys.zip(witnesses) { if witness.is_empty() { @@ -696,10 +781,11 @@ 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), - (9, has_received_tx_signatures, required), + (3, has_received_commitment_signed, required), + (5, holder_tx_signatures, required), + (7, has_received_tx_signatures, required), + (9, holder_sends_tx_signatures_first, required), + (11, shared_input_signature, required), }); #[derive(Debug)] @@ -1278,12 +1364,33 @@ macro_rules! define_state_transitions { impl StateTransition for $tx_complete_state { fn transition(self, _data: &msgs::TxComplete) -> StateTransitionResult { let context = self.into_negotiation_context(); + let shared_input_signature = context + .shared_funding_input + .as_ref() + .map(|shared_input| SharedInputSignature { + holder_signature_first: shared_input.holder_sig_first, + counterparty_signature: None, + witness_script: shared_input.witness_script.clone(), + }); + let holder_node_id = context.holder_node_id; + let counterparty_node_id = context.counterparty_node_id; + let tx = context.validate_tx()?; + + // Strict ordering prevents deadlocks during tx_signatures exchange + let holder_sends_tx_signatures_first = + if tx.local_inputs_value_satoshis == tx.remote_inputs_value_satoshis { + holder_node_id.serialize() < counterparty_node_id.serialize() + } else { + tx.local_inputs_value_satoshis < tx.remote_inputs_value_satoshis + }; + let signing_session = InteractiveTxSigningSession { - holder_sends_tx_signatures_first: tx.holder_sends_tx_signatures_first, unsigned_tx: tx, + holder_sends_tx_signatures_first, has_received_commitment_signed: false, has_received_tx_signatures: false, + shared_input_signature, holder_tx_signatures: None, }; Ok(NegotiationComplete(signing_session)) @@ -1450,10 +1557,15 @@ pub(super) struct SharedOwnedInput { input: TxIn, prev_output: TxOut, local_owned: u64, + holder_sig_first: bool, + witness_script: ScriptBuf, } impl SharedOwnedInput { - pub fn new(input: TxIn, prev_output: TxOut, local_owned: u64) -> Self { + pub fn new( + input: TxIn, prev_output: TxOut, local_owned: u64, holder_sig_first: bool, + witness_script: ScriptBuf, + ) -> Self { let value = prev_output.value.to_sat(); debug_assert!( local_owned <= value, @@ -1461,7 +1573,7 @@ impl SharedOwnedInput { local_owned, value, ); - Self { input, prev_output, local_owned } + Self { input, prev_output, local_owned, holder_sig_first, witness_script } } fn remote_owned(&self) -> u64 { @@ -2270,6 +2382,8 @@ mod tests { }, prev_output, lo, + true, // holder_sig_first + generate_funding_script_pubkey(), // witness_script for test ) }), shared_funding_output: SharedOwnedOutput::new( @@ -2307,6 +2421,8 @@ mod tests { }, prev_output, lo, + false, // holder_sig_first + generate_funding_script_pubkey(), // witness_script for test ) }), shared_funding_output: SharedOwnedOutput::new( @@ -3291,7 +3407,7 @@ mod tests { remote_inputs_value_satoshis: 0, // N/A for test remote_outputs_value_satoshis: 0, // N/A for test lock_time: transaction.lock_time, - holder_sends_tx_signatures_first: false, + shared_input_index: None, }; let secp_ctx = Secp256k1::new(); @@ -3301,6 +3417,7 @@ mod tests { holder_sends_tx_signatures_first: false, // N/A for test has_received_commitment_signed: false, // N/A for test has_received_tx_signatures: false, // N/A for test + shared_input_signature: None, holder_tx_signatures: None, } .verify_interactive_tx_signatures( From e664b7e5e6a2a399e77cc14c87711751fe643068 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 21 Aug 2025 09:40:38 -0700 Subject: [PATCH 6/6] Sign splice shared input when producing holder tx_signatures We also remove the `Result` to make it clear that this method does not support async operations yet and rename the method to clarify that it is only intended to be used for the shared input of a splice. --- lightning/src/ln/channel.rs | 51 ++++++++++++++++++++++- lightning/src/ln/channelmanager.rs | 4 +- lightning/src/sign/ecdsa.rs | 9 ++-- lightning/src/sign/mod.rs | 6 +-- lightning/src/util/dyn_signer.rs | 4 +- lightning/src/util/test_channel_signer.rs | 6 +-- 6 files changed, 64 insertions(+), 16 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9c274057f6b..75e597d2311 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7984,6 +7984,35 @@ where pub fn funding_transaction_signed( &mut self, funding_txid_signed: Txid, witnesses: Vec, ) -> Result<(Option, Option), APIError> { + if !self.context.channel_state.is_interactive_signing() { + let err = + format!("Channel {} not expecting funding signatures", self.context.channel_id); + return Err(APIError::APIMisuseError { err }); + } + if self.context.channel_state.is_our_tx_signatures_ready() { + let err = + format!("Channel {} already received funding signatures", self.context.channel_id); + return Err(APIError::APIMisuseError { err }); + } + #[cfg(splicing)] + if let Some(pending_splice) = self.pending_splice.as_ref() { + if !pending_splice + .funding_negotiation + .as_ref() + .map(|funding_negotiation| { + matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures(_)) + }) + .unwrap_or(false) + { + debug_assert!(false); + let err = format!( + "Channel {} with pending splice is not expecting funding signatures yet", + self.context.channel_id + ); + return Err(APIError::APIMisuseError { err }); + } + } + let (tx_signatures_opt, funding_tx_opt) = self .interactive_tx_signing_session .as_mut() @@ -8001,11 +8030,31 @@ where }); } + let shared_input_signature = if let Some(splice_input_index) = + signing_session.unsigned_tx().shared_input_index() + { + let sig = match &self.context.holder_signer { + ChannelSignerType::Ecdsa(signer) => signer.sign_splice_shared_input( + &self.funding.channel_transaction_parameters, + &tx, + splice_input_index as usize, + &self.context.secp_ctx, + ), + #[cfg(taproot)] + ChannelSignerType::Taproot(_) => todo!(), + }; + Some(sig) + } else { + None + }; + #[cfg(splicing)] + debug_assert_eq!(self.pending_splice.is_some(), shared_input_signature.is_some()); + let tx_signatures = msgs::TxSignatures { channel_id: self.context.channel_id, tx_hash: funding_txid_signed, witnesses, - shared_input_signature: None, + shared_input_signature, }; signing_session .provide_holder_witnesses(tx_signatures, &self.context.secp_ctx) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 28f770b1386..075bce34421 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9044,7 +9044,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Ok((None, _)) => { debug_assert!(false, "If our tx_signatures is empty, then we should send it first!"); }, - Err(err) => debug_assert!(false, "We should not error here but we got: {:?}", err), + Err(err) => { + log_warn!(logger, "Failed signing interactive funding transaction: {err:?}"); + }, } } } diff --git a/lightning/src/sign/ecdsa.rs b/lightning/src/sign/ecdsa.rs index a25d5df2761..bfa3a27acb1 100644 --- a/lightning/src/sign/ecdsa.rs +++ b/lightning/src/sign/ecdsa.rs @@ -242,7 +242,7 @@ pub trait EcdsaChannelSigner: ChannelSigner { msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1, ) -> Result; - /// Signs the input of a splicing funding transaction with our funding key. + /// Signs the shared input of a splice transaction with our funding key. /// /// In splicing, the previous funding transaction output is spent as the input of /// the new funding transaction, and is a 2-of-2 multisig. @@ -253,11 +253,8 @@ pub trait EcdsaChannelSigner: ChannelSigner { /// /// `input_index`: The index of the input within the new funding transaction `tx`, /// spending the previous funding transaction's output - /// - /// This method is *not* asynchronous. If an `Err` is returned, the channel will be immediately - /// closed. - fn sign_splicing_funding_input( + fn sign_splice_shared_input( &self, channel_parameters: &ChannelTransactionParameters, tx: &Transaction, input_index: usize, secp_ctx: &Secp256k1, - ) -> Result; + ) -> Signature; } diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index abdb03c0e8b..67b602a654e 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -1753,10 +1753,10 @@ impl EcdsaChannelSigner for InMemorySigner { Ok(secp_ctx.sign_ecdsa(&msghash, &funding_key)) } - fn sign_splicing_funding_input( + fn sign_splice_shared_input( &self, channel_parameters: &ChannelTransactionParameters, tx: &Transaction, input_index: usize, secp_ctx: &Secp256k1, - ) -> Result { + ) -> Signature { assert!(channel_parameters.is_populated(), "Channel parameters must be fully populated"); assert_eq!( tx.input[input_index].previous_output, @@ -1782,7 +1782,7 @@ impl EcdsaChannelSigner for InMemorySigner { ) .unwrap()[..]; let msg = hash_to_message!(sighash); - Ok(sign(secp_ctx, &msg, &funding_key)) + sign(secp_ctx, &msg, &funding_key) } } diff --git a/lightning/src/util/dyn_signer.rs b/lightning/src/util/dyn_signer.rs index baa4eeda7df..8bba856c219 100644 --- a/lightning/src/util/dyn_signer.rs +++ b/lightning/src/util/dyn_signer.rs @@ -159,8 +159,8 @@ delegate!(DynSigner, EcdsaChannelSigner, inner, secp_ctx: &Secp256k1) -> Result, fn sign_holder_htlc_transaction(, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1) -> Result, - fn sign_splicing_funding_input(, channel_parameters: &ChannelTransactionParameters, - tx: &Transaction, input_index: usize, secp_ctx: &Secp256k1) -> Result + fn sign_splice_shared_input(, channel_parameters: &ChannelTransactionParameters, + tx: &Transaction, input_index: usize, secp_ctx: &Secp256k1) -> Signature ); delegate!(DynSigner, ChannelSigner, diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index 7e89650a480..0b472b9300e 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -484,11 +484,11 @@ impl EcdsaChannelSigner for TestChannelSigner { self.inner.sign_channel_announcement_with_funding_key(channel_parameters, msg, secp_ctx) } - fn sign_splicing_funding_input( + fn sign_splice_shared_input( &self, channel_parameters: &ChannelTransactionParameters, tx: &Transaction, input_index: usize, secp_ctx: &Secp256k1, - ) -> Result { - self.inner.sign_splicing_funding_input(channel_parameters, tx, input_index, secp_ctx) + ) -> Signature { + self.inner.sign_splice_shared_input(channel_parameters, tx, input_index, secp_ctx) } }