clippy + comments

Author: tgeoghegan

src/circuit.rs

@@ -576,7 +576,7 @@ pub(crate) mod tests {
         pub(crate) fn linear_constraint_rhs<FE: CodecFieldElement>(&self) -> Vec<FE> {
             self.linear_rhs
                 .iter()
-                .map(|element| FE::try_from(hex::decode(&element).unwrap().as_slice()).unwrap())
+                .map(|element| FE::try_from(hex::decode(element).unwrap().as_slice()).unwrap())
                 .collect()
         }
     }

src/fields/mod.rs

@@ -119,6 +119,13 @@ pub trait CodecFieldElement:
 
 /// Elements of a field in which we can interpolate polynomials up to degree three. We precompute
 /// the denominators of the basis polynomials to avoid needing division.
+///
+/// # Bugs
+///
+/// The methods `sumcheck_p2_mul_inv`, `one_minus_sumcheck_p2_mul_inv`,
+/// `sumcheck_p2_squared_minus_sumcheck_p2_mul_inv` should be constants ([1]).
+///
+/// [1]: https://github.com/abetterinternet/zk-cred-longfellow/issues/40
 pub trait LagrangePolynomialFieldElement: FieldElement {
     /// Evaluate the ith Lagrange basis polynomial at x. From [Section 6.6][1]:
     ///

src/sumcheck/bind.rs

@@ -247,10 +247,15 @@ impl<FE: FieldElement> ElementwiseSum for FE {
     }
 }
 
-/// Naive implementation of bindeq() from 6.2. This binds `input` of length `l` to the implicit
-/// `EQ_2^l` array.
+/// Naive implementation of bindeq() from 6.2 ([1]). This binds `input` of length `l` to the
+/// implicit `EQ_2^l` array.
 ///
-/// We should rework this to avoid recursion.
+/// # Bugs
+///
+/// We should rework this to avoid recursion ([2]).
+///
+/// [1]: https://datatracker.ietf.org/doc/html/draft-google-cfrg-libzk-01#section-6.2
+/// [2]: https://github.com/abetterinternet/zk-cred-longfellow/issues/41
 pub fn bindeq<FE: FieldElement>(input: &[FE]) -> Vec<FE> {
     let output_len = 2usize.pow(
         input
@@ -725,9 +730,10 @@ mod tests {
         // 6.2: bindv(EQ_{n}, X) = bindeq(l, X) for n = 2^l
         fn construct_eq(n: usize) -> Vec<Vec<FieldP256>> {
             let mut eq_n = vec![vec![FieldP256::ZERO; n]; n];
-            for i in 0..n {
-                for j in 0..n {
-                    eq_n[i][j] = if i == j {
+
+            for (i, row) in eq_n.iter_mut().enumerate() {
+                for (j, element) in row.iter_mut().enumerate() {
+                    *element = if i == j {
                         FieldP256::ONE
                     } else {
                         FieldP256::ZERO

src/sumcheck/constraints.rs

@@ -15,13 +15,11 @@ use crate::{
 };
 use serde::Deserialize;
 
-/// A term of a  linear constraint consisting of a triple (c, j, k), per [4.4.2][1]. This is one
+/// A term of a linear constraint consisting of a triple (c, j, k), per [4.4.2][1]. This is one
 /// element of the the constraint matrix A for verifying that A * W = b. Several of these terms sum
 /// together into one of the elements of `ProofConstraints::linear_constraint_rhs`.
 ///
 /// [1]: https://datatracker.ietf.org/doc/html/draft-google-cfrg-libzk-01#section-4.4.2
-// We don't yet examine these outside of test code, so allow dead code for now.
-#[allow(dead_code)]
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct LinearConstraintLhsTerm<FieldElement> {
     /// The constraint number or row of A. This is an index into the vector `b`, which we represent
@@ -63,7 +61,11 @@ impl<FE: CodecFieldElement + LagrangePolynomialFieldElement> ProofConstraints<FE
     /// `sym_private_inputs`, but since the whole point is that we don't know what those values are,
     /// it doesn't make sense to represent them as arguments.
     ///
+    /// `ligero_commitment` is the commitment computed per [4.3][1], which is needed to initialize
+    /// the transcript.
+    ///
     /// [1]: https://datatracker.ietf.org/doc/html/draft-google-cfrg-libzk-01#section-6.6
+    /// [2]: https://datatracker.ietf.org/doc/html/draft-google-cfrg-libzk-01#section-4.3
     pub fn from_proof(
         circuit: &Circuit,
         public_inputs: &[FE],
@@ -139,15 +141,15 @@ impl<FE: CodecFieldElement + LagrangePolynomialFieldElement> ProofConstraints<FE
             // =
             //  Q * layer_proof.vl * layer_proof.vl - known_part
             //
-            // The LHS of this constraint will consist of two LinearConstraintTerms (p0 and p2) for
-            // each polynomial (for the symbolic manipulations needed to compute symbolic_part),\
-            // plus three more terms for this layer's vl, vr, vl*vr.
+            // The LHS of this constraint will consist of two LinearConstraintTerms for the previous
+            // layer's vl and vr (to compute this layer's claims), plus two terms (p0 and p2) for
+            // each polynomial in the layer, plus three more terms for this layer's vl, vr, vl*vr.
             //
             // The RHS can be computed straightforwardly and so we get one element per layer.
             //
             // Q is the quad once reduced down to a single value, which is bound_quad[0][0] for us.
 
-            // Known portion of initial claim.
+            // Known portion of layer's initial claim.
             let mut claim_known = claims[0] + alpha * claims[1];
 
             if layer_index > 0 {
@@ -478,7 +480,7 @@ mod tests {
 
         let test_vector_constraints = test_vector.constraints.unwrap();
         let test_vector_ligero_commitment =
-            hex::decode(&test_vector.ligero_commitment.as_ref().unwrap()).unwrap();
+            hex::decode(test_vector.ligero_commitment.as_ref().unwrap()).unwrap();
 
         let evaluation: Evaluation<FieldP128> = circuit
             .evaluate(&test_vector.valid_inputs.unwrap())