Make sign_holder_htlc_transaction generic over signature type

Author: tankyleo

lightning/src/chain/onchaintx.rs

@@ -1227,10 +1227,8 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 				preimage: preimage.clone(),
 				counterparty_sig: counterparty_htlc_sig.clone(),
 			};
-			if let Ok(htlc_sig) = self.signer.sign_holder_htlc_transaction(&htlc_tx, 0, &htlc_descriptor, &self.secp_ctx) {
-				htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness(
-					htlc_idx, &counterparty_htlc_sig, &htlc_sig, preimage,
-				);
+			if let Ok(tx) = self.signer.sign_holder_htlc_transaction(&htlc_tx, 0, &htlc_descriptor, &self.secp_ctx) {
+				htlc_tx = tx;
 			}
 			Some(MaybeSignedTransaction(htlc_tx))
 		};

lightning/src/events/bump_transaction.rs

@@ -189,7 +189,7 @@ pub enum BumpTransactionEvent {
 	/// 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 [`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
+	/// computed with [`ChannelSigner::sign_holder_htlc_transaction`], which can then be
 	/// provided to [`HTLCDescriptor::tx_input_witness`] to obtain the fully signed witness required
 	/// to spend.
 	///
@@ -204,7 +204,7 @@ pub enum BumpTransactionEvent {
 	/// to the HTLC transaction is greater in value than the HTLCs being claimed.
 	///
 	/// [`EcdsaChannelSigner`]: crate::sign::ecdsa::EcdsaChannelSigner
-	/// [`EcdsaChannelSigner::sign_holder_htlc_transaction`]: crate::sign::ecdsa::EcdsaChannelSigner::sign_holder_htlc_transaction
+	/// [`ChannelSigner::sign_holder_htlc_transaction`]: crate::sign::ChannelSigner::sign_holder_htlc_transaction
 	HTLCResolution {
 		/// The `channel_id` of the channel which has been closed.
 		channel_id: ChannelId,
@@ -799,9 +799,7 @@ where
 		for (idx, htlc_descriptor) in htlc_descriptors.iter().enumerate() {
 			// Unwrap because we derived the corresponding signers for all htlc descriptors further above
 			let signer  = &signers_and_revokeable_spks.get(&htlc_descriptor.channel_derivation_parameters.keys_id).unwrap().0;
-			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);
+			htlc_tx = signer.sign_holder_htlc_transaction(&htlc_tx, idx, htlc_descriptor, &self.secp)?;
 		}
 
 		#[cfg(debug_assertions)] {

lightning/src/ln/channel.rs

@@ -10891,7 +10891,7 @@ mod tests {
 		use bitcoin::hash_types::Txid;
 		use bitcoin::hex::DisplayHex;
 		use bitcoin::secp256k1::Message;
-		use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, ecdsa::EcdsaChannelSigner};
+		use crate::sign::{ChannelDerivationParameters, HTLCDescriptor};
 		use crate::types::payment::PaymentPreimage;
 		use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys};
 		use crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint};
@@ -11047,8 +11047,12 @@ mod tests {
 						assert!(preimage.is_some());
 					}
 
+					let num_anchors = if $opt_anchors.supports_anchors_zero_fee_htlc_tx() { 2 } else { 0 };
+					assert_eq!(htlc.transaction_output_index, Some($htlc_idx + num_anchors), "output index");
+
 					let htlc_counterparty_sig = htlc_counterparty_sig_iter.next().unwrap();
-					let htlc_holder_sig = chan.context.holder_signer.as_ecdsa().unwrap().sign_holder_htlc_transaction(&htlc_tx, 0, &HTLCDescriptor {
+
+					htlc_tx = chan.context.holder_signer.as_ref().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,
@@ -11062,13 +11066,7 @@ mod tests {
 						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.transaction_output_index, Some($htlc_idx + num_anchors), "output index");
 
-					let signature = Signature::from_der(&<Vec<u8>>::from_hex($htlc_sig_hex).unwrap()[..]).unwrap();
-					assert_eq!(signature, htlc_holder_sig, "htlc sig");
-					let trusted_tx = holder_commitment_tx.trust();
-					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 = {}", serialize(&htlc_tx).as_hex());
 					assert_eq!(serialize(&htlc_tx)[..], <Vec<u8>>::from_hex($htlc_tx_hex).unwrap()[..], "htlc tx");
 				})*

lightning/src/ln/monitor_tests.rs

