Improvements to error handling and naming (microsoft#309)

* rename and introduce checks about length

* introduce a test about public IO

benches/compressed-snark.rs

@@ -72,7 +72,8 @@ fn bench_compressed_snark_internal<S1: RelaxedR1CSSNARKTrait<E1>, S2: RelaxedR1C
     &c_secondary,
     &*S1::ck_floor(),
     &*S2::ck_floor(),
-  );
+  )
+  .unwrap();
 
   // Produce prover and verifier keys for CompressedSNARK
   let (pk, vk) = CompressedSNARK::<_, _, _, _, S1, S2>::setup(&pp).unwrap();

benches/recursive-snark.rs

@@ -75,7 +75,8 @@ fn bench_recursive_snark(c: &mut Criterion) {
       &c_secondary,
       &*default_ck_hint(),
       &*default_ck_hint(),
-    );
+    )
+    .unwrap();
 
     // Bench time to produce a recursive SNARK;
     // we execute a certain number of warm-up steps since executing

benches/sha256.rs

@@ -163,7 +163,8 @@ fn bench_recursive_snark(c: &mut Criterion) {
       &ttc,
       &*default_ck_hint(),
       &*default_ck_hint(),
-    );
+    )
+    .unwrap();
 
     let circuit_secondary = TrivialCircuit::default();
     let z0_primary = vec![<E1 as Engine>::Scalar::from(2u64)];

examples/and.rs

@@ -231,7 +231,8 @@ fn main() {
       &circuit_secondary,
       &*S1::ck_floor(),
       &*S2::ck_floor(),
-    );
+    )
+    .unwrap();
     println!("PublicParams::setup, took {:?} ", start.elapsed());
 
     println!(

examples/minroot.rs

@@ -175,7 +175,8 @@ fn main() {
       &circuit_secondary,
       &*S1::ck_floor(),
       &*S2::ck_floor(),
-    );
+    )
+    .unwrap();
     println!("PublicParams::setup, took {:?} ", start.elapsed());
 
     println!(

src/errors.rs

@@ -9,9 +9,10 @@ pub enum NovaError {
   /// returned if the supplied row or col in (row,col,val) tuple is out of range
   #[error("InvalidIndex")]
   InvalidIndex,
-  /// returned if the supplied input is not even-sized
-  #[error("OddInputLength")]
-  OddInputLength,
+  /// returned if the step circuit calls inputize or alloc_io in its synthesize method
+  /// instead of passing output with the return value
+  #[error("InvalidStepCircuitIO")]
+  InvalidStepCircuitIO,
   /// returned if the supplied input is not of the right length
   #[error("InvalidInputLength")]
   InvalidInputLength,
@@ -33,9 +34,9 @@ pub enum NovaError {
   /// returned if the provided number of steps is zero
   #[error("InvalidNumSteps")]
   InvalidNumSteps,
-  /// returned when an invalid inner product argument is provided
-  #[error("InvalidIPA")]
-  InvalidIPA,
+  /// returned when an invalid PCS evaluation argument is provided
+  #[error("InvalidPCS")]
+  InvalidPCS,
   /// returned when an invalid sum-check proof is provided
   #[error("InvalidSumcheckProof")]
   InvalidSumcheckProof,

src/lib.rs

@@ -143,7 +143,7 @@ where
     c_secondary: &C2,
     ck_hint1: &CommitmentKeyHint<E1>,
     ck_hint2: &CommitmentKeyHint<E2>,
-  ) -> Self {
+  ) -> Result<Self, NovaError> {
     let augmented_circuit_params_primary =
       NovaAugmentedCircuitParams::new(BN_LIMB_WIDTH, BN_N_LIMBS, true);
     let augmented_circuit_params_secondary =
@@ -181,7 +181,11 @@ where
     let _ = circuit_secondary.synthesize(&mut cs);
     let (r1cs_shape_secondary, ck_secondary) = cs.r1cs_shape(ck_hint2);
 
-    PublicParams {
+    if r1cs_shape_primary.num_io != 2 || r1cs_shape_secondary.num_io != 2 {
+      return Err(NovaError::InvalidStepCircuitIO);
+    }
+
+    Ok(PublicParams {
       F_arity_primary,
       F_arity_secondary,
       ro_consts_primary,
@@ -196,7 +200,7 @@ where
       augmented_circuit_params_secondary,
       digest: OnceCell::new(),
       _p: Default::default(),
-    }
+    })
   }
 
   /// Retrieve the digest of the public parameters.
@@ -924,7 +928,7 @@ mod tests {
     // this tests public parameters with a size specifically intended for a spark-compressed SNARK
     let ck_hint1 = &*SPrime::<E1, EE<E1>>::ck_floor();
     let ck_hint2 = &*SPrime::<E2, EE<E2>>::ck_floor();
-    let pp = PublicParams::<E1, E2, T1, T2>::setup(circuit1, circuit2, ck_hint1, ck_hint2);
+    let pp = PublicParams::<E1, E2, T1, T2>::setup(circuit1, circuit2, ck_hint1, ck_hint2).unwrap();
 
     let digest_str = pp
       .digest()
@@ -978,7 +982,8 @@ mod tests {
       &test_circuit2,
       &*default_ck_hint(),
       &*default_ck_hint(),
-    );
+    )
+    .unwrap();
 
     let num_steps = 1;
 
@@ -1032,7 +1037,8 @@ mod tests {
       &circuit_secondary,
       &*default_ck_hint(),
       &*default_ck_hint(),
-    );
+    )
+    .unwrap();
 
     let num_steps = 3;
 
@@ -1114,7 +1120,8 @@ mod tests {
       &circuit_secondary,
       &*default_ck_hint(),
       &*default_ck_hint(),
-    );
+    )
+    .unwrap();
 
     let num_steps = 3;
 
