feat/#1665 Precompile ECRECOVER #1720

KimiWu123 · 2023-12-25T09:37:46Z

This PR was ported from

which manly includes,

the main logic in ecrecover gadget (ecrecover.rs)
signature verifcation circuiit (sig_circuit.rs). What I did is to change rlc to word lo/hi.
a new table, sig_table.rs
ecc circuit (ecc_circuit.rs). It's not be used in ecRecover, but it was implemented in Scroll's PR. If I removed ecc_circuit, it would be inconvienent for people porting ecAdd, ecMul or ecPairing. That's why I keep it here.
dependencies update, using halo2lib (includes halo2-base and halo2-ecc)

* feat: preliminary work * wip * finish assignment to ecrecover * fix: input length may be different than call data length (for precompiles) * fix: input bytes to ecrecover * minor edits * Fix ecrecover input rlc comparison (right-padding zeroes) (#585) * potential approach (pow of rand lookup) * add lookup for pow of rand * fix: right pad only if needed * fix: missing constraint on padded_rlc * constrain pow of rand table * Update step.rs * fix: sig_v sanity check (remove assertions to allow garbage input) * fix: calldata length == 0 handled * chore: renaming precompile_* and parallel iter for tests --------- Co-authored-by: Zhang Zhuo <[email protected]> Precompile ECRECOVER (#529) * feat: preliminary work * wip * finish assignment to ecrecover * fix: input length may be different than call data length (for precompiles) * fix: input bytes to ecrecover * minor edits * Fix ecrecover input rlc comparison (right-padding zeroes) (#585) * potential approach (pow of rand lookup) * add lookup for pow of rand * fix: right pad only if needed * fix: missing constraint on padded_rlc * constrain pow of rand table * Update step.rs * fix: sig_v sanity check (remove assertions to allow garbage input) * fix: calldata length == 0 handled * chore: renaming precompile_* and parallel iter for tests --------- Co-authored-by: Zhang Zhuo <[email protected]>

* wip: preliminary work sig circuit/lookp * feat: handle panics and edge cases in sig circuit * fix: handle v not boolean * fix: fq modulus not assigned and bus-mapping handle overflowing r/s * fix: sig_v handle invalid values (lookup) and is_valid dev_load fix * refactor (review comment) * fix: no sig table lookup for invalid sig_v * remove wrong test case (assertion fail)

…elegate_call yet

This reverts commit 2e2625c.

miha-stopar

Added some nitpicks and questions. Looks great to me, but I have yet to read ECC circuit more carefully.

miha-stopar · 2024-01-31T10:27:54Z

zkevm-circuits/src/sig_circuit.rs

+        // - limb_bits: 88
+        // - num_limbs: 3
+        //
+        // TODO: make those parameters tunable from a config file


Is this to be done in a new PR?

fixed in aed502b

miha-stopar · 2024-02-02T10:18:17Z

zkevm-circuits/src/sig_circuit.rs

+            },
+        )?;
+
+        layouter.assign_region(


There are some TODOs in the Scroll version, like // TODO: is this correct? before assign_region function. I guess it's a leftover?

Yep, I believe so. So, I removed it.

miha-stopar · 2024-02-02T10:21:17Z

zkevm-circuits/src/ecc_circuit.rs

+        powers_of_256: &[QuantumCell<F>],
+        op: &EcAddOp,
+    ) -> EcAddDecomposed<F> {
+        log::trace!("[ECC] ==> EcAdd Assignmnet START:");


Nitpick: Assignmnet -> Assignment

miha-stopar · 2024-02-02T10:21:56Z

zkevm-circuits/src/ecc_circuit.rs

+
+        ecc_chip.assert_equal(ctx, &rand_point, &sum3);
+
+        log::trace!("[ECC] EcAdd Assignmnet END:");


Nitpick: Assignment

Nice catch! fixed in 4d5070f

miha-stopar · 2024-02-02T10:22:29Z

zkevm-circuits/src/ecc_circuit.rs

+        powers_of_256: &[QuantumCell<F>],
+        op: &EcMulOp,
+    ) -> EcMulDecomposed<F> {
+        log::trace!("[ECC] ==> EcMul Assignmnet START:");


Nitpick: Assignment

fixed in 4d5070f

miha-stopar · 2024-02-02T10:22:58Z

zkevm-circuits/src/ecc_circuit.rs

+        powers_of_256: &[QuantumCell<F>],
+        op: &EcPairingOp,
+    ) -> EcPairingDecomposed<F> {
+        log::trace!("[ECC] ==> EcPairing Assignment START:");


Nitpick: same as above

fixed in 4d5070f

miha-stopar

It seems all good to me. I would need some more time to check every detail of the ecc_circuit, but there are tests covering all edge cases and it was peer reviewed by Scroll people, so I think it should be all right.

ChihChengLiang

First pass.

bus-mapping/src/circuit_input_builder/input_state_ref.rs

zkevm-circuits/src/lib.rs

ChihChengLiang

LGTM.

github-actions bot added crate-bus-mapping Issues related to the bus-mapping workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member crate-eth-types Issues related to the eth-types workspace member labels Dec 25, 2023

KimiWu123 force-pushed the feat/#1665-ecrecover branch 3 times, most recently from 4b80960 to 99d2b1c Compare December 28, 2023 04:41

KimiWu123 force-pushed the feat/#1665-ecrecover branch from de1790e to 01b82f7 Compare January 4, 2024 07:54

KimiWu123 marked this pull request as ready for review January 8, 2024 03:06

KimiWu123 marked this pull request as draft January 8, 2024 03:40

github-actions bot added crate-mock Issues related to the mock workspace member crate-integration-tests Issues related to the integration-tests workspace member labels Jan 10, 2024

KimiWu123 force-pushed the feat/#1665-ecrecover branch from 9f248ba to 0e7d8a1 Compare January 11, 2024 02:29

KimiWu123 force-pushed the feat/#1665-ecrecover branch from ca8eb40 to ecbc924 Compare January 26, 2024 02:33

roynalnaruto and others added 14 commits January 26, 2024 11:35

fix: resolve build errors due to cherry-pick

c0d6123

feat: compiling error free

b76fd2f

test: pass first basic testing

2e72daf

test: pass all testing for call, not for static_call, call_code and d…

5851481

…elegate_call yet

test: pass all the normal testing

831556d

feat: support ecrecover oog case

ce4d6a6

feat: upload missing ecc circult realted files

187768e

feat: commit missing sig_circuit

ea52925

dep: add halo2-base and halo2-ecc as dependencies

21d2d19

fix: build error due to importing halo2_ecc and halo2_base

b88a8af

feat: using sign_verify for sig_circuit, compilation error free

2e2625c

fix error due to rebase

d0d3c4a

KimiWu123 force-pushed the feat/#1665-ecrecover branch from ecbc924 to d0d3c4a Compare January 26, 2024 03:02

github-actions bot removed the crate-mock Issues related to the mock workspace member label Jan 26, 2024

Revert "feat: using sign_verify for sig_circuit, compilation error free"

772ae18

This reverts commit 2e2625c.

KimiWu123 force-pushed the feat/#1665-ecrecover branch 4 times, most recently from afd1526 to be813d0 Compare January 29, 2024 11:39

ci: set fixed number of threads for light unit test to avoid OOM

fc1065c

KimiWu123 force-pushed the feat/#1665-ecrecover branch from be813d0 to fc1065c Compare January 29, 2024 11:58

chore: some leftovers

4f09a7d

KimiWu123 force-pushed the feat/#1665-ecrecover branch from 93ecb19 to 4f09a7d Compare January 30, 2024 01:15

test: split out a time-consuming test to a separated one

359ad7e

KimiWu123 force-pushed the feat/#1665-ecrecover branch from bbdf449 to 359ad7e Compare January 30, 2024 01:54

KimiWu123 changed the title ~~[WIP] feat/#1665 Precompile ECRECOVER~~ feat/#1665 Precompile ECRECOVER Jan 30, 2024

miha-stopar self-requested a review January 30, 2024 08:56

miha-stopar reviewed Feb 2, 2024

View reviewed changes

KimiWu123 added 2 commits February 5, 2024 10:14

chore: remove 'onephase' config and declare two constants for limbs

aed502b

Merge branch 'main' into feat/#1665-ecrecover

7af89c8

KimiWu123 force-pushed the feat/#1665-ecrecover branch from e30ce15 to 7af89c8 Compare February 5, 2024 03:44

chore: fix typo as review feedback

4d5070f

miha-stopar approved these changes Feb 6, 2024

View reviewed changes

ChihChengLiang self-requested a review February 7, 2024 09:07

ChihChengLiang suggested changes Feb 16, 2024

View reviewed changes

bus-mapping/src/circuit_input_builder/input_state_ref.rs Show resolved Hide resolved

zkevm-circuits/src/lib.rs Show resolved Hide resolved

Merge remote-tracking branch 'origin/main' into feat/#1665-ecrecover

d6bc6c1

KimiWu123 force-pushed the feat/#1665-ecrecover branch from 47e9d16 to d6bc6c1 Compare February 17, 2024 02:52

refactor to WordLoHi

a9858ef

KimiWu123 force-pushed the feat/#1665-ecrecover branch from 375181b to a9858ef Compare February 17, 2024 03:58

ChihChengLiang self-requested a review February 19, 2024 06:56

ChihChengLiang approved these changes Feb 19, 2024

View reviewed changes

KimiWu123 added this pull request to the merge queue Feb 19, 2024

Merged via the queue into main with commit e19d163 Feb 19, 2024
15 checks passed

KimiWu123 deleted the feat/#1665-ecrecover branch February 19, 2024 10:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat/#1665 Precompile ECRECOVER #1720

feat/#1665 Precompile ECRECOVER #1720

KimiWu123 commented Dec 25, 2023 •

edited

Loading

miha-stopar left a comment

miha-stopar Jan 31, 2024

KimiWu123 Feb 5, 2024

miha-stopar Feb 2, 2024

KimiWu123 Feb 5, 2024

miha-stopar Feb 2, 2024

miha-stopar Feb 2, 2024

KimiWu123 Feb 5, 2024

miha-stopar Feb 2, 2024

KimiWu123 Feb 5, 2024

miha-stopar Feb 2, 2024

KimiWu123 Feb 5, 2024

miha-stopar left a comment

ChihChengLiang left a comment

ChihChengLiang left a comment


		ecc_chip.assert_equal(ctx, &rand_point, &sum3);

		log::trace!("[ECC] EcAdd Assignmnet END:");

feat/#1665 Precompile ECRECOVER #1720

feat/#1665 Precompile ECRECOVER #1720

Conversation

KimiWu123 commented Dec 25, 2023 • edited Loading

miha-stopar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miha-stopar left a comment

Choose a reason for hiding this comment

ChihChengLiang left a comment

Choose a reason for hiding this comment

ChihChengLiang left a comment

Choose a reason for hiding this comment

KimiWu123 commented Dec 25, 2023 •

edited

Loading