Skip to content
This repository was archived by the owner on Jul 24, 2023. It is now read-only.

Sync halo ecc 1013#35

Open
zhenfeizhang wants to merge 29 commits into
scroll-dev-0920from
sync-halo-ecc-1013
Open

Sync halo ecc 1013#35
zhenfeizhang wants to merge 29 commits into
scroll-dev-0920from
sync-halo-ecc-1013

Conversation

@zhenfeizhang
Copy link
Copy Markdown
Contributor

No description provided.

jonathanpwang and others added 25 commits September 15, 2022 21:25
tests for single proof verify in chip for lookup, mul_add pass
…o2_proofs, so they can be uniformly patched at root level
constants during keygen_vk

now keygen_vk actually works for tests/mul_add_{single, aggregate}
Made changes because vkey write doesn't currently work.
now passes zkevm_bench with MockProver

this resolves an issues where the final_pair computation in
verify_circuit was incorrect because halo2_ecc did not detect (0,0)
elliptic curve points, which should be treated as identity in the group
circuit and 1 state circuit

* switched zkevm and halo2 repos to scroll-dev to get state circuit to
  prove correctly
@zhenfeizhang zhenfeizhang marked this pull request as ready for review October 17, 2022 15:22
@zhenfeizhang zhenfeizhang mentioned this pull request Oct 17, 2022
@zhenfeizhang zhenfeizhang reopened this Oct 17, 2022
silathdiir and others added 2 commits October 19, 2022 10:26
* Update Github CI with `secrets.SECRET_REPO_DEPLOY_KEY`.

* remove repo token

* update cargo lock

* switch rust to nightly

Co-authored-by: zhenfei <zhenfei.zhang@hotmail.com>
* use embedded template for solidity

* fix tera escape

* check pairing in soli gen

* add pairing check

* more log

* fix halo2

* log final pair check in instance

* fix all...

* ..
@lispc lispc changed the base branch from scroll-dev-0920 to main October 25, 2022 03:58
@lispc lispc changed the base branch from main to scroll-dev-0920 October 25, 2022 03:58
@lispc
Copy link
Copy Markdown
Contributor

lispc commented Oct 25, 2022

the diffs here is a bit strange... for example most changes in halo2-snark-aggregator-solidity/src/lib.rs seems already in main. the strange history is difficult for reviewing. We can continue development using this branch but before final review & merge we should refactor the git history

@lispc
Copy link
Copy Markdown
Contributor

lispc commented Oct 25, 2022

other case is in halo2-snark-aggregator-solidity/src/lib.rs, 63 becomes 33 again in poseidon param..

Also in halo2-snark-aggregator-circuit/src/verify_circuit.rs

@lispc
Copy link
Copy Markdown
Contributor

lispc commented Oct 25, 2022

btw later the deleted function print_points_profiling should be recovered, it is useful

@lispc
Copy link
Copy Markdown
Contributor

lispc commented Oct 26, 2022

seems fixed.

TODOs before merge of this PR

  1. clean println
  2. poseidon parameter is inconsistent. 63 <-> 33
  3. embed "verify_circuit.config" using "include_str" marco
  4. as a lib, halo2-snark-aggregator better not directly write&read filesystem in get_params_cached
  5. some changes are incorrectly reverted/missed. in verify_circuit.rs, key: format!("{}_p{}", self.0[ci].name, i), reverted to key: format!("c{}p{}", ci, i), incorrectly.
  6. recover the deleted print_points_profiling function.

@zhenfeizhang
Copy link
Copy Markdown
Contributor Author

seems fixed.

TODOs before merge of this PR

1. clean println

2. poseidon parameter is inconsistent. 63 <-> 33

3. embed "verify_circuit.config" using "include_str" marco

4. as a lib, halo2-snark-aggregator better not directly write&read filesystem in get_params_cached

5. some changes are incorrectly reverted/missed. in [verify_circuit.rs](https://github.com/scroll-tech/halo2-snark-aggregator/pull/35/files#diff-9d3e9bc089e1acbaf6a290358cb32f7247fb281afd9ec5656ae29591f2a8c199), `key: format!("{}_p{}", self.0[ci].name, i),` reverted to `key: format!("c{}p{}", ci, i),` incorrectly.

6. recover the deleted `print_points_profiling` function.

fixed in c5ce8eb

Comment thread halo2-snark-aggregator-circuit/src/verify_circuit.rs Outdated
Comment thread halo2-snark-aggregator-api/src/tests/systems/halo2.rs Outdated
Comment thread halo2-snark-aggregator-api/src/tests/systems/halo2/add_mul_test/verify_single.rs Outdated
Comment thread halo2-snark-aggregator-circuit/src/tests/mul_add.rs Outdated
Comment thread halo2-snark-aggregator-circuit/src/tests/mul_add.rs Outdated
Comment thread halo2-snark-aggregator-api/src/systems/halo2/verify.rs
@lispc
Copy link
Copy Markdown
Contributor

lispc commented Oct 28, 2022

LGTM.

I will use this branch for some real tests using common-rs next week.

@JohnWick2ETH do you have time to have a look at this since it is a huge PR?

@kunxian-xia
Copy link
Copy Markdown

sure.

@zhenfeizhang
Copy link
Copy Markdown
Contributor Author

This PR is not to be merged.
Renaming this branch as halo2-ecc and close this PR.

@zhenfeizhang zhenfeizhang deleted the sync-halo-ecc-1013 branch October 29, 2022 01:03
@zhenfeizhang zhenfeizhang restored the sync-halo-ecc-1013 branch November 1, 2022 01:39
@zhenfeizhang zhenfeizhang reopened this Nov 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants