Small modifications based on JP feedback.

Author: damrobi

mithril-stm/src/schnorr_signature/mod.rs

@@ -43,7 +43,6 @@ mod tests {
         signing_key::SchnorrSigningKey, verification_key::SchnorrVerificationKey,
     };
 
-    // Testing conversion from arbitrary message to scalar field element
     #[test]
     fn test_hash_msg_to_jubjubbase() {
         let msg = vec![0, 0, 0, 1];
@@ -53,23 +52,21 @@ mod tests {
             179, 7, 17, 168, 141, 112, 57, 117, 112, 92, 169, 56, 36, 70, 1, 217, 9, 13, 255, 42,
             100, 207, 166, 110, 188, 47, 35, 211, 35, 168, 100, 25,
         ];
+
         let field_elem = JubjubBase::from_bytes_le(&bytes_le).unwrap();
+
         assert_eq!(h, field_elem)
     }
 
-    // Testing conversion from EC point to scalar coordinates
-    // For now only printing, next step is to try to generate a point
-    // from x and y values to check if they match with the result of the function
     #[test]
     fn test_get_coordinates() {
         let seed = [0u8; 32];
         let mut rng = ChaCha20Rng::from_seed(seed);
         let point = JubjubSubgroup::random(&mut rng);
+
         let (_x, _y) = get_coordinates(point);
-        // println!("{:?}", (x, y));
     }
 
-    // Testing conversion from BLS12-381 base field to Jubjub base field
     // TODO: Add randomness to val
     #[test]
     fn test_jubjub_base_to_scalar() {
@@ -89,7 +86,9 @@ mod tests {
         let mut rng = ChaCha20Rng::from_seed(seed);
         let sk = SchnorrSigningKey::generate(&mut rng);
         let vk = SchnorrVerificationKey::from(&sk);
+
         let sig = sk.sign(&msg, &mut rng).unwrap();
+
         sig.verify(&msg, &vk).unwrap();
     }
 
@@ -99,26 +98,24 @@ mod tests {
         let msg2 = vec![0, 0, 0, 2];
         let seed = [0u8; 32];
         let mut rng = ChaCha20Rng::from_seed(seed);
-
         let sk = SchnorrSigningKey::generate(&mut rng);
         let vk = SchnorrVerificationKey::from(&sk);
-
         let sk2 = SchnorrSigningKey::generate(&mut rng);
         let vk2 = SchnorrVerificationKey::from(&sk2);
 
         let sig = sk.sign(&msg, &mut rng).unwrap();
         let sig2 = sk.sign(&msg2, &mut rng).unwrap();
 
         // Wrong verification key is used
-        let result = sig.verify(&msg, &vk2);
+        let result1 = sig.verify(&msg, &vk2);
+        let result2 = sig2.verify(&msg, &vk);
+
         assert!(
-            result.is_err(),
+            result1.is_err(),
             "Wrong verfication key used, test should fail."
         );
-
         // Wrong message is verified
-        let result = sig2.verify(&msg, &vk);
-        assert!(result.is_err(), "Wrong message used, test should fail.");
+        assert!(result2.is_err(), "Wrong message used, test should fail.");
     }
 
     #[test]
@@ -127,8 +124,10 @@ mod tests {
         let mut rng = rand_chacha::ChaCha8Rng::seed_from_u64(seed);
         let sk = SchnorrSigningKey::generate(&mut rng);
         let vk = SchnorrVerificationKey::from(&sk);
+
         let vk_bytes = vk.to_bytes();
         let vk2 = SchnorrVerificationKey::from_bytes(&vk_bytes).unwrap();
+
         assert_eq!(vk.0, vk2.0);
     }
 
@@ -137,8 +136,10 @@ mod tests {
         let seed = 0;
         let mut rng = rand_chacha::ChaCha8Rng::seed_from_u64(seed);
         let sk = SchnorrSigningKey::generate(&mut rng);
+
         let sk_bytes: [u8; 32] = sk.to_bytes();
         let sk2 = SchnorrSigningKey::from_bytes(&sk_bytes).unwrap();
+
         assert_eq!(sk, sk2);
     }
 
@@ -150,9 +151,9 @@ mod tests {
         let sk = SchnorrSigningKey::generate(&mut rng);
 
         let sig = sk.sign(&msg, &mut rng).unwrap();
-
         let sig_bytes: [u8; 96] = sig.clone().to_bytes();
         let sig2 = SchnorrSignature::from_bytes(&sig_bytes).unwrap();
+
         assert_eq!(sig, sig2);
     }
 }

mithril-stm/src/schnorr_signature/signature.rs

@@ -15,18 +15,23 @@ use crate::{
 };
 
 /// Structure of the Schnorr signature to use with the SNARK
+///
 /// This signature includes a value `sigma` which depends only on
 /// the message and the signing key.
 /// This value is used in the lottery process to determine the correct indices.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) struct SchnorrSignature {
+    /// Deterministic value depending on the message and secret key
     pub(crate) sigma: JubjubSubgroup,
+    /// Part of the Schnorr signature depending on the secret key
     pub(crate) signature: JubjubScalar,
+    /// Part of the Schnorr signature NOT depending on the secret key
     pub(crate) challenge: JubjubBase,
 }
 
 impl SchnorrSignature {
     /// Description of the verification for Schnorr
+    ///
     /// This function performs the verification of a Schnorr signature given the signature, the signed message
     /// and a verification key derived from the secret key used to sign.
     ///
@@ -98,8 +103,8 @@ impl SchnorrSignature {
         Ok(())
     }
 
-    /// Dense mapping function indexed by the index to be evaluated
-    /// adapted to the Schnorr signature.
+    /// Dense mapping function indexed by the index to be evaluated adapted to the Schnorr signature.
+    ///
     /// We need to convert the inputs to fit in a Poseidon hash.
     /// The order of the hash input must be the same as the one in the SNARK circuit
     /// `ev = H(DST || msg || index || σ) <- MSP.Eval(msg,index,σ)` given in paper.
@@ -126,6 +131,7 @@ impl SchnorrSignature {
     }
 
     /// Convert a string of bytes into a `SchnorrSignature`.
+    ///
     /// Not sure the sigma, s and c creation can fail if the 96 bytes are correctly extracted.
     /// TODO: Do we want to fail conversion if there are more than 96 bytes?
     pub(crate) fn from_bytes(bytes: &[u8]) -> Result<Self> {

mithril-stm/src/schnorr_signature/signing_key.rs

@@ -24,9 +24,9 @@ impl SchnorrSigningKey {
         SchnorrSigningKey(JubjubScalar::random(rng))
     }
 
-    // TODO: Check if we want the sign function to handle the randomness by itself
     /// This function is an adapted version of the Schnorr signature scheme
     /// and works with the Jubjub elliptic curve and the Poseidon hash function.
+    ///
     /// The scheme works as follows:
     /// Input:
     ///     - a message: some bytes
@@ -61,6 +61,8 @@ impl SchnorrSigningKey {
     /// The verification algorithm consists in recomputing the challenge from the signature value and
     /// checking it matches the challenge value in the Schnorr signature. It is described in more
     /// details in the implementation of the SchnorrSignature.
+    ///
+    // TODO: Check if we want the sign function to handle the randomness by itself
     pub(crate) fn sign(
         &self,
         msg: &[u8],
@@ -124,6 +126,7 @@ impl SchnorrSigningKey {
     }
 
     /// Convert a string of bytes into a `SchnorrSigningKey`.
+    ///
     /// The bytes must represent a Jubjub scalar or the conversion will fail
     // TODO: Maybe rework this function, do we want to allow any bytes representation
     // to be convertible to a sk?
@@ -168,8 +171,10 @@ mod tests {
     fn test_to_from_bytes() {
         let mut rng = ChaCha20Rng::from_seed([0u8; 32]);
         let sk = SchnorrSigningKey::generate(&mut rng);
+
         let bytes = sk.to_bytes();
         let recovered_sk = SchnorrSigningKey::from_bytes(&bytes).unwrap();
+
         assert_eq!(sk, recovered_sk);
     }
 
@@ -182,6 +187,7 @@ mod tests {
         rng.fill_bytes(&mut sk);
         // Setting the msb to 1 to make sk bigger than the modulus
         sk[0] |= 0xff;
+
         let result = SchnorrSigningKey::from_bytes(&sk);
 
         assert!(

mithril-stm/src/schnorr_signature/utils.rs

@@ -6,13 +6,13 @@ use sha2::{Digest, Sha256};
 use anyhow::{Result, anyhow};
 
 /// Convert an arbitrary array of bytes into a Jubjub scalar field element
+///
 /// First hash the message to 256 bits use Sha256 then perform the conversion
 pub(crate) fn hash_msg_to_jubjubbase(msg: &[u8]) -> Result<JubjubBase> {
     let mut hash = Sha256::new();
     hash.update(msg);
     let hmsg = hash.finalize();
     let mut output = [0u8; 32];
-    // Adding a check here but this
     if hmsg.len() == output.len() {
         output.copy_from_slice(&hmsg);
     } else {
@@ -30,18 +30,18 @@ pub(crate) fn hash_msg_to_jubjubbase(msg: &[u8]) -> Result<JubjubBase> {
 }
 
 /// Extract the coordinates of a given point
+///
 /// This is mainly use to feed the Poseidon hash function
 pub(crate) fn get_coordinates(point: JubjubSubgroup) -> (JubjubBase, JubjubBase) {
-    let extended = JubjubExtended::from(point); // Convert to JubjubExtended
-    let affine = JubjubAffine::from(extended); // Convert to JubjubAffine (affine coordinates)
-    let x = affine.get_u(); // Get x-coordinate
-    let y = affine.get_v(); // Get y-coordinate
+    let point_extended_representation = JubjubExtended::from(point);
+    let point_affine_representation = JubjubAffine::from(point_extended_representation);
+    let x_coordinate = point_affine_representation.get_u();
+    let y_coordinate = point_affine_representation.get_v();
 
-    (x, y)
+    (x_coordinate, y_coordinate)
 }
 
-/// Convert an element of the BLS12-381 base field to
-/// one of the Jubjub base field
+/// Convert an element of the BLS12-381 base field to one of the Jubjub base field
 pub(crate) fn jubjub_base_to_scalar(x: &JubjubBase) -> Result<JubjubScalar> {
     let bytes = x.to_bytes_le();