From 9510e298f9246fd197105f2ceae9c16a64214d47 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Oct 2023 13:49:50 -0700 Subject: [PATCH 1/6] Support signing non-anchors HTLCs with HTLCDescriptor We plan to use `EcdsaChannelSigner::sign_holder_htlc_transaction` to also sign holder HTLC transactions on non-anchor outputs channels. `HTLCDescriptor` was only used in an anchor outputs context, so a few things needed changing, mostly to handle the different scripts and feerate. --- lightning/src/chain/channelmonitor.rs | 1 + lightning/src/events/bump_transaction.rs | 33 ++++++++++++++---------- lightning/src/sign/mod.rs | 4 +-- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index bd0c1548428..acb735e151e 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2835,6 +2835,7 @@ impl ChannelMonitorImpl { per_commitment_point: self.onchain_tx_handler.signer.get_per_commitment_point( htlc.per_commitment_number, &self.onchain_tx_handler.secp_ctx, ), + feerate_per_kw: 0, htlc: htlc.htlc, preimage: htlc.preimage, counterparty_sig: htlc.counterparty_sig, diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 55c12d23e74..5d5161a67bb 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -23,7 +23,6 @@ use crate::ln::chan_utils::{ ANCHOR_INPUT_WITNESS_WEIGHT, HTLC_SUCCESS_INPUT_ANCHOR_WITNESS_WEIGHT, HTLC_TIMEOUT_INPUT_ANCHOR_WITNESS_WEIGHT, ChannelTransactionParameters, HTLCOutputInCommitment }; -use crate::ln::features::ChannelTypeFeatures; use crate::ln::PaymentPreimage; use crate::prelude::*; use crate::sign::{EcdsaChannelSigner, SignerProvider, WriteableEcdsaChannelSigner, P2WPKH_WITNESS_WEIGHT}; @@ -136,6 +135,10 @@ pub struct HTLCDescriptor { /// /// See for more info. pub per_commitment_point: PublicKey, + /// The feerate to use on the HTLC claiming transaction. This is always `0` for HTLCs + /// originating from a channel supporting anchor outputs, otherwise it is the channel's + /// negotiated feerate at the time the commitment transaction was built. + pub feerate_per_kw: u32, /// The details of the HTLC as it appears in the commitment transaction. pub htlc: HTLCOutputInCommitment, /// The preimage, if `Some`, to claim the HTLC output with. If `None`, the timeout path must be @@ -146,13 +149,14 @@ pub struct HTLCDescriptor { } impl_writeable_tlv_based!(HTLCDescriptor, { - (0, channel_derivation_parameters, required), - (2, commitment_txid, required), - (4, per_commitment_number, required), - (6, per_commitment_point, required), - (8, htlc, required), - (10, preimage, option), - (12, counterparty_sig, required), + (0, channel_derivation_parameters, required), + (1, feerate_per_kw, (default_value, 0)), + (2, commitment_txid, required), + (4, per_commitment_number, required), + (6, per_commitment_point, required), + (8, htlc, required), + (10, preimage, option), + (12, counterparty_sig, required), }); impl HTLCDescriptor { @@ -177,7 +181,9 @@ impl HTLCDescriptor { /// Returns the unsigned transaction input spending the HTLC output in the commitment /// transaction. pub fn unsigned_tx_input(&self) -> TxIn { - chan_utils::build_htlc_input(&self.commitment_txid, &self.htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()) + chan_utils::build_htlc_input( + &self.commitment_txid, &self.htlc, &self.channel_derivation_parameters.transaction_parameters.channel_type_features + ) } /// Returns the delayed output created as a result of spending the HTLC output in the commitment @@ -193,8 +199,8 @@ impl HTLCDescriptor { secp, &self.per_commitment_point, &counterparty_keys.revocation_basepoint ); chan_utils::build_htlc_output( - 0 /* feerate_per_kw */, channel_params.contest_delay(), &self.htlc, - &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &broadcaster_delayed_key, &counterparty_revocation_key + self.feerate_per_kw, channel_params.contest_delay(), &self.htlc, + channel_params.channel_type_features(), &broadcaster_delayed_key, &counterparty_revocation_key ) } @@ -213,7 +219,7 @@ impl HTLCDescriptor { secp, &self.per_commitment_point, &counterparty_keys.revocation_basepoint ); chan_utils::get_htlc_redeemscript_with_explicit_keys( - &self.htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &broadcaster_htlc_key, &counterparty_htlc_key, + &self.htlc, channel_params.channel_type_features(), &broadcaster_htlc_key, &counterparty_htlc_key, &counterparty_revocation_key, ) } @@ -222,7 +228,8 @@ impl HTLCDescriptor { /// transaction. pub fn tx_input_witness(&self, signature: &Signature, witness_script: &Script) -> Witness { chan_utils::build_htlc_input_witness( - signature, &self.counterparty_sig, &self.preimage, witness_script, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() /* opt_anchors */ + signature, &self.counterparty_sig, &self.preimage, witness_script, + &self.channel_derivation_parameters.transaction_parameters.channel_type_features ) } diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 04c4446e2c0..d9992585819 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -552,9 +552,7 @@ pub trait EcdsaChannelSigner: ChannelSigner { secp_ctx: &Secp256k1) -> Result; /// Computes the signature for a commitment transaction's HTLC output used as an input within /// `htlc_tx`, which spends the commitment transaction at index `input`. The signature returned - /// must be be computed using [`EcdsaSighashType::All`]. Note that this should only be used to - /// sign HTLC transactions from channels supporting anchor outputs after all additional - /// inputs/outputs have been added to the transaction. + /// must be be computed using [`EcdsaSighashType::All`]. /// /// [`EcdsaSighashType::All`]: bitcoin::blockdata::transaction::EcdsaSighashType::All fn sign_holder_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, From 5958604ea67b01ce69e4502bf2c35254838abab9 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Oct 2023 13:47:45 -0700 Subject: [PATCH 2/6] Provide missing derivation parameters to OnchainTxHandler `OnchainTxHandler` will need to construct `HTLCDescriptor`s for holder HTLCs, but it did not have access to all of the derivation parameters that need to be provided. --- lightning/src/chain/channelmonitor.rs | 7 ++++--- lightning/src/chain/onchaintx.rs | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index acb735e151e..aa53e6fd5d9 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1172,9 +1172,10 @@ impl ChannelMonitor { (holder_commitment_tx, trusted_tx.commitment_number()) }; - let onchain_tx_handler = - OnchainTxHandler::new(destination_script.clone(), keys, - channel_parameters.clone(), initial_holder_commitment_tx, secp_ctx); + let onchain_tx_handler = OnchainTxHandler::new( + channel_value_satoshis, channel_keys_id, destination_script.clone(), keys, + channel_parameters.clone(), initial_holder_commitment_tx, secp_ctx + ); let mut outputs_to_watch = HashMap::new(); outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]); diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 5daa2463de9..857b6b9a6b0 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -215,6 +215,8 @@ pub(crate) enum OnchainClaim { /// do RBF bumping if possible. #[derive(Clone)] pub struct OnchainTxHandler { + channel_value_satoshis: u64, + channel_keys_id: [u8; 32], destination_script: Script, holder_commitment: HolderCommitmentTransaction, // holder_htlc_sigs and prev_holder_htlc_sigs are in the order as they appear in the commitment @@ -276,7 +278,9 @@ pub struct OnchainTxHandler { impl PartialEq for OnchainTxHandler { fn eq(&self, other: &Self) -> bool { // `signer`, `secp_ctx`, and `pending_claim_events` are excluded on purpose. - self.destination_script == other.destination_script && + self.channel_value_satoshis == other.channel_value_satoshis && + self.channel_keys_id == other.channel_keys_id && + self.destination_script == other.destination_script && self.holder_commitment == other.holder_commitment && self.holder_htlc_sigs == other.holder_htlc_sigs && self.prev_holder_commitment == other.prev_holder_commitment && @@ -418,6 +422,8 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes()); Ok(OnchainTxHandler { + channel_value_satoshis, + channel_keys_id, destination_script, holder_commitment, holder_htlc_sigs, @@ -436,8 +442,14 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP } impl OnchainTxHandler { - pub(crate) fn new(destination_script: Script, signer: ChannelSigner, channel_parameters: ChannelTransactionParameters, holder_commitment: HolderCommitmentTransaction, secp_ctx: Secp256k1) -> Self { + pub(crate) fn new( + channel_value_satoshis: u64, channel_keys_id: [u8; 32], destination_script: Script, + signer: ChannelSigner, channel_parameters: ChannelTransactionParameters, + holder_commitment: HolderCommitmentTransaction, secp_ctx: Secp256k1 + ) -> Self { OnchainTxHandler { + channel_value_satoshis, + channel_keys_id, destination_script, holder_commitment, holder_htlc_sigs: None, From 03ec74631fcaf3cf8434432259d61571865820b0 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Oct 2023 13:52:23 -0700 Subject: [PATCH 3/6] Use sign_holder_htlc_transaction to sign non-anchors holder HTLCs We want to ensure we use fresh random signatures to prevent certain classes of transaction replacement attacks at the bitcoin P2P layer. This was already covered for commitment transactions and zero fee holder HTLC transactions, but was missing for holder HTLC transactions on non-anchors channels. We can easily do this by reusing the existing `EcdsaChannelSigner::sign_holder_htlc_transaction` method and circumventing the existing `holder_htlc_sigs/prev_holder_htlc_sigs` caches, which will be removed in a later commit anyway. --- lightning/src/chain/onchaintx.rs | 65 ++++++++++-------- lightning/src/ln/chan_utils.rs | 41 ++++++++--- lightning/src/ln/monitor_tests.rs | 110 ++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 39 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 857b6b9a6b0..b26d4d8724f 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -23,6 +23,7 @@ use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; use bitcoin::secp256k1; use crate::chain::chaininterface::compute_feerate_sat_per_1000_weight; +use crate::events::bump_transaction::{ChannelDerivationParameters, HTLCDescriptor}; use crate::sign::{ChannelSigner, EntropySource, SignerProvider}; use crate::ln::msgs::DecodeError; use crate::ln::PaymentPreimage; @@ -1157,35 +1158,43 @@ impl OnchainTxHandler } pub(crate) fn get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option) -> Option { - let mut htlc_tx = None; - let commitment_txid = self.holder_commitment.trust().txid(); - // Check if the HTLC spends from the current holder commitment - if commitment_txid == outp.txid { - self.sign_latest_holder_htlcs(); - if let &Some(ref htlc_sigs) = &self.holder_htlc_sigs { - let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap(); - let trusted_tx = self.holder_commitment.trust(); - let counterparty_htlc_sig = self.holder_commitment.counterparty_htlc_sigs[*htlc_idx]; - htlc_tx = Some(trusted_tx - .get_signed_htlc_tx(&self.channel_transaction_parameters.as_holder_broadcastable(), *htlc_idx, &counterparty_htlc_sig, htlc_sig, preimage)); - } - } - // If the HTLC doesn't spend the current holder commitment, check if it spends the previous one - if htlc_tx.is_none() && self.prev_holder_commitment.is_some() { - let commitment_txid = self.prev_holder_commitment.as_ref().unwrap().trust().txid(); - if commitment_txid == outp.txid { - self.sign_prev_holder_htlcs(); - if let &Some(ref htlc_sigs) = &self.prev_holder_htlc_sigs { - let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap(); - let holder_commitment = self.prev_holder_commitment.as_ref().unwrap(); - let trusted_tx = holder_commitment.trust(); - let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[*htlc_idx]; - htlc_tx = Some(trusted_tx - .get_signed_htlc_tx(&self.channel_transaction_parameters.as_holder_broadcastable(), *htlc_idx, &counterparty_htlc_sig, htlc_sig, preimage)); - } + let get_signed_htlc_tx = |holder_commitment: &HolderCommitmentTransaction| { + let trusted_tx = holder_commitment.trust(); + if trusted_tx.txid() != outp.txid { + return None; } - } - htlc_tx + let (htlc_idx, htlc) = trusted_tx.htlcs().iter().enumerate() + .find(|(_, htlc)| htlc.transaction_output_index.unwrap() == outp.vout) + .unwrap(); + let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[htlc_idx]; + let mut htlc_tx = trusted_tx.build_unsigned_htlc_tx( + &self.channel_transaction_parameters.as_holder_broadcastable(), htlc_idx, preimage, + ); + + let htlc_descriptor = HTLCDescriptor { + channel_derivation_parameters: ChannelDerivationParameters { + value_satoshis: self.channel_value_satoshis, + keys_id: self.channel_keys_id, + transaction_parameters: self.channel_transaction_parameters.clone(), + }, + commitment_txid: trusted_tx.txid(), + per_commitment_number: trusted_tx.commitment_number(), + per_commitment_point: trusted_tx.per_commitment_point(), + feerate_per_kw: trusted_tx.feerate_per_kw(), + htlc: htlc.clone(), + preimage: preimage.clone(), + counterparty_sig: counterparty_htlc_sig.clone(), + }; + let htlc_sig = self.signer.sign_holder_htlc_transaction(&htlc_tx, 0, &htlc_descriptor, &self.secp_ctx).unwrap(); + htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness( + htlc_idx, &counterparty_htlc_sig, &htlc_sig, preimage, + ); + Some(htlc_tx) + }; + + // Check if the HTLC spends from the current holder commitment first, or the previous. + get_signed_htlc_tx(&self.holder_commitment) + .or_else(|| self.prev_holder_commitment.as_ref().and_then(|prev_holder_commitment| get_signed_htlc_tx(prev_holder_commitment))) } pub(crate) fn generate_external_htlc_claim( diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index d1489e27168..1c068025b8f 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -1599,6 +1599,11 @@ impl CommitmentTransaction { self.commitment_number } + /// The per commitment point used by the broadcaster. + pub fn per_commitment_point(&self) -> PublicKey { + self.keys.per_commitment_point + } + /// The value to be sent to the broadcaster pub fn to_broadcaster_value_sat(&self) -> u64 { self.to_broadcaster_value_sat @@ -1720,26 +1725,40 @@ impl<'a> TrustedCommitmentTransaction<'a> { Ok(ret) } - /// Gets a signed HTLC transaction given a preimage (for !htlc.offered) and the holder HTLC transaction signature. - pub(crate) fn get_signed_htlc_tx(&self, channel_parameters: &DirectedChannelTransactionParameters, htlc_index: usize, counterparty_signature: &Signature, signature: &Signature, preimage: &Option) -> Transaction { - let inner = self.inner; - let keys = &inner.keys; - let txid = inner.built.txid; - let this_htlc = &inner.htlcs[htlc_index]; + /// Builds the second-level holder HTLC transaction for the HTLC with index `htlc_index`. + pub(crate) fn build_unsigned_htlc_tx( + &self, channel_parameters: &DirectedChannelTransactionParameters, htlc_index: usize, + preimage: &Option, + ) -> Transaction { + let keys = &self.inner.keys; + let this_htlc = &self.inner.htlcs[htlc_index]; assert!(this_htlc.transaction_output_index.is_some()); // if we don't have preimage for an HTLC-Success, we can't generate an HTLC transaction. if !this_htlc.offered && preimage.is_none() { unreachable!(); } // Further, we should never be provided the preimage for an HTLC-Timeout transaction. if this_htlc.offered && preimage.is_some() { unreachable!(); } - let mut htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, &self.channel_type_features, &keys.broadcaster_delayed_payment_key, &keys.revocation_key); + build_htlc_transaction( + &self.inner.built.txid, self.inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, + &self.channel_type_features, &keys.broadcaster_delayed_payment_key, &keys.revocation_key + ) + } - let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc, &self.channel_type_features, &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key); - htlc_tx.input[0].witness = chan_utils::build_htlc_input_witness( - signature, counterparty_signature, preimage, &htlc_redeemscript, &self.channel_type_features, + /// Builds the witness required to spend the input for the HTLC with index `htlc_index` in a + /// second-level holder HTLC transaction. + pub(crate) fn build_htlc_input_witness( + &self, htlc_index: usize, counterparty_signature: &Signature, signature: &Signature, + preimage: &Option + ) -> Witness { + let keys = &self.inner.keys; + let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys( + &self.inner.htlcs[htlc_index], &self.channel_type_features, &keys.broadcaster_htlc_key, + &keys.countersignatory_htlc_key, &keys.revocation_key ); - htlc_tx + chan_utils::build_htlc_input_witness( + signature, counterparty_signature, preimage, &htlc_redeemscript, &self.channel_type_features, + ) } /// Returns the index of the revokeable output, i.e. the `to_local` output sending funds to diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 0399ed38251..5be498e918b 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -2715,3 +2715,113 @@ fn test_anchors_monitor_fixes_counterparty_payment_script_on_reload() { do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(false); do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(true); } + +#[cfg(not(feature = "_test_vectors"))] +fn do_test_monitor_claims_with_random_signatures(anchors: bool, confirm_counterparty_commitment: bool) { + // Tests that our monitor claims will always use fresh random signatures (ensuring a unique + // wtxid) to prevent certain classes of transaction replacement at the bitcoin P2P layer. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut user_config = test_default_channel_config(); + if anchors { + user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + user_config.manually_accept_inbound_channels = true; + } + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let coinbase_tx = Transaction { + version: 2, + lock_time: PackedLockTime::ZERO, + input: vec![TxIn { ..Default::default() }], + output: vec![ + TxOut { + value: Amount::ONE_BTC.to_sat(), + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }, + ], + }; + if anchors { + nodes[0].wallet_source.add_utxo(bitcoin::OutPoint { txid: coinbase_tx.txid(), vout: 0 }, coinbase_tx.output[0].value); + } + + // Open a channel and route a payment. We'll let it timeout to claim it. + let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + route_payment(&nodes[0], &[&nodes[1]], 1_000_000); + + let (closing_node, other_node) = if confirm_counterparty_commitment { + (&nodes[1], &nodes[0]) + } else { + (&nodes[0], &nodes[1]) + }; + + closing_node.node.force_close_broadcasting_latest_txn(&chan_id, &other_node.node.get_our_node_id()).unwrap(); + + // The commitment transaction comes first. + let commitment_tx = { + let mut txn = closing_node.tx_broadcaster.unique_txn_broadcast(); + assert_eq!(txn.len(), 1); + check_spends!(txn[0], funding_tx); + txn.pop().unwrap() + }; + + mine_transaction(closing_node, &commitment_tx); + check_added_monitors!(closing_node, 1); + check_closed_broadcast!(closing_node, true); + check_closed_event!(closing_node, 1, ClosureReason::HolderForceClosed, [other_node.node.get_our_node_id()], 1_000_000); + + mine_transaction(other_node, &commitment_tx); + check_added_monitors!(other_node, 1); + check_closed_broadcast!(other_node, true); + check_closed_event!(other_node, 1, ClosureReason::CommitmentTxConfirmed, [closing_node.node.get_our_node_id()], 1_000_000); + + // If we update the best block to the new height before providing the confirmed transactions, + // we'll see another broadcast of the commitment transaction. + if anchors && !confirm_counterparty_commitment && nodes[0].connect_style.borrow().updates_best_block_first() { + let _ = nodes[0].tx_broadcaster.txn_broadcast(); + } + + // Then comes the HTLC timeout transaction. + if confirm_counterparty_commitment { + connect_blocks(&nodes[0], 5); + test_spendable_output(&nodes[0], &commitment_tx, false); + connect_blocks(&nodes[0], TEST_FINAL_CLTV - 5); + } else { + connect_blocks(&nodes[0], TEST_FINAL_CLTV); + } + if anchors && !confirm_counterparty_commitment { + handle_bump_htlc_event(&nodes[0], 1); + } + let htlc_timeout_tx = { + let mut txn = nodes[0].tx_broadcaster.txn_broadcast(); + assert_eq!(txn.len(), 1); + let tx = if txn[0].input[0].previous_output.txid == commitment_tx.txid() { + txn[0].clone() + } else { + txn[1].clone() + }; + check_spends!(tx, commitment_tx, coinbase_tx); + tx + }; + + // Check we rebroadcast it with a different wtxid. + nodes[0].chain_monitor.chain_monitor.rebroadcast_pending_claims(); + if anchors && !confirm_counterparty_commitment { + handle_bump_htlc_event(&nodes[0], 1); + } + { + let mut txn = nodes[0].tx_broadcaster.txn_broadcast(); + assert_eq!(txn.len(), 1); + assert_eq!(txn[0].txid(), htlc_timeout_tx.txid()); + assert_ne!(txn[0].wtxid(), htlc_timeout_tx.wtxid()); + } +} + +#[cfg(not(feature = "_test_vectors"))] +#[test] +fn test_monitor_claims_with_random_signatures() { + do_test_monitor_claims_with_random_signatures(false, false); + do_test_monitor_claims_with_random_signatures(false, true); + do_test_monitor_claims_with_random_signatures(true, false); + do_test_monitor_claims_with_random_signatures(true, true); +} From a9d9d266557abccb4ac55aa9fc2ba90ebb755c55 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Oct 2023 13:58:59 -0700 Subject: [PATCH 4/6] Remove caching of holder HTLC signatures Since we want our holder HTLC signatures to be randomly generated and not reused, our existing caches are useless now, so we opt to remove them. --- lightning/src/chain/channelmonitor.rs | 2 +- lightning/src/chain/onchaintx.rs | 72 +++------------------------ 2 files changed, 7 insertions(+), 67 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index aa53e6fd5d9..d855fdd819b 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -3356,7 +3356,7 @@ impl ChannelMonitorImpl { continue; } } else { None }; - if let Some(htlc_tx) = self.onchain_tx_handler.unsafe_get_fully_signed_htlc_tx( + if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx( &::bitcoin::OutPoint { txid, vout }, &preimage) { holder_transactions.push(htlc_tx); } diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index b26d4d8724f..5d44ea640f0 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -220,12 +220,7 @@ pub struct OnchainTxHandler { channel_keys_id: [u8; 32], destination_script: Script, holder_commitment: HolderCommitmentTransaction, - // holder_htlc_sigs and prev_holder_htlc_sigs are in the order as they appear in the commitment - // transaction outputs (hence the Option<>s inside the Vec). The first usize is the index in - // the set of HTLCs in the HolderCommitmentTransaction. - holder_htlc_sigs: Option>>, prev_holder_commitment: Option, - prev_holder_htlc_sigs: Option>>, pub(super) signer: ChannelSigner, pub(crate) channel_transaction_parameters: ChannelTransactionParameters, @@ -283,9 +278,7 @@ impl PartialEq for OnchainTxHandler< self.channel_keys_id == other.channel_keys_id && self.destination_script == other.destination_script && self.holder_commitment == other.holder_commitment && - self.holder_htlc_sigs == other.holder_htlc_sigs && self.prev_holder_commitment == other.prev_holder_commitment && - self.prev_holder_htlc_sigs == other.prev_holder_htlc_sigs && self.channel_transaction_parameters == other.channel_transaction_parameters && self.pending_claim_requests == other.pending_claim_requests && self.claimable_outpoints == other.claimable_outpoints && @@ -303,9 +296,9 @@ impl OnchainTxHandler self.destination_script.write(writer)?; self.holder_commitment.write(writer)?; - self.holder_htlc_sigs.write(writer)?; + None::>>>.write(writer)?; // holder_htlc_sigs self.prev_holder_commitment.write(writer)?; - self.prev_holder_htlc_sigs.write(writer)?; + None::>>>.write(writer)?; // prev_holder_htlc_sigs self.channel_transaction_parameters.write(writer)?; @@ -360,9 +353,9 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP let destination_script = Readable::read(reader)?; let holder_commitment = Readable::read(reader)?; - let holder_htlc_sigs = Readable::read(reader)?; + let _holder_htlc_sigs: Option>> = Readable::read(reader)?; let prev_holder_commitment = Readable::read(reader)?; - let prev_holder_htlc_sigs = Readable::read(reader)?; + let _prev_holder_htlc_sigs: Option>> = Readable::read(reader)?; let channel_parameters = Readable::read(reader)?; @@ -427,9 +420,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP channel_keys_id, destination_script, holder_commitment, - holder_htlc_sigs, prev_holder_commitment, - prev_holder_htlc_sigs, signer, channel_transaction_parameters: channel_parameters, claimable_outpoints, @@ -453,9 +444,7 @@ impl OnchainTxHandler channel_keys_id, destination_script, holder_commitment, - holder_htlc_sigs: None, prev_holder_commitment: None, - prev_holder_htlc_sigs: None, signer, channel_transaction_parameters: channel_parameters, pending_claim_requests: HashMap::new(), @@ -1101,39 +1090,6 @@ impl OnchainTxHandler pub(crate) fn provide_latest_holder_tx(&mut self, tx: HolderCommitmentTransaction) { self.prev_holder_commitment = Some(replace(&mut self.holder_commitment, tx)); - self.holder_htlc_sigs = None; - } - - // Normally holder HTLCs are signed at the same time as the holder commitment tx. However, - // in some configurations, the holder commitment tx has been signed and broadcast by a - // ChannelMonitor replica, so we handle that case here. - fn sign_latest_holder_htlcs(&mut self) { - if self.holder_htlc_sigs.is_none() { - let (_sig, sigs) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment"); - self.holder_htlc_sigs = Some(Self::extract_holder_sigs(&self.holder_commitment, sigs)); - } - } - - // Normally only the latest commitment tx and HTLCs need to be signed. However, in some - // configurations we may have updated our holder commitment but a replica of the ChannelMonitor - // broadcast the previous one before we sync with it. We handle that case here. - fn sign_prev_holder_htlcs(&mut self) { - if self.prev_holder_htlc_sigs.is_none() { - if let Some(ref holder_commitment) = self.prev_holder_commitment { - let (_sig, sigs) = self.signer.sign_holder_commitment_and_htlcs(holder_commitment, &self.secp_ctx).expect("sign previous holder commitment"); - self.prev_holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs)); - } - } - } - - fn extract_holder_sigs(holder_commitment: &HolderCommitmentTransaction, sigs: Vec) -> Vec> { - let mut ret = Vec::new(); - for (htlc_idx, (holder_sig, htlc)) in sigs.iter().zip(holder_commitment.htlcs().iter()).enumerate() { - let tx_idx = htlc.transaction_output_index.unwrap(); - if ret.len() <= tx_idx as usize { ret.resize(tx_idx as usize + 1, None); } - ret[tx_idx as usize] = Some((htlc_idx, holder_sig.clone())); - } - ret } pub(crate) fn get_unsigned_holder_commitment_tx(&self) -> &Transaction { @@ -1145,15 +1101,13 @@ impl OnchainTxHandler // before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing // to monitor before. pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction { - let (sig, htlc_sigs) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment"); - self.holder_htlc_sigs = Some(Self::extract_holder_sigs(&self.holder_commitment, htlc_sigs)); + let (sig, _) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment"); self.holder_commitment.add_holder_sig(funding_redeemscript, sig) } #[cfg(any(test, feature="unsafe_revoked_tx_signing"))] pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction { - let (sig, htlc_sigs) = self.signer.unsafe_sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment"); - self.holder_htlc_sigs = Some(Self::extract_holder_sigs(&self.holder_commitment, htlc_sigs)); + let (sig, _) = self.signer.unsafe_sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment"); self.holder_commitment.add_holder_sig(funding_redeemscript, sig) } @@ -1230,18 +1184,4 @@ impl OnchainTxHandler pub(crate) fn channel_type_features(&self) -> &ChannelTypeFeatures { &self.channel_transaction_parameters.channel_type_features } - - #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))] - pub(crate) fn unsafe_get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option) -> Option { - let latest_had_sigs = self.holder_htlc_sigs.is_some(); - let prev_had_sigs = self.prev_holder_htlc_sigs.is_some(); - let ret = self.get_fully_signed_htlc_tx(outp, preimage); - if !latest_had_sigs { - self.holder_htlc_sigs = None; - } - if !prev_had_sigs { - self.prev_holder_htlc_sigs = None; - } - ret - } } From aae4e7c0cab7f30e38b06a2314eea505389eefe1 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Oct 2023 14:09:37 -0700 Subject: [PATCH 5/6] Don't sign holder HTLCs along with holder commitments `sign_holder_commitment_and_htlcs` never really made sense. Unlike `sign_counterparty_commitment`, the signatures for holder HTLC transactions may be required much later than the commitment transaction's. While it was nice for us to only reach the signer once to obtain all holder signatures, it's not really ideal anymore as we want our signatures to be random and not reused. We no longer return all holder HTLC signatures and instead defer to obtaining them via `EcdsaChannelSigner::sign_holder_htlc_transaction`. --- lightning/src/chain/onchaintx.rs | 4 +- lightning/src/ln/channel.rs | 37 ++++++++++----- lightning/src/ln/monitor_tests.rs | 2 +- lightning/src/sign/mod.rs | 44 ++++++++--------- lightning/src/util/test_channel_signer.rs | 58 ++++++++++++----------- 5 files changed, 77 insertions(+), 68 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 5d44ea640f0..95824f910db 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -1101,13 +1101,13 @@ impl OnchainTxHandler // before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing // to monitor before. pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction { - let (sig, _) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment"); + let sig = self.signer.sign_holder_commitment(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment"); self.holder_commitment.add_holder_sig(funding_redeemscript, sig) } #[cfg(any(test, feature="unsafe_revoked_tx_signing"))] pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction { - let (sig, _) = self.signer.unsafe_sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment"); + let sig = self.signer.unsafe_sign_holder_commitment(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment"); self.holder_commitment.add_holder_sig(funding_redeemscript, sig) } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a61a8de82de..7b14573a1a4 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8184,6 +8184,7 @@ mod tests { use bitcoin::hashes::hex::FromHex; use bitcoin::hash_types::Txid; use bitcoin::secp256k1::Message; + use crate::events::bump_transaction::{ChannelDerivationParameters, HTLCDescriptor}; use crate::sign::EcdsaChannelSigner; use crate::ln::PaymentPreimage; use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys}; @@ -8309,7 +8310,7 @@ mod tests { &chan.context.holder_signer.as_ref().pubkeys().funding_pubkey, chan.context.counterparty_funding_pubkey() ); - let (holder_sig, htlc_sigs) = signer.sign_holder_commitment_and_htlcs(&holder_commitment_tx, &secp_ctx).unwrap(); + let holder_sig = signer.sign_holder_commitment(&holder_commitment_tx, &secp_ctx).unwrap(); assert_eq!(Signature::from_der(&hex::decode($sig_hex).unwrap()[..]).unwrap(), holder_sig, "holder_sig"); let funding_redeemscript = chan.context.get_funding_redeemscript(); @@ -8317,14 +8318,14 @@ mod tests { assert_eq!(serialize(&tx)[..], hex::decode($tx_hex).unwrap()[..], "tx"); // ((htlc, counterparty_sig), (index, holder_sig)) - let mut htlc_sig_iter = holder_commitment_tx.htlcs().iter().zip(&holder_commitment_tx.counterparty_htlc_sigs).zip(htlc_sigs.iter().enumerate()); + let mut htlc_counterparty_sig_iter = holder_commitment_tx.counterparty_htlc_sigs.iter(); $({ log_trace!(logger, "verifying htlc {}", $htlc_idx); let remote_signature = Signature::from_der(&hex::decode($counterparty_htlc_sig_hex).unwrap()[..]).unwrap(); let ref htlc = htlcs[$htlc_idx]; - let htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw, + let mut htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw, chan.context.get_counterparty_selected_contest_delay().unwrap(), &htlc, $opt_anchors, &keys.broadcaster_delayed_payment_key, &keys.revocation_key); let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, $opt_anchors, &keys); @@ -8344,20 +8345,32 @@ mod tests { assert!(preimage.is_some()); } - let htlc_sig = htlc_sig_iter.next().unwrap(); + let htlc_counterparty_sig = htlc_counterparty_sig_iter.next().unwrap(); + let htlc_holder_sig = signer.sign_holder_htlc_transaction(&htlc_tx, 0, &HTLCDescriptor { + channel_derivation_parameters: ChannelDerivationParameters { + value_satoshis: chan.context.channel_value_satoshis, + keys_id: chan.context.channel_keys_id, + transaction_parameters: chan.context.channel_transaction_parameters.clone(), + }, + commitment_txid: trusted_tx.txid(), + per_commitment_number: trusted_tx.commitment_number(), + per_commitment_point: trusted_tx.per_commitment_point(), + feerate_per_kw: trusted_tx.feerate_per_kw(), + htlc: htlc.clone(), + preimage: preimage.clone(), + counterparty_sig: *htlc_counterparty_sig, + }, &secp_ctx).unwrap(); let num_anchors = if $opt_anchors.supports_anchors_zero_fee_htlc_tx() { 2 } else { 0 }; - assert_eq!((htlc_sig.0).0.transaction_output_index, Some($htlc_idx + num_anchors), "output index"); + assert_eq!(htlc.transaction_output_index, Some($htlc_idx + num_anchors), "output index"); let signature = Signature::from_der(&hex::decode($htlc_sig_hex).unwrap()[..]).unwrap(); - assert_eq!(signature, *(htlc_sig.1).1, "htlc sig"); - let index = (htlc_sig.1).0; - let channel_parameters = chan.context.channel_transaction_parameters.as_holder_broadcastable(); + assert_eq!(signature, htlc_holder_sig, "htlc sig"); let trusted_tx = holder_commitment_tx.trust(); - log_trace!(logger, "htlc_tx = {}", hex::encode(serialize(&trusted_tx.get_signed_htlc_tx(&channel_parameters, index, &(htlc_sig.0).1, (htlc_sig.1).1, &preimage)))); - assert_eq!(serialize(&trusted_tx.get_signed_htlc_tx(&channel_parameters, index, &(htlc_sig.0).1, (htlc_sig.1).1, &preimage))[..], - hex::decode($htlc_tx_hex).unwrap()[..], "htlc tx"); + htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness($htlc_idx, htlc_counterparty_sig, &htlc_holder_sig, &preimage); + log_trace!(logger, "htlc_tx = {}", hex::encode(serialize(&htlc_tx))); + assert_eq!(serialize(&htlc_tx)[..], hex::decode($htlc_tx_hex).unwrap()[..], "htlc tx"); })* - assert!(htlc_sig_iter.next().is_none()); + assert!(htlc_counterparty_sig_iter.next().is_none()); } } } diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 5be498e918b..37fdc647a4c 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -1680,7 +1680,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { // secret to the counterparty. However, because we always immediately take the revocation // secret from the keys_manager, we would panic at broadcast as we're trying to sign a // transaction which, from the point of view of our keys_manager, is revoked. - chanmon_cfgs[1].keys_manager.disable_revocation_policy_check = true; + chanmon_cfgs[0].keys_manager.disable_revocation_policy_check = true; let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let mut user_config = test_default_channel_config(); if anchors { diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index d9992585819..6abf4bd254c 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -487,31 +487,26 @@ pub trait EcdsaChannelSigner: ChannelSigner { /// This is required in order for the signer to make sure that the state has moved /// forward and it is safe to sign the next counterparty commitment. fn validate_counterparty_revocation(&self, idx: u64, secret: &SecretKey) -> Result<(), ()>; - /// Creates a signature for a holder's commitment transaction and its claiming HTLC transactions. + /// Creates a signature for a holder's commitment transaction. /// /// This will be called /// - with a non-revoked `commitment_tx`. /// - with the latest `commitment_tx` when we initiate a force-close. - /// - with the previous `commitment_tx`, just to get claiming HTLC - /// signatures, if we are reacting to a [`ChannelMonitor`] - /// [replica](https://github.com/lightningdevkit/rust-lightning/blob/main/GLOSSARY.md#monitor-replicas) - /// that decided to broadcast before it had been updated to the latest `commitment_tx`. /// /// This may be called multiple times for the same transaction. /// /// An external signer implementation should check that the commitment has not been revoked. - /// - /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor + // // TODO: Document the things someone using this interface should enforce before signing. - fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, - secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; - /// Same as [`sign_holder_commitment_and_htlcs`], but exists only for tests to get access to - /// holder commitment transactions which will be broadcasted later, after the channel has moved - /// on to a newer state. Thus, needs its own method as [`sign_holder_commitment_and_htlcs`] may - /// enforce that we only ever get called once. + fn sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, + secp_ctx: &Secp256k1) -> Result; + /// Same as [`sign_holder_commitment`], but exists only for tests to get access to holder + /// commitment transactions which will be broadcasted later, after the channel has moved on to a + /// newer state. Thus, needs its own method as [`sign_holder_commitment`] may enforce that we + /// only ever get called once. #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))] - fn unsafe_sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, - secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; + fn unsafe_sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, + secp_ctx: &Secp256k1) -> Result; /// Create a signature for the given input in a transaction spending an HTLC transaction output /// or a commitment transaction `to_local` output when our counterparty broadcasts an old state. /// @@ -554,7 +549,12 @@ pub trait EcdsaChannelSigner: ChannelSigner { /// `htlc_tx`, which spends the commitment transaction at index `input`. The signature returned /// must be be computed using [`EcdsaSighashType::All`]. /// + /// Note that this may be called for HTLCs in the penultimate commitment transaction if a + /// [`ChannelMonitor`] [replica](https://github.com/lightningdevkit/rust-lightning/blob/main/GLOSSARY.md#monitor-replicas) + /// broadcasts it before receiving the update for the latest commitment transaction. + /// /// [`EcdsaSighashType::All`]: bitcoin::blockdata::transaction::EcdsaSighashType::All + /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor fn sign_holder_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1 ) -> Result; @@ -1118,27 +1118,21 @@ impl EcdsaChannelSigner for InMemorySigner { Ok(()) } - fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key); let counterparty_keys = self.counterparty_pubkeys().expect(MISSING_PARAMS_ERR); let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_keys.funding_pubkey); let trusted_tx = commitment_tx.trust(); - let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx); - let channel_parameters = self.get_channel_parameters().expect(MISSING_PARAMS_ERR); - let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?; - Ok((sig, htlc_sigs)) + Ok(trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx)) } #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))] - fn unsafe_sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn unsafe_sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key); let counterparty_keys = self.counterparty_pubkeys().expect(MISSING_PARAMS_ERR); let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_keys.funding_pubkey); let trusted_tx = commitment_tx.trust(); - let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx); - let channel_parameters = self.get_channel_parameters().expect(MISSING_PARAMS_ERR); - let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?; - Ok((sig, htlc_sigs)) + Ok(trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx)) } fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1) -> Result { diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index c57c5a9d6fd..99240873f32 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -155,11 +155,8 @@ impl EcdsaChannelSigner for TestChannelSigner { Ok(()) } - fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { let trusted_tx = self.verify_holder_commitment_tx(commitment_tx, secp_ctx); - let commitment_txid = trusted_tx.txid(); - let holder_csv = self.inner.counterparty_selected_contest_delay().unwrap(); - let state = self.state.lock().unwrap(); let commitment_number = trusted_tx.commitment_number(); if state.last_holder_revoked_commitment - 1 != commitment_number && state.last_holder_revoked_commitment - 2 != commitment_number { @@ -168,33 +165,12 @@ impl EcdsaChannelSigner for TestChannelSigner { state.last_holder_revoked_commitment, commitment_number, self.inner.commitment_seed[0]) } } - - for (this_htlc, sig) in trusted_tx.htlcs().iter().zip(&commitment_tx.counterparty_htlc_sigs) { - assert!(this_htlc.transaction_output_index.is_some()); - let keys = trusted_tx.keys(); - let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, trusted_tx.feerate_per_kw(), holder_csv, &this_htlc, self.channel_type_features(), &keys.broadcaster_delayed_payment_key, &keys.revocation_key); - - let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&this_htlc, self.channel_type_features(), &keys); - - let sighash_type = if self.channel_type_features().supports_anchors_zero_fee_htlc_tx() { - EcdsaSighashType::SinglePlusAnyoneCanPay - } else { - EcdsaSighashType::All - }; - let sighash = hash_to_message!( - &sighash::SighashCache::new(&htlc_tx).segwit_signature_hash( - 0, &htlc_redeemscript, this_htlc.amount_msat / 1000, sighash_type, - ).unwrap()[..] - ); - secp_ctx.verify_ecdsa(&sighash, sig, &keys.countersignatory_htlc_key).unwrap(); - } - - Ok(self.inner.sign_holder_commitment_and_htlcs(commitment_tx, secp_ctx).unwrap()) + Ok(self.inner.sign_holder_commitment(commitment_tx, secp_ctx).unwrap()) } #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))] - fn unsafe_sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { - Ok(self.inner.unsafe_sign_holder_commitment_and_htlcs(commitment_tx, secp_ctx).unwrap()) + fn unsafe_sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { + Ok(self.inner.unsafe_sign_holder_commitment(commitment_tx, secp_ctx).unwrap()) } fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1) -> Result { @@ -209,8 +185,34 @@ impl EcdsaChannelSigner for TestChannelSigner { &self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1 ) -> Result { + let state = self.state.lock().unwrap(); + if state.last_holder_revoked_commitment - 1 != htlc_descriptor.per_commitment_number && + state.last_holder_revoked_commitment - 2 != htlc_descriptor.per_commitment_number + { + if !self.disable_revocation_policy_check { + panic!("can only sign the next two unrevoked commitment numbers, revoked={} vs requested={} for {}", + state.last_holder_revoked_commitment, htlc_descriptor.per_commitment_number, self.inner.commitment_seed[0]) + } + } assert_eq!(htlc_tx.input[input], htlc_descriptor.unsigned_tx_input()); assert_eq!(htlc_tx.output[input], htlc_descriptor.tx_output(secp_ctx)); + { + let witness_script = htlc_descriptor.witness_script(secp_ctx); + let sighash_type = if self.channel_type_features().supports_anchors_zero_fee_htlc_tx() { + EcdsaSighashType::SinglePlusAnyoneCanPay + } else { + EcdsaSighashType::All + }; + let sighash = &sighash::SighashCache::new(&*htlc_tx).segwit_signature_hash( + input, &witness_script, htlc_descriptor.htlc.amount_msat / 1000, sighash_type + ).unwrap(); + let countersignatory_htlc_key = chan_utils::derive_public_key( + &secp_ctx, &htlc_descriptor.per_commitment_point, &self.inner.counterparty_pubkeys().unwrap().htlc_basepoint + ); + secp_ctx.verify_ecdsa( + &hash_to_message!(&sighash), &htlc_descriptor.counterparty_sig, &countersignatory_htlc_key + ).unwrap(); + } Ok(self.inner.sign_holder_htlc_transaction(htlc_tx, input, htlc_descriptor, secp_ctx).unwrap()) } From b06a652e585735b3d37a795b03c0246026baa1e2 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 16 Oct 2023 12:21:52 -0700 Subject: [PATCH 6/6] Move HTLCDescriptor to sign module Now that `HTLCDescriptor` is no longer specific to anchors, it doesn't make sense for it to live in the `bump_transaction` module anymore. --- lightning/src/chain/channelmonitor.rs | 4 +- lightning/src/chain/onchaintx.rs | 4 +- lightning/src/events/bump_transaction.rs | 160 +--------------------- lightning/src/ln/channel.rs | 3 +- lightning/src/sign/mod.rs | 148 +++++++++++++++++++- lightning/src/util/test_channel_signer.rs | 2 +- 6 files changed, 158 insertions(+), 163 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index d855fdd819b..37c3383a3ad 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -42,7 +42,7 @@ use crate::chain; use crate::chain::{BestBlock, WatchedOutput}; use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator}; use crate::chain::transaction::{OutPoint, TransactionData}; -use crate::sign::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, WriteableEcdsaChannelSigner, SignerProvider, EntropySource}; +use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, WriteableEcdsaChannelSigner, SignerProvider, EntropySource}; use crate::chain::onchaintx::{ClaimEvent, OnchainTxHandler}; use crate::chain::package::{CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedOutput, RevokedHTLCOutput}; use crate::chain::Filter; @@ -50,7 +50,7 @@ use crate::util::logger::Logger; use crate::util::ser::{Readable, ReadableArgs, RequiredWrapper, MaybeReadable, UpgradableRequired, Writer, Writeable, U48}; use crate::util::byte_utils; use crate::events::{Event, EventHandler}; -use crate::events::bump_transaction::{ChannelDerivationParameters, AnchorDescriptor, HTLCDescriptor, BumpTransactionEvent}; +use crate::events::bump_transaction::{AnchorDescriptor, BumpTransactionEvent}; use crate::prelude::*; use core::{cmp, mem}; diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 95824f910db..e1949354079 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -23,15 +23,13 @@ use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; use bitcoin::secp256k1; use crate::chain::chaininterface::compute_feerate_sat_per_1000_weight; -use crate::events::bump_transaction::{ChannelDerivationParameters, HTLCDescriptor}; -use crate::sign::{ChannelSigner, EntropySource, SignerProvider}; +use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, ChannelSigner, EntropySource, SignerProvider, WriteableEcdsaChannelSigner}; use crate::ln::msgs::DecodeError; use crate::ln::PaymentPreimage; use crate::ln::chan_utils::{self, ChannelTransactionParameters, HTLCOutputInCommitment, HolderCommitmentTransaction}; use crate::chain::ClaimId; use crate::chain::chaininterface::{ConfirmationTarget, FeeEstimator, BroadcasterInterface, LowerBoundedFeeEstimator}; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER}; -use crate::sign::WriteableEcdsaChannelSigner; use crate::chain::package::{PackageSolvingData, PackageTemplate}; use crate::util::logger::Logger; use crate::util::ser::{Readable, ReadableArgs, MaybeReadable, UpgradableRequired, Writer, Writeable, VecWriter}; diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 5d5161a67bb..1fd533d2fd2 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -21,19 +21,21 @@ use crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI; use crate::ln::chan_utils; use crate::ln::chan_utils::{ ANCHOR_INPUT_WITNESS_WEIGHT, HTLC_SUCCESS_INPUT_ANCHOR_WITNESS_WEIGHT, - HTLC_TIMEOUT_INPUT_ANCHOR_WITNESS_WEIGHT, ChannelTransactionParameters, HTLCOutputInCommitment + HTLC_TIMEOUT_INPUT_ANCHOR_WITNESS_WEIGHT, HTLCOutputInCommitment }; -use crate::ln::PaymentPreimage; use crate::prelude::*; -use crate::sign::{EcdsaChannelSigner, SignerProvider, WriteableEcdsaChannelSigner, P2WPKH_WITNESS_WEIGHT}; +use crate::sign::{ + ChannelDerivationParameters, HTLCDescriptor, EcdsaChannelSigner, SignerProvider, + WriteableEcdsaChannelSigner, P2WPKH_WITNESS_WEIGHT +}; use crate::sync::Mutex; use crate::util::logger::Logger; -use bitcoin::{OutPoint, PackedLockTime, PubkeyHash, Sequence, Script, Transaction, Txid, TxIn, TxOut, Witness, WPubkeyHash}; +use bitcoin::{OutPoint, PackedLockTime, PubkeyHash, Sequence, Script, Transaction, TxIn, TxOut, Witness, WPubkeyHash}; use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; use bitcoin::consensus::Encodable; use bitcoin::secp256k1; -use bitcoin::secp256k1::{PublicKey, Secp256k1}; +use bitcoin::secp256k1::Secp256k1; use bitcoin::secp256k1::ecdsa::Signature; const EMPTY_SCRIPT_SIG_WEIGHT: u64 = 1 /* empty script_sig */ * WITNESS_SCALE_FACTOR as u64; @@ -42,26 +44,6 @@ const BASE_INPUT_SIZE: u64 = 32 /* txid */ + 4 /* vout */ + 4 /* sequence */; const BASE_INPUT_WEIGHT: u64 = BASE_INPUT_SIZE * WITNESS_SCALE_FACTOR as u64; -/// The parameters required to derive a channel signer via [`SignerProvider`]. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct ChannelDerivationParameters { - /// The value in satoshis of the channel we're attempting to spend the anchor output of. - pub value_satoshis: u64, - /// The unique identifier to re-derive the signer for the associated channel. - pub keys_id: [u8; 32], - /// The necessary channel parameters that need to be provided to the re-derived signer through - /// [`ChannelSigner::provide_channel_parameters`]. - /// - /// [`ChannelSigner::provide_channel_parameters`]: crate::sign::ChannelSigner::provide_channel_parameters - pub transaction_parameters: ChannelTransactionParameters, -} - -impl_writeable_tlv_based!(ChannelDerivationParameters, { - (0, value_satoshis, required), - (2, keys_id, required), - (4, transaction_parameters, required), -}); - /// A descriptor used to sign for a commitment transaction's anchor output. #[derive(Clone, Debug, PartialEq, Eq)] pub struct AnchorDescriptor { @@ -120,133 +102,6 @@ impl AnchorDescriptor { } } -/// A descriptor used to sign for a commitment transaction's HTLC output. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct HTLCDescriptor { - /// The parameters required to derive the signer for the HTLC input. - pub channel_derivation_parameters: ChannelDerivationParameters, - /// The txid of the commitment transaction in which the HTLC output lives. - pub commitment_txid: Txid, - /// The number of the commitment transaction in which the HTLC output lives. - pub per_commitment_number: u64, - /// The key tweak corresponding to the number of the commitment transaction in which the HTLC - /// output lives. This tweak is applied to all the basepoints for both parties in the channel to - /// arrive at unique keys per commitment. - /// - /// See for more info. - pub per_commitment_point: PublicKey, - /// The feerate to use on the HTLC claiming transaction. This is always `0` for HTLCs - /// originating from a channel supporting anchor outputs, otherwise it is the channel's - /// negotiated feerate at the time the commitment transaction was built. - pub feerate_per_kw: u32, - /// The details of the HTLC as it appears in the commitment transaction. - pub htlc: HTLCOutputInCommitment, - /// The preimage, if `Some`, to claim the HTLC output with. If `None`, the timeout path must be - /// taken. - pub preimage: Option, - /// The counterparty's signature required to spend the HTLC output. - pub counterparty_sig: Signature -} - -impl_writeable_tlv_based!(HTLCDescriptor, { - (0, channel_derivation_parameters, required), - (1, feerate_per_kw, (default_value, 0)), - (2, commitment_txid, required), - (4, per_commitment_number, required), - (6, per_commitment_point, required), - (8, htlc, required), - (10, preimage, option), - (12, counterparty_sig, required), -}); - -impl HTLCDescriptor { - /// Returns the outpoint of the HTLC output in the commitment transaction. This is the outpoint - /// being spent by the HTLC input in the HTLC transaction. - pub fn outpoint(&self) -> OutPoint { - OutPoint { - txid: self.commitment_txid, - vout: self.htlc.transaction_output_index.unwrap(), - } - } - - /// Returns the UTXO to be spent by the HTLC input, which can be obtained via - /// [`Self::unsigned_tx_input`]. - pub fn previous_utxo(&self, secp: &Secp256k1) -> TxOut { - TxOut { - script_pubkey: self.witness_script(secp).to_v0_p2wsh(), - value: self.htlc.amount_msat / 1000, - } - } - - /// Returns the unsigned transaction input spending the HTLC output in the commitment - /// transaction. - pub fn unsigned_tx_input(&self) -> TxIn { - chan_utils::build_htlc_input( - &self.commitment_txid, &self.htlc, &self.channel_derivation_parameters.transaction_parameters.channel_type_features - ) - } - - /// Returns the delayed output created as a result of spending the HTLC output in the commitment - /// transaction. - pub fn tx_output(&self, secp: &Secp256k1) -> TxOut { - let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); - let broadcaster_keys = channel_params.broadcaster_pubkeys(); - let counterparty_keys = channel_params.countersignatory_pubkeys(); - let broadcaster_delayed_key = chan_utils::derive_public_key( - secp, &self.per_commitment_point, &broadcaster_keys.delayed_payment_basepoint - ); - let counterparty_revocation_key = chan_utils::derive_public_revocation_key( - secp, &self.per_commitment_point, &counterparty_keys.revocation_basepoint - ); - chan_utils::build_htlc_output( - self.feerate_per_kw, channel_params.contest_delay(), &self.htlc, - channel_params.channel_type_features(), &broadcaster_delayed_key, &counterparty_revocation_key - ) - } - - /// Returns the witness script of the HTLC output in the commitment transaction. - pub fn witness_script(&self, secp: &Secp256k1) -> Script { - let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); - let broadcaster_keys = channel_params.broadcaster_pubkeys(); - let counterparty_keys = channel_params.countersignatory_pubkeys(); - let broadcaster_htlc_key = chan_utils::derive_public_key( - secp, &self.per_commitment_point, &broadcaster_keys.htlc_basepoint - ); - let counterparty_htlc_key = chan_utils::derive_public_key( - secp, &self.per_commitment_point, &counterparty_keys.htlc_basepoint - ); - let counterparty_revocation_key = chan_utils::derive_public_revocation_key( - secp, &self.per_commitment_point, &counterparty_keys.revocation_basepoint - ); - chan_utils::get_htlc_redeemscript_with_explicit_keys( - &self.htlc, channel_params.channel_type_features(), &broadcaster_htlc_key, &counterparty_htlc_key, - &counterparty_revocation_key, - ) - } - - /// Returns the fully signed witness required to spend the HTLC output in the commitment - /// transaction. - pub fn tx_input_witness(&self, signature: &Signature, witness_script: &Script) -> Witness { - chan_utils::build_htlc_input_witness( - signature, &self.counterparty_sig, &self.preimage, witness_script, - &self.channel_derivation_parameters.transaction_parameters.channel_type_features - ) - } - - /// Derives the channel signer required to sign the HTLC input. - pub fn derive_channel_signer(&self, signer_provider: &SP) -> S - where - SP::Target: SignerProvider - { - let mut signer = signer_provider.derive_channel_signer( - self.channel_derivation_parameters.value_satoshis, - self.channel_derivation_parameters.keys_id, - ); - signer.provide_channel_parameters(&self.channel_derivation_parameters.transaction_parameters); - signer - } -} - /// Represents the different types of transactions, originating from LDK, to be bumped. #[derive(Clone, Debug, PartialEq, Eq)] pub enum BumpTransactionEvent { @@ -342,7 +197,6 @@ pub enum BumpTransactionEvent { /// /// [`EcdsaChannelSigner`]: crate::sign::EcdsaChannelSigner /// [`EcdsaChannelSigner::sign_holder_htlc_transaction`]: crate::sign::EcdsaChannelSigner::sign_holder_htlc_transaction - /// [`HTLCDescriptor::tx_input_witness`]: HTLCDescriptor::tx_input_witness HTLCResolution { /// The unique identifier for the claim of the HTLCs in the confirmed commitment /// transaction. diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 7b14573a1a4..10ce637701c 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8184,8 +8184,7 @@ mod tests { use bitcoin::hashes::hex::FromHex; use bitcoin::hash_types::Txid; use bitcoin::secp256k1::Message; - use crate::events::bump_transaction::{ChannelDerivationParameters, HTLCDescriptor}; - use crate::sign::EcdsaChannelSigner; + use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, EcdsaChannelSigner}; use crate::ln::PaymentPreimage; use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys}; use crate::ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters}; diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 6abf4bd254c..5912b7b84e1 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -30,13 +30,12 @@ use bitcoin::secp256k1::{KeyPair, PublicKey, Scalar, Secp256k1, SecretKey, Signi use bitcoin::secp256k1::ecdh::SharedSecret; use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature}; use bitcoin::secp256k1::schnorr; -use bitcoin::{PackedLockTime, secp256k1, Sequence, Witness}; +use bitcoin::{PackedLockTime, secp256k1, Sequence, Witness, Txid}; use crate::util::transaction_utils; use crate::util::crypto::{hkdf_extract_expand_twice, sign, sign_with_aux_rand}; use crate::util::ser::{Writeable, Writer, Readable, ReadableArgs}; use crate::chain::transaction::OutPoint; -use crate::events::bump_transaction::HTLCDescriptor; use crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI; use crate::ln::{chan_utils, PaymentPreimage}; use crate::ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, ChannelTransactionParameters, CommitmentTransaction, ClosingTransaction}; @@ -401,6 +400,151 @@ impl SpendableOutputDescriptor { } } +/// The parameters required to derive a channel signer via [`SignerProvider`]. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ChannelDerivationParameters { + /// The value in satoshis of the channel we're attempting to spend the anchor output of. + pub value_satoshis: u64, + /// The unique identifier to re-derive the signer for the associated channel. + pub keys_id: [u8; 32], + /// The necessary channel parameters that need to be provided to the re-derived signer through + /// [`ChannelSigner::provide_channel_parameters`]. + pub transaction_parameters: ChannelTransactionParameters, +} + +impl_writeable_tlv_based!(ChannelDerivationParameters, { + (0, value_satoshis, required), + (2, keys_id, required), + (4, transaction_parameters, required), +}); + +/// A descriptor used to sign for a commitment transaction's HTLC output. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct HTLCDescriptor { + /// The parameters required to derive the signer for the HTLC input. + pub channel_derivation_parameters: ChannelDerivationParameters, + /// The txid of the commitment transaction in which the HTLC output lives. + pub commitment_txid: Txid, + /// The number of the commitment transaction in which the HTLC output lives. + pub per_commitment_number: u64, + /// The key tweak corresponding to the number of the commitment transaction in which the HTLC + /// output lives. This tweak is applied to all the basepoints for both parties in the channel to + /// arrive at unique keys per commitment. + /// + /// See for more info. + pub per_commitment_point: PublicKey, + /// The feerate to use on the HTLC claiming transaction. This is always `0` for HTLCs + /// originating from a channel supporting anchor outputs, otherwise it is the channel's + /// negotiated feerate at the time the commitment transaction was built. + pub feerate_per_kw: u32, + /// The details of the HTLC as it appears in the commitment transaction. + pub htlc: HTLCOutputInCommitment, + /// The preimage, if `Some`, to claim the HTLC output with. If `None`, the timeout path must be + /// taken. + pub preimage: Option, + /// The counterparty's signature required to spend the HTLC output. + pub counterparty_sig: Signature +} + +impl_writeable_tlv_based!(HTLCDescriptor, { + (0, channel_derivation_parameters, required), + (1, feerate_per_kw, (default_value, 0)), + (2, commitment_txid, required), + (4, per_commitment_number, required), + (6, per_commitment_point, required), + (8, htlc, required), + (10, preimage, option), + (12, counterparty_sig, required), +}); + +impl HTLCDescriptor { + /// Returns the outpoint of the HTLC output in the commitment transaction. This is the outpoint + /// being spent by the HTLC input in the HTLC transaction. + pub fn outpoint(&self) -> bitcoin::OutPoint { + bitcoin::OutPoint { + txid: self.commitment_txid, + vout: self.htlc.transaction_output_index.unwrap(), + } + } + + /// Returns the UTXO to be spent by the HTLC input, which can be obtained via + /// [`Self::unsigned_tx_input`]. + pub fn previous_utxo(&self, secp: &Secp256k1) -> TxOut { + TxOut { + script_pubkey: self.witness_script(secp).to_v0_p2wsh(), + value: self.htlc.amount_msat / 1000, + } + } + + /// Returns the unsigned transaction input spending the HTLC output in the commitment + /// transaction. + pub fn unsigned_tx_input(&self) -> TxIn { + chan_utils::build_htlc_input( + &self.commitment_txid, &self.htlc, &self.channel_derivation_parameters.transaction_parameters.channel_type_features + ) + } + + /// Returns the delayed output created as a result of spending the HTLC output in the commitment + /// transaction. + pub fn tx_output(&self, secp: &Secp256k1) -> TxOut { + let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); + let broadcaster_keys = channel_params.broadcaster_pubkeys(); + let counterparty_keys = channel_params.countersignatory_pubkeys(); + let broadcaster_delayed_key = chan_utils::derive_public_key( + secp, &self.per_commitment_point, &broadcaster_keys.delayed_payment_basepoint + ); + let counterparty_revocation_key = chan_utils::derive_public_revocation_key( + secp, &self.per_commitment_point, &counterparty_keys.revocation_basepoint + ); + chan_utils::build_htlc_output( + self.feerate_per_kw, channel_params.contest_delay(), &self.htlc, + channel_params.channel_type_features(), &broadcaster_delayed_key, &counterparty_revocation_key + ) + } + + /// Returns the witness script of the HTLC output in the commitment transaction. + pub fn witness_script(&self, secp: &Secp256k1) -> Script { + let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); + let broadcaster_keys = channel_params.broadcaster_pubkeys(); + let counterparty_keys = channel_params.countersignatory_pubkeys(); + let broadcaster_htlc_key = chan_utils::derive_public_key( + secp, &self.per_commitment_point, &broadcaster_keys.htlc_basepoint + ); + let counterparty_htlc_key = chan_utils::derive_public_key( + secp, &self.per_commitment_point, &counterparty_keys.htlc_basepoint + ); + let counterparty_revocation_key = chan_utils::derive_public_revocation_key( + secp, &self.per_commitment_point, &counterparty_keys.revocation_basepoint + ); + chan_utils::get_htlc_redeemscript_with_explicit_keys( + &self.htlc, channel_params.channel_type_features(), &broadcaster_htlc_key, &counterparty_htlc_key, + &counterparty_revocation_key, + ) + } + + /// Returns the fully signed witness required to spend the HTLC output in the commitment + /// transaction. + pub fn tx_input_witness(&self, signature: &Signature, witness_script: &Script) -> Witness { + chan_utils::build_htlc_input_witness( + signature, &self.counterparty_sig, &self.preimage, witness_script, + &self.channel_derivation_parameters.transaction_parameters.channel_type_features + ) + } + + /// Derives the channel signer required to sign the HTLC input. + pub fn derive_channel_signer(&self, signer_provider: &SP) -> S + where + SP::Target: SignerProvider + { + let mut signer = signer_provider.derive_channel_signer( + self.channel_derivation_parameters.value_satoshis, + self.channel_derivation_parameters.keys_id, + ); + signer.provide_channel_parameters(&self.channel_derivation_parameters.transaction_parameters); + signer + } +} + /// A trait to handle Lightning channel key material without concretizing the channel type or /// the signature mechanism. pub trait ChannelSigner { diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index 99240873f32..942671cf46b 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -23,7 +23,7 @@ use bitcoin::util::sighash; use bitcoin::secp256k1; use bitcoin::secp256k1::{SecretKey, PublicKey}; use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; -use crate::events::bump_transaction::HTLCDescriptor; +use crate::sign::HTLCDescriptor; use crate::util::ser::{Writeable, Writer}; use crate::io::Error; use crate::ln::features::ChannelTypeFeatures;