From a100ed0098702bbdf1089a3d160046e1785877ba Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Sun, 2 Jul 2023 13:44:53 -0700 Subject: [PATCH 1/6] Accept BumpTransactionEvent in handle_event There's no reason to accept the general `Event` enum. --- lightning/src/events/bump_transaction.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 6d8a5bb6c65..91a069324dd 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -7,14 +7,15 @@ // You may not use this file except in accordance with one or both of these // licenses. -//! Utitilies for bumping transactions originating from [`super::Event`]s. +//! Utilities for bumping transactions originating from [`Event`]s. +//! +//! [`Event`]: crate::events::Event use core::convert::TryInto; use core::ops::Deref; use crate::chain::chaininterface::BroadcasterInterface; use crate::chain::ClaimId; -use crate::events::Event; use crate::io_extras::sink; use crate::ln::chan_utils; use crate::ln::chan_utils::{ @@ -553,6 +554,8 @@ impl CoinSelectionSource for Wallet where W::Target: WalletSource { /// A handler for [`Event::BumpTransaction`] events that sources confirmed UTXOs from a /// [`CoinSelectionSource`] to fee bump transactions via Child-Pays-For-Parent (CPFP) or /// Replace-By-Fee (RBF). +/// +/// [`Event::BumpTransaction`]: crate::events::Event::BumpTransaction pub struct BumpTransactionEventHandler where B::Target: BroadcasterInterface, @@ -575,6 +578,8 @@ where L::Target: Logger, { /// Returns a new instance capable of handling [`Event::BumpTransaction`] events. + /// + /// [`Event::BumpTransaction`]: crate::events::Event::BumpTransaction pub fn new(broadcaster: B, utxo_source: C, signer_provider: SP, logger: L) -> Self { Self { broadcaster, @@ -749,13 +754,8 @@ where Ok(()) } - /// Handles all variants of [`BumpTransactionEvent`], immediately returning otherwise. - pub fn handle_event(&self, event: &Event) { - let event = if let Event::BumpTransaction(event) = event { - event - } else { - return; - }; + /// Handles all variants of [`BumpTransactionEvent`]. + pub fn handle_event(&self, event: &BumpTransactionEvent) { match event { BumpTransactionEvent::ChannelClose { claim_id, package_target_feerate_sat_per_1000_weight, commitment_tx, From ae701a0d20fc5c7db925b88d4a8bcb4e6b16fdc2 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Sun, 2 Jul 2023 13:45:26 -0700 Subject: [PATCH 2/6] Expose CoinSelection struct members These are meant to be provided by the user, so they need to be exposed in the API. --- lightning/src/events/bump_transaction.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 91a069324dd..edb146ba215 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -361,12 +361,12 @@ impl Utxo { pub struct CoinSelection { /// The set of UTXOs (with at least 1 confirmation) to spend and use within a transaction /// requiring additional fees. - confirmed_utxos: Vec, + pub confirmed_utxos: Vec, /// An additional output tracking whether any change remained after coin selection. This output /// should always have a value above dust for its given `script_pubkey`. It should not be /// spent until the transaction it belongs to confirms to ensure mempool descendant limits are /// not met. This implies no other party should be able to spend it except us. - change_output: Option, + pub change_output: Option, } /// An abstraction over a bitcoin wallet that can perform coin selection over a set of UTXOs and can From 72c42ee786d11ea1913d2898069f2c6b15285a2a Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 6 Jul 2023 10:04:50 -0700 Subject: [PATCH 3/6] Cache HTLC per_commitment_point in descriptor This allows us to obtain the HTLC input and output from its descriptor without needing to derive the `per_commitment_point` through the signer. --- lightning/src/chain/channelmonitor.rs | 3 ++ lightning/src/events/bump_transaction.rs | 36 +++++++++------------ lightning/src/ln/monitor_tests.rs | 27 ++++++---------- lightning/src/sign/mod.rs | 7 ++-- lightning/src/util/enforcing_trait_impls.rs | 3 +- 5 files changed, 31 insertions(+), 45 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 36e3bc46bfc..1bc160354ba 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2632,6 +2632,9 @@ impl ChannelMonitorImpl { channel_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(), commitment_txid: htlc.commitment_txid, per_commitment_number: htlc.per_commitment_number, + per_commitment_point: self.onchain_tx_handler.signer.get_per_commitment_point( + htlc.per_commitment_number, &self.onchain_tx_handler.secp_ctx, + ), 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 edb146ba215..0e3152d7b0a 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -93,6 +93,12 @@ pub struct HTLCDescriptor { 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 @@ -111,17 +117,15 @@ impl HTLCDescriptor { /// Returns the delayed output created as a result of spending the HTLC output in the commitment /// transaction. - pub fn tx_output( - &self, per_commitment_point: &PublicKey, secp: &Secp256k1 - ) -> TxOut { + pub fn tx_output(&self, secp: &Secp256k1) -> TxOut { let channel_params = self.channel_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, per_commitment_point, &broadcaster_keys.delayed_payment_basepoint + secp, &self.per_commitment_point, &broadcaster_keys.delayed_payment_basepoint ); let counterparty_revocation_key = chan_utils::derive_public_revocation_key( - secp, per_commitment_point, &counterparty_keys.revocation_basepoint + secp, &self.per_commitment_point, &counterparty_keys.revocation_basepoint ); chan_utils::build_htlc_output( 0 /* feerate_per_kw */, channel_params.contest_delay(), &self.htlc, @@ -130,20 +134,18 @@ impl HTLCDescriptor { } /// Returns the witness script of the HTLC output in the commitment transaction. - pub fn witness_script( - &self, per_commitment_point: &PublicKey, secp: &Secp256k1 - ) -> Script { + pub fn witness_script(&self, secp: &Secp256k1) -> Script { let channel_params = self.channel_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, per_commitment_point, &broadcaster_keys.htlc_basepoint + secp, &self.per_commitment_point, &broadcaster_keys.htlc_basepoint ); let counterparty_htlc_key = chan_utils::derive_public_key( - secp, per_commitment_point, &counterparty_keys.htlc_basepoint + secp, &self.per_commitment_point, &counterparty_keys.htlc_basepoint ); let counterparty_revocation_key = chan_utils::derive_public_revocation_key( - secp, per_commitment_point, &counterparty_keys.revocation_basepoint + 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, @@ -696,15 +698,12 @@ where let mut signers = HashMap::new(); let mut must_spend = Vec::with_capacity(htlc_descriptors.len()); for htlc_descriptor in htlc_descriptors { - let signer = signers.entry(htlc_descriptor.channel_keys_id) + signers.entry(htlc_descriptor.channel_keys_id) .or_insert_with(|| self.signer_provider.derive_channel_signer( htlc_descriptor.channel_value_satoshis, htlc_descriptor.channel_keys_id, ) ); - let per_commitment_point = signer.get_per_commitment_point( - htlc_descriptor.per_commitment_number, &self.secp - ); let htlc_input = htlc_descriptor.unsigned_tx_input(); must_spend.push(Input { @@ -716,7 +715,7 @@ where }, }); tx.input.push(htlc_input); - let htlc_output = htlc_descriptor.tx_output(&per_commitment_point, &self.secp); + let htlc_output = htlc_descriptor.tx_output(&self.secp); tx.output.push(htlc_output); } @@ -743,10 +742,7 @@ where let htlc_sig = signer.sign_holder_htlc_transaction( &htlc_tx, idx, htlc_descriptor, &self.secp )?; - let per_commitment_point = signer.get_per_commitment_point( - htlc_descriptor.per_commitment_number, &self.secp - ); - let witness_script = htlc_descriptor.witness_script(&per_commitment_point, &self.secp); + let witness_script = htlc_descriptor.witness_script(&self.secp); htlc_tx.input[idx].witness = htlc_descriptor.tx_input_witness(&htlc_sig, &witness_script); } diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 2a3f5849b95..4d37f936970 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -1781,12 +1781,9 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) { let signer = nodes[0].keys_manager.derive_channel_keys( descriptor.channel_value_satoshis, &descriptor.channel_keys_id, ); - let per_commitment_point = signer.get_per_commitment_point( - descriptor.per_commitment_number, &secp - ); - tx.output.push(descriptor.tx_output(&per_commitment_point, &secp)); + tx.output.push(descriptor.tx_output(&secp)); let our_sig = signer.sign_holder_htlc_transaction(&mut tx, 0, &descriptor, &secp).unwrap(); - let witness_script = descriptor.witness_script(&per_commitment_point, &secp); + let witness_script = descriptor.witness_script(&secp); tx.input[0].witness = descriptor.tx_input_witness(&our_sig, &witness_script); target_feerate_sat_per_1000_weight as u64 } else { panic!("unexpected event"); }; @@ -1943,10 +1940,6 @@ fn test_yield_anchors_events() { Event::BumpTransaction(BumpTransactionEvent::HTLCResolution { htlc_descriptors, tx_lock_time, .. }) => { assert_eq!(htlc_descriptors.len(), 1); let htlc_descriptor = &htlc_descriptors[0]; - let signer = nodes[0].keys_manager.derive_channel_keys( - htlc_descriptor.channel_value_satoshis, &htlc_descriptor.channel_keys_id - ); - let per_commitment_point = signer.get_per_commitment_point(htlc_descriptor.per_commitment_number, &secp); let mut htlc_tx = Transaction { version: 2, lock_time: tx_lock_time, @@ -1955,15 +1948,18 @@ fn test_yield_anchors_events() { TxIn { ..Default::default() } // Fee input ], output: vec![ - htlc_descriptor.tx_output(&per_commitment_point, &secp), // HTLC output + htlc_descriptor.tx_output(&secp), // HTLC output TxOut { // Fee input change value: Amount::ONE_BTC.to_sat(), script_pubkey: Script::new_op_return(&[]), } ] }; + let signer = nodes[0].keys_manager.derive_channel_keys( + htlc_descriptor.channel_value_satoshis, &htlc_descriptor.channel_keys_id + ); let our_sig = signer.sign_holder_htlc_transaction(&mut htlc_tx, 0, htlc_descriptor, &secp).unwrap(); - let witness_script = htlc_descriptor.witness_script(&per_commitment_point, &secp); + let witness_script = htlc_descriptor.witness_script(&secp); htlc_tx.input[0].witness = htlc_descriptor.tx_input_witness(&our_sig, &witness_script); htlc_txs.push(htlc_tx); }, @@ -2227,12 +2223,8 @@ fn test_anchors_aggregated_revoked_htlc_tx() { assert_eq!(htlc_descriptors.len(), 2); for htlc_descriptor in &htlc_descriptors { assert!(!htlc_descriptor.htlc.offered); - let signer = nodes[1].keys_manager.derive_channel_keys( - htlc_descriptor.channel_value_satoshis, &htlc_descriptor.channel_keys_id - ); - let per_commitment_point = signer.get_per_commitment_point(htlc_descriptor.per_commitment_number, &secp); htlc_tx.input.push(htlc_descriptor.unsigned_tx_input()); - htlc_tx.output.push(htlc_descriptor.tx_output(&per_commitment_point, &secp)); + htlc_tx.output.push(htlc_descriptor.tx_output(&secp)); } descriptors.append(&mut htlc_descriptors); htlc_tx.lock_time = tx_lock_time; @@ -2246,8 +2238,7 @@ fn test_anchors_aggregated_revoked_htlc_tx() { htlc_descriptor.channel_value_satoshis, &htlc_descriptor.channel_keys_id ); let our_sig = signer.sign_holder_htlc_transaction(&htlc_tx, htlc_input_idx, &htlc_descriptor, &secp).unwrap(); - let per_commitment_point = signer.get_per_commitment_point(htlc_descriptor.per_commitment_number, &secp); - let witness_script = htlc_descriptor.witness_script(&per_commitment_point, &secp); + let witness_script = htlc_descriptor.witness_script(&secp); htlc_tx.input[htlc_input_idx].witness = htlc_descriptor.tx_input_witness(&our_sig, &witness_script); } let fee_utxo_sig = { diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 66217de28ac..c11d47e9ff9 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -1030,15 +1030,12 @@ impl EcdsaChannelSigner for InMemorySigner { &self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1 ) -> Result { - let per_commitment_point = self.get_per_commitment_point( - htlc_descriptor.per_commitment_number, &secp_ctx - ); - let witness_script = htlc_descriptor.witness_script(&per_commitment_point, secp_ctx); + let witness_script = htlc_descriptor.witness_script(secp_ctx); let sighash = &sighash::SighashCache::new(&*htlc_tx).segwit_signature_hash( input, &witness_script, htlc_descriptor.htlc.amount_msat / 1000, EcdsaSighashType::All ).map_err(|_| ())?; let our_htlc_private_key = chan_utils::derive_private_key( - &secp_ctx, &per_commitment_point, &self.htlc_base_key + &secp_ctx, &htlc_descriptor.per_commitment_point, &self.htlc_base_key ); Ok(sign_with_aux_rand(&secp_ctx, &hash_to_message!(sighash), &our_htlc_private_key, &self)) } diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index a9a6e8e0933..df0f13bc3ad 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -209,9 +209,8 @@ impl EcdsaChannelSigner for EnforcingSigner { &self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1 ) -> Result { - let per_commitment_point = self.get_per_commitment_point(htlc_descriptor.per_commitment_number, secp_ctx); assert_eq!(htlc_tx.input[input], htlc_descriptor.unsigned_tx_input()); - assert_eq!(htlc_tx.output[input], htlc_descriptor.tx_output(&per_commitment_point, secp_ctx)); + assert_eq!(htlc_tx.output[input], htlc_descriptor.tx_output(secp_ctx)); Ok(self.inner.sign_holder_htlc_transaction(htlc_tx, input, htlc_descriptor, secp_ctx).unwrap()) } From 690ad18b22fbc1917b77ae5ef820a9a7fd967020 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 6 Jul 2023 10:05:34 -0700 Subject: [PATCH 4/6] Provide missing post-derivation signer parameters Users may expect these to be provided manually after derivation in the event they need to perform any enforcement prior to signing. --- lightning/src/chain/channelmonitor.rs | 17 ++- lightning/src/events/bump_transaction.rs | 134 ++++++++++++----------- lightning/src/ln/monitor_tests.rs | 20 +--- 3 files changed, 85 insertions(+), 86 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 1bc160354ba..be8dcd90443 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -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::{AnchorDescriptor, HTLCDescriptor, BumpTransactionEvent}; +use crate::events::bump_transaction::{ChannelDerivationParameters, AnchorDescriptor, HTLCDescriptor, BumpTransactionEvent}; use crate::prelude::*; use core::{cmp, mem}; @@ -2611,8 +2611,11 @@ impl ChannelMonitorImpl { commitment_tx, commitment_tx_fee_satoshis, anchor_descriptor: AnchorDescriptor { - channel_keys_id: self.channel_keys_id, - channel_value_satoshis: self.channel_value_satoshis, + channel_derivation_parameters: ChannelDerivationParameters { + keys_id: self.channel_keys_id, + value_satoshis: self.channel_value_satoshis, + transaction_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(), + }, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: anchor_output_idx, @@ -2627,9 +2630,11 @@ impl ChannelMonitorImpl { let mut htlc_descriptors = Vec::with_capacity(htlcs.len()); for htlc in htlcs { htlc_descriptors.push(HTLCDescriptor { - channel_keys_id: self.channel_keys_id, - channel_value_satoshis: self.channel_value_satoshis, - channel_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(), + channel_derivation_parameters: ChannelDerivationParameters { + keys_id: self.channel_keys_id, + value_satoshis: self.channel_value_satoshis, + transaction_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(), + }, commitment_txid: htlc.commitment_txid, per_commitment_number: htlc.per_commitment_number, per_commitment_point: self.onchain_tx_handler.signer.get_per_commitment_point( diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 0e3152d7b0a..5ea9bdb8e29 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -11,6 +11,7 @@ //! //! [`Event`]: crate::events::Event +use alloc::collections::BTreeMap; use core::convert::TryInto; use core::ops::Deref; @@ -50,45 +51,50 @@ const fn fee_for_weight(feerate_sat_per_1000_weight: u32, weight: u64) -> u64 { ((feerate_sat_per_1000_weight as u64 * weight) + 1000 - 1) / 1000 } +/// 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, +} + /// A descriptor used to sign for a commitment transaction's anchor output. #[derive(Clone, Debug, PartialEq, Eq)] pub struct AnchorDescriptor { - /// A unique identifier used along with `channel_value_satoshis` to re-derive the - /// [`InMemorySigner`] required to sign `input`. - /// - /// [`InMemorySigner`]: crate::sign::InMemorySigner - pub channel_keys_id: [u8; 32], - /// The value in satoshis of the channel we're attempting to spend the anchor output of. This is - /// used along with `channel_keys_id` to re-derive the [`InMemorySigner`] required to sign - /// `input`. - /// - /// [`InMemorySigner`]: crate::sign::InMemorySigner - pub channel_value_satoshis: u64, + /// The parameters required to derive the signer for the anchor input. + pub channel_derivation_parameters: ChannelDerivationParameters, /// The transaction input's outpoint corresponding to the commitment transaction's anchor /// output. pub outpoint: OutPoint, } +impl AnchorDescriptor { + /// Derives the channel signer required to sign the anchor input. + pub fn derive_channel_signer(&self, signer_provider: &SP) -> ::Signer + 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 descriptor used to sign for a commitment transaction's HTLC output. #[derive(Clone, Debug, PartialEq, Eq)] pub struct HTLCDescriptor { - /// A unique identifier used along with `channel_value_satoshis` to re-derive the - /// [`InMemorySigner`] required to sign `input`. - /// - /// [`InMemorySigner`]: crate::sign::InMemorySigner - pub channel_keys_id: [u8; 32], - /// The value in satoshis of the channel we're attempting to spend the anchor output of. This is - /// used along with `channel_keys_id` to re-derive the [`InMemorySigner`] required to sign - /// `input`. - /// - /// [`InMemorySigner`]: crate::sign::InMemorySigner - pub channel_value_satoshis: u64, - /// The necessary channel parameters that need to be provided to the re-derived - /// [`InMemorySigner`] through [`ChannelSigner::provide_channel_parameters`]. - /// - /// [`InMemorySigner`]: crate::sign::InMemorySigner - /// [`ChannelSigner::provide_channel_parameters`]: crate::sign::ChannelSigner::provide_channel_parameters - pub channel_parameters: ChannelTransactionParameters, + /// 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. @@ -118,7 +124,7 @@ impl HTLCDescriptor { /// 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_parameters.as_holder_broadcastable(); + 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( @@ -135,7 +141,7 @@ impl HTLCDescriptor { /// 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_parameters.as_holder_broadcastable(); + 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( @@ -160,6 +166,19 @@ impl HTLCDescriptor { 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) -> ::Signer + 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. @@ -178,12 +197,11 @@ pub enum BumpTransactionEvent { /// broadcast first, as the child anchor transaction depends on it. /// /// The consumer should be able to sign for any of the additional inputs included within the - /// child anchor transaction. To sign its anchor input, an [`InMemorySigner`] should be - /// re-derived through [`KeysManager::derive_channel_keys`] with the help of - /// [`AnchorDescriptor::channel_keys_id`] and [`AnchorDescriptor::channel_value_satoshis`]. The - /// anchor input signature can be computed with [`EcdsaChannelSigner::sign_holder_anchor_input`], - /// which can then be provided to [`build_anchor_input_witness`] along with the `funding_pubkey` - /// to obtain the full witness required to spend. + /// child anchor transaction. To sign its anchor input, an [`EcdsaChannelSigner`] should be + /// re-derived through [`AnchorDescriptor::derive_channel_signer`]. The anchor input signature + /// can be computed with [`EcdsaChannelSigner::sign_holder_anchor_input`], which can then be + /// provided to [`build_anchor_input_witness`] along with the `funding_pubkey` to obtain the + /// full witness required to spend. /// /// It is possible to receive more than one instance of this event if a valid child anchor /// transaction is never broadcast or is but not with a sufficient fee to be mined. Care should @@ -202,8 +220,7 @@ pub enum BumpTransactionEvent { /// an empty `pending_htlcs`), confirmation of the commitment transaction can be considered to /// be not urgent. /// - /// [`InMemorySigner`]: crate::sign::InMemorySigner - /// [`KeysManager::derive_channel_keys`]: crate::sign::KeysManager::derive_channel_keys + /// [`EcdsaChannelSigner`]: crate::sign::EcdsaChannelSigner /// [`EcdsaChannelSigner::sign_holder_anchor_input`]: crate::sign::EcdsaChannelSigner::sign_holder_anchor_input /// [`build_anchor_input_witness`]: crate::ln::chan_utils::build_anchor_input_witness ChannelClose { @@ -241,11 +258,11 @@ pub enum BumpTransactionEvent { /// broadcast by the consumer of the event. /// /// The consumer should be able to sign for any of the non-HTLC inputs added to the resulting - /// HTLC transaction. To sign HTLC inputs, an [`InMemorySigner`] should be re-derived through - /// [`KeysManager::derive_channel_keys`] with the help of `channel_keys_id` and - /// `channel_value_satoshis`. Each HTLC input's signature can be computed with - /// [`EcdsaChannelSigner::sign_holder_htlc_transaction`], which can then be provided to - /// [`HTLCDescriptor::tx_input_witness`] to obtain the fully signed witness required to spend. + /// HTLC transaction. To sign HTLC inputs, an [`EcdsaChannelSigner`] should be re-derived + /// through [`HTLCDescriptor::derive_channel_signer`]. Each HTLC input's signature can be + /// computed with [`EcdsaChannelSigner::sign_holder_htlc_transaction`], which can then be + /// provided to [`HTLCDescriptor::tx_input_witness`] to obtain the fully signed witness required + /// to spend. /// /// It is possible to receive more than one instance of this event if a valid HTLC transaction /// is never broadcast or is but not with a sufficient fee to be mined. Care should be taken by @@ -257,8 +274,7 @@ pub enum BumpTransactionEvent { /// longer able to commit external confirmed funds to the HTLC transaction or the fee committed /// to the HTLC transaction is greater in value than the HTLCs being claimed. /// - /// [`InMemorySigner`]: crate::sign::InMemorySigner - /// [`KeysManager::derive_channel_keys`]: crate::sign::KeysManager::derive_channel_keys + /// [`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 { @@ -670,9 +686,7 @@ where debug_assert_eq!(anchor_tx.output.len(), 1); self.utxo_source.sign_tx(&mut anchor_tx)?; - let signer = self.signer_provider.derive_channel_signer( - anchor_descriptor.channel_value_satoshis, anchor_descriptor.channel_keys_id, - ); + let signer = anchor_descriptor.derive_channel_signer(&self.signer_provider); let anchor_sig = signer.sign_holder_anchor_input(&anchor_tx, 0, &self.secp)?; anchor_tx.input[0].witness = chan_utils::build_anchor_input_witness(&signer.pubkeys().funding_pubkey, &anchor_sig); @@ -686,25 +700,15 @@ where fn build_htlc_tx( &self, claim_id: ClaimId, target_feerate_sat_per_1000_weight: u32, htlc_descriptors: &[HTLCDescriptor], tx_lock_time: PackedLockTime, - ) -> Result<(Transaction, HashMap<[u8; 32], ::Signer>), ()> { + ) -> Result { let mut tx = Transaction { version: 2, lock_time: tx_lock_time, input: vec![], output: vec![], }; - // Unfortunately, we need to derive the signer for each HTLC ahead of time to obtain its - // input. - let mut signers = HashMap::new(); let mut must_spend = Vec::with_capacity(htlc_descriptors.len()); for htlc_descriptor in htlc_descriptors { - signers.entry(htlc_descriptor.channel_keys_id) - .or_insert_with(|| - self.signer_provider.derive_channel_signer( - htlc_descriptor.channel_value_satoshis, htlc_descriptor.channel_keys_id, - ) - ); - let htlc_input = htlc_descriptor.unsigned_tx_input(); must_spend.push(Input { outpoint: htlc_input.previous_output.clone(), @@ -723,7 +727,7 @@ where claim_id, &must_spend, &tx.output, target_feerate_sat_per_1000_weight, )?; self.process_coin_selection(&mut tx, coin_selection); - Ok((tx, signers)) + Ok(tx) } /// Handles a [`BumpTransactionEvent::HTLCResolution`] event variant by producing a @@ -732,16 +736,16 @@ where &self, claim_id: ClaimId, target_feerate_sat_per_1000_weight: u32, htlc_descriptors: &[HTLCDescriptor], tx_lock_time: PackedLockTime, ) -> Result<(), ()> { - let (mut htlc_tx, signers) = self.build_htlc_tx( + let mut htlc_tx = self.build_htlc_tx( claim_id, target_feerate_sat_per_1000_weight, htlc_descriptors, tx_lock_time, )?; self.utxo_source.sign_tx(&mut htlc_tx)?; + let mut signers = BTreeMap::new(); for (idx, htlc_descriptor) in htlc_descriptors.iter().enumerate() { - let signer = signers.get(&htlc_descriptor.channel_keys_id).unwrap(); - let htlc_sig = signer.sign_holder_htlc_transaction( - &htlc_tx, idx, htlc_descriptor, &self.secp - )?; + let signer = signers.entry(htlc_descriptor.channel_derivation_parameters.keys_id) + .or_insert_with(|| htlc_descriptor.derive_channel_signer(&self.signer_provider)); + let htlc_sig = signer.sign_holder_htlc_transaction(&htlc_tx, idx, htlc_descriptor, &self.secp)?; let witness_script = htlc_descriptor.witness_script(&self.secp); htlc_tx.input[idx].witness = htlc_descriptor.tx_input_witness(&htlc_sig, &witness_script); } diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 4d37f936970..41ecd9da0fb 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -1778,10 +1778,8 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) { // feerate for the test, we just want to make sure the feerates we receive from // the events never decrease. tx.input.push(descriptor.unsigned_tx_input()); - let signer = nodes[0].keys_manager.derive_channel_keys( - descriptor.channel_value_satoshis, &descriptor.channel_keys_id, - ); tx.output.push(descriptor.tx_output(&secp)); + let signer = descriptor.derive_channel_signer(&nodes[0].keys_manager); let our_sig = signer.sign_holder_htlc_transaction(&mut tx, 0, &descriptor, &secp).unwrap(); let witness_script = descriptor.witness_script(&secp); tx.input[0].witness = descriptor.tx_input_witness(&our_sig, &witness_script); @@ -1907,9 +1905,7 @@ fn test_yield_anchors_events() { script_pubkey: Script::new_op_return(&[]), }], }; - let signer = nodes[0].keys_manager.derive_channel_keys( - anchor_descriptor.channel_value_satoshis, &anchor_descriptor.channel_keys_id, - ); + let signer = anchor_descriptor.derive_channel_signer(&nodes[0].keys_manager); let funding_sig = signer.sign_holder_anchor_input(&mut anchor_tx, 0, &secp).unwrap(); anchor_tx.input[0].witness = chan_utils::build_anchor_input_witness( &signer.pubkeys().funding_pubkey, &funding_sig @@ -1955,9 +1951,7 @@ fn test_yield_anchors_events() { } ] }; - let signer = nodes[0].keys_manager.derive_channel_keys( - htlc_descriptor.channel_value_satoshis, &htlc_descriptor.channel_keys_id - ); + let signer = htlc_descriptor.derive_channel_signer(&nodes[0].keys_manager); let our_sig = signer.sign_holder_htlc_transaction(&mut htlc_tx, 0, htlc_descriptor, &secp).unwrap(); let witness_script = htlc_descriptor.witness_script(&secp); htlc_tx.input[0].witness = htlc_descriptor.tx_input_witness(&our_sig, &witness_script); @@ -2118,9 +2112,7 @@ fn test_anchors_aggregated_revoked_htlc_tx() { previous_output: anchor_descriptor.outpoint, ..Default::default() }); - let signer = nodes[1].keys_manager.derive_channel_keys( - anchor_descriptor.channel_value_satoshis, &anchor_descriptor.channel_keys_id, - ); + let signer = anchor_descriptor.derive_channel_signer(&nodes[1].keys_manager); signers.push(signer); }, _ => panic!("Unexpected event"), @@ -2234,9 +2226,7 @@ fn test_anchors_aggregated_revoked_htlc_tx() { } for (idx, htlc_descriptor) in descriptors.into_iter().enumerate() { let htlc_input_idx = idx + 1; - let signer = nodes[1].keys_manager.derive_channel_keys( - htlc_descriptor.channel_value_satoshis, &htlc_descriptor.channel_keys_id - ); + let signer = htlc_descriptor.derive_channel_signer(&nodes[1].keys_manager); let our_sig = signer.sign_holder_htlc_transaction(&htlc_tx, htlc_input_idx, &htlc_descriptor, &secp).unwrap(); let witness_script = htlc_descriptor.witness_script(&secp); htlc_tx.input[htlc_input_idx].witness = htlc_descriptor.tx_input_witness(&our_sig, &witness_script); From 0dbfe245a9a561059d06e9988fe318cb08a05598 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 5 Jul 2023 09:47:05 -0700 Subject: [PATCH 5/6] Add transaction-related helpers to AnchorDescriptor This provides a similar interface as `HTLCDescriptor` for users which choose to implement their own bump transaction event handler. --- lightning/src/events/bump_transaction.rs | 34 ++++++++++++++++++------ 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 5ea9bdb8e29..9d086a26d27 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -76,6 +76,30 @@ pub struct AnchorDescriptor { } impl AnchorDescriptor { + /// Returns the unsigned transaction input spending the anchor output in the commitment + /// transaction. + pub fn unsigned_tx_input(&self) -> TxIn { + TxIn { + previous_output: self.outpoint.clone(), + script_sig: Script::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + } + } + + /// Returns the witness script of the anchor output in the commitment transaction. + pub fn witness_script(&self) -> Script { + let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); + chan_utils::get_anchor_redeemscript(&channel_params.broadcaster_pubkeys().funding_pubkey) + } + + /// Returns the fully signed witness required to spend the anchor output in the commitment + /// transaction. + pub fn tx_input_witness(&self, signature: &Signature) -> Witness { + let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); + chan_utils::build_anchor_input_witness(&channel_params.broadcaster_pubkeys().funding_pubkey, signature) + } + /// Derives the channel signer required to sign the anchor input. pub fn derive_channel_signer(&self, signer_provider: &SP) -> ::Signer where @@ -649,12 +673,7 @@ where let mut tx = Transaction { version: 2, lock_time: PackedLockTime::ZERO, // TODO: Use next best height. - input: vec![TxIn { - previous_output: anchor_descriptor.outpoint, - script_sig: Script::new(), - sequence: Sequence::ZERO, - witness: Witness::new(), - }], + input: vec![anchor_descriptor.unsigned_tx_input()], output: vec![], }; self.process_coin_selection(&mut tx, coin_selection); @@ -688,8 +707,7 @@ where self.utxo_source.sign_tx(&mut anchor_tx)?; let signer = anchor_descriptor.derive_channel_signer(&self.signer_provider); let anchor_sig = signer.sign_holder_anchor_input(&anchor_tx, 0, &self.secp)?; - anchor_tx.input[0].witness = - chan_utils::build_anchor_input_witness(&signer.pubkeys().funding_pubkey, &anchor_sig); + anchor_tx.input[0].witness = anchor_descriptor.tx_input_witness(&anchor_sig); self.broadcaster.broadcast_transactions(&[&commitment_tx, &anchor_tx]); Ok(()) From 4c7883c831749895d5a1307112377e6dfc68af5d Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 5 Jul 2023 09:45:20 -0700 Subject: [PATCH 6/6] Expose previous UTXO for anchor and HTLC inputs This may be required by some wallets that rely on PSBTs internally to create/sign transactions. --- lightning/src/events/bump_transaction.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 9d086a26d27..1f28bcfb938 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -18,6 +18,7 @@ use core::ops::Deref; use crate::chain::chaininterface::BroadcasterInterface; use crate::chain::ClaimId; use crate::io_extras::sink; +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, @@ -76,6 +77,15 @@ pub struct AnchorDescriptor { } impl AnchorDescriptor { + /// Returns the UTXO to be spent by the anchor input, which can be obtained via + /// [`Self::unsigned_tx_input`]. + pub fn previous_utxo(&self) -> TxOut { + TxOut { + script_pubkey: self.witness_script().to_v0_p2wsh(), + value: ANCHOR_OUTPUT_VALUE_SATOSHI, + } + } + /// Returns the unsigned transaction input spending the anchor output in the commitment /// transaction. pub fn unsigned_tx_input(&self) -> TxIn { @@ -139,6 +149,15 @@ pub struct HTLCDescriptor { } impl HTLCDescriptor { + /// 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 { @@ -325,6 +344,8 @@ pub enum BumpTransactionEvent { pub struct Input { /// The unique identifier of the input. pub outpoint: OutPoint, + /// The UTXO being spent by the input. + pub previous_utxo: TxOut, /// The upper-bound weight consumed by the input's full [`TxIn::script_sig`] and /// [`TxIn::witness`], each with their lengths included, required to satisfy the output's /// script. @@ -664,6 +685,7 @@ where ) -> Result { let must_spend = vec![Input { outpoint: anchor_descriptor.outpoint, + previous_utxo: anchor_descriptor.previous_utxo(), satisfaction_weight: commitment_tx.weight() as u64 + ANCHOR_INPUT_WITNESS_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT, }]; let coin_selection = self.utxo_source.select_confirmed_utxos( @@ -730,6 +752,7 @@ where let htlc_input = htlc_descriptor.unsigned_tx_input(); must_spend.push(Input { outpoint: htlc_input.previous_output.clone(), + previous_utxo: htlc_descriptor.previous_utxo(&self.secp), satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + if htlc_descriptor.preimage.is_some() { HTLC_SUCCESS_INPUT_ANCHOR_WITNESS_WEIGHT } else {