fix: Address review comments

src/ccs/cccs.rs

@@ -31,37 +31,28 @@ use super::util::virtual_poly::VirtualPolynomial;
 use super::CCS;
 
 /// A type that holds the shape of a Committed CCS (CCCS) instance
-#[derive(Clone, Debug, PartialEq, Eq /*Serialize, Deserialize*/)]
-//#[serde(bound(deserialize = "'de: 'a"))]
+#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(bound = "")]
 pub struct CCCS<G: Group> {
   // The `z` vector used as input for this instance.
   pub(crate) z: Vec<G::Scalar>,
   // Commitment to the witness of `z`.
   pub(crate) w_comm: Commitment<G>,
-  // Commitment to the public inputs of `z`.
-  pub(crate) x_comm: Commitment<G>,
 }
 
 impl<G: Group> CCCS<G> {
-  /// Generates a new CCCS given a reference to it's original CCS repr & the multilinear extension of it's matrixes.
-  /// Then, given the input vector `z`.
+  /// Generates a new CCCS given a reference to it's original CCS repr and it's public and private inputs.
   pub(crate) fn new(
     ccs: &CCS<G>,
     ccs_matrix_mle: &Vec<MultilinearPolynomial<G::Scalar>>,
     z: Vec<G::Scalar>,
     ck: &CommitmentKey<G>,
   ) -> Self {
-    let x_comm = if ccs.l != 0 {
-      CE::<G>::commit(ck, &z[1..ccs.l + 1])
-    } else {
-      Commitment::<G>::default()
-    };
     let w_comm = CE::<G>::commit(ck, &z[(1 + ccs.l)..]);
 
     Self {
       z: z.to_vec(),
       w_comm,
-      x_comm,
     }
   }
 
@@ -142,14 +133,6 @@ impl<G: Group> CCCS<G> {
 
     Ok(())
   }
-
-  // XXX: Pending to add a commit fn:
-
-  // let w: Vec<G::Scalar> = z[(1 + self.l)..].to_vec();
-  // XXX: API doesn't offer a way to handle this apparently?
-  // Need to investigate
-  // let _r_w = G::Scalar::random(rng);
-  // let C = <<G as Group>::CE as CommitmentEngineTrait<G>>::commit(ck, &w);
 }
 
 #[cfg(test)]

src/ccs/lcccs.rs

@@ -44,6 +44,7 @@ pub struct LCCCS<G: Group> {
 impl<G: Group> LCCCS<G> {
   /// Generates a new LCCCS instance from a given randomness, CommitmentKey & witness input vector.
   /// This should only be used to probably test or setup the initial NIMFS instance.
+  #[cfg(test)]
   pub(crate) fn new<R: RngCore>(
     ccs: &CCS<G>,
     ccs_m_mle: &[MultilinearPolynomial<G::Scalar>],
@@ -69,12 +70,15 @@ impl<G: Group> LCCCS<G> {
     ccs_m_mle: &[MultilinearPolynomial<G::Scalar>],
     ck: &CommitmentKey<G>,
   ) -> Result<(), NovaError> {
+    dbg!(self.z.clone());
     let w = &self.z[(1 + ccs.l)..];
     // check that C is the commitment of w. Notice that this is not verifying a Pedersen
     // opening, but checking that the Commmitment comes from committing to the witness.
     let comm_eq = self.w_comm == CE::<G>::commit(ck, w);
 
     let computed_v = compute_all_sum_Mz_evals::<G>(ccs_m_mle, &self.z, &self.r_x, ccs.s_prime);
+    dbg!(self.v.clone());
+    dbg!(computed_v.clone());
     let vs_eq = computed_v == self.v;
 
     if vs_eq && comm_eq {
@@ -84,8 +88,7 @@ impl<G: Group> LCCCS<G> {
     }
   }
 
-  /// Compute all L_j(x) polynomials
-  // Can we recieve the MLE of z directy?
+  /// Compute all L_j(x) polynomials.
   pub fn compute_Ls(
     &self,
     ccs: &CCS<G>,

src/ccs/multifolding.rs

@@ -32,7 +32,7 @@ use sha3::{Digest, Sha3_256};
 use std::ops::{Add, Mul};
 use std::sync::Arc;
 
-/// The NIMFS structure is the center of operations of the folding scheme.
+/// The NIMFS (Non-Interactive MultiFolding Scheme) structure is the center of operations of the folding scheme.
 /// Once generated, it allows us to fold any upcomming CCCS instances within it without needing to do much.
 // XXX: Pending to add doc examples.
 #[derive(Debug)]
@@ -44,7 +44,7 @@ pub struct NIMFS<G: Group> {
 }
 
 impl<G: Group> NIMFS<G> {
-  /// Generates a new NIMFS instance based on the given CCS.
+  /// Generates a new NIMFS instance based on the given CCS instance, it's matrix mle's, an existing LCCCS instance and a commitment key to the CCS.
   pub fn new(
     ccs: CCS<G>,
     ccs_mle: Vec<MultilinearPolynomial<G::Scalar>>,
@@ -176,7 +176,7 @@ impl<G: Group> NIMFS<G> {
     g
   }
 
-  /// This folds an upcomming CCCS instance into the LCCCS instance contained within the NIMFS object.
+  /// This folds an upcoming CCCS instance into the running LCCCS instance contained within the NIMFS object.
   pub fn fold<R: RngCore>(&mut self, mut rng: &mut R, cccs: CCCS<G>) {
     // Compute r_x_prime and rho from a given randomnes.
     let r_x_prime = vec![G::Scalar::random(&mut rng); self.ccs.s];
@@ -197,6 +197,7 @@ impl<G: Group> NIMFS<G> {
       .map(|(a_i, b_i)| *a_i + b_i)
       .collect();
 
+    // Here we perform steps 7 & 8 of the section 5 of the paper. Were we actually fold LCCCS & CCCS instances.
     self.lcccs.w_comm += cccs.w_comm.mul(rho);
     self.lcccs.v = folded_v;
     self.lcccs.r_x = r_x_prime;
@@ -372,17 +373,19 @@ mod tests {
     assert!(ccs.is_sat(&ck, &ccs_instance_1, &ccs_witness_1).is_ok());
     assert!(ccs.is_sat(&ck, &ccs_instance_2, &ccs_witness_2).is_ok());
 
-    let cccs = CCCS::new(&ccs, &mles, z2, &ck);
-    assert!(cccs.is_sat(&ccs, &mles, &ck).is_ok());
-
     // Generate a new NIMFS instance
-    let mut nimfs = NIMFS::init(&mut rng, ccs, mles, z1);
+    let mut nimfs = NIMFS::init(&mut rng, ccs.clone(), mles.clone(), z1);
     assert!(nimfs.is_sat().is_ok());
 
-    let rho = Fq::random(&mut rng);
+    // Folding garbage should cause a failure
+    let cccs = nimfs.gen_cccs(vec![Fq::ONE, Fq::ONE, Fq::ONE]);
     nimfs.fold(&mut rng, cccs);
+    assert!(nimfs.is_sat().is_err());
 
     // check folding correct stuff still alows the NIMFS to be satisfied correctly.
+    let cccs = nimfs.gen_cccs(z2);
+    assert!(cccs.is_sat(&ccs, &mles, &ck).is_ok());
+    nimfs.fold(&mut rng, cccs);
     assert!(nimfs.is_sat().is_ok());
   }
 }