No more assert on result (#346)

* refactor: a few zip_with applications * refactor: Replace assertion checks with direct unwraps for error handling - Replaced assertions throughout multiple functions and tests with directly unwrapping results for better error handling and clarity on failures. - Applied this change to methods like `prove_step`, `verify`, `test_inner`, and in areas like RecursiveSNARK, CompressedSNARK, and test functions across different scripts. - Enhanced the error feedback mechanism during benchmarks for RecursiveSNARK, and CompressedSNARK. - Simplified code further by removing unnecessary checks and variable assignments, and performing inline verification with cleanercode. - Updated `.cargo/config.toml` with a new project-wide Clippy lint rule for assertion on result states.
argumentcomputer · Feb 25, 2024 · e73a98e · e73a98e
1 parent 6089294
commit e73a98e
Show file tree

Hide file tree

Showing 20 changed files with 178 additions and 200 deletions.
diff --git a/.cargo/config.toml b/.cargo/config.toml
@@ -7,6 +7,7 @@
 xclippy = [
     "clippy", "--all-targets", "--",
     "-Wclippy::all",
+    "-Wclippy::assertions_on_result_states",
     "-Wclippy::cast_lossless",
     "-Wclippy::checked_conversions",
     "-Wclippy::dbg_macro",

diff --git a/benches/common/supernova/bench.rs b/benches/common/supernova/bench.rs
@@ -99,56 +99,55 @@ pub fn bench_snark_internal_with_arity<
       // Benchmark the prove time
       group.bench_function(bench_params.bench_id("Prove"), |b| {
         b.iter(|| {
-          assert!(CompressedSNARK::<_, S1, S2>::prove(
+          CompressedSNARK::<_, S1, S2>::prove(
             black_box(&pp),
             black_box(&prover_key),
-            black_box(&recursive_snark)
+            black_box(&recursive_snark),
           )
-          .is_ok());
+          .unwrap();
         })
       });
 
-      let res = CompressedSNARK::<_, S1, S2>::prove(&pp, &prover_key, &recursive_snark);
-      assert!(res.is_ok());
-      let compressed_snark = res.unwrap();
+      let compressed_snark =
+        CompressedSNARK::<_, S1, S2>::prove(&pp, &prover_key, &recursive_snark).unwrap();
       // Benchmark the verification time
       group.bench_function(bench_params.bench_id("Verify"), |b| {
         b.iter(|| {
-          assert!(black_box(&compressed_snark)
+          black_box(&compressed_snark)
             .verify(
               black_box(&pp),
               black_box(&verifier_key),
               black_box(&z0_primary),
               black_box(&z0_secondary),
             )
-            .is_ok());
+            .unwrap();
         })
       });
     }
     SnarkType::Recursive => {
       // Benchmark the prove time
       group.bench_function(bench_params.bench_id("Prove"), |b| {
         b.iter(|| {
-          assert!(black_box(&mut recursive_snark.clone())
+          black_box(&mut recursive_snark.clone())
             .prove_step(
               black_box(&pp),
               &bench.primary_circuit(0),
-              &bench.secondary_circuit()
+              &bench.secondary_circuit(),
             )
-            .is_ok());
+            .unwrap();
         })
       });
 
       // Benchmark the verification time
       group.bench_function(bench_params.bench_id("Verify"), |b| {
         b.iter(|| {
-          assert!(black_box(&mut recursive_snark.clone())
+          black_box(&mut recursive_snark.clone())
             .verify(
               black_box(&pp),
               black_box(&[<Bn256EngineKZG as Engine>::Scalar::from(2u64)]),
               black_box(&[<GrumpkinEngine as Engine>::Scalar::from(2u64)]),
             )
-            .is_ok());
+            .unwrap();
         })
       });
     }

diff --git a/benches/compressed-snark.rs b/benches/compressed-snark.rs
@@ -86,17 +86,19 @@ fn bench_compressed_snark_internal<S1: RelaxedR1CSSNARKTrait<E1>, S2: RelaxedR1C
   .unwrap();
 
   for i in 0..num_steps {
-    let res = recursive_snark.prove_step(&pp, &c_primary, &c_secondary);
-    assert!(res.is_ok());
+    recursive_snark
+      .prove_step(&pp, &c_primary, &c_secondary)
+      .unwrap();
 
     // verify the recursive snark at each step of recursion
-    let res = recursive_snark.verify(
-      &pp,
-      i + 1,
-      &[<E1 as Engine>::Scalar::from(2u64)],
-      &[<E2 as Engine>::Scalar::from(2u64)],
-    );
-    assert!(res.is_ok());
+    recursive_snark
+      .verify(
+        &pp,
+        i + 1,
+        &[<E1 as Engine>::Scalar::from(2u64)],
+        &[<E2 as Engine>::Scalar::from(2u64)],
+      )
+      .unwrap();
   }
 
   let bench_params = BenchParams {
@@ -108,29 +110,27 @@ fn bench_compressed_snark_internal<S1: RelaxedR1CSSNARKTrait<E1>, S2: RelaxedR1C
   // Bench time to produce a compressed SNARK
   group.bench_function(bench_params.bench_id("Prove"), |b| {
     b.iter(|| {
-      assert!(CompressedSNARK::<_, S1, S2>::prove(
+      CompressedSNARK::<_, S1, S2>::prove(
         black_box(&pp),
         black_box(&pk),
-        black_box(&recursive_snark)
+        black_box(&recursive_snark),
       )
-      .is_ok());
+      .unwrap();
     })
   });
-  let res = CompressedSNARK::<_, S1, S2>::prove(&pp, &pk, &recursive_snark);
-  assert!(res.is_ok());
-  let compressed_snark = res.unwrap();
+  let compressed_snark = CompressedSNARK::<_, S1, S2>::prove(&pp, &pk, &recursive_snark).unwrap();
 
   // Benchmark the verification time
   group.bench_function(bench_params.bench_id("Verify"), |b| {
     b.iter(|| {
-      assert!(black_box(&compressed_snark)
+      black_box(&compressed_snark)
         .verify(
           black_box(&vk),
           black_box(num_steps),
           black_box(&[<E1 as Engine>::Scalar::from(2u64)]),
           black_box(&[<E2 as Engine>::Scalar::from(2u64)]),
         )
-        .is_ok());
+        .unwrap();
     })
   });
 }

diff --git a/benches/recursive-snark.rs b/benches/recursive-snark.rs
@@ -94,17 +94,19 @@ fn bench_recursive_snark(c: &mut Criterion) {
     .unwrap();
 
     for i in 0..num_warmup_steps {
-      let res = recursive_snark.prove_step(&pp, &c_primary, &c_secondary);
-      assert!(res.is_ok());
+      recursive_snark
+        .prove_step(&pp, &c_primary, &c_secondary)
+        .unwrap();
 
       // verify the recursive snark at each step of recursion
-      let res = recursive_snark.verify(
-        &pp,
-        i + 1,
-        &[<E1 as Engine>::Scalar::from(2u64)],
-        &[<E2 as Engine>::Scalar::from(2u64)],
-      );
-      assert!(res.is_ok());
+      recursive_snark
+        .verify(
+          &pp,
+          i + 1,
+          &[<E1 as Engine>::Scalar::from(2u64)],
+          &[<E2 as Engine>::Scalar::from(2u64)],
+        )
+        .unwrap();
     }
 
     let bench_params = BenchParams {
@@ -116,27 +118,27 @@ fn bench_recursive_snark(c: &mut Criterion) {
     group.bench_function(bench_params.bench_id("Prove"), |b| {
       b.iter(|| {
         // produce a recursive SNARK for a step of the recursion
-        assert!(black_box(&mut recursive_snark.clone())
+        black_box(&mut recursive_snark.clone())
           .prove_step(
             black_box(&pp),
             black_box(&c_primary),
             black_box(&c_secondary),
           )
-          .is_ok());
+          .unwrap();
       })
     });
 
     // Benchmark the verification time
     group.bench_function(bench_params.bench_id("Verify"), |b| {
       b.iter(|| {
-        assert!(black_box(&recursive_snark)
+        black_box(&recursive_snark)
           .verify(
             black_box(&pp),
             black_box(num_warmup_steps),
             black_box(&[<E1 as Engine>::Scalar::from(2u64)]),
             black_box(&[<E2 as Engine>::Scalar::from(2u64)]),
           )
-          .is_ok());
+          .unwrap();
       });
     });
     group.finish();

diff --git a/benches/sha256.rs b/benches/sha256.rs
@@ -179,13 +179,13 @@ fn bench_recursive_snark(c: &mut Criterion) {
         .unwrap();
 
         // produce a recursive SNARK for a step of the recursion
-        assert!(recursive_snark
+        recursive_snark
           .prove_step(
             black_box(&pp),
             black_box(&circuit_primary),
             black_box(&circuit_secondary),
           )
-          .is_ok());
+          .unwrap();
       })
     });
     group.finish();

diff --git a/examples/and.rs b/examples/and.rs
@@ -270,8 +270,9 @@ fn main() {
 
     let start = Instant::now();
     for circuit_primary in circuits.iter() {
-      let res = recursive_snark.prove_step(&pp, circuit_primary, &circuit_secondary);
-      assert!(res.is_ok());
+      recursive_snark
+        .prove_step(&pp, circuit_primary, &circuit_secondary)
+        .unwrap();
     }
     println!(
       "RecursiveSNARK::prove {} AND ops: took {:?} ",
@@ -288,7 +289,7 @@ fn main() {
       &[<E2 as Engine>::Scalar::ZERO],
     );
     println!("RecursiveSNARK::verify: {:?}", res.is_ok(),);
-    assert!(res.is_ok());
+    res.unwrap();
 
     // produce a compressed SNARK
     println!("Generating a CompressedSNARK using Spartan with HyperKZG...");
@@ -327,7 +328,7 @@ fn main() {
       res.is_ok(),
       start.elapsed()
     );
-    assert!(res.is_ok());
+    res.unwrap();
     println!("=========================================================");
   }
 }
diff --git a/examples/hashchain.rs b/examples/hashchain.rs
@@ -167,8 +167,9 @@ fn main() {
 
     for (i, circuit_primary) in circuits.iter().enumerate() {
       let start = Instant::now();
-      let res = recursive_snark.prove_step(&pp, circuit_primary, &circuit_secondary);
-      assert!(res.is_ok());
+      recursive_snark
+        .prove_step(&pp, circuit_primary, &circuit_secondary)
+        .unwrap();
 
       println!("RecursiveSNARK::prove {} : took {:?} ", i, start.elapsed());
     }
@@ -182,7 +183,7 @@ fn main() {
       &[<E2 as Engine>::Scalar::ZERO],
     );
     println!("RecursiveSNARK::verify: {:?}", res.is_ok(),);
-    assert!(res.is_ok());
+    res.unwrap();
 
     // produce a compressed SNARK
     println!("Generating a CompressedSNARK using Spartan with HyperKZG...");
@@ -221,7 +222,7 @@ fn main() {
       res.is_ok(),
       start.elapsed()
     );
-    assert!(res.is_ok());
+    res.unwrap();
     println!("=========================================================");
   }
 }
diff --git a/examples/minroot.rs b/examples/minroot.rs
@@ -314,7 +314,6 @@ fn main() {
     for (i, circuit_primary) in minroot_circuits.iter().enumerate() {
       let start = Instant::now();
       let res = recursive_snark.prove_step(&pp, circuit_primary, &circuit_secondary);
-      assert!(res.is_ok());
       println!(
         "RecursiveSNARK::prove_step {}: {:?}, took {:?} ",
         i,
@@ -332,7 +331,7 @@ fn main() {
       res.is_ok(),
       start.elapsed()
     );
-    assert!(res.is_ok());
+    res.unwrap();
 
     // produce a compressed SNARK
     println!("Generating a CompressedSNARK using Spartan with HyperKZG...");
@@ -372,7 +371,7 @@ fn main() {
       res.is_ok(),
       start.elapsed()
     );
-    assert!(res.is_ok());
+    res.unwrap();
     println!("=========================================================");
   }
 }
diff --git a/src/bellpepper/mod.rs b/src/bellpepper/mod.rs
@@ -53,7 +53,7 @@ mod tests {
     let (inst, witness) = cs.r1cs_instance_and_witness(&shape, &ck).unwrap();
 
     // Make sure that this is satisfiable
-    assert!(shape.is_sat(&ck, &inst, &witness).is_ok());
+    shape.is_sat(&ck, &inst, &witness).unwrap();
   }
 
   #[test]

diff --git a/src/circuit.rs b/src/circuit.rs
@@ -427,7 +427,7 @@ mod tests {
     let _ = circuit1.synthesize(&mut cs1);
     let (inst1, witness1) = cs1.r1cs_instance_and_witness(&shape1, &ck1).unwrap();
     // Make sure that this is satisfiable
-    assert!(shape1.is_sat(&ck1, &inst1, &witness1).is_ok());
+    shape1.is_sat(&ck1, &inst1, &witness1).unwrap();
 
     // Execute the base case for the secondary
     let zero2 = <<E1 as Engine>::Base as Field>::ZERO;
@@ -446,7 +446,7 @@ mod tests {
     let _ = circuit2.synthesize(&mut cs2);
     let (inst2, witness2) = cs2.r1cs_instance_and_witness(&shape2, &ck2).unwrap();
     // Make sure that it is satisfiable
-    assert!(shape2.is_sat(&ck2, &inst2, &witness2).is_ok());
+    shape2.is_sat(&ck2, &inst2, &witness2).unwrap();
   }
 
   #[test]

diff --git a/src/gadgets/ecc.rs b/src/gadgets/ecc.rs
@@ -1056,7 +1056,7 @@ mod tests {
     let e_new = a_p.scalar_mul(&s);
     assert!(e_p.x == e_new.x && e_p.y == e_new.y);
     // Make sure that this is satisfiable
-    assert!(shape.is_sat(&ck, &inst, &witness).is_ok());
+    shape.is_sat(&ck, &inst, &witness).unwrap();
   }
 
   fn synthesize_add_equal<G, CS>(mut cs: CS) -> (AllocatedPoint<G>, AllocatedPoint<G>)
@@ -1111,7 +1111,7 @@ mod tests {
     let e_new = a_p.add(&a_p);
     assert!(e_p.x == e_new.x && e_p.y == e_new.y);
     // Make sure that it is satisfiable
-    assert!(shape.is_sat(&ck, &inst, &witness).is_ok());
+    shape.is_sat(&ck, &inst, &witness).unwrap();
   }
 
   fn synthesize_add_negation<G, CS>(mut cs: CS) -> AllocatedPoint<G>
@@ -1180,6 +1180,6 @@ mod tests {
     );
     assert!(e_p.is_infinity);
     // Make sure that it is satisfiable
-    assert!(shape.is_sat(&ck, &inst, &witness).is_ok());
+    shape.is_sat(&ck, &inst, &witness).unwrap();
   }
 }