Improve arithmetic checks and update verifier API (#63)

This PR checks for an arithmetic side effect [lint](https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects), which can help prevent unintended behavior.

It also updates the API such that proof verification now returns `Result<(), ProofError>` instead of `bool`. This simplifies the implementation and seems like a safer design.

BREAKING CHANGE: Updates the API relating to proof verification.

Author: AaronFeickert

benches/triptych.rs

@@ -29,6 +29,7 @@ const BATCH_SIZES: [usize; 1] = [2];
 
 // Generate a batch of witnesses, statements, and transcripts
 #[allow(non_snake_case)]
+#[allow(clippy::arithmetic_side_effects)]
 fn generate_data<R: CryptoRngCore>(
     params: &Arc<Parameters>,
     b: usize,
@@ -166,7 +167,7 @@ fn verify_proof(c: &mut Criterion) {
                     || transcripts[0].clone(),
                     |t| {
                         // Verify the proof
-                        assert!(proof.verify(&statements[0], t));
+                        assert!(proof.verify(&statements[0], t).is_ok());
                     },
                     BatchSize::SmallInput,
                 )
@@ -209,7 +210,7 @@ fn verify_batch_proof(c: &mut Criterion) {
                         || transcripts.clone(),
                         |t| {
                             // Verify the proofs in a batch
-                            assert!(Proof::verify_batch(&statements, &proofs, t));
+                            assert!(Proof::verify_batch(&statements, &proofs, t).is_ok());
                         },
                         BatchSize::SmallInput,
                     )

clippy.toml

@@ -0,0 +1 @@
+arithmetic-side-effects-allowed = [ "curve25519_dalek::Scalar", "curve25519_dalek::RistrettoPoint" ]

lints.toml

@@ -43,7 +43,10 @@ deny = [
     'clippy::cast_possible_truncation',
     'clippy::cast_possible_wrap',
     'clippy::cast_precision-loss',
-    'clippy::cast_sign_loss'
+    'clippy::cast_sign_loss',
+
+    # Mathematical mistakes
+    'clippy::arithmetic_side_effects',
 ]
 
 warn = [

src/gray.rs

@@ -54,17 +54,18 @@ impl GrayIterator {
         // Get a base-`N` decomposition
         let mut base_N = Vec::with_capacity(M as usize);
         for _ in 0..M {
-            base_N.push(v % N);
-            v /= N;
+            // These are always defined since `N > 0`
+            base_N.push(v.checked_rem(N)?);
+            v = v.checked_div(N)?;
         }
 
         // Now get the Gray decomposition from the base-`N` decomposition
         let mut shift = 0;
         let mut digits = vec![0; M as usize];
 
         for i in (0..M).rev() {
-            digits[i as usize] = (base_N[i as usize] + shift) % N;
-            shift = shift + N - digits[i as usize];
+            digits[i as usize] = (base_N[i as usize].checked_add(shift)?).checked_rem(N)?;
+            shift = shift.checked_add(N)?.checked_sub(digits[i as usize])?;
         }
 
         Some(digits)
@@ -128,7 +129,7 @@ impl Iterator for GrayIterator {
     #[allow(non_snake_case)]
     fn next(&mut self) -> Option<Self::Item> {
         if self.i == 0 {
-            self.i += 1;
+            self.i = 1;
             return Some((0, 0, 0));
         }
 
@@ -150,7 +151,7 @@ impl Iterator for GrayIterator {
         let new = next[index];
 
         // Update the state
-        self.i += 1;
+        self.i = self.i.checked_add(1)?;
         self.last = next;
 
         Some((index, old, new))

src/lib.rs

@@ -103,7 +103,7 @@
 //! let proof = Proof::prove(&witness, &statement, &mut transcript.clone()).unwrap();
 //!
 //! // The proof should verify against the same statement and transcript
-//! assert!(proof.verify(&statement, &mut transcript));
+//! assert!(proof.verify(&statement, &mut transcript).is_ok());
 //! # }
 //! ```
 

src/parameters.rs

@@ -100,7 +100,7 @@ impl Parameters {
         hasher.update(&m.to_le_bytes());
         let mut hasher_xof = hasher.finalize_xof();
         let mut CommitmentG_bytes = [0u8; 64];
-        let CommitmentG = (0..n * m)
+        let CommitmentG = (0..n.checked_mul(m).ok_or(ParameterError::InvalidParameter)?)
             .map(|_| {
                 hasher_xof.fill(&mut CommitmentG_bytes);
                 RistrettoPoint::from_uniform_bytes(&CommitmentG_bytes)