Modifications according to reviews.

Author: damrobi

mithril-stm/Cargo.toml

@@ -19,23 +19,30 @@ rug-backend = ["rug/default"]
 num-integer-backend = ["num-bigint", "num-rational", "num-traits"]
 benchmark-internals = [] # For benchmarking multi_sig
 future_proof_system = [] # For activating future proof systems
-future_snark = ["ff", "midnight-circuits", "midnight-curves", "sha2", "group", "num-traits"] # For activating snark features
+future_snark = [
+    "ff",
+    "midnight-circuits",
+    "midnight-curves",
+    "sha2",
+    "group",
+    "num-traits",
+] # For activating snark features
 
 [dependencies]
 anyhow = { workspace = true }
 blake2 = "0.10.6"
 # Enforce blst portable feature for runtime detection of Intel ADX instruction set.
 blst = { version = "0.3.16", features = ["portable"] }
 digest = { workspace = true }
-ff = {version = "0.13.1", optional = true}
-group = {version = "0.13.0", optional = true }
+ff = { version = "0.13.1", optional = true }
+group = { version = "0.13.0", optional = true }
 midnight-circuits = { git = "https://github.com/midnightntwrk/midnight-zk", rev = "c88a50c2169f060120a52ad0980de90f08bc9535", optional = true }
-midnight-curves = { git = "https://github.com/midnightntwrk/midnight-zk", rev = "c88a50c2169f060120a52ad0980de90f08bc9535", optional = true  }
-num-traits = {version = "0.2.19", optional = true}
+midnight-curves = { git = "https://github.com/midnightntwrk/midnight-zk", rev = "c88a50c2169f060120a52ad0980de90f08bc9535", optional = true }
+num-traits = { version = "0.2.19", optional = true }
 rand_core = { workspace = true }
 rayon = { workspace = true }
 serde = { workspace = true }
-sha2 = {version = "0.10.9", optional = true }
+sha2 = { version = "0.10.9", optional = true }
 thiserror = { workspace = true }
 
 [target.'cfg(any(target_family = "wasm", target_env = "musl", windows))'.dependencies]

mithril-stm/src/schnorr_signature/mod.rs

@@ -3,23 +3,23 @@
 
 mod signature;
 mod signing_key;
-mod utils;
+pub(super) mod utils;
 mod verification_key;
 
+pub use signature::*;
+pub use utils::*;
+pub use verification_key::*;
+
 use midnight_circuits::{
     ecc::{hash_to_curve::HashToCurveGadget, native::EccChip},
     hash::poseidon::PoseidonChip,
     types::AssignedNative,
 };
 use midnight_curves::{Fq as JubjubBase, JubjubExtended};
 
-use signature::*;
-use utils::*;
-use verification_key::*;
-
 /// A DST (Domain Separation Tag) to distinguish between use of Poseidon hash
-pub const DST_SIGNATURE: JubjubBase = JubjubBase::from_raw([0u64, 0, 0, 0]);
-pub const DST_LOTTERY: JubjubBase = JubjubBase::from_raw([1u64, 0, 0, 0]);
+pub(crate) const DST_SIGNATURE: JubjubBase = JubjubBase::from_raw([0u64, 0, 0, 0]);
+pub(crate) const DST_LOTTERY: JubjubBase = JubjubBase::from_raw([1u64, 0, 0, 0]);
 
 /// Defining a type for the CPU hash to curve gadget
 pub(crate) type JubjubHashToCurve = HashToCurveGadget<
@@ -151,9 +151,18 @@ 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 sig_bytes: [u8; 96] = sig.to_bytes();
         let sig2 = SchnorrSignature::from_bytes(&sig_bytes).unwrap();
 
         assert_eq!(sig, sig2);
     }
+
+    #[test]
+    fn test_from_bytes_signature_too_many_bytes() {
+        let msg = vec![0u8; 97];
+
+        let result = SchnorrSignature::from_bytes(&msg);
+
+        assert!(result.is_err());
+    }
 }

mithril-stm/src/schnorr_signature/signature.rs

