Feat/hybrid snark agg #1438
Feat/hybrid snark agg #1438
Conversation
* pin rust to 1.77 * pick changes from v0.13 * update l2geth and ci * lint * disable "default" ci
|
some high level comments: in aggregator crate: in prover crate: |
| @@ -37,7 +41,8 @@ impl<'params> Prover<'params> { | |||
| env::set_var("KECCAK_ROWS", BATCH_KECCAK_ROW.to_string()); | |||
|
|
|||
| let prover_impl = common::Prover::from_params_map(params_map); | |||
| let chunk_protocol = force_to_read(assets_dir, &CHUNK_PROTOCOL_FILENAME); | |||
| let halo2_protocol = force_to_read(assets_dir, &FD_HALO2_CHUNK_PROTOCOL); | |||
| let sp1_protocol = force_to_read(assets_dir, &FD_SP1_CHUNK_PROTOCOL); | |||
There was a problem hiding this comment.
so we usually generate this file inside sp1-halo2-wrapper repo, and copy it here?
There was a problem hiding this comment.
Yes, we load the SNARK protocol from the halo2-backend for sp1. Since that won't change post-e2e tests, this will also be published as a part of the assets for that release
| ); | ||
| config | ||
| .flex_gate() | ||
| .assert_is_const(&mut ctx, &preprocessed_check, Fr::ONE); |
There was a problem hiding this comment.
this may be a bit not very sound. Not each poly be either sp1 or halo2, but, all poly, either all be sp1 or all be halo2. I feel your constaints will allow poly[0] == sp1_poly[0] while poly[1] == halo2_poly[1]. It is not sound.
Though in real case, this unsoundness may not affect (not sure). better fix it later. (not high priority)
ok i see. Here the design is, you verify sp1 proof & protocol, you verify halo2 proof & protocol, and you assert one of them to be true. this design is ok. But not efficient. We should optimize it later. (Actually using this design do you really need loaded_preprocessed_as_witness?) the optimized design is, we could make sp1 chunk protocol and halo2 chunk protocol, the circuits shapes be same (only fixed_columns/vk be different). So the Plonk::<PCS>::succinct_verify is called once, not twice. And assert protocol_digest outside.
There was a problem hiding this comment.
oh, I thought chunks in a batch could be either sp1 or halo2. Isn't that true?
There was a problem hiding this comment.
Do you mean, a batch is all sp1 or all halo2?
Description
[PR description]
Issue Link
[link issue here]
Type of change
Contents
Rationale
[design decisions and extended information]
How Has This Been Tested?
[explanation]
How to fill a PR description
Please give a concise description of your PR.
The target readers could be future developers, reviewers, and auditors. By reading your description, they should easily understand the changes proposed in this pull request.
MUST: Reference the issue to resolve
Single responsibility
Is RECOMMENDED to create single responsibility commits, but not mandatory.
Anyway, you MUST enumerate the changes in a unitary way, e.g.
Design choices
RECOMMENDED to: