diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index bd0c1548428..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}; @@ -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())]); @@ -2835,6 +2836,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, @@ -3354,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 5daa2463de9..e1949354079 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -23,14 +23,13 @@ use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; use bitcoin::secp256k1; use crate::chain::chaininterface::compute_feerate_sat_per_1000_weight; -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}; @@ -215,14 +214,11 @@ 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 - // 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, @@ -276,11 +272,11 @@ 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 && - 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 && @@ -298,9 +294,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)?; @@ -355,9 +351,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)?; @@ -418,11 +414,11 @@ 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, prev_holder_commitment, - prev_holder_htlc_sigs, signer, channel_transaction_parameters: channel_parameters, claimable_outpoints, @@ -436,13 +432,17 @@ 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, prev_holder_commitment: None, - prev_holder_htlc_sigs: None, signer, channel_transaction_parameters: channel_parameters, pending_claim_requests: HashMap::new(), @@ -1088,39 +1088,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 { @@ -1132,48 +1099,54 @@ 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(&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(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment"); self.holder_commitment.add_holder_sig(funding_redeemscript, sig) } 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( @@ -1209,18 +1182,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 - } } diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 55c12d23e74..1fd533d2fd2 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -21,20 +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::features::ChannelTypeFeatures; -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; @@ -43,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 { @@ -121,125 +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 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), - (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, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()) - } - - /// 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( - 0 /* feerate_per_kw */, channel_params.contest_delay(), &self.htlc, - &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &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, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &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, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() /* opt_anchors */ - ) - } - - /// 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 { @@ -335,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/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/channel.rs b/lightning/src/ln/channel.rs index a61a8de82de..10ce637701c 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8184,7 +8184,7 @@ mod tests { use bitcoin::hashes::hex::FromHex; use bitcoin::hash_types::Txid; use bitcoin::secp256k1::Message; - 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}; @@ -8309,7 +8309,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 +8317,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 +8344,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 0399ed38251..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 { @@ -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); +} diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 04c4446e2c0..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 { @@ -487,31 +631,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. /// @@ -552,11 +691,14 @@ 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`]. + /// + /// 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; @@ -1120,27 +1262,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..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; @@ -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()) }