Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Proof Generation for KECCAK & SUPER circuit fails with error: RootCircuit::new: AssertionFailure("e(lhs, g2)·e(rhs, -s_g2) == O") {'error': 'RootCircuit::new: AssertionFailure("e(lhs, g2)·e(rhs, -s_g2) == O")', 'result': 'FAILED'} #1335

Open
AronisAt79 opened this issue Mar 26, 2023 · 7 comments
Labels
T-bug Type: bug

Comments

@AronisAt79
Copy link
Contributor

What command(s) is the bug in?

No response

Describe the bug

Proof request for block consisting of single l2 transaction (eth withddraw)
Proof request options:

('{"jsonrpc":"2.0", "method":"proof", '
'"params":[{"block":37,"circuit":"keccak","aggregate":true,"mock":false,"mock_feedback":true,"rpc":"http://leader-testnet-geth:8545/", '
'"retry":false}], "id":37}')

Error:
RootCircuit::new: AssertionFailure("e(lhs, g2)·e(rhs, -s_g2) == O")
{'error': 'RootCircuit::new: AssertionFailure("e(lhs, g2)·e(rhs, -s_g2) == O")',
'result': 'FAILED'}

Circuit Commit: https://github.com/privacy-scaling-explorations/zkevm-circuits.git@branch=main
Chain Commit: eaab99aeb31b92ed8870548414c37148e18ac78d

Full logs:

https://zkevm-chain-testing.s3.eu-central-1.amazonaws.com/proveSingeCrossChainTx_keccak_True_372-maronis.tar.gz

Concrete steps to reproduce the bug

No response

@AronisAt79 AronisAt79 added the T-bug Type: bug label Mar 26, 2023
@AronisAt79 AronisAt79 changed the title Proof Generation for KECCAK circuit fails with error: RootCircuit::new: AssertionFailure("e(lhs, g2)·e(rhs, -s_g2) == O") {'error': 'RootCircuit::new: AssertionFailure("e(lhs, g2)·e(rhs, -s_g2) == O")', 'result': 'FAILED'} Proof Generation for KECCAK & SUPER circuit fails with error: RootCircuit::new: AssertionFailure("e(lhs, g2)·e(rhs, -s_g2) == O") {'error': 'RootCircuit::new: AssertionFailure("e(lhs, g2)·e(rhs, -s_g2) == O")', 'result': 'FAILED'} Apr 1, 2023
@AronisAt79
Copy link
Contributor Author

proof generation for super circuit also fails similarly
Logs here:
https://zkevm-chain-testing.s3.eu-central-1.amazonaws.com/proveSingeCrossChainTx_super_True_377-maronis.tar.gz

@AronisAt79
Copy link
Contributor Author

Look like this might be related to #1330
Will retest once merged to main

@ed255
Copy link
Member

ed255 commented Apr 20, 2023

Look like this might be related to #1330
Will retest once merged to main

So the issue that #1330 was resolving had already been solved via another PR #1315 which was merged on March 23rd so it seems this issue may be unrelated.

@ChihChengLiang
Copy link
Collaborator

Close for inactivity

@ChihChengLiang ChihChengLiang closed this as not planned Won't fix, can't repro, duplicate, stale May 18, 2023
@AronisAt79
Copy link
Contributor Author

KECCAK circuit proof in latest main is now passing

@ed255
Copy link
Member

ed255 commented Jun 1, 2023

Reopening the issue to track the failure of the Super Circuit with aggregation.

@ed255 ed255 reopened this Jun 1, 2023
@ed255
Copy link
Member

ed255 commented Jun 5, 2023

The next step to continue debugging this issue is to update the zkevm-chain to use the latest release of halo2_proofs so that it can run with the current main of zkevm-circuits.

On a second note, I think it would be great to add root circuit tests into the integration tests from the zkevm-circuits, so that we have easy to run integration tests that include the root circuit (I think currently we don't have any)

ed255 added a commit that referenced this issue Jun 20, 2023
### Description

Introduce two new sets of tests in the Integration tests with the root
circuit: one where the root circuit is verified with the MockProver and
the other where a real proof is generated. The input proof to the root
circuit is the output of each subcircuit test we had before.

### Issue Link

Resolve
#1334
Resolve
#1460

### Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)

### Contents

Issue
#1335
has been resolved by assigning the fixed columns in the ExpTable and
CopyTable in their `load` methods. Previously they were either not
assigned, or assigned dynamically. This was not affecting the
SuperCircuit or the Exp/Copy Circuit because in those cases, the
assignment is done with a different method. Only for SubCircuit tests
where the SubCircuit requires the external table the `load` method is
used.

### Rationale

Testing a Root Circuit (no mater if it's with the MockProver or with a
real proof) requires a real proof of the underlying circuit. This means
that now for each SubCircuit we have 3 tests that require the SubCircuit
proof:
- normal real prover test
- root mock prover test
- root real prover test

I implemented a cache to keep the SubCircuit proofs for each SubCircuit
and block so that we have the possibility of running all the tests in a
single run while reusing the SubCircuit proofs.

### How Has This Been Tested?

I have only tested the SubCircuits (minus SuperCircuit) + Root Circuit
via MockProver.
- The machine I use has 64 GiB of memory, and that's not enough to
generate a SuperCircuit proof
- I have as a pending task to try the Root Circuit real prover

But now we can run these tests under the github CI, so we can defer full
testing after the PR is merged.
cameron1024 pushed a commit to cameron1024/zkevm-circuits that referenced this issue Jun 30, 2024
…ions#1335)

* fix degree pow of rand table in agg circuit

* change some log level from info to debug
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-bug Type: bug
Projects
Status: 🆕 Product Backlog Items
Development

No branches or pull requests

3 participants