From 1f36390ccf4897678ac2d1aee3b0db0bc791bbb9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?=
 <4142+huitseeker@users.noreply.github.com>
Date: Thu, 25 Jan 2024 13:50:06 -0500
Subject: [PATCH] refactor: add missing Debug implementations (#282)

* refactor: use zip_with in batch evaluation

- Refactored iterated zip operations in batch evaluation verification function in `src/spartan/snark.rs`
- Used `zip_with!` macro to enhance code readability and maintainability

* refactor: slightly faster evaluation

- Enhanced the efficiency of polynomial evaluation in `prove` function in `src/spartan/snark.rs` by eliminating needless cloning.

* feat: Implement Debug trait across various structs and traits

- `Debug` trait added across multiple files to enhance log and error handling capacity; applies to structures like `ProverKey`, `VerifierKey`, `R1CSWithArity`, `NovaPublicParameters`, `CompressedSNARK`, and others.
- Expanded `Debug` trait implementation to more complex structures like `CircuitDigests`, `PublicParams`, associated types in `ROTrait` and `ROCircuitTrait`, and several structures in the `spartan` and `gadgets` module.
- An additional project-wide compilation check for missing `Debug` implementations has been added to cargo build settings.
---
 .cargo/config                  |  1 +
 src/gadgets/ecc.rs             |  4 ++--
 src/lib.rs                     |  8 ++++----
 src/provider/poseidon.rs       |  6 +++---
 src/r1cs/sparse.rs             |  1 +
 src/spartan/batched.rs         |  4 ++--
 src/spartan/batched_ppsnark.rs |  4 ++--
 src/spartan/mod.rs             |  2 ++
 src/spartan/ppsnark.rs         |  8 ++++----
 src/spartan/snark.rs           | 16 ++++++++--------
 src/supernova/mod.rs           |  6 +++---
 src/supernova/snark.rs         |  4 ++--
 src/traits/mod.rs              |  6 ++++--
 13 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/.cargo/config b/.cargo/config
index 50df778a..1051f0fd 100644
--- a/.cargo/config
+++ b/.cargo/config
@@ -36,6 +36,7 @@ xclippy = [
     "-Wclippy::unnecessary_mut_passed",
     "-Wclippy::unnecessary_wraps",
     "-Wclippy::use_self",
+    "-Wmissing_debug_implementations",
     "-Wnonstandard_style",
     "-Wrust_2018_idioms",
     "-Wtrivial_numeric_casts",
diff --git a/src/gadgets/ecc.rs b/src/gadgets/ecc.rs
index 692f0391..f174f6f0 100644
--- a/src/gadgets/ecc.rs
+++ b/src/gadgets/ecc.rs
@@ -17,7 +17,7 @@ use bellpepper_core::{
 use ff::{Field, PrimeField};
 
 /// `AllocatedPoint` provides an elliptic curve abstraction inside a circuit.
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct AllocatedPoint<E: Engine> {
   pub(crate) x: AllocatedNum<E::Base>,
   pub(crate) y: AllocatedNum<E::Base>,
@@ -590,7 +590,7 @@ where
   }
 }
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 /// `AllocatedPoint` but one that is guaranteed to be not infinity
 pub struct AllocatedPointNonInfinity<E: Engine> {
   x: AllocatedNum<E::Base>,
diff --git a/src/lib.rs b/src/lib.rs
index 6e066d03..c28942c7 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -60,7 +60,7 @@ use traits::{
 };
 
 /// A type that holds parameters for the primary and secondary circuits of Nova and SuperNova
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Abomonation)]
+#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where <E::Scalar as PrimeField>::Repr: Abomonation)]
 pub struct R1CSWithArity<E: Engine> {
@@ -87,7 +87,7 @@ impl<E: Engine> R1CSWithArity<E> {
 }
 
 /// A type that holds public parameters of Nova
-#[derive(Clone, PartialEq, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(
 where
@@ -706,7 +706,7 @@ where
 }
 
 /// A type that holds the verifier key for `CompressedSNARK`
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(
   where
@@ -739,7 +739,7 @@ where
 }
 
 /// A SNARK that proves the knowledge of a valid `RecursiveSNARK`
-#[derive(Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(bound = "")]
 pub struct CompressedSNARK<E1, E2, C1, C2, S1, S2>
 where
diff --git a/src/provider/poseidon.rs b/src/provider/poseidon.rs
index cd98a7f8..cb48528e 100644
--- a/src/provider/poseidon.rs
+++ b/src/provider/poseidon.rs
@@ -23,7 +23,7 @@ use neptune::{
 use serde::{Deserialize, Serialize};
 
 /// All Poseidon Constants that are used in Nova
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Abomonation)]
 #[abomonation_bounds(where Scalar::Repr: Abomonation)]
 pub struct PoseidonConstantsCircuit<Scalar: PrimeField>(PoseidonConstants<Scalar, U24>);
 
@@ -35,7 +35,7 @@ impl<Scalar: PrimeField> Default for PoseidonConstantsCircuit<Scalar> {
 }
 
 /// A Poseidon-based RO to use outside circuits
