Implement RootCircuit for SuperCircuit aggregation#1000
Conversation
github-actions
Bot
added
crate-bus-mapping
|
@lispc can you help us get another reviewer for this task. Thanks! |
han0110
force-pushed
the
feature/root-circuit
branch
from
eb76f8f to
b0fd687
Compare
|
Does the evm-verifier-with-accumulator in that snark-verifier repo work for this root circuit?? Thanks! |
CPerezz
left a comment
CPerezz
left a comment
There was a problem hiding this comment.
This work is awesome @han0110
It's quite clear what you're doing all the time. I really like it!
I left some questions and minor nits.
Also, are there any specs for the Root circuit somewhere? As I think we should have some. And also, to have them related to the code we see here.
Again, awesome work mate!
| use itertools::Itertools; | ||
| use rand::rngs::OsRng; | ||
|
|
||
| #[ignore = "Due to high memory requirement"] |
There was a problem hiding this comment.
Would this be a candidate to run in the serial tests section of our testsuite? Or even with this the machine would OOM?
There was a problem hiding this comment.
Since it's trying to create a SuperCircuit proof with k >= 18, so I believe it requires a tons of memory to run, so I didn't bother to try enable this to test the CI machine.
@smtmfft Yes I think so. |
@smtmfft Sorry that I haven't added too much document yet, but I'm planning to do that soon, the paper I followed is this one https://eprint.iacr.org/2020/499. Simply speaking it's trying to verify it without doing pairing, and defer the final pairing input to the evm verifier. |
|
Note: Han is working on documentation tasks first that will help to review this one. |
|
Note: after the document is finished, this review can continue. privacy-ethereum/snark-verifier#22 |
han0110
force-pushed
the
feature/root-circuit
branch
from
b0fd687 to
1435a30
Compare
github-actions
Bot
removed
crate-integration-tests
ChihChengLiang
left a comment
ChihChengLiang
left a comment
There was a problem hiding this comment.
Amazing work! LGTM. I added a comment.
| let a = region.assign_advice(|| "", w_l, 0, || Value::known(self.0))?; | ||
| region.assign_fixed(|| "", q_l, 0, || Value::known(-F::one()))?; | ||
| a.copy_advice(|| "", &mut region, w_r, 1)?; | ||
| a.copy_advice(|| "", &mut region, w_o, 2)?; | ||
| region.assign_advice(|| "", w_l, 3, || Value::known(-F::from(5)))?; | ||
| for (column, idx) in [q_l, q_r, q_o, q_m, q_c].iter().zip(1..) { | ||
| region.assign_fixed(|| "", *column, 3, || Value::known(F::from(idx)))?; | ||
| } |
There was a problem hiding this comment.
I don't follow the cell assignments here. Can we add a comment on why we are assigning this way?
There was a problem hiding this comment.
It's because currently there is some problem for halo2wrong if there is identity point in preprocessed or proof (kind of forget to investigate for a long time), so here it's trying to make sure the fixed columns are not identity by assigning some random values.
I have added some comments and error handling in 54935c5, and will try to dig into that issue.
…roof, and comments why we need to assign some random values in testing circuit `StandardPlonk`
han0110
force-pushed
the
feature/root-circuit
branch
from
c0ba028 to
54935c5
Compare
* create_op: fix memory size (privacy-ethereum#990) * create: refactor lifetime * fmt * add more tests * update access list in create * refactor create * build * fix * try * ci
6 participants
This PR aims to implement
RootCircuitforSuperCircuitaggregation, to replace current one inzkevm-chainrepo.It uses the latest
snark-verifier(previouslyplonk-verifier) so it requires to upgrade thehalo2andhalo2wrongtag, tho it doesn't require any change (it's just that we could use stable channel rust now butzkevm-circuitsitself still requires nightly).Resolves #664.