We no longer H3(msg) to create the challange, only to create the binding_factor

frost-ristretto255/src/frost.rs

@@ -273,9 +273,17 @@ pub struct ShareCommitment(pub(crate) Vec<CoefficientCommitment>);
 
 /// The product of all signers' individual commitments, published as part of the
 /// final signature.
-#[derive(Debug, PartialEq)]
+#[derive(PartialEq)]
 pub struct GroupCommitment(pub(crate) RistrettoPoint);
 
+impl Debug for GroupCommitment {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_tuple("GroupCommitment")
+            .field(&hex::encode(self.0.compress().to_bytes()))
+            .finish()
+    }
+}
+
 impl TryFrom<&SigningPackage> for GroupCommitment {
     type Error = &'static str;
 
@@ -686,12 +694,21 @@ impl TryFrom<[u8; 32]> for Rho {
 
 /// A representation of a single signature share used in FROST structures and messages, including
 /// the group commitment share.
-#[derive(Clone, Copy, Debug, Default, PartialEq)]
+#[derive(Clone, Copy, Default, PartialEq)]
 pub struct SignatureResponse {
     pub(crate) R_share: RistrettoPoint,
     pub(crate) z_share: Scalar,
 }
 
+impl Debug for SignatureResponse {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        f.debug_struct("SignatureResponse")
+            .field("R_share", &hex::encode(self.R_share.compress().to_bytes()))
+            .field("z_share", &hex::encode(self.z_share.to_bytes()))
+            .finish()
+    }
+}
+
 impl From<SignatureResponse> for [u8; 64] {
     fn from(sig: SignatureResponse) -> [u8; 64] {
         let mut bytes = [0; 64];
@@ -727,7 +744,7 @@ impl From<SignatureResponse> for [u8; 64] {
 
 /// A participant's signature share, which the coordinator will use to aggregate
 /// with all other signer's shares into the joint signature.
-#[derive(Clone, Copy, Debug, Default, PartialEq)]
+#[derive(Clone, Copy, Default, PartialEq)]
 pub struct SignatureShare {
     /// Represents the participant index.
     pub(crate) index: u16,
@@ -735,6 +752,15 @@ pub struct SignatureShare {
     pub(crate) signature: SignatureResponse,
 }
 
+impl Debug for SignatureShare {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct("SignatureShare")
+            .field("index", &self.index)
+            .field("signature", &self.signature)
+            .finish()
+    }
+}
+
 // Zeroizes `SignatureShare` to be the `Default` value on drop (when it goes out
 // of scope).  Luckily the derived `Default` includes the `Default` impl of
 // Scalar, which is four 0u64's under the hood, and u32, which is
@@ -750,8 +776,6 @@ impl SignatureShare {
         lambda_i: Scalar,
         challenge: Scalar,
     ) -> Result<(), &'static str> {
-        println!("checking sig share: {:?}", self);
-
         if (RISTRETTO_BASEPOINT_POINT * self.signature.z_share)
             != (self.signature.R_share + (public_key.0 * challenge * lambda_i))
         {
@@ -848,25 +872,15 @@ pub fn sign(
 
     let lambda_i = generate_lagrange_coeff(key_package.index, signing_package)?;
 
-    println!("signer_nonces: {:?}", signer_nonces);
-
     // The Schnorr signature share
     let z_share: Scalar = signer_nonces.hiding.0
         + (signer_nonces.binding.0 * rho.0)
         + (lambda_i * key_package.secret_share.0 * challenge);
 
-    println!(
-        "z_share_{:?}: {:?}",
-        key_package.index,
-        hex::encode(z_share.to_bytes())
-    );
-
     // The Schnorr signature commitment share
     let R_share: RistrettoPoint =
         signer_commitments.hiding.0 + (signer_commitments.binding.0 * rho.0);
 
-    println!("R_share_{:?}: {:?}", key_package.index, R_share.compress());
-
     let signature_share = SignatureShare {
         index: key_package.index,
         signature: SignatureResponse { z_share, R_share },
@@ -903,28 +917,11 @@ pub fn aggregate(
         signing_package.message.as_slice(),
     );
 
-    let rho = Rho::from(signing_package);
-
     for signing_share in signing_shares {
         let signer_pubkey = pubkeys.signer_pubkeys.get(&signing_share.index).unwrap();
         let lambda_i = generate_lagrange_coeff(signing_share.index, signing_package)?;
-        let signer_commitments = signing_package
-            .signing_commitments
-            .iter()
-            .find(|comm| comm.index == signing_share.index)
-            .ok_or("No matching signing commitment for signer")?;
 
-        let commitment_i = signer_commitments.hiding.0 + (signer_commitments.binding.0 * rho.0);
+        signing_share.verify(signer_pubkey, lambda_i, challenge)?;
-
-        println!("calc'd: {:?}", commitment_i.compress());
-        println!("passed: {:?}", signing_share.signature.R_share.compress());
-
-        let verify_result = signing_share.verify(signer_pubkey, lambda_i, challenge);
-
-        println!(
-            "sig share {:?} verify result: {:?}",
-            signing_share.index, verify_result
-        );
     }
 
     // The aggregation of the signature shares by summing them up, resulting in
@@ -934,12 +931,10 @@ pub fn aggregate(
 
     for signature_share in signing_shares {
         z += signature_share.signature.z_share;
+        // TODO(dconnolly): enforce this == group_commitment? This is not in the spec.
         R += signature_share.signature.R_share;
     }
 
-    println!("R: {:?}", R);
-    println!("group_commitment: {:?}", group_commitment);
-
     Ok(Signature {
         R_bytes: group_commitment.0.compress().to_bytes(),
         z_bytes: z.to_bytes(),

frost-ristretto255/src/frost/tests.rs

@@ -84,7 +84,7 @@ fn check_sign_with_test_vectors() {
     }
 
     /////////////////////////////////////////////////////////////////////////////
-    // Round 1
+    // Round 1: generating nonces and signing commitments for each participant
     /////////////////////////////////////////////////////////////////////////////
 
     for (i, _) in signer_commitments.clone() {
@@ -104,7 +104,7 @@ fn check_sign_with_test_vectors() {
     }
 
     /////////////////////////////////////////////////////////////////////////////
-    // Round 2
+    // Round 2: each participant generates their signature share
     /////////////////////////////////////////////////////////////////////////////
 
     let signer_commitments_vec = signer_commitments
@@ -124,8 +124,6 @@ fn check_sign_with_test_vectors() {
     let mut our_signature_shares: Vec<frost::SignatureShare> = Vec::new();
 
     // Each participant generates their signature share
-    println!("{:?}", signer_nonces.keys());
-
     for index in signer_nonces.keys() {
         let key_package = key_packages[index];
         let nonces = signer_nonces[index];
@@ -138,14 +136,8 @@ fn check_sign_with_test_vectors() {
         our_signature_shares.push(signature_share);
     }
 
-    for _sig_share in our_signature_shares {
+    for sig_share in our_signature_shares.clone() {
-        // sig_share.check_is_valid()?;
+        assert_eq!(sig_share, signature_shares[&sig_share.index]);
-        // println!("Ours: {:?}", sig_share);
-
-        // // !!! DIVERGANCE !!!
-        // println!("Test vector: {:?}", signature_shares[&sig_share.index]);
-
-        // assert_eq!(sig_share, signature_shares[&sig_share.index]);
     }
 
     let signer_pubkeys = key_packages
@@ -158,8 +150,12 @@ fn check_sign_with_test_vectors() {
         group_public,
     };
 
-    // The aggregator collects the signing shares from all participants and generates the final
+    ////////////////////////////////////////////////////////////////////////////
-    // signature.
+    // Aggregation:  collects the signing shares from all participants,
+    // generates the final signature.
+    ////////////////////////////////////////////////////////////////////////////
+
+    // Aggregate the FROST signature from test vector sig shares
     let group_signature_result = frost::aggregate(
         &signing_package,
         &signature_shares
@@ -169,11 +165,21 @@ fn check_sign_with_test_vectors() {
         &pubkey_package,
     );
 
-    println!("{:?}", group_signature_result);
+    // Check that the aggregation passed signature share verification and generation
-
     assert!(group_signature_result.is_ok());
 
+    // Check that the generated signature matches the test vector signature
     let group_signature = group_signature_result.unwrap();
+    assert_eq!(group_signature, signature);
 
+    // Aggregate the FROST signature from our signature shares
+    let group_signature_result =
+        frost::aggregate(&signing_package, &our_signature_shares, &pubkey_package);
+
+    // Check that the aggregation passed signature share verification and generation
+    assert!(group_signature_result.is_ok());
+
+    // Check that the generated signature matches the test vector signature
+    let group_signature = group_signature_result.unwrap();
     assert_eq!(group_signature, signature);
 }

frost-ristretto255/src/lib.rs

@@ -90,7 +90,7 @@ fn generate_challenge(R_bytes: &[u8; 32], pubkey_bytes: &[u8; 32], msg: &[u8]) -
 
     preimage.extend_from_slice(R_bytes);
     preimage.extend_from_slice(pubkey_bytes);
-    preimage.extend_from_slice(&H3(msg));
+    preimage.extend_from_slice(msg);
 
     let challenge_wide = H2(&preimage[..]);
 

frost-ristretto255/src/signature.rs

@@ -11,13 +11,22 @@
 //! Schnorr signatures on the Ristretto group
 
 /// A Schnorr signature on the Ristretto group.
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+#[derive(Copy, Clone, Eq, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Signature {
     pub(crate) R_bytes: [u8; 32],
     pub(crate) z_bytes: [u8; 32],
 }
 
+impl std::fmt::Debug for Signature {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        f.debug_struct("Signature")
+            .field("R", &hex::encode(self.R_bytes))
+            .field("z", &hex::encode(self.z_bytes))
+            .finish()
+    }
+}
+
 impl From<[u8; 64]> for Signature {
     fn from(bytes: [u8; 64]) -> Signature {
         let mut R_bytes = [0; 32];

frost-ristretto255/tests/frost.rs

@@ -21,7 +21,7 @@ fn check_sign_with_dealer() {
     let mut commitments: HashMap<u16, Vec<frost::SigningCommitments>> = HashMap::new();
 
     ////////////////////////////////////////////////////////////////////////////
-    // Round 1, generating nonces and signing commitments for each participant.
+    // Round 1: generating nonces and signing commitments for each participant
     ////////////////////////////////////////////////////////////////////////////
 
     for participant_index in 1..(threshold + 1) {
@@ -65,28 +65,27 @@ fn check_sign_with_dealer() {
     }
 
     ////////////////////////////////////////////////////////////////////////////
-    // Aggregator collects the signing shares from all participants and
+    // Aggregation:  collects the signing shares from all participants,
     // generates the final signature.
     ////////////////////////////////////////////////////////////////////////////
 
-    // Aggregate
+    // Aggregate (also verifies the signature shares)
     let group_signature_res = frost::aggregate(&signing_package, &signature_shares[..], &pubkeys);
+
     assert!(group_signature_res.is_ok());
 
     let group_signature = group_signature_res.unwrap();
 
     // Check that the threshold signature can be verified by the group public
-    // key (aka verification key).
+    // key (the verification key).
     assert!(pubkeys
         .group_public
         .verify(message, &group_signature)
         .is_ok());
 
-    let nonces_2 = nonces.clone();
-
     // Check that the threshold signature can be verified by the group public
-    // key (aka verification key) from SharePackage.group_public
+    // key (the verification key) from SharePackage.group_public
-    for (participant_index, _) in nonces_2 {
+    for (participant_index, _) in nonces.clone() {
         let key_package = key_packages.get(participant_index as usize).unwrap();
 
         assert!(key_package