Comments, lints, and formatting from on July 29 (#69)

Signed-off-by: Michele Orrù <[email protected]>
Co-authored-by: Michele Orrù <[email protected]>

Author: nategraf

src/composition.rs

@@ -225,18 +225,13 @@ impl<G: PrimeGroup> ComposedRelation<G> {
         // Calculate the real challenge by subtracting all simulated challenges
         let (child_states, simulated_challenges, simulated_responses) = prover_state;
 
-        let mut real_challenge = challenge;
-        for some_challenge in simulated_challenges.iter() {
-            if let Some(challenge) = some_challenge {
-                real_challenge -= challenge;
-            }
-        }
+        let real_challenge = challenge - simulated_challenges.iter().flatten().sum::<G::Scalar>();
 
         let it = instances
-            .into_iter()
-            .zip(child_states.into_iter())
-            .zip(simulated_challenges.into_iter())
-            .zip(simulated_responses.into_iter());
+            .iter()
+            .zip(child_states)
+            .zip(simulated_challenges)
+            .zip(simulated_responses);
         for (((i, prover_state), simulated_challenge), simulated_response) in it {
             if let Some(state) = prover_state {
                 // Real case: compute response with real challenge
@@ -310,7 +305,7 @@ impl<G: PrimeGroup> SigmaProtocol for ComposedRelation<G> {
                 ComposedRelation::Simple(p),
                 ComposedCommitment::Simple(c),
                 ComposedResponse::Simple(r),
-            ) => p.verifier(c, &challenge, r),
+            ) => p.verifier(c, challenge, r),
             (
                 ComposedRelation::And(ps),
                 ComposedCommitment::And(commitments),
@@ -328,10 +323,10 @@ impl<G: PrimeGroup> SigmaProtocol for ComposedRelation<G> {
                 let last_challenge = *challenge - challenges.iter().sum::<G::Scalar>();
                 ps.iter()
                     .zip(commitments)
-                    .zip(challenges.into_iter().chain(&Some(last_challenge)))
+                    .zip(challenges.iter().chain(&Some(last_challenge)))
                     .zip(responses)
                     .try_for_each(|(((p, commitment), challenge), response)| {
-                        p.verifier(commitment, &challenge, response)
+                        p.verifier(commitment, challenge, response)
                     })
             }
             _ => Err(Error::InvalidInstanceWitnessPair),
@@ -548,7 +543,7 @@ impl<G: PrimeGroup> SigmaProtocolSimulator for ComposedRelation<G> {
                 let mut challenges = Vec::with_capacity(ps.len());
                 let mut responses = Vec::with_capacity(ps.len());
                 for _ in 0..ps.len() {
-                    challenges.push(G::Scalar::random(&mut *rng).into());
+                    challenges.push(G::Scalar::random(&mut *rng));
                 }
                 for p in ps.iter() {
                     responses.push(p.simulate_response(&mut *rng));

src/linear_relation/canonical.rs

@@ -26,7 +26,12 @@ pub struct CanonicalLinearRelation<G: PrimeGroup> {
     pub num_scalars: usize,
 }
 
-type GroupExpr<G> = Vec<(<G as group::Group>::Scalar, GroupVar<G>)>;
+/// Private type alias used to simplify function signatures below.
+///
+/// The cache is essentially a mapping (GroupVar, Scalar) => GroupVar, which maps the original
+/// weighted group vars to a new assignment, such that if a pair appears more than once, it will
+/// map to the same group variable in the canonical linear relation.
+type WeightedGroupCache<G> = HashMap<GroupVar<G>, Vec<(<G as group::Group>::Scalar, GroupVar<G>)>>;
 
 impl<G: PrimeGroup> CanonicalLinearRelation<G> {
     /// Create a new empty canonical linear relation
@@ -45,7 +50,7 @@ impl<G: PrimeGroup> CanonicalLinearRelation<G> {
         group_var: GroupVar<G>,
         weight: &G::Scalar,
         original_group_elements: &GroupMap<G>,
-        weighted_group_cache: &mut HashMap<GroupVar<G>, GroupExpr<G>>,
+        weighted_group_cache: &mut WeightedGroupCache<G>,
     ) -> Result<GroupVar<G>, InvalidInstance> {
         // Check if we already have this (weight, group_var) combination
         let entry = weighted_group_cache.entry(group_var).or_default();
@@ -74,7 +79,7 @@ impl<G: PrimeGroup> CanonicalLinearRelation<G> {
         &image_var: &GroupVar<G>,
         equation: &LinearCombination<G>,
         original_relation: &LinearRelation<G>,
-        weighted_group_cache: &mut HashMap<GroupVar<G>, GroupExpr<G>>,
+        weighted_group_cache: &mut WeightedGroupCache<G>,
     ) -> Result<(), InvalidInstance> {
         let mut rhs_terms = Vec::new();
 
@@ -130,7 +135,12 @@ impl<G: PrimeGroup> CanonicalLinearRelation<G> {
     pub fn label(&self) -> Vec<u8> {
         let mut out = Vec::new();
 
-        // Replicate the original LinearRelationReprBuilder ordering behavior
+        // Create an ordered list of unique group element representations. Elements are ordered
+        // based on the order they appear in the canonical linear relation, as seen by the loop
+        // below. Note that this is dependent on the building order in TryFrom<LinearRelation>.
+        // QUESTION: Does anything depend on this order being stable? This seems difficult to
+        // maintain across versions of this library, and changes to the relation definition may
+        // have difficult to predict effects on the order.
         let mut group_repr_mapping: HashMap<Box<[u8]>, u32> = HashMap::new();
         let mut group_elements_ordered = Vec::new();
 
@@ -146,8 +156,9 @@ impl<G: PrimeGroup> CanonicalLinearRelation<G> {
             new_index
         };
 
-        // Build constraint data in the same order as original
-        let mut constraint_data = Vec::new();
+        // Build constraint data in the same order as original, as a nested list of group and
+        // scalar indices. Note that the group indices are into group_elements_ordered.
+        let mut constraint_data = Vec::<(u32, Vec<(u32, u32)>)>::new();
 
         for (image_elem, constraint_terms) in iter::zip(&self.image, &self.linear_combinations) {
             // First, add the left-hand side (image) element

src/linear_relation/mod.rs

@@ -160,7 +160,7 @@ impl<G: PrimeGroup> GroupMap<G> {
 
     /// Get the element value assigned to the given point var.
     ///
-    /// Returns [`Error::UnassignedGroupVar`] if a value is not assigned.
+    /// Returns [`InvalidInstance`] if a value is not assigned.
     pub fn get(&self, var: GroupVar<G>) -> Result<G, InvalidInstance> {
         match self.0.get(var.0) {
             Some(Some(elem)) => Ok(*elem),

src/linear_relation/ops.rs

@@ -784,9 +784,9 @@ mod tests {
 
         let diff = x - y;
         assert_eq!(diff.terms().len(), 2);
-        assert_eq!(diff.terms()[0].term, y.into());
+        assert_eq!(diff.terms()[0].term, y);
         assert_eq!(diff.terms()[0].weight, -Scalar::ONE);
-        assert_eq!(diff.terms()[1].term, x.into());
+        assert_eq!(diff.terms()[1].term, x);
         assert_eq!(diff.terms()[1].weight, Scalar::ONE);
     }