-#[derive(Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Serialize, Deserialize, Abomonation)]
 #[abomonation_bounds(
   where
     Base: PrimeField,
@@ -115,7 +115,7 @@ where
 }
 
 /// A Poseidon-based RO gadget to use inside the verifier circuit.
-#[derive(Serialize, Deserialize)]
+#[derive(Debug, Serialize, Deserialize)]
 pub struct PoseidonROCircuit<Scalar: PrimeField> {
   // Internal state
   state: Vec<AllocatedNum<Scalar>>,
diff --git a/src/r1cs/sparse.rs b/src/r1cs/sparse.rs
index 0334ab61..98ce0a42 100644
--- a/src/r1cs/sparse.rs
+++ b/src/r1cs/sparse.rs
@@ -223,6 +223,7 @@ impl<F: PrimeField> SparseMatrix<F> {
 }
 
 /// Iterator for sparse matrix
+#[derive(Debug)]
 pub struct Iter<'a, F: PrimeField> {
   matrix: &'a SparseMatrix<F>,
   row: usize,
diff --git a/src/spartan/batched.rs b/src/spartan/batched.rs
index 0036955d..abd076d2 100644
--- a/src/spartan/batched.rs
+++ b/src/spartan/batched.rs
@@ -59,7 +59,7 @@ pub struct BatchedRelaxedR1CSSNARK<E: Engine, EE: EvaluationEngineTrait<E>> {
 }
 
 /// A type that represents the prover's key
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where <E::Scalar as ff::PrimeField>::Repr: Abomonation)]
 pub struct ProverKey<E: Engine, EE: EvaluationEngineTrait<E>> {
@@ -69,7 +69,7 @@ pub struct ProverKey<E: Engine, EE: EvaluationEngineTrait<E>> {
 }
 
 /// A type that represents the verifier's key
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where <E::Scalar as ff::PrimeField>::Repr: Abomonation)]
 pub struct VerifierKey<E: Engine, EE: EvaluationEngineTrait<E>> {
diff --git a/src/spartan/batched_ppsnark.rs b/src/spartan/batched_ppsnark.rs
index 5c5f2a4c..7abcc217 100644
--- a/src/spartan/batched_ppsnark.rs
+++ b/src/spartan/batched_ppsnark.rs
@@ -43,7 +43,7 @@ use rayon::prelude::*;
 use serde::{Deserialize, Serialize};
 
 /// A type that represents the prover's key
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where < E::Scalar as PrimeField >::Repr: Abomonation)]
 pub struct ProverKey<E: Engine, EE: EvaluationEngineTrait<E>> {
@@ -55,7 +55,7 @@ pub struct ProverKey<E: Engine, EE: EvaluationEngineTrait<E>> {
 }
 
 /// A type that represents the verifier's key
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where < E::Scalar as PrimeField >::Repr: Abomonation)]
 pub struct VerifierKey<E: Engine, EE: EvaluationEngineTrait<E>> {
diff --git a/src/spartan/mod.rs b/src/spartan/mod.rs
index 175c00b3..d3231fca 100644
--- a/src/spartan/mod.rs
+++ b/src/spartan/mod.rs
@@ -35,6 +35,7 @@ fn powers<E: Engine>(s: &E::Scalar, n: usize) -> Vec<E::Scalar> {
 }
 
 /// A type that holds a witness to a polynomial evaluation instance
+#[derive(Debug)]
 struct PolyEvalWitness<E: Engine> {
   p: Vec<E::Scalar>, // polynomial
 }
@@ -114,6 +115,7 @@ impl<E: Engine> PolyEvalWitness<E> {
 }
 
 /// A type that holds a polynomial evaluation instance
+#[derive(Debug)]
 struct PolyEvalInstance<E: Engine> {
   c: Commitment<E>,  // commitment to the polynomial
   x: Vec<E::Scalar>, // evaluation point
diff --git a/src/spartan/ppsnark.rs b/src/spartan/ppsnark.rs
index 2d6cfc64..88cb239a 100644
--- a/src/spartan/ppsnark.rs
+++ b/src/spartan/ppsnark.rs
@@ -52,7 +52,7 @@ fn padded<E: Engine>(v: &[E::Scalar], n: usize, e: &E::Scalar) -> Vec<E::Scalar>
 }
 
 /// A type that holds `R1CSShape` in a form amenable to memory checking
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where <E::Scalar as PrimeField>::Repr: Abomonation)]
 pub struct R1CSShapeSparkRepr<E: Engine> {
@@ -78,7 +78,7 @@ pub struct R1CSShapeSparkRepr<E: Engine> {
 }
 
 /// A type that holds a commitment to a sparse polynomial
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where <E::Scalar as PrimeField>::Repr: Abomonation)]
 pub struct R1CSShapeSparkCommitment<E: Engine> {
@@ -255,7 +255,7 @@ impl<E: Engine> R1CSShapeSparkRepr<E> {
 }
 
 /// A type that represents the prover's key
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where <E::Scalar as PrimeField>::Repr: Abomonation)]
 pub struct ProverKey<E: Engine, EE: EvaluationEngineTrait<E>> {
@@ -267,7 +267,7 @@ pub struct ProverKey<E: Engine, EE: EvaluationEngineTrait<E>> {
 }
 
 /// A type that represents the verifier's key
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where <E::Scalar as PrimeField>::Repr: Abomonation)]
 pub struct VerifierKey<E: Engine, EE: EvaluationEngineTrait<E>> {
diff --git a/src/spartan/snark.rs b/src/spartan/snark.rs
index df09387f..7d9bb8dd 100644
--- a/src/spartan/snark.rs
+++ b/src/spartan/snark.rs
@@ -33,7 +33,7 @@ use rayon::prelude::*;
 use serde::{Deserialize, Serialize};
 
 /// A type that represents the prover's key
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where <E::Scalar as ff::PrimeField>::Repr: Abomonation)]
 pub struct ProverKey<E: Engine, EE: EvaluationEngineTrait<E>> {
@@ -43,7 +43,7 @@ pub struct ProverKey<E: Engine, EE: EvaluationEngineTrait<E>> {
 }
 
 /// A type that represents the verifier's key
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(where <E::Scalar as ff::PrimeField>::Repr: Abomonation)]
 pub struct VerifierKey<E: Engine, EE: EvaluationEngineTrait<E>> {
@@ -189,7 +189,7 @@ where
     // claims from the end of sum-check
     let (claim_Az, claim_Bz): (E::Scalar, E::Scalar) = (claims_outer[1], claims_outer[2]);
     let claim_Cz = poly_Cz.evaluate(&r_x);
-    let eval_E = MultilinearPolynomial::new(W.E.clone()).evaluate(&r_x);
+    let eval_E = MultilinearPolynomial::evaluate_with(&W.E, &r_x);
     transcript.absorb(
       b"claims_outer",
       &[claim_Az, claim_Bz, claim_Cz, eval_E].as_slice(),
@@ -536,11 +536,11 @@ pub(in crate::spartan) fn batch_eval_verify<E: Engine>(
       EqPolynomial::new(r_hi.to_vec()).evaluate(&u.x)
     });
 
-    evals_r
-      .zip_eq(evals_batch.iter())
-      .zip_eq(powers_of_rho.iter())
-      .map(|((e_i, p_i), rho_i)| e_i * *p_i * rho_i)
-      .sum()
+    zip_with!(
+      (evals_r, evals_batch.iter(), powers_of_rho.iter()),
+      |e_i, p_i, rho_i| e_i * *p_i * rho_i
+    )
+    .sum()
   };
 
   if claim_batch_final != claim_batch_final_expected {
diff --git a/src/supernova/mod.rs b/src/supernova/mod.rs
index 07203735..66dff3a8 100644
--- a/src/supernova/mod.rs
+++ b/src/supernova/mod.rs
@@ -46,7 +46,7 @@ use circuit::{
 use error::SuperNovaError;
 
 /// A struct that manages all the digests of the primary circuits of a SuperNova instance
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct CircuitDigests<E: Engine> {
   digests: Vec<E::Scalar>,
 }
@@ -75,7 +75,7 @@ impl<E: Engine> CircuitDigests<E> {
 }
 
 /// A vector of [R1CSWithArity] adjoined to a set of [PublicParams]
-#[derive(Clone, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(bound = "")]
 pub struct PublicParams<E1, E2, C1, C2>
 where
@@ -107,7 +107,7 @@ where
 /// Auxiliary [PublicParams] information about the commitment keys and
 /// secondary circuit. This is used as a helper struct when reconstructing
 /// [PublicParams] downstream in lurk.
-#[derive(Clone, PartialEq, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(
 where
diff --git a/src/supernova/snark.rs b/src/supernova/snark.rs
index 1b0b0568..8a99bbbb 100644
--- a/src/supernova/snark.rs
+++ b/src/supernova/snark.rs
@@ -19,7 +19,7 @@ use serde::{Deserialize, Serialize};
 use std::marker::PhantomData;
 
 /// A type that holds the prover key for `CompressedSNARK`
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(
   where
@@ -46,7 +46,7 @@ where
 }
 
 /// A type that holds the verifier key for `CompressedSNARK`
-#[derive(Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(
   where
diff --git a/src/traits/mod.rs b/src/traits/mod.rs
index 3327c14e..f5e1ae01 100644
--- a/src/traits/mod.rs
+++ b/src/traits/mod.rs
@@ -67,7 +67,8 @@ pub trait ROTrait<Base: PrimeField, Scalar> {
   type CircuitRO: ROCircuitTrait<Base, Constants = Self::Constants>;
 
   /// A type representing constants/parameters associated with the hash function
-  type Constants: Default
+  type Constants: Debug
+    + Default
     + Clone
     + PartialEq
     + Send
@@ -92,7 +93,8 @@ pub trait ROCircuitTrait<Base: PrimeField> {
   type NativeRO<T: PrimeField>: ROTrait<Base, T, Constants = Self::Constants>;
 
   /// A type representing constants/parameters associated with the hash function on this Base field
-  type Constants: Default
+  type Constants: Debug
+    + Default
     + Clone
     + PartialEq
     + Send