From 39b61ec9da75c73476f1573a0781fb87069d86b1 Mon Sep 17 00:00:00 2001
From: Armin Sabouri <armins88@gmail.com>
Date: Mon, 30 Dec 2024 08:30:48 -0500
Subject: [PATCH] fix(frost-secp256k1-tr): empty merkle root tweak should still
 hash the x-only pk (#815)

Per BIP-341 if there is no script paths the internal key should still be
tapTweak'd by tG where t = TaggedHash(P_x). Before this commit the
internal key and the taproot output key are the same if no script paths
are used. This is because the tweak is the 0 scalar value so Q = P + tG
= P.

It is worth noting that Bitcoin's consensus would still accept a
non-taptweak'd internal key as it verifies a signature against whatever
pk is used in the witness program. So the outputs are still spendable, however it deviates from the spec.
---
 frost-secp256k1-tr/src/lib.rs | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/frost-secp256k1-tr/src/lib.rs b/frost-secp256k1-tr/src/lib.rs
index 3aa51605..f566f72e 100644
--- a/frost-secp256k1-tr/src/lib.rs
+++ b/frost-secp256k1-tr/src/lib.rs
@@ -208,7 +208,11 @@ fn tweak<T: AsRef<[u8]>>(
     merkle_root: Option<T>,
 ) -> Scalar {
     match merkle_root {
-        None => Secp256K1ScalarField::zero(),
+        None => {
+            let mut hasher = tagged_hash("TapTweak");
+            hasher.update(public_key.to_affine().x());
+            hasher_to_scalar(hasher)
+        }
         Some(root) => {
             let mut hasher = tagged_hash("TapTweak");
             hasher.update(public_key.to_affine().x());
@@ -481,10 +485,9 @@ impl Ciphersuite for Secp256K1Sha256TR {
         // > key should commit to an unspendable script path instead of having
         // > no script path. This can be achieved by computing the output key
         // > point as Q = P + int(hashTapTweak(bytes(P)))G.
-        let merkle_root = [0u8; 0];
         Ok((
-            key_package.tweak(Some(&merkle_root)),
-            public_key_package.tweak(Some(&merkle_root)),
+            key_package.tweak::<&[u8]>(None),
+            public_key_package.tweak::<&[u8]>(None),
         ))
     }
 }
@@ -862,12 +865,8 @@ pub mod round2 {
         key_package: &keys::KeyPackage,
         merkle_root: Option<&[u8]>,
     ) -> Result<SignatureShare, Error> {
-        if merkle_root.is_some() {
-            let key_package = key_package.clone().tweak(merkle_root);
-            frost::round2::sign(signing_package, signer_nonces, &key_package)
-        } else {
-            frost::round2::sign(signing_package, signer_nonces, key_package)
-        }
+        let key_package = key_package.clone().tweak(merkle_root);
+        frost::round2::sign(signing_package, signer_nonces, &key_package)
     }
 }
 
@@ -904,12 +903,8 @@ pub fn aggregate_with_tweak(
     public_key_package: &keys::PublicKeyPackage,
     merkle_root: Option<&[u8]>,
 ) -> Result<Signature, Error> {
-    if merkle_root.is_some() {
-        let public_key_package = public_key_package.clone().tweak(merkle_root);
-        frost::aggregate(signing_package, signature_shares, &public_key_package)
-    } else {
-        frost::aggregate(signing_package, signature_shares, public_key_package)
-    }
+    let public_key_package = public_key_package.clone().tweak(merkle_root);
+    frost::aggregate(signing_package, signature_shares, &public_key_package)
 }
 
 /// A signing key for a Schnorr signature on FROST(secp256k1, SHA-256).