[WIP] Halo2 24-08-2022 tag update by CPerezz · Pull Request #724 · privacy-ethereum/zkevm-circuits

CPerezz · 2022-08-31T07:53:50Z

This PR does several things although all follow the same rationale.

Update halo2 version used across the workspace to: https://github.com/privacy-scaling-explorations/halo2/releases/tag/v2022_08_19
Update halo2wrong version used across the workspace to: https://github.com/privacy-scaling-explorations/halo2wrong/releases/tag/v2022_08_24
Update the ff, group, pairing and other deps to use the halo2curves re-exported ones.
Remove all the circuit-related things from keccak crate and port them to https://github.com/privacy-scaling-explorations/keccak_circuit in favour of the implementations added by Brecht on [keccak] Alternative implementations #634
Remove all unused deps across all crates (mainly found after passing cargo-udeps.
Use halo2_proofs::circuit::Value everywhere instead of || Option<F>.
EDIT:
Tried to follow the same approach as in cargo: patch halo2_proofs on crates.io #555 with the patch but Cargo is unable to resolve the dependency correctly.
So even was pointed out by @ChihChengLiang [WIP] Halo2 24-08-2022 tag update #724 there's nothing we can do unless we explore resolver 2 and other similar stuff which might be the cause of this.

Resolves: #702

As decided on #634 we're going to use the bit-approach merged there. And so, most of the circuits here are no longer required/ will be added back when needed.

The first attempt to update the `prover` crate was to basically use full generics everywhere. The issue is that we ended up hitting trait safety and the complexity started to increase significantly. Hence, since we are always using the same commitment scheme and curve, it's easier to switch to concrete types.

CPerezz · 2022-08-31T21:12:47Z

d2ed5bb should fix the issue of composition table is not set, bit lenght: 8. But it doesn't.

I was under the impression that the load of the RangeChip tables was the issue inside of the SignVerify and therefore Tx configs.
I'll need to look at this more carefully tomorrow. Maybe @han0110 has some hint on what can be causing the issue as he did the re-write of the halo2wrong main gate.

CPerezz · 2022-09-01T09:58:19Z

d2ed5bb should fix the issue of composition table is not set, bit lenght: 8. But it doesn't.

I was under the impression that the load of the RangeChip tables was the issue inside of the SignVerify and therefore Tx configs. I'll need to look at this more carefully tomorrow. Maybe @han0110 has some hint on what can be causing the issue as he did the re-write of the halo2wrong main gate.

Fixed in 7d753bb.
Now the last error is related to unsatisfied constraints in the signature verification.

Co-authored-by: Han <tinghan0110@gmail.com>

CPerezz · 2022-09-01T11:01:27Z

I highly appreciate all the details you found @han0110 .

I copy pasted the expect messages and did not realize that the types changed..
Also, the error was indeed in the Compressed usage. Thanks for working on it to help me close this!

ChihChengLiang

LGTM with some nitpicks. Great PR, this can unblock many other prs

ChihChengLiang · 2022-09-01T15:22:42Z


 [dependencies]
-halo2_proofs = { version = "0.1.0-beta.1" }
+halo2_proofs = { git = "https://github.com/privacy-scaling-explorations/halo2.git", tag = "v2022_08_19" }


Do we still need to change here if we already patched halo2_proofs in the [patch.crates-io] section of root Cargo.toml? I remember that's how we did in #555 right?

Tied here and Cargo is not able to resolve it..

This isn't so important but I got it to resolve correctly by also updating the version from "0.1.0-beta.1" to "0.2.0"

Wow Thanks! I've never seen this behaviour in Cargo. It should also work with 0.1.0-beta no?? LOL

Thanks anyway @pinkiebell . Filled #729 with it!

ChihChengLiang · 2022-09-01T15:51:48Z


 pub mod arith_helpers;
-pub mod circuit;
 pub mod common;


How come we still keep some parts of the keccak crate?

We need them to perform the hashes outside of the circuit and compute witnesses in the CircuitInputBuilder still :)

ChihChengLiang · 2022-09-01T15:59:53Z

                region,
                offset,
