privacy-scaling-explorations / snark-verifier

Apache License 2.0
144 stars 137 forks source link

[Question] verification of the aggregation circuit fails on EVM when it has instances other than the accumulator. #33

Closed SoraSuegami closed 1 year ago

SoraSuegami commented 1 year ago

Issue

Referring to the AggregationCircuit in the example code, I made an aggregation circuit named PublicAggregationCircuit that exposes the accumulator and the instances of snarks to be aggregated. https://github.com/zkemail/halo2-zk-email/blob/feat/main_gate_base_sha2/src/snark_verifier_sdk.rs#L137-L396

I generated proof and a verifier contract with the snark-verifier. However, the contract rejected the proof even if the prover and contract have the same values for the instance and accumulators. In more detail, after generating a yul code of the verifier code, I converted it to a function-equivalent Solidity code with a fix_verifier_sol function in the ezkl repository and tested it in a hardhat local network. The aggregated snarks were generated for DefaultEmailVerifyCircuit, which verifies a DKIM RSA signature and substrings in the email header and body. The verifier contract returned false as a verification result without any revert error. (While the original yul code reverts when the proof is invalid, the converted Solidity code returns false.)

What I tried

I confirmed that proof for the AggregationCircuit is accepted when it is locally verified by a verify_proof function in halo2 with EvmTranscript. In addition, when the proof and the verifier contract are generated directly for the DefaultEmailVerifyCircuit, it passed the verification also on EVM. When I commented out the following lines in the Solidity code of the verifier contract for the AggregationCircuit, the contract accepted the proof with any instances but rejected an invalid proof and invalid accumulator values. https://github.com/zkemail/halo2-zk-email/blob/feat/main_gate_base_sha2/test_data/Verifier.sol#L7700-L7733 Therefore, I think the error is due to the verification of the additional instances.

Questions

  1. Does the current snark-verifier support additional instances other than the accumulator in the proof aggregation?
  2. If it is supported, is my implementation that exposes additional instances in the PublicAggregationCircuit correct? https://github.com/zkemail/halo2-zk-email/blob/feat/main_gate_base_sha2/src/snark_verifier_sdk.rs#L368-L392
han0110 commented 1 year ago

Thanks for reporting the bug!

Just had a quick look at the codebase, I see that in gen_param we are using thread_rng as the prng, and I guess we'd generate 2 params with different k for application and aggregation, if that's the case I think it makes sense the verifier doesn't work, because we need the 2 different proof to use the same setup to random linearly combine them (otherwise the s_g2 will be different then the pairing will fail).

https://github.com/zkemail/halo2-zk-email/blob/37574a089a3f33b029571a4ae002a4749ee71c7a/src/helpers.rs#L55-L63

Could this be the issue?

SoraSuegami commented 1 year ago

Thank you very much for answering my questions! Yes, I am using different setup parameters for application and aggregation. I will try it again with the same parameter.

SoraSuegami commented 1 year ago

With the same setup parameter, the aggregation proof passed the verification on EVM! Thank you very much for your advice!