Organize errors. (#523)

src/core/commitment_scheme/quotients.rs

@@ -124,9 +124,11 @@ pub fn fri_answers_for_log_size(
 ) -> Result<SparseCircleEvaluation<SecureField>, VerificationError> {
     let commitment_domain = CanonicCoset::new(log_size).circle_domain();
     let batched_samples = ColumnSampleBatch::new(samples);
-    for x in queried_values_per_column {
-        if x.len() != query_domain.flatten().len() {
-            return Err(VerificationError::InvalidStructure);
+    for queried_values in queried_values_per_column {
+        if queried_values.len() != query_domain.flatten().len() {
+            return Err(VerificationError::InvalidStructure(
+                "Insufficient number of queried values".to_string(),
+            ));
         }
     }
     let mut queried_values_per_column = queried_values_per_column
@@ -164,7 +166,9 @@ pub fn fri_answers_for_log_size(
 
     let res = SparseCircleEvaluation::new(evals);
     if !queried_values_per_column.iter().all(|x| x.is_empty()) {
-        return Err(VerificationError::InvalidStructure);
+        return Err(VerificationError::InvalidStructure(
+            "Too many queried values".to_string(),
+        ));
     }
     Ok(res)
 }

src/core/commitment_scheme/verifier.rs

@@ -73,8 +73,7 @@ impl CommitmentSchemeVerifier {
 
         // FRI commitment phase on OODS quotients.
         let fri_config = FriConfig::new(LOG_LAST_LAYER_DEGREE_BOUND, LOG_BLOWUP_FACTOR, N_QUERIES);
-        let mut fri_verifier =
-            FriVerifier::commit(channel, fri_config, proof.fri_proof, bounds).unwrap();
+        let mut fri_verifier = FriVerifier::commit(channel, fri_config, proof.fri_proof, bounds)?;
 
         // Verify proof of work.
         ProofOfWork::new(PROOF_OF_WORK_BITS).verify(channel, &proof.proof_of_work)?;

src/core/prover/mod.rs

@@ -82,22 +82,22 @@ pub fn prove(
     // Draw OODS point.
     let oods_point = CirclePoint::<SecureField>::get_random_point(channel);
 
-    // Open mask values at oods points.
-    let open_points = air.mask_points(oods_point);
+    // Get mask sample points relative to oods point.
+    let sample_points = air.mask_points(oods_point);
 
     // TODO(spapini): Change when we support multiple interactions.
     // First tree - trace.
-    let mut open_points = TreeVec::new(vec![open_points.flatten()]);
+    let mut sample_points = TreeVec::new(vec![sample_points.flatten()]);
     // Second tree - composition polynomial.
-    open_points.push(vec![vec![oods_point]; 4]);
+    sample_points.push(vec![vec![oods_point]; 4]);
 
-    let commitment_scheme_proof = commitment_scheme.prove_values(open_points, channel);
+    let commitment_scheme_proof = commitment_scheme.prove_values(sample_points, channel);
 
     // Evaluate composition polynomial at OODS point and check that it matches the trace OODS
     // values. This is a sanity check.
     // TODO(spapini): Save clone.
     let (trace_oods_values, composition_oods_value) =
-        opened_values_to_mask(air, commitment_scheme_proof.sampled_values.clone()).unwrap();
+        sampled_values_to_mask(air, commitment_scheme_proof.sampled_values.clone()).unwrap();
 
     if composition_oods_value
         != air.eval_composition_polynomial_at_point(oods_point, &trace_oods_values, random_coeff)
@@ -131,37 +131,42 @@ pub fn verify(
     // Draw OODS point.
     let oods_point = CirclePoint::<SecureField>::get_random_point(channel);
 
-    // Open mask values at oods points.
-    let open_points = air.mask_points(oods_point);
+    // Get mask sample points relative to oods point.
+    let trace_sample_points = air.mask_points(oods_point);
 
     // TODO(spapini): Change when we support multiple interactions.
     // First tree - trace.
-    let mut open_points = TreeVec::new(vec![open_points.flatten()]);
+    let mut sample_points = TreeVec::new(vec![trace_sample_points.flatten()]);
     // Second tree - composition polynomial.
-    open_points.push(vec![vec![oods_point]; 4]);
+    sample_points.push(vec![vec![oods_point]; 4]);
 
     // TODO(spapini): Save clone.
-    let (trace_oods_values, composition_oods_value) =
-        opened_values_to_mask(air, proof.commitment_scheme_proof.sampled_values.clone())
-            .map_err(|_| VerificationError::InvalidStructure)?;
+    let (trace_oods_values, composition_oods_value) = sampled_values_to_mask(
+        air,
+        proof.commitment_scheme_proof.sampled_values.clone(),
+    )
+    .map_err(|_| {
+        VerificationError::InvalidStructure("Unexpected sampled_values structure".to_string())
+    })?;
 
     if composition_oods_value
         != air.eval_composition_polynomial_at_point(oods_point, &trace_oods_values, random_coeff)
     {
         return Err(VerificationError::OodsNotMatching);
     }
 
-    commitment_scheme.verify_values(open_points, proof.commitment_scheme_proof, channel)
+    commitment_scheme.verify_values(sample_points, proof.commitment_scheme_proof, channel)
 }
 
-fn opened_values_to_mask(
+/// Structures the tree-wise sampled values into component-wise OODS values and a composition
+/// polynomial OODS value.
+fn sampled_values_to_mask(
     air: &impl Air<CPUBackend>,
-    mut opened_values: TreeVec<ColumnVec<Vec<SecureField>>>,
+    mut sampled_values: TreeVec<ColumnVec<Vec<SecureField>>>,
 ) -> Result<(ComponentVec<Vec<SecureField>>, SecureField), ()> {
-    let composition_oods_values = SecureCirclePoly::eval_from_partial_evals(
-        opened_values
-            .pop()
-            .unwrap()
+    let composition_partial_sampled_values = sampled_values.pop().ok_or(())?;
+    let composition_oods_value = SecureCirclePoly::eval_from_partial_evals(
+        composition_partial_sampled_values
             .iter()
             .flatten()
             .cloned()
@@ -170,16 +175,16 @@ fn opened_values_to_mask(
             .map_err(|_| ())?,
     );
 
-    // Retrieved open mask values for each component.
-    let flat_trace_values = &mut opened_values.pop().unwrap().into_iter();
+    // Retrieve sampled mask values for each component.
+    let flat_trace_values = &mut sampled_values.pop().ok_or(())?.into_iter();
     let trace_oods_values = ComponentVec(
         air.components()
             .iter()
             .map(|c| flat_trace_values.take(c.mask().len()).collect_vec())
             .collect(),
     );
 
-    Ok((trace_oods_values, composition_oods_values))
+    Ok((trace_oods_values, composition_oods_value))
 }
 
 #[derive(Clone, Copy, Debug, Error)]
@@ -198,10 +203,10 @@ pub enum ProvingError {
     ConstraintsNotSatisfied,
 }
 
-#[derive(Clone, Copy, Debug, Error)]
+#[derive(Clone, Debug, Error)]
 pub enum VerificationError {
-    #[error("Proof has invalid structure.")]
-    InvalidStructure,
+    #[error("Proof has invalid structure: {0}.")]
+    InvalidStructure(String),
     #[error("Merkle verification failed.")]
     MerkleVerificationFailed,
     #[error(

src/fibonacci/mod.rs

@@ -120,6 +120,7 @@ mod tests {
     use crate::core::fields::m31::BaseField;
     use crate::core::fields::qm31::SecureField;
     use crate::core::poly::circle::CanonicCoset;
+    use crate::core::prover::VerificationError;
     use crate::core::queries::Queries;
     use crate::core::utils::bit_reverse;
     use crate::{m31, qm31};
@@ -209,11 +210,10 @@ mod tests {
         let mut invalid_proof = fib.prove().unwrap();
         invalid_proof.commitment_scheme_proof.queried_values.0[0][0][4] += BaseField::one();
 
-        fib.verify(invalid_proof).unwrap_err();
+        let error = fib.verify(invalid_proof).unwrap_err();
+        assert!(matches!(error, VerificationError::Fri(_)));
     }
 
-    // TODO(AlonH): Check the correct error occurs after introducing errors instead of
-    // #[should_panic].
     #[test]
     fn test_prove_invalid_trace_oods_values() {
         const FIB_LOG_SIZE: u32 = 5;
@@ -225,11 +225,13 @@ mod tests {
             .sampled_values
             .swap(0, 1);
 
-        fib.verify(invalid_proof).unwrap_err();
+        let error = fib.verify(invalid_proof).unwrap_err();
+        assert!(matches!(error, VerificationError::InvalidStructure(_)));
+        assert!(error
+            .to_string()
+            .contains("Unexpected sampled_values structure"));
     }
 
-    // TODO(AlonH): Check the correct error occurs after introducing errors instead of
-    // #[should_panic].
     #[test]
     fn test_prove_insufficient_trace_values() {
         const FIB_LOG_SIZE: u32 = 5;
@@ -238,7 +240,11 @@ mod tests {
         let mut invalid_proof = fib.prove().unwrap();
         invalid_proof.commitment_scheme_proof.queried_values.0[0][0].pop();
 
-        fib.verify(invalid_proof).unwrap_err();
+        let error = fib.verify(invalid_proof).unwrap_err();
+        assert!(matches!(error, VerificationError::InvalidStructure(_)));
+        assert!(error
+            .to_string()
+            .contains("Insufficient number of queried values"));
     }
 
     #[test]