From 5ed50a526247055a07d066b0d57d1e4660508995 Mon Sep 17 00:00:00 2001 From: Alon Haramati Date: Thu, 21 Mar 2024 17:03:56 +0200 Subject: [PATCH] Organize errors. --- src/core/commitment_scheme/verifier.rs | 17 +++++++------ src/core/prover/mod.rs | 35 +++++++++++++++----------- src/fibonacci/mod.rs | 20 +++++++++------ 3 files changed, 43 insertions(+), 29 deletions(-) diff --git a/src/core/commitment_scheme/verifier.rs b/src/core/commitment_scheme/verifier.rs index a0fd34810..10971def5 100644 --- a/src/core/commitment_scheme/verifier.rs +++ b/src/core/commitment_scheme/verifier.rs @@ -70,8 +70,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)?; @@ -148,7 +147,9 @@ fn eval_quotients_on_sparse_domain( .map(|subdomain| { let values = queried_values.take(1 << subdomain.log_size).collect_vec(); if values.len() != 1 << subdomain.log_size { - return Err(VerificationError::InvalidStructure); + return Err(VerificationError::InvalidStructure( + "Insufficient number of queried values".to_string(), + )); } let subeval = CircleEvaluation::new(subdomain.to_circle_domain(&commitment_domain), values); @@ -156,10 +157,12 @@ fn eval_quotients_on_sparse_domain( }) .collect::>()?, ); - assert!( - queried_values.is_empty(), - "Not all queried values were used" - ); + if !queried_values.is_empty() { + return Err(VerificationError::InvalidStructure( + "Too many queried values".to_string(), + )); + } + Ok(res) } diff --git a/src/core/prover/mod.rs b/src/core/prover/mod.rs index d8ace6156..f32dfbbcd 100644 --- a/src/core/prover/mod.rs +++ b/src/core/prover/mod.rs @@ -97,7 +97,7 @@ pub fn prove( // 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.proved_values.clone()).unwrap(); + proved_values_to_mask(air, commitment_scheme_proof.proved_values.clone()).unwrap(); if composition_oods_value != air.eval_composition_polynomial_at_point(oods_point, &trace_oods_values, random_coeff) @@ -141,9 +141,13 @@ pub fn verify( open_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.proved_values.clone()) - .map_err(|_| VerificationError::InvalidStructure)?; + let (trace_oods_values, composition_oods_value) = proved_values_to_mask( + air, + proof.commitment_scheme_proof.proved_values.clone(), + ) + .map_err(|_| { + VerificationError::InvalidStructure("Unexpected proved_values structure".to_string()) + })?; if composition_oods_value != air.eval_composition_polynomial_at_point(oods_point, &trace_oods_values, random_coeff) @@ -154,14 +158,15 @@ pub fn verify( commitment_scheme.verify_values(open_points, proof.commitment_scheme_proof, channel) } -fn opened_values_to_mask( +/// Structures the tree-wise proved values into component-wise OODS values and a composition +/// polynomial OODS value. +fn proved_values_to_mask( air: &impl Air, - mut opened_values: TreeVec>>, + mut proved_values: TreeVec>>, ) -> Result<(ComponentVec>, SecureField), ()> { - let composition_oods_values = SecureCirclePoly::eval_from_partial_evals( - opened_values - .pop() - .unwrap() + let composition_partial_proved_values = proved_values.pop().ok_or(())?; + let composition_oods_value = SecureCirclePoly::eval_from_partial_evals( + composition_partial_proved_values .iter() .flatten() .cloned() @@ -171,7 +176,7 @@ fn opened_values_to_mask( ); // Retrieved open mask values for each component. - let flat_trace_values = &mut opened_values.pop().unwrap().into_iter(); + let flat_trace_values = &mut proved_values.pop().ok_or(())?.into_iter(); let trace_oods_values = ComponentVec( air.components() .iter() @@ -179,7 +184,7 @@ fn opened_values_to_mask( .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( diff --git a/src/fibonacci/mod.rs b/src/fibonacci/mod.rs index e92beae37..d1454769c 100644 --- a/src/fibonacci/mod.rs +++ b/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 { .proved_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 proved_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]