New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Implement comprehensive error handling in verify.cairo #23

Closed

hexiao-NM wants to merge 6 commits into NethermindEth:main from hexiao-NM:error-handling

Collaborator

hexiao-NM commented Nov 21, 2024

Error Handling Implementation for Cairo PLONK Verifier

Overview

This PR implements comprehensive error handling for the PLONK verifier in Cairo, transforming the codebase to use Result types and providing detailed error information for debugging and validation.

Key Changes

Error Handling System

Implemented Result-based error handling throughout verify.cairo
Added granular error checks for:
- Curve point validation
- Field element validation
- Public input verification
- Transcript operations
- Pairing verification

Code Improvements

Transformed all verification functions to return Result types
Added detailed error messages for debugging
Implemented proper error propagation using the ? operator
Enhanced input validation with specific error cases

Error Types

InvalidCurvePoint: For curve point validation failures
InvalidFieldElement: For field element validation issues
InvalidPublicInput: For public input validation errors
TranscriptError: For transcript operation failures
PairingError: For pairing check failures
VerificationFailed: For general verification failures

Testing

Verified error propagation works correctly
Confirmed error messages are helpful for debugging
Tested all error paths in the verification process


          feat: Implement comprehensive error handling in verify.cairo

132aec1

Key changes:
- Added Result-based error handling
- Implemented detailed error checks for curve points and field elements
- Added granular error messages for debugging
- Updated all functions to use proper error propagation
- Improved error reporting for verification failures

danielntmd requested changes

View reviewed changes

plonk-verifier/src/plonk/verify.cairo Outdated

-              use plonk_verifier::pairing::tate_bkls::{tate_pairing, tate_miller_loop};
-              use plonk_verifier::pairing::optimal_ate::{single_ate_pairing, ate_miller_loop};
+              use plonk_verifier::plonk::transcript::{TranscriptTrait, Keccak256Transcript};
+              use plonk_verifier::errors::{PlonkError, Result};

Collaborator

danielntmd Nov 23, 2024

Can you link this file into the PR

plonk-verifier/src/plonk/verify.cairo Outdated


		result
		Result::Ok(true)

Collaborator

danielntmd Nov 23, 2024

It looks like we are returning True here? Let's fix with proper result that we want

Collaborator

danielntmd Nov 23, 2024

Let's also reduce all Result::Ok() into just Ok()

plonk-verifier/src/plonk/verify.cairo Outdated

Comment on lines 22 to 73

+                      if !Self::is_on_curve(proof.A) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point A is not on curve'));
+                      }
+                      if !Self::is_on_curve(proof.B) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point B is not on curve'));
+                      }
+                      if !Self::is_on_curve(proof.C) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point C is not on curve'));
+                      }
+                      if !Self::is_on_curve(proof.Z) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point Z is not on curve'));
+                      }
+                      if !Self::is_on_curve(proof.T1) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point T1 is not on curve'));
+                      }
+                      if !Self::is_on_curve(proof.T2) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point T2 is not on curve'));
+                      }
+                      if !Self::is_on_curve(proof.T3) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point T3 is not on curve'));
+                      }
+                      if !Self::is_on_curve(proof.Wxi) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point Wxi is not on curve'));
+                      }
+                      if !Self::is_on_curve(proof.Wxiw) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point Wxiw is not on curve'));
+                      }
+                      // Validate field elements
+                      if !Self::is_in_field(proof.eval_a) {
+                          return Result::Err(PlonkError::InvalidFieldElement('eval_a not in field'));
+                      }
+                      if !Self::is_in_field(proof.eval_b) {
+                          return Result::Err(PlonkError::InvalidFieldElement('eval_b not in field'));
+                      }
+                      if !Self::is_in_field(proof.eval_c) {
+                          return Result::Err(PlonkError::InvalidFieldElement('eval_c not in field'));
+                      }
+                      if !Self::is_in_field(proof.eval_s1) {
+                          return Result::Err(PlonkError::InvalidFieldElement('eval_s1 not in field'));
+                      }
+                      if !Self::is_in_field(proof.eval_s2) {
+                          return Result::Err(PlonkError::InvalidFieldElement('eval_s2 not in field'));
+                      }
+                      if !Self::is_in_field(proof.eval_zw) {
+                          return Result::Err(PlonkError::InvalidFieldElement('eval_zw not in field'));
+                      }
+                      // Validate public inputs
+                      if !Self::check_public_inputs_length(verification_key.nPublic, publicSignals.len().into()) {
+                          return Result::Err(PlonkError::InvalidPublicInput('Public inputs length mismatch'));
+                      }

Collaborator

danielntmd Nov 23, 2024

Let's make this cleaner by creating a helper function to wrap each call (probably place this in the error file you have made) and have it so that this helper function outputs as a Result type. This way we can just call this helper function and error handle it with "?" all in one line.

plonk-verifier/src/plonk/verify.cairo Outdated

Comment on lines 22 to 24

+                      if !Self::is_on_curve(proof.A) {
+                          return Result::Err(PlonkError::InvalidCurvePoint('Point A is not on curve'));
+                      }

Collaborator

danielntmd Nov 23, 2024

Let's make the code more visible by importing the Result::Errr and PlonkError::InvalidCurvePoint directly. This way we can just have a simple Err(InvalidCurvePoint(' '));

plonk-verifier/src/plonk/verify.cairo

-                      result
+                      Result::Ok(true)
                   }

Collaborator

danielntmd Nov 23, 2024

Clean same thing as before in the code below.

hexiao-NM added 2 commits

November 29, 2024 08:56


          refactor: Move error handling to separate file and improve syntax

11dec60


          refactor: Simplify Result::Ok to Ok after importing Ok variant

741b2cc

danielntmd reviewed

View reviewed changes

plonk-verifier/src/plonk/verify.cairo Outdated


		result
		Ok(())

Collaborator

danielntmd Nov 29, 2024

Return Ok(result)

hexiao-NM added 3 commits

December 2, 2024 09:18


          Return result from verify function and add polynomial commitment veri…

acaa406

…fication


          Fix verify function return type to Result<bool>

cbe1f54


          fix: update verify return type handling to return Ok(true) after vali…

2811cb9

…dation

hexiao-NM closed this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet