Various clean ups (#715)

fastcrypto-zkp/src/bls12381/conversions.rs

@@ -149,8 +149,8 @@ pub fn blst_fp12_to_bls_fq12(f: &blst_fp12) -> Fq12 {
     Fq12::new(c0, c1)
 }
 
-/// Affine point translations: those mostly allow us to receive the
-/// proof points, provided in affine form.
+// Affine point translations: those mostly allow us to receive the
+// proof points, provided in affine form.
 
 fn blst_g1_affine_infinity() -> blst_p1_affine {
     blst_p1_affine {
@@ -190,10 +190,8 @@ pub fn bls_g1_affine_to_blst_g1_affine(pt: &BlsG1Affine) -> blst_p1_affine {
     };
 
     let mut g1 = blst_p1_affine::default();
-    debug_assert_eq!(
-        unsafe { blst_p1_deserialize(&mut g1, tmp2.as_ptr()) },
-        BLST_ERROR::BLST_SUCCESS
-    );
+    let result = unsafe { blst_p1_deserialize(&mut g1, tmp2.as_ptr()) };
+    debug_assert_eq!(result, BLST_ERROR::BLST_SUCCESS);
     g1
 }
 
@@ -260,10 +258,8 @@ pub fn bls_g2_affine_to_blst_g2_affine(pt: &BlsG2Affine) -> blst_p2_affine {
     };
 
     let mut g2 = blst_p2_affine::default();
-    debug_assert_eq!(
-        unsafe { blst_p2_deserialize(&mut g2, tmp2.as_ptr()) },
-        BLST_ERROR::BLST_SUCCESS
-    );
+    let result = unsafe { blst_p2_deserialize(&mut g2, tmp2.as_ptr()) };
+    debug_assert_eq!(result, BLST_ERROR::BLST_SUCCESS);
     g2
 }
 
@@ -434,7 +430,7 @@ pub fn bls_g2_affine_to_zcash_bytes(p: &BlsG2Affine) -> [u8; G2_COMPRESSED_SIZE]
     bytes
 }
 
-// Attempt to obtain x_coordinate
+/// Attempt to obtain x_coordinate
 fn obtain_x_coordinate(bytes: &[u8]) -> Option<Fq> {
     let mut tmp = [0; G1_COMPRESSED_SIZE];
     // this is safe as the private obtain_x_coordinate function is only invoked with G1 (48) and

fastcrypto-zkp/src/circom.rs

@@ -1,58 +1,70 @@
 // Copyright (c) 2022, Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use ark_bn254::{Fq, Fq2, G1Affine, G1Projective, G2Affine};
+use ark_bn254::{Fq, Fq2, G1Affine, G1Projective, G2Affine, G2Projective};
 use fastcrypto::error::FastCryptoError;
 
+/// A G1 point in BN254 serialized as a vector of three strings which is the canonical decimal
+/// representation of the projective coordinates in Fq.
 pub type CircomG1 = Vec<String>;
+
+/// A G2 point in BN254 serialized as a vector of three vectors each being a vector of two strings
+/// which are the canonical decimal representation of the coefficients of the projective coordinates
+/// in Fq2.
 pub type CircomG2 = Vec<Vec<String>>;
 
+/// Parse a string as a field element in BN254. Return an `FastCryptoError::InvalidInput` error if
+/// the parsing fails.
+fn parse_field_element(s: &str) -> Result<Fq, FastCryptoError> {
+    s.parse::<Fq>().map_err(|_| FastCryptoError::InvalidInput)
+}
+
+/// Deserialize a G1 projective point in BN254 serialized as a vector of three strings into an affine
+/// G1 point in arkworks format. Return an error if the input is not a vector of three strings or if
+/// any of the strings cannot be parsed as a field element.
 pub fn g1_affine_from_str_projective(s: &CircomG1) -> Result<G1Affine, FastCryptoError> {
+    // There are three projective coordinates
     if s.len() != 3 {
         return Err(FastCryptoError::InvalidInput);
     }
+
     Ok(G1Projective::new_unchecked(
-        s[0].parse::<Fq>()
-            .map_err(|_| FastCryptoError::InvalidInput)?,
-        s[1].parse::<Fq>()
-            .map_err(|_| FastCryptoError::InvalidInput)?,
-        s[2].parse::<Fq>()
-            .map_err(|_| FastCryptoError::InvalidInput)?,
+        parse_field_element(&s[0])?,
+        parse_field_element(&s[1])?,
+        parse_field_element(&s[2])?,
     )
     .into())
 }
 
+/// Deserialize a G2 projective point from the BN254 construction serialized as a vector of three
+/// vectors each being a vector of two strings into an affine G2 point in arkworks format. Return an
+/// error if the input is not a vector of the right format or if any of the strings cannot be parsed
+/// as a field element.
 pub fn g2_affine_from_str_projective(s: &CircomG2) -> Result<G2Affine, FastCryptoError> {
-    use ark_bn254::G2Projective;
+    // There are three projective coordinates
     if s.len() != 3 {
         return Err(FastCryptoError::InvalidInput);
     }
 
+    // Each coordinate in Fq2 is represented by two coefficients in Fq
     for x in s {
         if x.len() != 2 {
             return Err(FastCryptoError::InvalidInput);
         }
     }
+
     Ok(G2Projective::new_unchecked(
         Fq2::new(
-            s[0][0]
-                .parse::<Fq>()
-                .map_err(|_| FastCryptoError::InvalidInput)?,
-            s[0][1]
-                .parse::<Fq>()
-                .map_err(|_| FastCryptoError::InvalidInput)?,
+            parse_field_element(&s[0][0])?,
+            parse_field_element(&s[0][1])?,
         ),
         Fq2::new(
-            s[1][0]
-                .parse::<Fq>()
-                .map_err(|_| FastCryptoError::InvalidInput)?,
-            s[1][1]
-                .parse::<Fq>()
-                .map_err(|_| FastCryptoError::InvalidInput)?,
+            parse_field_element(&s[1][0])?,
+            parse_field_element(&s[1][1])?,
         ),
         Fq2::new(
-            s[2][0].parse::<Fq>().unwrap(),
-            s[2][1].parse::<Fq>().unwrap(),
+            parse_field_element(&s[2][0])?,
+            parse_field_element(&s[2][1])?,
         ),
     )
     .into())

fastcrypto-zkp/src/lib.rs

@@ -23,4 +23,5 @@ pub mod bn254;
 /// Simple circuits used in benchmarks and demos
 pub mod dummy_circuits;
 
+/// Circom-compatible deserialization of points
 pub mod circom;