fix v1 and v2 taproot tests; return tweaked public key and schnorr pr…

…oof from aggregator taproot sign; don't try to verify signature if we're signing taproot just verify the schnorr proof
Trust-Machines · Sep 21, 2023 · 14ed7c7 · 14ed7c7
1 parent e3cf9d4
commit 14ed7c7
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 58 deletions.
diff --git a/src/errors.rs b/src/errors.rs
@@ -1,6 +1,8 @@
 use p256k1::{point::Error as PointError, scalar::Scalar};
 use thiserror::Error;
 
+use crate::taproot::Error as TaprootError;
+
 #[derive(Error, Debug, Clone)]
 /// Errors which can happen during distributed key generation
 pub enum DkgError {
@@ -48,4 +50,13 @@ pub enum AggregatorError {
     #[error("bad group sig")]
     /// The aggregate group signature failed to verify
     BadGroupSig,
+    #[error("taproot {0:?}")]
+    /// Taproot error
+    Taproot(TaprootError),
+}
+
+impl From<TaprootError> for AggregatorError {
+    fn from(e: TaprootError) -> Self {
+        AggregatorError::Taproot(e)
+    }
 }
diff --git a/src/taproot.rs b/src/taproot.rs
@@ -7,7 +7,7 @@ use p256k1::{
 use crate::{common::Signature, compute};
 
 /// Errors from BIP-340 operations
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub enum Error {
     /// Point R is odd
     OddR,
@@ -31,10 +31,10 @@ impl SchnorrProof {
         /*if !sig.R.has_even_y() {
             Err(Error::OddR)
         } else {*/
-            Ok(Self {
-                r: sig.R.x(),
-                s: sig.z,
-            })
+        Ok(Self {
+            r: sig.R.x(),
+            s: sig.z,
+        })
         //}
     }
 
@@ -154,8 +154,7 @@ pub mod test_helpers {
     ) -> (Vec<u32>, Vec<u32>, Vec<PublicNonce>) {
         let signer_ids: Vec<u32> = signers.iter().map(|s| s.get_id()).collect();
         let key_ids: Vec<u32> = signers.iter().flat_map(|s| s.get_key_ids()).collect();
-        let nonces: Vec<PublicNonce> =
-            signers.iter_mut().flat_map(|s| s.gen_nonces(rng)).collect();
+        let nonces: Vec<PublicNonce> = signers.iter_mut().flat_map(|s| s.gen_nonces(rng)).collect();
 
         (signer_ids, key_ids, nonces)
     }
@@ -231,16 +230,13 @@ mod test {
         let mut S = [signers[0].clone(), signers[1].clone(), signers[3].clone()].to_vec();
         let mut sig_agg =
             v1::SignatureAggregator::new(N, T, A.clone()).expect("aggregator ctor failed");
-        let aggregate_public_key = sig_agg.poly[0];
-        let tweaked_public_key = compute::tweaked_public_key(&aggregate_public_key, merkle_root);
-        let (nonces, sig_shares) = test_helpers::sign(&msg, &mut S, &mut rng, merkle_root);
-        let sig = match sig_agg.sign_taproot(&msg, &nonces, &sig_shares, merkle_root) {
-            Err(e) => panic!("Aggregator sign failed: {:?}", e),
-            Ok(sig) => sig,
-        };
 
-        // now create a SchnorrProof from the frost signature
-        let proof = SchnorrProof::new(&sig).unwrap();
+        let (nonces, sig_shares) = test_helpers::sign(&msg, &mut S, &mut rng, merkle_root);
+        let (tweaked_public_key, proof) =
+            match sig_agg.sign_taproot(&msg, &nonces, &sig_shares, merkle_root) {
+                Err(e) => panic!("Aggregator sign failed: {:?}", e),
+                Ok((key, proof)) => (key, proof),
+            };
 
         assert!(proof.verify(&tweaked_public_key.x(), msg));
 
@@ -300,17 +296,12 @@ mod test {
         let key_ids = S.iter().flat_map(|s| s.get_key_ids()).collect::<Vec<u32>>();
         let mut sig_agg =
             v2::SignatureAggregator::new(Nk, T, A.clone()).expect("aggregator ctor failed");
-        let aggregate_public_key = sig_agg.poly[0];
-        let tweaked_public_key = compute::tweaked_public_key(&aggregate_public_key, merkle_root);
-
         let (nonces, sig_shares) = test_helpers::sign(&msg, &mut S, &mut rng, merkle_root);
-        let sig = match sig_agg.sign_taproot(&msg, &nonces, &sig_shares, &key_ids, merkle_root) {
-            Err(e) => panic!("Aggregator sign failed: {:?}", e),
-            Ok(sig) => sig,
-        };
-
-        // now create a SchnorrProof from the frost signature
-        let proof = SchnorrProof::new(&sig).unwrap();
+        let (tweaked_public_key, proof) =
+            match sig_agg.sign_taproot(&msg, &nonces, &sig_shares, &key_ids, merkle_root) {
+                Err(e) => panic!("Aggregator sign failed: {:?}", e),
+                Ok((key, proof)) => (key, proof),
+            };
 
         assert!(proof.verify(&tweaked_public_key.x(), msg));
 

diff --git a/src/v1.rs b/src/v1.rs
@@ -13,6 +13,7 @@ use crate::{
     compute,
     errors::{AggregatorError, DkgError},
     schnorr::ID,
+    taproot::SchnorrProof,
     vss::VSS,
 };
 
@@ -277,7 +278,13 @@ impl SignatureAggregator {
         nonces: &[PublicNonce],
         sig_shares: &[SignatureShare],
     ) -> Result<Signature, AggregatorError> {
-        self.sign_with_tweak(msg, nonces, sig_shares, &Scalar::zero())
+        let (key, sig) = self.sign_with_tweak(msg, nonces, sig_shares, &Scalar::zero())?;
+
+        if sig.verify(&key, msg) {
+            Ok(sig)
+        } else {
+            Err(AggregatorError::BadGroupSig)
+        }
     }
 
     /// Check and aggregate the party signatures using a merke root to make a tweak
@@ -287,9 +294,16 @@ impl SignatureAggregator {
         nonces: &[PublicNonce],
         sig_shares: &[SignatureShare],
         merkle_root: Option<[u8; 32]>,
-    ) -> Result<Signature, AggregatorError> {
+    ) -> Result<(Point, SchnorrProof), AggregatorError> {
         let tweak = compute::tweak(&self.poly[0], merkle_root);
-        self.sign_with_tweak(msg, nonces, sig_shares, &tweak)
+        let (key, sig) = self.sign_with_tweak(msg, nonces, sig_shares, &tweak)?;
+        let proof = SchnorrProof::new(&sig)?;
+
+        if proof.verify(&key.x(), msg) {
+            Ok((key, proof))
+        } else {
+            Err(AggregatorError::BadGroupSig)
+        }
     }
 
     #[allow(non_snake_case)]
@@ -300,7 +314,7 @@ impl SignatureAggregator {
         nonces: &[PublicNonce],
         sig_shares: &[SignatureShare],
         tweak: &Scalar,
-    ) -> Result<Signature, AggregatorError> {
+    ) -> Result<(Point, Signature), AggregatorError> {
         if nonces.len() != sig_shares.len() {
             return Err(AggregatorError::BadNonceLen(nonces.len(), sig_shares.len()));
         }
@@ -314,8 +328,10 @@ impl SignatureAggregator {
         let tweaked_public_key = aggregate_public_key + tweak * G;
         let c = compute::challenge(&tweaked_public_key, &R, msg);
         let mut r_sign = Scalar::one();
-        if tweak != &Scalar::zero() && !R.has_even_y() { r_sign = -Scalar::one(); }
-
+        if tweak != &Scalar::zero() && !R.has_even_y() {
+            r_sign = -Scalar::one();
+        }
+
         for i in 0..sig_shares.len() {
             let id = compute::id(sig_shares[i].id);
             let public_key = match compute::poly(&id, &self.poly) {
@@ -328,7 +344,9 @@ impl SignatureAggregator {
 
             let z_i = sig_shares[i].z_i;
 
-            if z_i * G != r_sign * R_vec[i] + (compute::lambda(sig_shares[i].id, &signers) * c * public_key)
+            if z_i * G
+                != r_sign * R_vec[i]
+                    + (compute::lambda(sig_shares[i].id, &signers) * c * public_key)
             {
                 bad_party_sigs.push(sig_shares[i].id);
             }
@@ -338,23 +356,11 @@ impl SignatureAggregator {
 
         if tweak != &Scalar::zero() {
             z += c * tweak;
-            /*
-            if !R.has_even_y() {
-                let n = Scalar::from(p256k1::point::N);
-                z += n;
-            }*/
         }
-        
+
         if bad_party_sigs.is_empty() {
             let sig = Signature { R, z };
-            Ok(sig)
-            /*
-            if sig.verify(&tweaked_public_key, msg) {
-                Ok(sig)
-            } else {
-                Err(AggregatorError::BadGroupSig)
-            }
-             */
+            Ok((tweaked_public_key, sig))
         } else if !bad_party_keys.is_empty() {
             Err(AggregatorError::BadPartyKeys(bad_party_keys))
         } else {

diff --git a/src/v2.rs b/src/v2.rs
@@ -1,5 +1,5 @@
 use hashbrown::{HashMap, HashSet};
-use num_traits::Zero;
+use num_traits::{One, Zero};
 use p256k1::{
     point::{Point, G},
     scalar::Scalar,
@@ -12,6 +12,7 @@ use crate::common::{Nonce, PolyCommitment, PublicNonce, Signature, SignatureShar
 use crate::compute;
 use crate::errors::{AggregatorError, DkgError};
 use crate::schnorr::ID;
+use crate::taproot::SchnorrProof;
 use crate::vss::VSS;
 
 /// A map of private keys indexed by key ID
@@ -231,6 +232,10 @@ impl Party {
         let (_R_vec, R) = compute::intermediate(msg, party_ids, nonces);
         let c = compute::challenge(&(self.group_key + tweak * G), &R, msg);
         let mut z = &self.nonce.d + &self.nonce.e * compute::binding(&self.id(), nonces, msg);
+        if tweak != &Scalar::zero() && !R.has_even_y() {
+            z = -z;
+        }
+
         for key_id in self.key_ids.iter() {
             z += c * &self.private_keys[key_id] * compute::lambda(*key_id, key_ids);
         }
@@ -297,7 +302,13 @@ impl SignatureAggregator {
         sig_shares: &[SignatureShare],
         key_ids: &[u32],
     ) -> Result<Signature, AggregatorError> {
-        self.sign_with_tweak(msg, nonces, sig_shares, key_ids, &Scalar::zero())
+        let (key, sig) = self.sign_with_tweak(msg, nonces, sig_shares, key_ids, &Scalar::zero())?;
+
+        if sig.verify(&key, msg) {
+            Ok(sig)
+        } else {
+            Err(AggregatorError::BadGroupSig)
+        }
     }
 
     /// Check and aggregate the party signatures
@@ -309,9 +320,16 @@ impl SignatureAggregator {
         sig_shares: &[SignatureShare],
         key_ids: &[u32],
         merkle_root: Option<[u8; 32]>,
-    ) -> Result<Signature, AggregatorError> {
+    ) -> Result<(Point, SchnorrProof), AggregatorError> {
         let tweak = compute::tweak(&self.poly[0], merkle_root);
-        self.sign_with_tweak(msg, nonces, sig_shares, key_ids, &tweak)
+        let (key, sig) = self.sign_with_tweak(msg, nonces, sig_shares, key_ids, &tweak)?;
+        let proof = SchnorrProof::new(&sig)?;
+
+        if proof.verify(&key.x(), msg) {
+            Ok((key, proof))
+        } else {
+            Err(AggregatorError::BadGroupSig)
+        }
     }
 
     /// Check and aggregate the party signatures
@@ -323,7 +341,7 @@ impl SignatureAggregator {
         sig_shares: &[SignatureShare],
         key_ids: &[u32],
         tweak: &Scalar,
-    ) -> Result<Signature, AggregatorError> {
+    ) -> Result<(Point, Signature), AggregatorError> {
         if nonces.len() != sig_shares.len() {
             return Err(AggregatorError::BadNonceLen(nonces.len(), sig_shares.len()));
         }
@@ -336,6 +354,10 @@ impl SignatureAggregator {
         let aggregate_public_key = self.poly[0];
         let tweaked_public_key = aggregate_public_key + tweak * G;
         let c = compute::challenge(&tweaked_public_key, &R, msg);
+        let mut r_sign = Scalar::one();
+        if tweak != &Scalar::zero() && !R.has_even_y() {
+            r_sign = -Scalar::one();
+        }
 
         for i in 0..sig_shares.len() {
             let z_i = sig_shares[i].z_i;
@@ -354,7 +376,7 @@ impl SignatureAggregator {
                 cx += compute::lambda(*key_id, key_ids) * c * public_key;
             }
 
-            if z_i * G != (Ris[i] + cx) {
+            if z_i * G != (r_sign * Ris[i] + cx) {
                 bad_party_sigs.push(sig_shares[i].id);
             }
 
@@ -365,11 +387,7 @@ impl SignatureAggregator {
 
         if bad_party_sigs.is_empty() {
             let sig = Signature { R, z };
-            if sig.verify(&tweaked_public_key, msg) {
-                Ok(sig)
-            } else {
-                Err(AggregatorError::BadGroupSig)
-            }
+            Ok((tweaked_public_key, sig))
         } else if !bad_party_keys.is_empty() {
             Err(AggregatorError::BadPartyKeys(bad_party_keys))
         } else {