handle generators refs and input validation

.github/workflows/build-and-test.yml

@@ -19,6 +19,9 @@ jobs:
     - name: Install stable Rust
       uses: dtolnay/rust-toolchain@stable
 
+    - name: Install cargo-nextest
+      uses: taiki-e/install-action@nextest
+
     - name: Cache cargo registry
       uses: actions/cache@v3
       with:
@@ -38,7 +41,7 @@ jobs:
           ${{ runner.os }}-target-
 
     - name: Run tests
-      run: cargo test --release --verbose
+      run: cargo nextest run --release
 
   build-wasm:
     runs-on: ubuntu-latest

ark-dlog-gadget/benches/dlog.rs

@@ -238,7 +238,8 @@ fn bench_blinding_with_discrete_log_verify<C, Params, B, S, BP>(
                     &mut verifier,
                     &all_divisor_commitments[i],
                     vc_len,
-                );
+                )
+                .unwrap();
 
                 discrete_log_blinding(
                     &mut verifier,
@@ -473,7 +474,8 @@ fn bench_blinding_with_discrete_log_combined_verify<C, Params, B, S, BP>(
                     &mut verifier,
                     &all_divisor_commitments[i],
                     vc_len,
-                );
+                )
+                .unwrap();
                 all_o_blind_claims.push((o_blind_claim, o_x_var, o_y_var));
             }
 

ark-dlog-gadget/src/dlog.rs

@@ -429,6 +429,7 @@ pub fn discrete_log_challenge<
         let z3 = vvv;
 
         // Normalize from XYZ to XY
+        // unwrap is as both points are chosen to be distinct and not negative of each other
         let z3_inv = z3.inverse().unwrap();
         let x3 = x3 * z3_inv;
         let y3 = y3 * z3_inv;
@@ -752,6 +753,7 @@ pub fn create_divisor_and_decomposition<
     )))
 }
 
+/// Assumes `vars_dlog` and `vars_divisor` are of appropriate length.
 fn dlog_and_divisor_vars<F: PrimeField, Parameters: DiscreteLogParameters>(
     mut vars_dlog: Vec<Variable<F>>,
     mut vars_divisor: Vec<Variable<F>>,
@@ -874,18 +876,30 @@ pub fn commit_witness_chunks_verifier<
     cs: &mut Verifier<MerlinTranscript, C>,
     comms: &DivisorComms<C>,
     chunk_len: usize,
-) -> Box<PointWithDlog<F, Parameters>> {
-    let mut vars = Vec::with_capacity(DECOMPOSITION_SIZE * 2);
+) -> Result<Box<PointWithDlog<F, Parameters>>, Error> {
+    if chunk_len == 0 {
+        return Err(Error::ZeroChunkSize);
+    }
+
+    let expected_vars_len = DECOMPOSITION_SIZE * 2;
+    let mut vars = Vec::with_capacity(expected_vars_len);
 
     for comm in &comms.0 {
         let chunk_vars = cs.commit_vec(chunk_len, *comm);
         vars.extend(chunk_vars);
     }
+    // Allow padding
+    if vars.len() < expected_vars_len {
+        return Err(Error::VerifierWitnessVarCountMismatch {
+            got: vars.len(),
+            expected: expected_vars_len,
+        });
+    }
 
     let vars_dlog = vars[0..DECOMPOSITION_SIZE].to_vec();
     let vars_divisor = vars[DECOMPOSITION_SIZE..].to_vec();
 
-    dlog_and_divisor_vars(vars_dlog, vars_divisor)
+    Ok(dlog_and_divisor_vars(vars_dlog, vars_divisor))
 }
 
 /// Each generator in `generator_sources` is multiplied by the scalar `blinding`
@@ -1056,13 +1070,25 @@ pub fn commit_witness_chunks_verifier_multi_gen<
     comms: &DivisorComms<C>,
     chunk_len: usize,
     num_generators: usize,