-                Some(if index == pair_index {
+                Value::known(if index == pair_index {


Do we now have an expressive way to turn bool to F?

Suggested change

Value::known(if index == pair_index {

Value::known(F::from(index == pair_index))

No that I know.. THe only is F::from(bool as u64)

ChihChengLiang · 2022-09-01T16:00:18Z

+        self.target_odd.assign(
+            region,
+            offset,
+            Value::known(if odd { F::one() } else { F::zero() }),


Suggested change

Value::known(if odd { F::one() } else { F::zero() }),

Value::known(F::from(odd)),

han0110

LGTM! Thanks for this great work!

@pinkiebell

In #724 the patch wasn't working with `0.1.0-beta` so it was left as TODO. But thanks to @pinkiebell who pointed out that it was required to bump the base dep in the multiple Cargo.tomls this was solved.

@han0110

* change: Update halo2 tag version in all Cargo.toml * change: Use `Value` instead of `AssignedCell`. * remove: Strip keccak circuits from the repo As decided on privacy-ethereum#634 we're going to use the bit-approach merged there. And so, most of the circuits here are no longer required/ will be added back when needed. * tmp * fix: RegionCtx new API adoption without &mut holding * change: Use KZG, SHPLONK and Bn256 instead of generics The first attempt to update the `prover` crate was to basically use full generics everywhere. The issue is that we ended up hitting trait safety and the complexity started to increase significantly. Hence, since we are always using the same commitment scheme and curve, it's easier to switch to concrete types. * remove: Delete prover crate as was migrated to zkevm-chain * remove: Remove legacy keccak bench file * remove: Remove `group` dep from all crates Cargo.toml * update: Port circuit-benchmarks to latest halo2 version * update: Port integration-tests to newest halo2 release * Fix errors in zkevm-circuits tests * fix: Errors introduced in `main` branch merge * fix: Load range_chip tables in SignVerigy and Tx Configs * Fix Synthesis and lookup table errors * fix: Apply suggestions from @han0110 review Co-authored-by: Han <tinghan0110@gmail.com> * Fix Signature errors introduced on Compressed treatment * fix: Fix tests and address review comments Co-authored-by: Han <tinghan0110@gmail.com>

* fix returndatacopy len * lint

CPerezz added 4 commits August 29, 2022 09:23

change: Update halo2 tag version in all Cargo.toml

cbe8eeb

change: Use Value instead of AssignedCell.

7c79eba

remove: Strip keccak circuits from the repo

1c8b47f

As decided on #634 we're going to use the bit-approach merged there. And so, most of the circuits here are no longer required/ will be added back when needed.

tmp

af1f43e

CPerezz added 3 commits August 31, 2022 09:58

fix: RegionCtx new API adoption without &mut holding

30abd92

remove: Delete prover crate as was migrated to zkevm-chain

eef1d1f

CPerezz force-pushed the halo2_24_08_2022_update branch from 7d101a7 to eef1d1f Compare August 31, 2022 12:02

remove: Remove legacy keccak bench file

b0f36d4

pinkiebell mentioned this pull request Aug 31, 2022

Implement halo2 verifier contract privacy-ethereum/zkevm-chain#18

Closed

CPerezz added 6 commits August 31, 2022 17:04

remove: Remove group dep from all crates Cargo.toml

15e8831

update: Port circuit-benchmarks to latest halo2 version

2bee754

update: Port integration-tests to newest halo2 release

9625019

Fix errors in zkevm-circuits tests

76c5f65

Merge branch 'main' into halo2_24_08_2022_update

9d504f9

fix: Errors introduced in main branch merge

b88480e

github-actions Bot removed the crate-prover Issues related to the prover workspace member label Aug 31, 2022

CPerezz marked this pull request as ready for review August 31, 2022 16:16

CPerezz requested review from ed255 and miha-stopar as code owners August 31, 2022 16:16

fix: Load range_chip tables in SignVerigy and Tx Configs

d2ed5bb

han0110 self-requested a review September 1, 2022 03:15

Fix Synthesis and lookup table errors

7d753bb

han0110 reviewed Sep 1, 2022

View reviewed changes

ChihChengLiang self-requested a review September 1, 2022 10:18

fix: Apply suggestions from @han0110 review

d30b3e6

Co-authored-by: Han <tinghan0110@gmail.com>

Fix Signature errors introduced on Compressed treatment

36d4884

CPerezz requested a review from han0110 September 1, 2022 11:19

ChihChengLiang approved these changes Sep 1, 2022

View reviewed changes

fix: Fix tests and address review comments

e4a724d

CPerezz force-pushed the halo2_24_08_2022_update branch from 217398b to 543fa54 Compare September 1, 2022 16:43

github-actions Bot removed the crate-bus-mapping Issues related to the bus-mapping workspace member label Sep 1, 2022

CPerezz force-pushed the halo2_24_08_2022_update branch from f46d44f to e4a724d Compare September 1, 2022 17:05

github-actions Bot added the crate-bus-mapping Issues related to the bus-mapping workspace member label Sep 1, 2022

Merge branch 'main' into halo2_24_08_2022_update

f039131

han0110 reviewed Sep 1, 2022

View reviewed changes

CPerezz merged commit 2783eba into main Sep 1, 2022

CPerezz deleted the halo2_24_08_2022_update branch September 1, 2022 17:36

CPerezz mentioned this pull request Sep 1, 2022

Upgrade halo2 #610

Closed

2 tasks

CPerezz mentioned this pull request Sep 2, 2022

fix: Return to patch usage for halo2 deps #729

Closed

ed255 mentioned this pull request Jun 26, 2023

Use RangeInstructions::decompose in Tx Circuit for field to bytes decomposition #624

Closed

lispc added a commit that referenced this pull request Aug 28, 2023

fix returndatacopy len (#724)

76989b3

* fix returndatacopy len * lint

	Value::known(if index == pair_index {
	Value::known(F::from(index == pair_index))

	Value::known(if odd { F::one() } else { F::zero() }),
	Value::known(F::from(odd)),

Conversation

CPerezz commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CPerezz commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CPerezz commented Sep 1, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CPerezz commented Sep 1, 2022

Uh oh!

ChihChengLiang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

han0110 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CPerezz commented Aug 31, 2022 •

edited

Loading

CPerezz commented Aug 31, 2022 •

edited

Loading