refactor: Enhance error handling and messaging across modules (#660)

* refactor: Enhance error handling and messaging across modules - Enhanced clarity and detail of error messages in multiple modules (`clutch/src/lib.rs`, `fcomm/src/lib.rs`, `src/z_data/z_ptr.rs`, and `src/public_parameters/mem_cache.rs`). - This includes displaying previously-silenced specific errors in proof key generation, expression/environment reading, base32 decoding, and disk write errors in the public parameters module. * fix: inline format arguments (clippy) * chore: remove uneeded lifetimes * fix: Remove unecessary panic - Enhanced error handling in `verify` function within `clutch/src/lib.rs` to improve program stability. * refactor: Refactor code while reading LEM - Updated import specifications across numerous files for clarity:, these include changes to `backend.rs`, `circuit.rs`, `path.rs`, `eval.rs`, `macros.rs`, and `pointers.rs`. - Modified and refactored code for improved error handling, examples include changes to `interpreter.rs`, `mod.rs`, `var_map.rs`, `writer.rs`, `store.rs`. - Included implementation of Debug traits in `var_map.rs` and `zstore.rs`. - Enhanced handling of invalid u16 values in `Tag` implementation - Implemented several lint checks in `.cargo/config` to ensure code quality. - Refined match syntax in the `num_paths_taken` method on `path.rs` and `fetch_symbol_path`, `read_maybe_meta`, `hash_ptr` on `store.rs` for better performance and error handling. * chore: clippy
argumentcomputer · Sep 20, 2023 · 078ceaa · 078ceaa
1 parent c91cc59
commit 078ceaa
Show file tree

Hide file tree

Showing 59 changed files with 284 additions and 316 deletions.
diff --git a/.cargo/config b/.cargo/config
@@ -11,6 +11,7 @@ xclippy = [
     "-Wclippy::dbg_macro",
     "-Wclippy::disallowed_methods",
     "-Wclippy::derive_partial_eq_without_eq",
+    "-Wclippy::enum_glob_use",
     "-Wclippy::filter_map_next",
     "-Wclippy::flat_map_option",
     "-Wclippy::inefficient_to_string",
@@ -21,6 +22,8 @@ xclippy = [
     "-Wclippy::map_unwrap_or",
     "-Wclippy::needless_borrow",
     "-Wclippy::checked_conversions",
+    "-Wclippy::trait_duplication_in_bounds",
     "-Wrust_2018_idioms",
+    "-Wtrivial_numeric_casts",
     "-Wunused_lifetimes",
 ]
diff --git a/benches/end2end.rs b/benches/end2end.rs
@@ -309,7 +309,7 @@ fn prove_benchmark(c: &mut Criterion) {
 
         b.iter(|| {
             let result = prover
-                .prove(&pp, &frames, &mut store, lang_pallas_rc.clone())
+                .prove(&pp, &frames, &store, lang_pallas_rc.clone())
                 .unwrap();
             black_box(result);
         })
@@ -362,7 +362,7 @@ fn prove_compressed_benchmark(c: &mut Criterion) {
 
         b.iter(|| {
             let (proof, _, _, _) = prover
-                .prove(&pp, &frames, &mut store, lang_pallas_rc.clone())
+                .prove(&pp, &frames, &store, lang_pallas_rc.clone())
                 .unwrap();
 
             let compressed_result = proof.compress(&pp).unwrap();
@@ -412,7 +412,7 @@ fn verify_benchmark(c: &mut Criterion) {
                 )
                 .unwrap();
             let (proof, z0, zi, num_steps) = prover
-                .prove(&pp, &frames, &mut store, lang_pallas_rc.clone())
+                .prove(&pp, &frames, &store, lang_pallas_rc.clone())
                 .unwrap();
 
             b.iter_batched(
@@ -470,7 +470,7 @@ fn verify_compressed_benchmark(c: &mut Criterion) {
                 )
                 .unwrap();
             let (proof, z0, zi, num_steps) = prover
-                .prove(&pp, &frames, &mut store, lang_pallas_rc.clone())
+                .prove(&pp, &frames, &store, lang_pallas_rc.clone())
                 .unwrap();
 
             let compressed_proof = proof.compress(&pp).unwrap();

diff --git a/benches/fibonacci.rs b/benches/fibonacci.rs
@@ -104,7 +104,7 @@ fn fibo_prove<M: measurement::Measurement>(
             b.iter_batched(
                 || (frames, lang_rc.clone()),
                 |(frames, lang_rc)| {
-                    let result = prover.prove(&pp, frames, &mut store, lang_rc);
+                    let result = prover.prove(&pp, frames, &store, lang_rc);
                     let _ = black_box(result);
                 },
                 BatchSize::LargeInput,

diff --git a/benches/sha256_ivc.rs b/benches/sha256_ivc.rs
@@ -54,7 +54,7 @@ fn sha256_ivc<F: LurkField>(
         .map(|i| format!("(sha256 . {i})"))
         .collect::<Vec<String>>()
         .join(" ");
-    let input = format!("'({})", input);
+    let input = format!("'({input})");
     let program = format!(
         r#"
 (letrec ((encode-1 (lambda (term) 

diff --git a/clutch/src/lib.rs b/clutch/src/lib.rs
@@ -510,14 +510,14 @@ impl ClutchState<F, Coproc<F>> {
         println!();
         Ok(None)
     }
-    fn proof_claim(&self, store: &mut Store<F>, rest: Ptr<F>) -> Result<Option<Ptr<F>>> {
+    fn proof_claim(&self, store: &Store<F>, rest: Ptr<F>) -> Result<Option<Ptr<F>>> {
         let proof = self.get_proof(store, rest)?;
 
         println!("{0:#?}", proof.claim);
         Ok(None)
     }
 
-    fn get_proof(&self, store: &mut Store<F>, rest: Ptr<F>) -> Result<Proof<'_, F>> {
+    fn get_proof(&self, store: &Store<F>, rest: Ptr<F>) -> Result<Proof<'_, F>> {
         let (proof_cid, _rest1) = store.car_cdr(&rest)?;
         let zptr_string = store
             .fetch_string(&proof_cid)
@@ -566,7 +566,7 @@ impl ClutchState<F, Coproc<F>> {
             let zptr_string = proof
                 .claim
                 .proof_key()
-                .map_err(|_| Error::msg("Failed to generate proof key"))?
+                .map_err(|e| Error::msg(format!("Failed to generate proof key: {}", e)))?
                 .to_base32();
             match proof.claim {
                 Claim::Evaluation(_) | Claim::Opening(_) => println!("{0:#?}", proof.claim),
@@ -579,7 +579,7 @@ impl ClutchState<F, Coproc<F>> {
             bail!("verification of new proof failed");
         }
     }
-    fn verify(&mut self, store: &mut Store<F>, rest: Ptr<F>) -> Result<Option<Ptr<F>>> {
+    fn verify(&mut self, store: &Store<F>, rest: Ptr<F>) -> Result<Option<Ptr<F>>> {
         let (proof_cid, _) = store.car_cdr(&rest)?;
 
         let zptr_string = store
@@ -592,7 +592,7 @@ impl ClutchState<F, Coproc<F>> {
             .ok_or_else(|| anyhow!("proof not found: {zptr_string}"))?;
 
         let pp = public_params(self.reduction_count, true, self.lang(), &public_param_dir())?;
-        let result = proof.verify(&pp, &self.lang()).unwrap();
+        let result = proof.verify(&pp, &self.lang())?;
 
         if result.verified {
             Ok(Some(lurk_sym_ptr!(store, t)))

diff --git a/examples/circom.rs b/examples/circom.rs
@@ -109,7 +109,7 @@ fn main() {
         )],
     );
 
-    let coproc_expr = format!("{}", sym_str);
+    let coproc_expr = format!("{sym_str}");
 
     let expr = format!("({coproc_expr})");
     let ptr = store.read(&expr).unwrap();
@@ -129,7 +129,7 @@ fn main() {
     .unwrap();
     let pp_end = pp_start.elapsed();
 
-    println!("Public parameters took {:?}", pp_end);
+    println!("Public parameters took {pp_end:?}");
 
     println!("Beginning proof step...");
 
@@ -139,15 +139,15 @@ fn main() {
         .unwrap();
     let proof_end = proof_start.elapsed();
 
-    println!("Proofs took {:?}", proof_end);
+    println!("Proofs took {proof_end:?}");
 
     println!("Verifying proof...");
 
     let verify_start = Instant::now();
     let res = proof.verify(&pp, num_steps, &z0, &zi).unwrap();
     let verify_end = verify_start.elapsed();
 
-    println!("Verify took {:?}", verify_end);
+    println!("Verify took {verify_end:?}");
 
     if res {
         println!(

diff --git a/examples/sha256.rs b/examples/sha256.rs
@@ -205,7 +205,7 @@ fn main() {
     .unwrap();
     let pp_end = pp_start.elapsed();
 
-    println!("Public parameters took {:?}", pp_end);
+    println!("Public parameters took {pp_end:?}");
 
     if setup_only {
         return;
@@ -222,15 +222,15 @@ fn main() {
         });
     let proof_end = proof_start.elapsed();
 
-    println!("Proofs took {:?}", proof_end);
+    println!("Proofs took {proof_end:?}");
 
     println!("Verifying proof...");
 
     let verify_start = Instant::now();
     let res = proof.verify(&pp, num_steps, &z0, &zi).unwrap();
     let verify_end = verify_start.elapsed();
 
-    println!("Verify took {:?}", verify_end);
+    println!("Verify took {verify_end:?}");
 
     if res {
         println!(

diff --git a/examples/sha256_ivc.rs b/examples/sha256_ivc.rs
@@ -36,7 +36,7 @@ fn sha256_ivc<F: LurkField>(store: &mut Store<F>, n: usize, input: Vec<usize>) -
         .map(|i| i.to_string())
         .collect::<Vec<String>>()
         .join(" ");
-    let input = format!("'({})", input);
+    let input = format!("'({input})");
     let program = format!(
         r#"
 (letrec ((encode-1 (lambda (term) 

diff --git a/fcomm/src/lib.rs b/fcomm/src/lib.rs
@@ -78,7 +78,7 @@ mod base64 {
 
 pub type NovaProofCache = FileMap<String, Proof<'static, S1>>;
 pub fn nova_proof_cache(reduction_count: usize) -> NovaProofCache {
-    FileMap::<String, Proof<'_, S1>>::new(format!("nova_proofs.{}", reduction_count)).unwrap()
+    FileMap::<String, Proof<'_, S1>>::new(format!("nova_proofs.{reduction_count}")).unwrap()
 }
 
 pub type CommittedExpressionMap = FileMap<Commitment<S1>, CommittedExpression<S1>>;
@@ -264,7 +264,7 @@ impl<F: LurkField> Eq for LurkPtr<F> {}
 #[cfg_attr(not(target_arch = "wasm32"), proptest(no_bound))]
 #[cfg_attr(not(target_arch = "wasm32"), serde_test(types(S1), zdata(true)))]
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
-pub struct CommittedExpression<F: LurkField + Serialize> {
+pub struct CommittedExpression<F: LurkField> {
     pub expr: LurkPtr<F>,
     #[cfg_attr(
         not(target_arch = "wasm32"),
@@ -405,7 +405,7 @@ impl ReductionCount {
 
 impl Evaluation {
     fn new<F: LurkField>(
-        s: &mut Store<F>,
+        s: &Store<F>,
         input: IO<F>,
         output: IO<F>,
         iterations: Option<usize>, // This might be padded, so is not quite 'iterations' in the sense of number of actual reduction steps required
@@ -491,7 +491,7 @@ impl<F: LurkField + Serialize + DeserializeOwned> PtrEvaluation<F> {
 }
 
 impl<F: LurkField + Serialize + DeserializeOwned> Commitment<F> {
-    pub fn from_comm(s: &mut Store<F>, ptr: &Ptr<F>) -> Result<Self, Error> {
+    pub fn from_comm(s: &Store<F>, ptr: &Ptr<F>) -> Result<Self, Error> {
         assert_eq!(ExprTag::Comm, ptr.tag);
 
         let digest = *s
@@ -589,7 +589,7 @@ impl<F: LurkField + Serialize + DeserializeOwned> LurkPtr<F> {
         }
     }
 
-    pub fn from_ptr(s: &mut Store<F>, ptr: &Ptr<F>) -> Self {
+    pub fn from_ptr(s: &Store<F>, ptr: &Ptr<F>) -> Self {
         let (z_store, z_ptr) = ZStore::new_with_expr(s, ptr);
         let z_ptr = z_ptr.unwrap();
         Self::ZStorePtr(ZStorePtr { z_store, z_ptr })
@@ -599,7 +599,7 @@ impl<F: LurkField + Serialize + DeserializeOwned> LurkPtr<F> {
 impl LurkCont {
     pub fn cont_ptr<F: LurkField + Serialize + DeserializeOwned>(
         &self,
-        s: &mut Store<F>,
+        s: &Store<F>,
     ) -> ContPtr<F> {
         match self {
             Self::Outermost => s.get_cont_outermost(),
@@ -969,11 +969,11 @@ impl<'a> Proof<'a, S1> {
         let input_io = {
             let expr = s
                 .read(&evaluation.expr)
-                .map_err(|_| Error::VerificationError("failed to read expr".into()))?;
+                .map_err(|e| Error::VerificationError(format!("failed to read expr: {}", e)))?;
 
             let env = s
                 .read(&evaluation.env)
-                .map_err(|_| Error::VerificationError("failed to read env".into()))?;
+                .map_err(|e| Error::VerificationError(format!("failed to read env: {}", e)))?;
 
             // FIXME: We ignore cont and assume Outermost, since we can't read a Cont.
             let cont = s.intern_cont_outermost();
@@ -984,11 +984,11 @@ impl<'a> Proof<'a, S1> {
         let output_io = {
             let expr = s
                 .read(&evaluation.expr_out)
-                .map_err(|_| Error::VerificationError("failed to read expr out".into()))?;
+                .map_err(|e| Error::VerificationError(format!("failed to read expr out: {}", e)))?;
 
             let env = s
                 .read(&evaluation.env_out)
-                .map_err(|_| Error::VerificationError("failed to read env out".into()))?;
+                .map_err(|e| Error::VerificationError(format!("failed to read env out: {}", e)))?;
             let cont = evaluation
                 .status
                 .to_cont(s)
@@ -1261,7 +1261,7 @@ mod test {
                 Some(c) => commitment = c,
                 _ => panic!("new commitment missing"),
             }
-            println!("Commitment: {:?}", commitment);
+            println!("Commitment: {commitment:?}");
         }
     }
 

diff --git a/fcomm/tests/makefile_tests.rs b/fcomm/tests/makefile_tests.rs
@@ -21,7 +21,7 @@ fn test_make_fcomm_examples() {
 
     let make_output = Command::new("make")
         .current_dir(examples_dir)
-        .arg(format!("-j{}", cpus))
+        .arg(format!("-j{cpus}"))
         .output()
         .expect("Failed to run the make command, is make installed?");
 

diff --git a/lurk-macros/src/lib.rs b/lurk-macros/src/lib.rs
@@ -298,10 +298,10 @@ pub fn serde_test(args: TokenStream, input: TokenStream) -> TokenStream {
                     }
                 }
 
-                _ => panic!("invalid attribute {:?}", path),
+                _ => panic!("invalid attribute {path:?}"),
             },
 
-            _ => panic!("invalid argument {:?}", arg),
+            _ => panic!("invalid argument {arg:?}"),
         }
     }
 
@@ -318,7 +318,7 @@ pub fn serde_test(args: TokenStream, input: TokenStream) -> TokenStream {
     for (i, ty) in types.into_iter().enumerate() {
         let serde_test = {
             let test_name = Ident::new(
-                &format!("test_serde_roundtrip_{}_{}", name, i),
+                &format!("test_serde_roundtrip_{name}_{i}"),
                 Span::mixed_site(),
             );
             quote! {
@@ -335,7 +335,7 @@ pub fn serde_test(args: TokenStream, input: TokenStream) -> TokenStream {
 
         let zdata_test = if test_zdata {
             let test_name = Ident::new(
-                &format!("test_zdata_roundtrip_{}_{}", name, i),
+                &format!("test_zdata_roundtrip_{name}_{i}"),
                 Span::mixed_site(),
             );
             quote! {
@@ -398,15 +398,15 @@ fn get_type_from_attrs(attrs: &[syn::Attribute], attr_name: &str) -> syn::Result
     }) else {
         return Err(syn::Error::new(
             proc_macro2::Span::call_site(),
-            format!("Could not find attribute {}", attr_name),
+            format!("Could not find attribute {attr_name}"),
         ));
     };
 
     match nested_arg {
         NestedMeta::Meta(Meta::Path(path)) => Ok(path),
         bad => Err(syn::Error::new_spanned(
             bad,
-            &format!("Could not parse {} attribute", attr_name)[..],
+            &format!("Could not parse {attr_name} attribute")[..],
         )),
     }
 }
@@ -447,10 +447,7 @@ pub fn derive_try_from_repr(input: TokenStream) -> TokenStream {
     match res_ty {
         Err(e) => {
             // If no explicit repr were given for us, we can't pursue
-            panic!(
-                "TryFromRepr macro requires a repr parameter, which couldn't be parsed: {:?}",
-                e
-            );
+            panic!("TryFromRepr macro requires a repr parameter, which couldn't be parsed: {e:?}");
         }
         Ok(ty) => {
             let match_arms = try_from_match_arms(name, &variants, ty.clone());

diff --git a/src/circuit/circuit_frame.rs b/src/circuit/circuit_frame.rs
@@ -413,7 +413,7 @@ impl<'a, F: LurkField, C: Coprocessor<F>> MultiFrame<'a, F, C> {
 
         let mut final_output = None;
 
-        for (frames_cs, output) in css.into_iter() {
+        for (frames_cs, output) in css {
             final_output = Some(output);
 
             let aux = frames_cs.aux_slice();
@@ -5194,10 +5194,7 @@ fn car_cdr_named<F: LurkField, CS: ConstraintSystem<F>>(
             maybe_cons.hash().get_value(),
             &allocated_digest.get_value()
         );
-        panic!(
-            "tried to take car_cdr of a non-dummy cons ({:?}) but supplied wrong value",
-            name
-        );
+        panic!("tried to take car_cdr of a non-dummy cons ({name:?}) but supplied wrong value");
     }
 
     implies!(cs, &cons_not_dummy, &real_cons);