-) -> PointsWithDlog<F, Parameters> {
-    let mut vars = Vec::new();
+) -> Result<PointsWithDlog<F, Parameters>, Error> {
+    if chunk_len == 0 {
+        return Err(Error::ZeroChunkSize);
+    }
+    let expected_vars_len =
+        MAX_BITS_SUPPORTED + num_generators + (num_generators * DECOMPOSITION_SIZE);
+    let mut vars = Vec::with_capacity(expected_vars_len);
     for comm in &comms.0 {
         let chunk_vars = cs.commit_vec(chunk_len, *comm);
         vars.extend(chunk_vars);
     }
-    dlog_and_divisor_vars_multi(vars, num_generators)
+    // Allow padding
+    if vars.len() < expected_vars_len {
+        return Err(Error::VerifierWitnessVarCountMismatch {
+            got: vars.len(),
+            expected: expected_vars_len,
+        });
+    }
+    Ok(dlog_and_divisor_vars_multi(vars, num_generators))
 }
 
 /// Similar to [`discrete_log`] but proves that given points have the specified discrete logarithm over
@@ -1434,6 +1460,64 @@ mod tests {
         aff.xy()
     }
 
+    #[test]
+    fn test_commit_witness_chunks_verifier_rejects_malformed_commitments() {
+        let mut rng = StdRng::seed_from_u64(11);
+        let pc_gens = PedersenGens::<VestaAffine>::default();
+        let bp_gens = BulletproofGens::<VestaAffine>::new(512, 1);
+        let generator = Projective::<PallasConfig>::rand(&mut rng);
+        let generator_table =
+            GeneratorTable::<PallasBase, PallasParams>::new::<PallasConfig>(generator);
+
+        let witness = create_divisor_and_decomposition::<_, PallasConfig, PallasParams>(
+            &generator_table,
+            PallasScalar::rand(&mut rng),
+        )
+        .unwrap();
+
+        let vc_len = 64;
+
+        let transcript = MerlinTranscript::new(b"malformed-dlog-prover");
+        let mut prover = Prover::new(&pc_gens, transcript);
+        let (comms, _, _) = commit_witness_chunks_prover::<_, _, _, PallasParams>(
+            &mut rng,
+            &mut prover,
+            &witness,
+            vc_len,
+            &bp_gens,
+        )
+        .unwrap();
+
+        let mut missing_chunk = comms.clone();
+        missing_chunk.0.pop();
+
+        let transcript = MerlinTranscript::new(b"malformed-dlog-verifier-short");
+        let mut verifier = Verifier::new(transcript);
+        let result = commit_witness_chunks_verifier::<_, _, PallasParams>(
+            &mut verifier,
+            &missing_chunk,
+            vc_len,
+        );
+        match result {
+            Err(err) => match err {
+                Error::VerifierWitnessVarCountMismatch {
+                    got: _,
+                    expected: _,
+                } => (),
+                other => panic!("unexpected error variant: {other:?}"),
+            },
+            _ => panic!("expected verifier witness var count mismatch"),
+        }
+
+        let transcript = MerlinTranscript::new(b"malformed-dlog-verifier-zero");
+        let mut verifier = Verifier::new(transcript);
+        let result = commit_witness_chunks_verifier::<_, _, PallasParams>(&mut verifier, &comms, 0);
+        match result {
+            Err(err) => assert!(matches!(err, Error::ZeroChunkSize)),
+            _ => panic!("expected zero chunk size error"),
+        }
+    }
+
     #[test]
     fn test_blinding_with_discrete_log() {
         fn check<
@@ -1543,7 +1627,8 @@ mod tests {
                 let o_x_var = vars_orig.pop().unwrap();
 
                 let o_blind_claim =
-                    commit_witness_chunks_verifier::<_, _, Params>(&mut verifier, &comms, vc_len);
+                    commit_witness_chunks_verifier::<_, _, Params>(&mut verifier, &comms, vc_len)
+                        .unwrap();
 
                 verifying_times_00.push(start.elapsed());
 
@@ -1750,7 +1835,8 @@ mod tests {
                     &mut verifier,
                     &all_divisor_commitments[i],
                     vc_len,
-                );
+                )
+                .unwrap();
                 all_o_blind_claims.push((o_blind_claim, O_x_var, O_y_var));
             }
 
@@ -1942,7 +2028,8 @@ mod tests {
                     &mut verifier,
                     &all_divisor_commitments[i],
                     vc_len,
-                );
+                )
+                .unwrap();
                 all_o_blind_claims.push((o_blind_claim, O_x_var, O_y_var));
             }
 
