Move htlc tx sig validation from FundedChannel to ChannelSigner

Author: tankyleo

lightning/src/ln/channel.rs

@@ -11,7 +11,6 @@ use bitcoin::amount::Amount;
 use bitcoin::constants::ChainHash;
 use bitcoin::script::{Script, ScriptBuf, Builder, WScriptHash};
 use bitcoin::transaction::{Transaction, TxIn};
-use bitcoin::sighash;
 use bitcoin::sighash::EcdsaSighashType;
 use bitcoin::consensus::encode;
 use bitcoin::absolute::LockTime;
@@ -5254,7 +5253,7 @@ impl<SP: Deref> FundedChannel<SP> where
 		}
 
 		let holder_commitment_tx = HolderCommitmentTransaction::new(
-			commitment_stats.tx.clone(),
+			commitment_stats.tx,
 			msg.signature,
 			msg.htlc_signatures.clone(),
 			&self.context.get_holder_pubkeys().funding_pubkey,
@@ -5264,11 +5263,6 @@ impl<SP: Deref> FundedChannel<SP> where
 		self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_stats.outbound_htlc_preimages, &self.context.secp_ctx)
 			.map_err(|_| ChannelError::close("Failed to validate our commitment".to_owned()))?;
 
-		let commitment_txid = {
-			let trusted_tx = commitment_stats.tx.trust();
-			let bitcoin_tx = trusted_tx.built_transaction();
-			bitcoin_tx.txid
-		};
 		let mut htlcs_cloned: Vec<_> = commitment_stats.htlcs_included.iter().map(|htlc| (htlc.0.clone(), htlc.1.map(|h| h.clone()))).collect();
 
 		// If our counterparty updated the channel fee in this commitment transaction, check that
@@ -5318,21 +5312,8 @@ impl<SP: Deref> FundedChannel<SP> where
 
 		let mut nondust_htlc_sources = Vec::with_capacity(htlcs_cloned.len());
 		let mut htlcs_and_sigs = Vec::with_capacity(htlcs_cloned.len());
-		let revokeable_spk = self.context.holder_signer.as_ref().get_revokeable_spk(true, commitment_stats.tx.commitment_number(), &commitment_stats.tx.per_commitment_point(), &self.context.secp_ctx);
 		for (idx, (htlc, mut source_opt)) in htlcs_cloned.drain(..).enumerate() {
 			if let Some(_) = htlc.transaction_output_index {
-				let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_stats.feerate_per_kw,
-					&htlc, &self.context.channel_type, revokeable_spk.clone());
-
-				let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &self.context.channel_type, &keys);
-				let htlc_sighashtype = if self.context.channel_type.supports_anchors_zero_fee_htlc_tx() { EcdsaSighashType::SinglePlusAnyoneCanPay } else { EcdsaSighashType::All };
-				let htlc_sighash = hash_to_message!(&sighash::SighashCache::new(&htlc_tx).p2wsh_signature_hash(0, &htlc_redeemscript, htlc.to_bitcoin_amount(), htlc_sighashtype).unwrap()[..]);
-				log_trace!(logger, "Checking HTLC tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} in channel {}.",
-					log_bytes!(msg.htlc_signatures[idx].serialize_compact()[..]), log_bytes!(keys.countersignatory_htlc_key.to_public_key().serialize()),
-					encode::serialize_hex(&htlc_tx), log_bytes!(htlc_sighash[..]), encode::serialize_hex(&htlc_redeemscript), &self.context.channel_id());
-				if let Err(_) = self.context.secp_ctx.verify_ecdsa(&htlc_sighash, &msg.htlc_signatures[idx], &keys.countersignatory_htlc_key.to_public_key()) {
-					return Err(ChannelError::close("Invalid HTLC tx signature from peer".to_owned()));
-				}
 				if !separate_nondust_htlc_sources {
 					htlcs_and_sigs.push((htlc, Some(msg.htlc_signatures[idx]), source_opt.take()));
 				}

lightning/src/sign/mod.rs

@@ -713,6 +713,62 @@ pub trait ChannelSigner {
 		{
 			return Err(());
 		}
+
+		if holder_tx.htlcs().len() != holder_tx.counterparty_htlc_sigs.len() {
+			return Err(());
+		}
+
+		let commitment_txid = bitcoin_tx.txid;
+		let revokeable_spk = self.get_revokeable_spk(
+			true,
+			holder_tx.commitment_number(),
+			&holder_tx.per_commitment_point(),
+			secp_ctx,
+		);
+		let params = self.get_channel_parameters().unwrap().as_holder_broadcastable();
+		let keys = TxCreationKeys::from_channel_static_keys(
+			&holder_tx.per_commitment_point(),
+			params.broadcaster_pubkeys(),
+			params.countersignatory_pubkeys(),
+			secp_ctx,
+		);
+		for (idx, htlc) in holder_tx.htlcs().iter().enumerate() {
+			if let Some(_) = htlc.transaction_output_index {
+				let htlc_tx = chan_utils::build_htlc_transaction(
+					&commitment_txid,
+					holder_tx.feerate_per_kw(),
+					&htlc,
+					params.channel_type_features(),
+					revokeable_spk.clone(),
+				);
+				let htlc_redeemscript =
+					chan_utils::get_htlc_redeemscript(&htlc, params.channel_type_features(), &keys);
+				let htlc_sighashtype =
+					if params.channel_type_features().supports_anchors_zero_fee_htlc_tx() {
+						EcdsaSighashType::SinglePlusAnyoneCanPay
+					} else {
+						EcdsaSighashType::All
+					};
+				let htlc_sighash = hash_to_message!(
+					&sighash::SighashCache::new(&htlc_tx)
+						.p2wsh_signature_hash(
+							0,
+							&htlc_redeemscript,
+							htlc.to_bitcoin_amount(),
+							htlc_sighashtype
+						)
+						.unwrap()[..]
+				);
+				secp_ctx
+					.verify_ecdsa(
+						&htlc_sighash,
+						&holder_tx.counterparty_htlc_sigs[idx],
+						&keys.countersignatory_htlc_key.to_public_key(),
+					)
+					.map_err(|_| ())?;
+			}
+		}
+
 		Ok(())
 	}