Cache HTLC per_commitment_point in descriptor

Author: wpaulino

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;
-use crate::events::bump_transaction::{AnchorDescriptor, HTLCDescriptor, BumpTransactionEvent};
+use crate::events::bump_transaction::{ChannelDerivationParameters, AnchorDescriptor, HTLCDescriptor, BumpTransactionEvent};
 
 use crate::prelude::*;
 use core::{cmp, mem};
@@ -2550,8 +2550,11 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 						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,
@@ -2566,11 +2569,16 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 					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(
+								htlc.per_commitment_number, &self.onchain_tx_handler.secp_ctx,
+							),
 							htlc: htlc.htlc,
 							preimage: htlc.preimage,
 							counterparty_sig: htlc.counterparty_sig,

lightning/src/events/bump_transaction.rs

@@ -9,6 +9,7 @@
 
 //! Utitilies for bumping transactions originating from [`super::Event`]s.
 
+use alloc::collections::BTreeMap;
 use core::convert::TryInto;
 use core::ops::Deref;
 
@@ -48,49 +49,57 @@ 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<SP: Deref>(&self, signer_provider: &SP) -> <SP::Target as SignerProvider>::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.
 	pub per_commitment_number: u64,
+	/// The public key corresponding to the number of the commitment transaction in which the HTLC
+	/// output lives.
+	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
@@ -109,17 +118,15 @@ impl HTLCDescriptor {
 
 	/// Returns the delayed output created as a result of spending the HTLC output in the commitment
 	/// transaction.
-	pub fn tx_output<C: secp256k1::Signing + secp256k1::Verification>(
-		&self, per_commitment_point: &PublicKey, secp: &Secp256k1<C>
-	) -> TxOut {
-		let channel_params = self.channel_parameters.as_holder_broadcastable();
+	pub fn tx_output<C: secp256k1::Signing + secp256k1::Verification>(&self, secp: &Secp256k1<C>) -> 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, 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,
@@ -128,20 +135,18 @@ impl HTLCDescriptor {
 	}
 
 	/// Returns the witness script of the HTLC output in the commitment transaction.
-	pub fn witness_script<C: secp256k1::Signing + secp256k1::Verification>(
-		&self, per_commitment_point: &PublicKey, secp: &Secp256k1<C>
-	) -> Script {
-		let channel_params = self.channel_parameters.as_holder_broadcastable();
+	pub fn witness_script<C: secp256k1::Signing + secp256k1::Verification>(&self, secp: &Secp256k1<C>) -> 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, 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,
@@ -156,6 +161,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<SP: Deref>(&self, signer_provider: &SP) -> <SP::Target as SignerProvider>::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.
@@ -662,9 +680,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);
@@ -678,7 +694,7 @@ 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], <SP::Target as SignerProvider>::Signer>), ()> {
+	) -> Result<Transaction, ()> {
 		let mut tx = Transaction {
 			version: 2,
 			lock_time: tx_lock_time,
@@ -687,19 +703,8 @@ where
 		};
 		// 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 {
-			let signer = 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 {
 				outpoint: htlc_input.previous_output.clone(),
@@ -710,15 +715,15 @@ 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);
 		}
 
 		let coin_selection = self.utxo_source.select_confirmed_utxos(
 			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
@@ -727,20 +732,22 @@ 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 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 signer = if let Some(signer) = signers.get(&htlc_descriptor.channel_derivation_parameters.keys_id) {
+				signer
+			} else {
+				let signer = htlc_descriptor.derive_channel_signer(&self.signer_provider);
+				signers.insert(htlc_descriptor.channel_derivation_parameters.keys_id, signer);
+				signers.get(&htlc_descriptor.channel_derivation_parameters.keys_id).expect("Signer was just added")
+			};
+			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);
 		}
 

lightning/src/ln/monitor_tests.rs

@@ -1778,15 +1778,10 @@ 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,
-				);
-				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 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(&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"); };
@@ -1910,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
@@ -1943,10 +1936,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 +1944,16 @@ 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 = 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(&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);
 			},
@@ -2122,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"),
@@ -2227,12 +2215,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;
@@ -2242,12 +2226,9 @@ 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 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 = {