@@ -2102,7 +2189,8 @@ mod tests {
                 &comms,
                 vc_len,
                 n,
-            );
+            )
+            .unwrap();
             let commit_time_v = start_v.elapsed();
 
             let original_point_vars: Vec<(Variable<B>, Variable<B>)> = (0..n)
@@ -2268,7 +2356,8 @@ mod tests {
                     &comms,
                     vc_len,
                     2,
-                );
+                )
+                .unwrap();
 
                 discrete_log_blinding_and_dlog(
                     &mut verifier,

ark-dlog-gadget/src/error.rs

@@ -39,7 +39,13 @@ pub enum Error {
     /// Combined witness length is not evenly divisible by chunk length.
     #[error("Combined witness length is not evenly divisible by chunk length: {0} % {1}")]
     WitnessChunkLengthMismatch(usize, usize),
+    /// Chunk size is zero.
+    #[error("Chunk size is zero")]
+    ZeroChunkSize,
     /// Mismatched size error.
     #[error("Mismatched size: got {0}, expected {1}")]
     MismatchedSize(usize, usize),
+    /// Verifier committed witness variables count mismatch.
+    #[error("Verifier committed witness variable count mismatch: got {got}, expected {expected}")]
+    VerifierWitnessVarCountMismatch { got: usize, expected: usize },
 }

ark-dlog-gadget/src/utils.rs

@@ -43,7 +43,7 @@ impl<C: DivisorCurve> ScalarMulAndDivisor<C> {
         let (x, y) = aff.xy().ok_or(Error::PointAtInfinity)?;
         let divisor = scalar
             .scalar_mul_divisor(generator_source)?
-            .normalize_x_coefficient();
+            .normalize_x_coefficient()?;
         Ok(ScalarMulAndDivisor {
             point,
             x,
@@ -131,6 +131,7 @@ pub fn incomplete_add_pub<F: PrimeField, Cs: ConstraintSystem<F>>(
     // slope of line through (b_x, b_y) and (a_x, a_y)
     let slope = cs.evaluate(&b_y_lc).map(|b_y| {
         let b_x = cs.evaluate(&b_x_lc).unwrap();
+        // unwrap only affects prover as verifier can't evaluate the lc
         let b_x_minus_a_x_inv = (b_x - a_x).inverse().unwrap();
         (b_y - a_y) * b_x_minus_a_x_inv
     });
@@ -171,6 +172,7 @@ pub fn inverse<F: Field, Cs: ConstraintSystem<F>>(
             assert_eq!(F::ONE, o.unwrap());
         }
     }
+    // unwrap only affects prover as verifier can't evaluate the lc
     let x_inv = cs.evaluate(&x_lc).map(|x| x.inverse().unwrap());
     let x_inv_lc: LinearCombination<F> = cs.allocate(x_inv).unwrap().into();
     let (_, _, o) = cs.multiply(x_lc, x_inv_lc.clone());

ark-ec-divisors/src/poly.rs

@@ -1,3 +1,4 @@
+use crate::error::Error;
 use ark_ff::PrimeField;
 use ark_std::{vec, vec::Vec};
 use core::ops::{Add, Mul, Neg, Sub};
@@ -364,9 +365,11 @@ impl<F: PrimeField> DivisorPoly<F> {
     ///
     /// Panics if there is no x coefficient to normalize or if it cannot be normalized to 1.
     #[must_use]
-    pub fn normalize_x_coefficient(self) -> Self {
-        let scalar = self.x_coefficients[0].inverse().unwrap();
-        self * scalar
+    pub fn normalize_x_coefficient(self) -> Result<Self, Error> {
+        let scalar = self.x_coefficients[0]
+            .inverse()
+            .ok_or_else(|| Error::InvertingZero)?;
+        Ok(self * scalar)
     }
 }
 

bulletproofs/src/errors.rs

@@ -41,6 +41,9 @@ pub enum ProofError {
     /// Hash to curve error
     #[error("Hash to curve error")]
     HashToCurveError,
+    /// Attempting to invert zero in challenge calculation.
+    #[error("Attempting to invert zero in challenge calculation")]
+    InvertingZero,
 }
 
 impl From<MPCError> for ProofError {

bulletproofs/src/inner_product_proof.rs

@@ -44,7 +44,7 @@ impl<C: AffineRepr> InnerProductProof<C> {
         mut H_vec: Vec<C>,
         mut a_vec: Vec<C::ScalarField>,
         mut b_vec: Vec<C::ScalarField>,
-    ) -> InnerProductProof<C> {
+    ) -> Result<InnerProductProof<C>, ProofError> {
         // Create slices G, H, a, b backed by their respective
         // vectors.  This lets us reslice as we compress the lengths
         // of the vectors in the main loop below.
@@ -129,7 +129,7 @@ impl<C: AffineRepr> InnerProductProof<C> {
             transcript.append_point(b"R", &R);
 
             let u = TranscriptProtocol::challenge_scalar::<C>(transcript, b"u");
-            let u_inv = u.inverse().expect("u challenge is zero");
+            let u_inv = u.inverse().ok_or_else(|| ProofError::InvertingZero)?;
 
             for i in 0..n {
                 a_L[i] = a_L[i] * u + u_inv * a_R[i];
@@ -201,7 +201,7 @@ impl<C: AffineRepr> InnerProductProof<C> {
             transcript.append_point(b"R", &R);
 
             let u = TranscriptProtocol::challenge_scalar::<C>(transcript, b"u");
-            let u_inv = u.inverse().expect("u challenge is zero");
+            let u_inv = u.inverse().ok_or_else(|| ProofError::InvertingZero)?;
 
             for i in 0..n {
                 a_L[i] = a_L[i] * u + u_inv * a_R[i];
@@ -217,12 +217,12 @@ impl<C: AffineRepr> InnerProductProof<C> {
             // todo collapse iteration one and rest?
         }
 
-        InnerProductProof {
+        Ok(InnerProductProof {
             L_vec,
             R_vec,
             a: a[0],
             b: b[0],
-        }
+        })
     }
 
     /// Computes three vectors of verification scalars \\([u\_{i}^{2}]\\), \\([u\_{i}^{-2}]\\) and \\([s\_{i}]\\) for combined multiscalar multiplication
@@ -466,7 +466,8 @@ mod tests {
             H.clone(),
             a.clone(),
             b.clone(),
-        );
+        )
+        .unwrap();
 
         let mut verifier = MerlinTranscript::new(b"innerproducttest");
         proof.verify(

bulletproofs/src/r1cs/prover.rs

@@ -1240,7 +1240,7 @@ impl<'g, T: BorrowMut<MerlinTranscript>, C: AffineRepr> Prover<'g, T, C> {
             gens.H(padded_n).copied().collect(),
             l_vec,
             r_vec,
-        );
+        )?;
 
         let second_phase = if A_I2.is_zero() && A_O2.is_zero() && S2.is_zero() {
             None

relations/benches/batched_curve_tree_new.rs

@@ -97,7 +97,7 @@ fn bench_batched_curve_tree_with_varying_batch_size_new<const L: usize, const M:
                     >(
                         &mut pallas_prover,
                         &mut vesta_prover,
-                        &SRProofParamsNewPallasLeaf,
+                        &*SRProofParamsNewPallasLeaf,
                         &mut rng,
                     )
                     .expect("Failed to prove batched select and rerandomize");
@@ -134,7 +134,7 @@ fn bench_batched_curve_tree_with_varying_batch_size_new<const L: usize, const M:
                 .batched_select_and_rerandomize_prover_gadget_new::<_, PallasParams, VestaParams>(
                     &mut pallas_prover,
                     &mut vesta_prover,
-                    &SRProofParamsNewPallasLeaf,
+                    &*SRProofParamsNewPallasLeaf,
                     &mut rng,
                 )
                 .expect("Failed to prove batched select and rerandomize");
@@ -178,7 +178,7 @@ fn bench_batched_curve_tree_with_varying_batch_size_new<const L: usize, const M:
                             &root,
                             &mut pallas_verifier,
                             &mut vesta_verifier,
-                            &SRProofParamsNewPallasLeaf,
+                            &*SRProofParamsNewPallasLeaf,
                         )
                         .unwrap();