@@ -20,7 +20,7 @@ use crate::{
 /// 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 {
+pub 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
@@ -58,24 +58,24 @@ impl SchnorrSignature {
     /// to their coordinates representation to feed them to the hash function.
     ///     - Check: c == c_tilde
     ///     
-    pub(crate) fn verify(&self, msg: &[u8], vk: &SchnorrVerificationKey) -> Result<()> {
-        let g = JubjubSubgroup::generator();
+    pub fn verify(&self, msg: &[u8], vk: &SchnorrVerificationKey) -> Result<()> {
+        let generator = JubjubSubgroup::generator();
 
         // First hashing the message to a scalar then hashing it to a curve point
-        let hash = JubjubHashToCurve::hash_to_curve(&[hash_msg_to_jubjubbase(msg)?]);
+        let hash_msg = JubjubHashToCurve::hash_to_curve(&[hash_msg_to_jubjubbase(msg)?]);
 
         // Computing R1 = H(msg) * s + sigma * c
-        let c_scalar = jubjub_base_to_scalar(&self.challenge)?;
-        let h_s = hash * self.signature;
-        let sigma_c = self.sigma * c_scalar;
+        let challenge_scalar = jubjub_base_to_scalar(&self.challenge)?;
+        let h_s = hash_msg * self.signature;
+        let sigma_c = self.sigma * challenge_scalar;
         let r1_tilde = h_s + sigma_c;
 
         // Computing R2 = g * s + vk * c
-        let g_s = g * self.signature;
-        let vk_c = vk.0 * c_scalar;
+        let g_s = generator * self.signature;
+        let vk_c = vk.0 * challenge_scalar;
         let r2_tilde = g_s + vk_c;
 
-        let (hashx, hashy) = get_coordinates(hash);
+        let (hashx, hashy) = get_coordinates(hash_msg);
         let (vkx, vky) = get_coordinates(vk.0);
         let (sigmax, sigmay) = get_coordinates(self.sigma);
         let (r1x, r1y) = get_coordinates(r1_tilde);
@@ -108,7 +108,7 @@ impl SchnorrSignature {
     /// 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.
-    fn evaluate_dense_mapping(&self, msg: &[u8], index: Index) -> Result<[u8; 32]> {
+    pub(crate) fn evaluate_dense_mapping(&self, msg: &[u8], index: Index) -> Result<[u8; 32]> {
         let hash = JubjubHashToCurve::hash_to_curve(&[hash_msg_to_jubjubbase(msg)?]);
         let (hashx, hashy) = get_coordinates(hash);
         // TODO: Check if this is the correct way to add the index
@@ -121,7 +121,7 @@ impl SchnorrSignature {
     }
 
     /// Convert an `SchnorrSignature` to a byte representation.
-    pub(crate) fn to_bytes(self) -> [u8; 96] {
+    pub fn to_bytes(self) -> [u8; 96] {
         let mut out = [0; 96];
         out[0..32].copy_from_slice(&self.sigma.to_bytes());
         out[32..64].copy_from_slice(&self.signature.to_bytes());
@@ -133,11 +133,8 @@ 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> {
-        let bytes = bytes
-            .get(..96)
-            .ok_or(anyhow!("Not enough bytes to create a signature."))?;
+    pub fn from_bytes(bytes: &[u8]) -> Result<Self> {
+        let bytes: [u8; 96] = bytes.try_into()?;
         let sigma = JubjubSubgroup::from_bytes(&bytes[0..32].try_into()?)
             .into_option()
             .ok_or(anyhow!("Unable to convert bytes into a sigma value."))?;

mithril-stm/src/schnorr_signature/signing_key.rs

@@ -1,23 +1,22 @@
 use anyhow::{Result, anyhow};
 use ff::Field;
-use midnight_circuits::hash::poseidon::PoseidonChip;
-use midnight_circuits::instructions::hash::HashCPU;
+use midnight_circuits::{
+    hash::poseidon::PoseidonChip,
+    instructions::{HashToCurveCPU, hash::HashCPU},
+};
 use midnight_curves::{Fq as JubjubBase, Fr as JubjubScalar, JubjubSubgroup};
 use rand_core::{CryptoRng, RngCore};
 
-use midnight_circuits::instructions::HashToCurveCPU;
-
 use group::Group;
 
-pub(crate) use crate::schnorr_signature::{
-    DST_SIGNATURE, JubjubHashToCurve,
+use crate::schnorr_signature::{
+    DST_SIGNATURE, JubjubHashToCurve, SchnorrSignature, SchnorrVerificationKey,
     utils::{get_coordinates, hash_msg_to_jubjubbase, jubjub_base_to_scalar},
 };
-use crate::schnorr_signature::{SchnorrSignature, SchnorrVerificationKey};
 
 /// Schnorr Signing key, it is essentially a random scalar of the Jubjub scalar field
 #[derive(Debug, Clone)]
-pub(crate) struct SchnorrSigningKey(pub(crate) JubjubScalar);
+pub struct SchnorrSigningKey(pub(crate) JubjubScalar);
 
 impl SchnorrSigningKey {
     pub(crate) fn generate(rng: &mut (impl RngCore + CryptoRng)) -> Self {

mithril-stm/src/schnorr_signature/utils.rs

@@ -8,7 +8,7 @@ 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> {
+pub fn hash_msg_to_jubjubbase(msg: &[u8]) -> Result<JubjubBase> {
     let mut hash = Sha256::new();
     hash.update(msg);
     let hmsg = hash.finalize();
@@ -32,7 +32,7 @@ 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) {
+pub fn get_coordinates(point: JubjubSubgroup) -> (JubjubBase, JubjubBase) {
     let point_extended_representation = JubjubExtended::from(point);
     let point_affine_representation = JubjubAffine::from(point_extended_representation);
     let x_coordinate = point_affine_representation.get_u();
@@ -42,7 +42,7 @@ pub(crate) fn get_coordinates(point: JubjubSubgroup) -> (JubjubBase, JubjubBase)
 }
 
 /// 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> {
+pub fn jubjub_base_to_scalar(x: &JubjubBase) -> Result<JubjubScalar> {
     let bytes = x.to_bytes_le();
 
     Ok(JubjubScalar::from_raw([

mithril-stm/src/schnorr_signature/verification_key.rs

@@ -2,12 +2,12 @@ use anyhow::{Result, anyhow};
 use group::{Group, GroupEncoding};
 pub use midnight_curves::JubjubSubgroup;
 
-use crate::schnorr_signature::signing_key::SchnorrSigningKey;
+pub(crate) use crate::schnorr_signature::signing_key::SchnorrSigningKey;
 
 /// Schnorr verification key, it consists of a point on the Jubjub curve
 /// vk = g * sk, where g is a generator
 #[derive(Debug, Clone, Copy, Default)]
-pub(crate) struct SchnorrVerificationKey(pub(crate) JubjubSubgroup);
+pub struct SchnorrVerificationKey(pub(crate) JubjubSubgroup);
 
 impl SchnorrVerificationKey {
     /// TODO: Make sure this is correct as the previous implementation is