@@ -2928,9 +2928,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 = 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);
+			htlc_tx = signer.sign_holder_htlc_transaction(&htlc_tx, htlc_input_idx, &htlc_descriptor, &secp).unwrap();
 		}
 		let fee_utxo_sig = {
 			let witness_script = ScriptBuf::new_p2pkh(&public_key.pubkey_hash());

lightning/src/sign/ecdsa.rs

@@ -110,27 +110,6 @@ pub trait EcdsaChannelSigner: ChannelSigner {
 		&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey,
 		htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> Result<Signature, ()>;
-	/// 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 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.
-	///
-	/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
-	/// signature and should be retried later. Once the signer is ready to provide a signature after
-	/// previously returning an `Err`, [`ChannelMonitor::signer_unblocked`] must be called on its
-	/// monitor or [`ChainMonitor::signer_unblocked`] called to attempt unblocking all monitors.
-	///
-	/// [`EcdsaSighashType::All`]: bitcoin::sighash::EcdsaSighashType::All
-	/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
-	/// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked
-	/// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked
-	fn sign_holder_htlc_transaction(
-		&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
-		secp_ctx: &Secp256k1<secp256k1::All>,
-	) -> Result<Signature, ()>;
 	/// Create a signature for a claiming transaction for a HTLC output on a counterparty's commitment
 	/// transaction, either offered or received.
 	///

lightning/src/sign/mod.rs

@@ -925,6 +925,27 @@ pub trait ChannelSigner {
 	fn unsafe_sign_holder_commitment(
 		&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> Result<Transaction, ()>;
+	/// 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 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.
+	///
+	/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
+	/// signature and should be retried later. Once the signer is ready to provide a signature after
+	/// previously returning an `Err`, [`ChannelMonitor::signer_unblocked`] must be called on its
+	/// monitor or [`ChainMonitor::signer_unblocked`] called to attempt unblocking all monitors.
+	///
+	/// [`EcdsaSighashType::All`]: bitcoin::sighash::EcdsaSighashType::All
+	/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
+	/// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked
+	/// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked
+	fn sign_holder_htlc_transaction(
+		&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
+		secp_ctx: &Secp256k1<secp256k1::All>,
+	) -> Result<Transaction, ()>;
 }
 
 /// Specifies the recipient of an invoice.
@@ -1713,6 +1734,40 @@ impl ChannelSigner for InMemorySigner {
 		);
 		Ok(commitment_tx.add_holder_sig(&funding_redeemscript, sig))
 	}
+
+	fn sign_holder_htlc_transaction(
+		&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
+		secp_ctx: &Secp256k1<secp256k1::All>,
+	) -> Result<Transaction, ()> {
+		let witness_script = htlc_descriptor.witness_script(secp_ctx);
+		let sighash = &sighash::SighashCache::new(&*htlc_tx)
+			.p2wsh_signature_hash(
+				input,
+				&witness_script,
+				htlc_descriptor.htlc.to_bitcoin_amount(),
+				EcdsaSighashType::All,
+			)
+			.map_err(|_| ())?;
+		let our_htlc_private_key = chan_utils::derive_private_key(
+			&secp_ctx,
+			&htlc_descriptor.per_commitment_point,
+			&self.htlc_base_key,
+		);
+		let sighash = hash_to_message!(sighash.as_byte_array());
+		let sig = sign_with_aux_rand(&secp_ctx, &sighash, &our_htlc_private_key, &self);
+
+		let features = &self.channel_parameters.as_ref().unwrap().channel_type_features;
+		let witness = chan_utils::build_htlc_input_witness(
+			&sig,
+			&htlc_descriptor.counterparty_sig,
+			&htlc_descriptor.preimage,
+			&witness_script,
+			features,
+		);
+		let mut signed_tx = htlc_tx.clone();
+		signed_tx.input[input].witness = witness;
+		Ok(signed_tx)
+	}
 }
 
 const MISSING_PARAMS_ERR: &'static str =
@@ -1879,28 +1934,6 @@ impl EcdsaChannelSigner for InMemorySigner {
 		return Ok(sign_with_aux_rand(secp_ctx, &sighash, &revocation_key, &self));
 	}
 
-	fn sign_holder_htlc_transaction(
-		&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
-		secp_ctx: &Secp256k1<secp256k1::All>,
-	) -> Result<Signature, ()> {
-		let witness_script = htlc_descriptor.witness_script(secp_ctx);
-		let sighash = &sighash::SighashCache::new(&*htlc_tx)
-			.p2wsh_signature_hash(
-				input,
-				&witness_script,
-				htlc_descriptor.htlc.to_bitcoin_amount(),
-				EcdsaSighashType::All,
-			)
-			.map_err(|_| ())?;
-		let our_htlc_private_key = chan_utils::derive_private_key(
-			&secp_ctx,
-			&htlc_descriptor.per_commitment_point,
-			&self.htlc_base_key,
-		);
-		let sighash = hash_to_message!(sighash.as_byte_array());
-		Ok(sign_with_aux_rand(&secp_ctx, &sighash, &our_htlc_private_key, &self))
-	}
-
 	fn sign_counterparty_htlc_transaction(
 		&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey,
 		htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>,
@@ -2041,13 +2074,6 @@ impl TaprootChannelSigner for InMemorySigner {
 		todo!()
 	}
 
-	fn sign_holder_htlc_transaction(
-		&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
-		secp_ctx: &Secp256k1<All>,
-	) -> Result<schnorr::Signature, ()> {
-		todo!()
-	}
-
 	fn sign_counterparty_htlc_transaction(
 		&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey,
 		htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<All>,

lightning/src/sign/taproot.rs

@@ -105,22 +105,6 @@ pub trait TaprootChannelSigner: ChannelSigner {
 		htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> Result<Signature, ()>;
 
-	/// 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 [`TapSighashType::Default`].
-	///
-	/// 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.
-	///
-	///
-	/// [`TapSighashType::Default`]: bitcoin::sighash::TapSighashType::Default
-	/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
-	fn sign_holder_htlc_transaction(
-		&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
-		secp_ctx: &Secp256k1<secp256k1::All>,
-	) -> Result<Signature, ()>;
-
 	/// Create a signature for a claiming transaction for a HTLC output on a counterparty's commitment
 	/// transaction, either offered or received.
 	///

lightning/src/util/test_channel_signer.rs

@@ -294,6 +294,48 @@ impl ChannelSigner for TestChannelSigner {
 	fn unsafe_sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Transaction, ()> {
 		Ok(self.inner.unsafe_sign_holder_commitment(commitment_tx, secp_ctx).unwrap())
 	}
+
+	fn sign_holder_htlc_transaction(
+		&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
+		secp_ctx: &Secp256k1<secp256k1::All>
+	) -> Result<Transaction, ()> {
+		#[cfg(test)]
+		if !self.is_signer_available(SignerOp::SignHolderHtlcTransaction) {
+			return Err(());
+		}
+		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());
+		let revokeable_spk = self.get_revokeable_spk(true, htlc_descriptor.per_commitment_number, &htlc_descriptor.per_commitment_point, secp_ctx);
+		assert_eq!(htlc_tx.output[input], htlc_descriptor.tx_output(revokeable_spk));
+		{
+			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).p2wsh_signature_hash(
+				input, &witness_script, htlc_descriptor.htlc.to_bitcoin_amount(), sighash_type
+			).unwrap();
+			let countersignatory_htlc_key = HtlcKey::from_basepoint(
+				&secp_ctx, &self.inner.counterparty_pubkeys().unwrap().htlc_basepoint, &htlc_descriptor.per_commitment_point,
+			);
+
+			secp_ctx.verify_ecdsa(
+				&hash_to_message!(sighash.as_byte_array()), &htlc_descriptor.counterparty_sig, &countersignatory_htlc_key.to_public_key()
+			).unwrap();
+		}
+		Ok(self.inner.sign_holder_htlc_transaction(htlc_tx, input, htlc_descriptor, secp_ctx).unwrap())
+	}
+
 }
 
 impl EcdsaChannelSigner for TestChannelSigner {
@@ -336,47 +378,6 @@ impl EcdsaChannelSigner for TestChannelSigner {
 		Ok(EcdsaChannelSigner::sign_justice_revoked_htlc(&self.inner, justice_tx, input, amount, per_commitment_key, htlc, secp_ctx).unwrap())
 	}
 
-	fn sign_holder_htlc_transaction(
-		&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
-		secp_ctx: &Secp256k1<secp256k1::All>
-	) -> Result<Signature, ()> {
-		#[cfg(test)]
-		if !self.is_signer_available(SignerOp::SignHolderHtlcTransaction) {
-			return Err(());
-		}
-		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());
-		let revokeable_spk = self.get_revokeable_spk(true, htlc_descriptor.per_commitment_number, &htlc_descriptor.per_commitment_point, secp_ctx);
-		assert_eq!(htlc_tx.output[input], htlc_descriptor.tx_output(revokeable_spk));
-		{
-			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).p2wsh_signature_hash(
-				input, &witness_script, htlc_descriptor.htlc.to_bitcoin_amount(), sighash_type
-			).unwrap();
-			let countersignatory_htlc_key = HtlcKey::from_basepoint(
-				&secp_ctx, &self.inner.counterparty_pubkeys().unwrap().htlc_basepoint, &htlc_descriptor.per_commitment_point,
-			);
-
-			secp_ctx.verify_ecdsa(
-				&hash_to_message!(sighash.as_byte_array()), &htlc_descriptor.counterparty_sig, &countersignatory_htlc_key.to_public_key()
-			).unwrap();
-		}
-		Ok(EcdsaChannelSigner::sign_holder_htlc_transaction(&self.inner, htlc_tx, input, htlc_descriptor, secp_ctx).unwrap())
-	}
-
 	fn sign_counterparty_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
 		#[cfg(test)]
 		if !self.is_signer_available(SignerOp::SignCounterpartyHtlcTransaction) {
@@ -446,10 +447,6 @@ impl TaprootChannelSigner for TestChannelSigner {
 		todo!()
 	}
 
-	fn sign_holder_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1<All>) -> Result<secp256k1::schnorr::Signature, ()> {
-		todo!()
-	}
-
 	fn sign_counterparty_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<All>) -> Result<secp256k1::schnorr::Signature, ()> {
 		todo!()
 	}