From 10677575430fe793fdd224e10ff0edb70440a488 Mon Sep 17 00:00:00 2001 From: Ryan Hall Date: Thu, 28 Nov 2024 07:17:14 -0700 Subject: [PATCH] modify groth verifier contracts for better test-ability and extendability (#414) This PR does the following: * adds the `virtual` tag to several functions in `Groth16VerifierExtension.sol` - this allows us to override the functions in a parent contract, which makes it much easier to test. With these changes, we can test as much of the response codepath as possible without actually having to generate proofs. This will also make it easier to tweak or extend verification logic for specific chains. Ex on polygon (and in the future on scroll) we want to skip blockhash verification. This will be much easier to do once we have this setup. * moves `verifyBlockhash()` to it's own function. This is specifically for the `isCDK()` case where we want to override this function. * renames `verifier.sol` => `Verifier.sol` and `Groth16VerifierExtensions.sol` => `Groth16VerifierExtension.sol` for consistency with the contracts codebase * Changes the name of the `Query` contract to `Groth16VerifierExtension` for consistency --- gnark-utils/lib/lib.go | 6 +-- gnark-utils/src/compile.rs | 2 +- gnark-utils/src/lib.rs | 2 +- groth16-framework/src/compiler.rs | 2 +- groth16-framework/src/lib.rs | 4 +- groth16-framework/src/test_utils.rs | 2 +- groth16-framework/src/utils.rs | 2 +- ...sions.sol => Groth16VerifierExtension.sol} | 39 +++++++++++++++---- groth16-framework/tests/query.rs | 8 ++-- 9 files changed, 45 insertions(+), 22 deletions(-) rename groth16-framework/test_data/{Groth16VerifierExtensions.sol => Groth16VerifierExtension.sol} (90%) diff --git a/gnark-utils/lib/lib.go b/gnark-utils/lib/lib.go index 43d766c19..67cb1d2e5 100644 --- a/gnark-utils/lib/lib.go +++ b/gnark-utils/lib/lib.go @@ -308,16 +308,16 @@ func SaveVerifierSolidity(assetDir string, vk groth16.VerifyingKey) error { } content := buf.String() - contractFile, err := os.Create(assetDir + "/verifier.sol") + contractFile, err := os.Create(assetDir + "/Verifier.sol") if err != nil { - return errors.Wrap(err, "create verifier.sol file") + return errors.Wrap(err, "create Verifier.sol file") } defer contractFile.Close() w := bufio.NewWriter(contractFile) // write the new content to the writer if _, err = w.Write([]byte(content)); err != nil { - return errors.Wrap(err, "write to verifier.sol") + return errors.Wrap(err, "write to Verifier.sol") } return nil diff --git a/gnark-utils/src/compile.rs b/gnark-utils/src/compile.rs index acb8b9780..c1d35df30 100644 --- a/gnark-utils/src/compile.rs +++ b/gnark-utils/src/compile.rs @@ -5,7 +5,7 @@ use anyhow::Result; use std::ffi::CString; /// Compile the circuit data and generate the asset files of `r1cs.bin`, -/// `pk.bin`, `vk.bin` and `verifier.sol`. +/// `pk.bin`, `vk.bin` and `Verifier.sol`. pub fn compile_and_generate_assets( common_circuit_data: &str, verifier_only_circuit_data: &str, diff --git a/gnark-utils/src/lib.rs b/gnark-utils/src/lib.rs index 53d05305a..3bc736d7f 100644 --- a/gnark-utils/src/lib.rs +++ b/gnark-utils/src/lib.rs @@ -15,7 +15,7 @@ mod go { extern "C" { /// Compile and generate the asset files from the circuit data to the /// specified dir. The generated files are `r1cs.bin`, `pk.bin`, - /// `vk.bin` and `verifier.sol`. + /// `vk.bin` and `Verifier.sol`. pub fn CompileAndGenerateAssets( common_circuit_data: *const c_char, verifier_only_circuit_data: *const c_char, diff --git a/groth16-framework/src/compiler.rs b/groth16-framework/src/compiler.rs index 3f6870a2d..6007571ce 100644 --- a/groth16-framework/src/compiler.rs +++ b/groth16-framework/src/compiler.rs @@ -22,7 +22,7 @@ use std::{ type WrapCircuit = WrappedCircuit; /// Compile the circuit data and generate the asset files of `r1cs.bin`, -/// `pk.bin`, `vk.bin` and `verifier.sol`. +/// `pk.bin`, `vk.bin` and `Verifier.sol`. /// This function returns the full file path of the Solidity verifier contract. pub fn compile_and_generate_assets( circuit_data: CircuitData, diff --git a/groth16-framework/src/lib.rs b/groth16-framework/src/lib.rs index 59a58857f..066a242f8 100644 --- a/groth16-framework/src/lib.rs +++ b/groth16-framework/src/lib.rs @@ -6,7 +6,7 @@ //! 1. Generate the asset files. //! //! The asset files are `circuit.bin`, `r1cs.bin`, `pk.bin`, `vk.bin` and -//! `verifier.sol`. User could call the `compile_and_generate_assets` +//! `Verifier.sol`. User could call the `compile_and_generate_assets` //! function to generate these files as below. //! //! `` @@ -76,7 +76,7 @@ pub mod utils; mod verifier; // The function is used to generate the asset files of `circuit.bin`, -// `r1cs.bin`, `pk.bin`, `vk.bin` and `verifier.sol`. It's only necessary to be +// `r1cs.bin`, `pk.bin`, `vk.bin` and `Verifier.sol`. It's only necessary to be // called for re-generating these asset files when the circuit code changes. pub use compiler::compile_and_generate_assets; diff --git a/groth16-framework/src/test_utils.rs b/groth16-framework/src/test_utils.rs index 6c75a6d2e..ccdd0c616 100644 --- a/groth16-framework/src/test_utils.rs +++ b/groth16-framework/src/test_utils.rs @@ -66,7 +66,7 @@ fn groth16_verify(asset_dir: &str, proof: &Groth16Proof) { /// Test the Solidity verification. fn evm_verify(asset_dir: &str, proof: &Groth16Proof) { let solidity_file_path = Path::new(asset_dir) - .join("verifier.sol") + .join("Verifier.sol") .to_string_lossy() .to_string(); diff --git a/groth16-framework/src/utils.rs b/groth16-framework/src/utils.rs index 6f6e629e6..77595558f 100644 --- a/groth16-framework/src/utils.rs +++ b/groth16-framework/src/utils.rs @@ -19,7 +19,7 @@ use std::{ pub const CIRCUIT_DATA_FILENAME: &str = "circuit.bin"; /// The filename of the exported Solidity verifier contract. -pub const SOLIDITY_VERIFIER_FILENAME: &str = "verifier.sol"; +pub const SOLIDITY_VERIFIER_FILENAME: &str = "Verifier.sol"; /// Convert a string with `0x` prefix to an U256. pub fn hex_to_u256(s: &str) -> Result { diff --git a/groth16-framework/test_data/Groth16VerifierExtensions.sol b/groth16-framework/test_data/Groth16VerifierExtension.sol similarity index 90% rename from groth16-framework/test_data/Groth16VerifierExtensions.sol rename to groth16-framework/test_data/Groth16VerifierExtension.sol index 69c533178..73725b687 100644 --- a/groth16-framework/test_data/Groth16VerifierExtensions.sol +++ b/groth16-framework/test_data/Groth16VerifierExtension.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; -import {Verifier} from "./verifier.sol"; +import {Verifier} from "./Verifier.sol"; // The query input struct passed into the processQuery function struct QueryInput { @@ -40,7 +40,7 @@ enum QueryErrorCode { ComputationOverflow } -contract Query is Verifier { +contract Groth16VerifierExtension is Verifier { // Top 3 bits mask. uint256 constant TOP_THREE_BIT_MASK = ~(uint256(7) << 253); @@ -94,7 +94,12 @@ contract Query is Verifier { // Then ensure this hash value equals to the last Groth16 input (groth16_inputs[2]). // 3. Parse the items from public inputs, and check as expected for query. // 4. Parse and return the query output from public inputs. - function processQuery(bytes32[] calldata data, QueryInput memory query) public view returns (QueryOutput memory) { + function processQuery(bytes32[] calldata data, QueryInput memory query) + public + view + virtual + returns (QueryOutput memory) + { // 1. Groth16 verification uint256[3] memory groth16Inputs = verifyGroth16Proof(data); @@ -109,7 +114,7 @@ contract Query is Verifier { } // Parse the Groth16 proofs and inputs, do verification, and returns the Groth16 inputs. - function verifyGroth16Proof(bytes32[] calldata data) internal view returns (uint256[3] memory) { + function verifyGroth16Proof(bytes32[] calldata data) internal view virtual returns (uint256[3] memory) { uint256[8] memory proofs; uint256[3] memory inputs; @@ -130,7 +135,7 @@ contract Query is Verifier { } // Compute sha256 on the public inputs, and ensure it equals to the last Groth16 input. - function verifyPublicInputs(bytes32[] calldata data, uint256[3] memory groth16Inputs) internal pure { + function verifyPublicInputs(bytes32[] calldata data, uint256[3] memory groth16Inputs) internal pure virtual { // Parse the public inputs from calldata. bytes memory pi = parsePublicInputs(data); @@ -166,13 +171,18 @@ contract Query is Verifier { } // Verify the public inputs with the expected query. - function verifyQuery(bytes32[] calldata data, QueryInput memory query) internal pure returns (QueryErrorCode) { + function verifyQuery(bytes32[] calldata data, QueryInput memory query) + internal + view + virtual + returns (QueryErrorCode) + { // Retrieve the last Uint256 of public inputs. bytes32 rem = data[PI_REM_OFFSET]; // Check the block hash and computational hash. bytes32 blockHash = convertToBlockHash(data[PI_OFFSET + BLOCK_HASH_POS]); - require(blockHash == query.blockHash, "Block hash must equal as expected."); + verifyBlockHash(blockHash, query.blockHash); bytes32 computationalHash = data[PI_OFFSET + COMPUTATIONAL_HASH_POS]; require(computationalHash == query.computationalHash, "Computational hash must equal as expected."); @@ -215,8 +225,21 @@ contract Query is Verifier { return QueryErrorCode.ComputationOverflow; } + /// @notice verifies two blockhashed are equal + /// @param blockHash the blockhash computed from the proof + /// @param expectedBlockHash the expected blockhash, retrieved from the query + /// @dev this function is virtual to allow for different implementations in different environments + function verifyBlockHash(bytes32 blockHash, bytes32 expectedBlockHash) internal view virtual { + require(blockHash == expectedBlockHash, "Block hash must equal as expected."); + } + // Parse the query output from the public inputs. - function parseOutput(bytes32[] calldata data, QueryErrorCode error) internal pure returns (QueryOutput memory) { + function parseOutput(bytes32[] calldata data, QueryErrorCode error) + internal + pure + virtual + returns (QueryOutput memory) + { bytes32 rem = data[PI_REM_OFFSET]; // Retrieve total number of the matched rows. diff --git a/groth16-framework/tests/query.rs b/groth16-framework/tests/query.rs index f9964b99a..f56543262 100644 --- a/groth16-framework/tests/query.rs +++ b/groth16-framework/tests/query.rs @@ -35,8 +35,8 @@ fn test_local_groth16_proof() { // Verify the query in the Solidity function. // The editing Solidity code is saved in `test_data/TestGroth16Verifier.sol`. - // TODO: In practice, the separate `Groth16VerifierExtensions.sol` and - // `verifier.sol` should be used, but the `revm` (Rust EVM) cannot support + // TODO: In practice, the separate `Groth16VerifierExtension.sol` and + // `Verifier.sol` should be used, but the `revm` (Rust EVM) cannot support // compilated contract deployment (as inheritance) for now. verify_query_in_solidity(ASSET_DIR); } @@ -64,8 +64,8 @@ fn test_groth16_proving_for_query() { // Verify the query in the Solidity function. // The editing Solidity code is saved in `test_data/TestGroth16Verifier.sol`. - // TODO: In practice, the separate `Groth16VerifierExtensions.sol` and - // `verifier.sol` should be used, but the `revm` (Rust EVM) cannot support + // TODO: In practice, the separate `Groth16VerifierExtension.sol` and + // `Verifier.sol` should be used, but the `revm` (Rust EVM) cannot support // compilated contract deployment (as inheritance) for now. verify_query_in_solidity(ASSET_DIR); }