@@ -1213,7 +1220,8 @@ mod tests {
       &circuit_secondary,
       &*SPrime::<E1, EE1>::ck_floor(),
       &*SPrime::<E2, EE2>::ck_floor(),
-    );
+    )
+    .unwrap();
 
     let num_steps = 3;
 
@@ -1375,7 +1383,8 @@ mod tests {
       &circuit_secondary,
       &*default_ck_hint(),
       &*default_ck_hint(),
-    );
+    )
+    .unwrap();
 
     let num_steps = 3;
 
@@ -1452,7 +1461,8 @@ mod tests {
       &test_circuit2,
       &*default_ck_hint(),
       &*default_ck_hint(),
-    );
+    )
+    .unwrap();
 
     let num_steps = 1;
 
@@ -1497,4 +1507,61 @@ mod tests {
     test_ivc_base_with::<Bn256EngineKZG, GrumpkinEngine>();
     test_ivc_base_with::<Secp256k1Engine, Secq256k1Engine>();
   }
+
+  fn test_setup_with<E1, E2>()
+  where
+    E1: Engine<Base = <E2 as Engine>::Scalar>,
+    E2: Engine<Base = <E1 as Engine>::Scalar>,
+  {
+    #[derive(Clone, Debug, Default)]
+    struct CircuitWithInputize<F: PrimeField> {
+      _p: PhantomData<F>,
+    }
+
+    impl<F: PrimeField> StepCircuit<F> for CircuitWithInputize<F> {
+      fn arity(&self) -> usize {
+        1
+      }
+
+      fn synthesize<CS: ConstraintSystem<F>>(
+        &self,
+        cs: &mut CS,
+        z: &[AllocatedNum<F>],
+      ) -> Result<Vec<AllocatedNum<F>>, SynthesisError> {
+        let x = &z[0];
+        let y = x.square(cs.namespace(|| "x_sq"))?;
+        y.inputize(cs.namespace(|| "y"))?; // inputize y
+        Ok(vec![y])
+      }
+    }
+
+    // produce public parameters with trivial secondary
+    let circuit = CircuitWithInputize::<<E1 as Engine>::Scalar>::default();
+    let pp =
+      PublicParams::<E1, E2, CircuitWithInputize<E1::Scalar>, TrivialCircuit<E2::Scalar>>::setup(
+        &circuit,
+        &TrivialCircuit::default(),
+        &*default_ck_hint(),
+        &*default_ck_hint(),
+      );
+    assert!(pp.is_err());
+    assert_eq!(pp.err(), Some(NovaError::InvalidStepCircuitIO));
+
+    // produce public parameters with the trivial primary
+    let circuit = CircuitWithInputize::<E2::Scalar>::default();
+    let pp =
+      PublicParams::<E1, E2, TrivialCircuit<E1::Scalar>, CircuitWithInputize<E2::Scalar>>::setup(
+        &TrivialCircuit::default(),
+        &circuit,
+        &*default_ck_hint(),
+        &*default_ck_hint(),
+      );
+    assert!(pp.is_err());
+    assert_eq!(pp.err(), Some(NovaError::InvalidStepCircuitIO));
+  }
+
+  #[test]
+  fn test_setup() {
+    test_setup_with::<Bn256EngineKZG, GrumpkinEngine>();
+  }
 }

src/provider/bn256_grumpkin.rs

@@ -1,5 +1,6 @@
 //! This module implements the Nova traits for `bn256::Point`, `bn256::Scalar`, `grumpkin::Point`, `grumpkin::Scalar`.
 use crate::{
+  errors::NovaError,
   impl_traits,
   provider::traits::{CompressedGroup, DlogGroup, PairingGroup},
   traits::{Group, PrimeFieldExt, TranscriptReprTrait},
@@ -56,7 +57,7 @@ impl PairingGroup for Bn256Point {
   type GT = Gt;
 
   fn pairing(p: &Self, q: &Self::G2) -> Self::GT {
-    pairing(&p.to_affine(), &q.to_affine())
+    pairing(&p.affine(), &q.affine())
   }
 }
 
@@ -84,28 +85,25 @@ impl Group for G2 {
 
 impl DlogGroup for G2 {
   type CompressedGroupElement = G2Compressed;
-  type PreprocessedGroupElement = G2Affine;
+  type AffineGroupElement = G2Affine;
 
-  fn vartime_multiscalar_mul(
-    scalars: &[Self::Scalar],
-    bases: &[Self::PreprocessedGroupElement],
-  ) -> Self {
+  fn vartime_multiscalar_mul(scalars: &[Self::Scalar], bases: &[Self::AffineGroupElement]) -> Self {
     best_multiexp(scalars, bases)
   }
 
-  fn preprocessed(&self) -> Self::PreprocessedGroupElement {
+  fn affine(&self) -> Self::AffineGroupElement {
     self.to_affine()
   }
 
-  fn group(p: &Self::PreprocessedGroupElement) -> Self {
+  fn group(p: &Self::AffineGroupElement) -> Self {
     G2::from(*p)
   }
 
   fn compress(&self) -> Self::CompressedGroupElement {
     self.to_bytes()
   }
 
-  fn from_label(_label: &'static [u8], _n: usize) -> Vec<Self::PreprocessedGroupElement> {
+  fn from_label(_label: &'static [u8], _n: usize) -> Vec<Self::AffineGroupElement> {
     unimplemented!()
   }
 
@@ -131,8 +129,13 @@ impl<G: DlogGroup> TranscriptReprTrait<G> for G2Compressed {
 impl CompressedGroup for G2Compressed {
   type GroupElement = G2;
 
-  fn decompress(&self) -> Option<G2> {
-    Some(G2::from_bytes(self).unwrap())
+  fn decompress(&self) -> Result<G2, NovaError> {
+    let d = G2::from_bytes(self);
+    if d.is_some().into() {
+      Ok(d.unwrap())
+    } else {
+      Err(NovaError::DecompressionError)
+    }
   }
 }