Feat/challenge evm circuit by adria0 · Pull Request #1020 · privacy-ethereum/zkevm-circuits

adria0 · 2023-01-02T16:56:24Z

The original PR was #990 but after renaming the branch to feat/challenge-evm-circuit github automatically closed it.

…circuits into tmp/challenge-evm-circuit

han0110

First round pass. Overall looks good to me! Only left some small questions.

han0110 · 2023-01-09T12:25:20Z

+    #[test]
+    fn callop_base() {
+        test_ok(
+            caller(&OpcodeId::CALL, Stack::default(), true),
+            callee(bytecode! {}),
+        );
+        // use crate::evm_circuit::util::print_op_count;
+        // print_op_count();
+    }
+


oops! fixed in 8aa678d

It seems to be still here 🤣

han0110 · 2023-01-09T14:31:33Z

+    pub(crate) fn storage_for_inv<F: FieldExt>(value: &Expression<F>) -> CellType {
+        match Self::expr_phase(value) {
+            0 => CellType::StoragePhase1,
+            1 => CellType::StoragePhase3,


I can't think about the reason to require phase 3 here? Would you like to explain it more?

Yes, the idea is is that which is the "storage requiered to store the inverse of an expression", and this function is really only used in IsZeroGadget.

IIRC, we need an extra phase when computing the inverse of an expression (this is why is_zero in the IsZeroGadget) when it was not in the first phase.

hmm but the expr_phase returns max_phase_challenge_usable_after, which seems to be "this expression will be evaluateable after such phase", so the inverse should also be in max_phase_challenge_usable_after + 1, right?

For example, an expression advice_at_phase_1 + challenge_usable_after_phase_1, we can store its inverse in phase 2.

Done in 8bfda72

hmm sorry I think I'm confused about this function now, so what's the difference between storage_for and storage_for_inv? Inverse of a value should be able to live in the same phase as the value, right?

The idea was:

storage_for -> storage for an expression

storage_for_inv -> storage for the inverse of an expression

I understand, my question is more like, what makes inverse storage need one extra phase to be stored? For example advice_at_phase_1 + challenge_usable_after_phase_1 we should be able to store its inverse in phase 2 right? Because this expression is evaluateable in phase 2.

Changed in 61710e8

ChihChengLiang

First pass

…circuits into feat/challenge-evm-circuit

…ng-explorations/zkevm-circuits into feat/challenge-evm-circuit

…circuits into feat/challenge-evm-circuit

han0110

Finished my second review, only left some nitpicks. Also I think now it might be a good time to try to run integration test for this PR before being merged.

han0110 · 2023-01-11T13:59:25Z

+    #[test]
+    fn callop_base() {
+        test_ok(
+            caller(&OpcodeId::CALL, Stack::default(), true),
+            callee(bytecode! {}),
+        );
+        // use crate::evm_circuit::util::print_op_count;
+        // print_op_count();
+    }
+


It seems to be still here 🤣

han0110

Nice work!

ChihChengLiang

LGTM. I added some nitpicks.

…circuits into feat/challenge-evm-circuit

adria0 added 7 commits December 13, 2022 12:48

Challange in EVM circuit WIP

6d1b258

Fixed copy_circuit_valid_sha3

f6da00f

Remove leftovers

eaafdf9

lookup_input wip

8120bd4

fix _dummy_cols, cleanup, fmt

74b9b8b

revert fast id, split_expression phase fix

c9dd231

Merge branch 'main' of github.com:privacy-scaling-explorations/zkevm-…

c390abf

…circuits into tmp/challenge-evm-circuit

adria0 linked an issue Jan 2, 2023 that may be closed by this pull request

Refactor EVM circuit with challenge API #750

Closed

adria0 mentioned this pull request Jan 2, 2023

Improve memory allocation when generating expression identifier privacy-ethereum/halo2#115

Merged

adria0 added 2 commits January 3, 2023 10:49

limit 12 pows in lookup_powers_of_randomness

d41b85c

EVM lookup columns in phase3

5ba2cf1

adria0 force-pushed the feat/challenge-evm-circuit branch from b5ee253 to 5ba2cf1 Compare January 3, 2023 13:20

han0110 self-requested a review January 4, 2023 15:14

andyguzmaneth requested a review from ChihChengLiang January 5, 2023 12:24

Extract Value contents to order instead ordering by Value

f23089a

han0110 reviewed Jan 9, 2023

View reviewed changes

adria0 added 3 commits January 9, 2023 16:14

Removed leftovers & misspells

8aa678d

Move Value::transpose from halo2 to this crate

3ded707

Fix phases in is_zero gagets

8bfda72

ChihChengLiang reviewed Jan 10, 2023

View reviewed changes

Comment thread Cargo.toml Outdated

github-actions Bot removed crate-bus-mapping Issues related to the bus-mapping workspace member crate-eth-types Issues related to the eth-types workspace member crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member labels Jan 10, 2023

github-actions Bot removed crate-gadgets Issues related to the gadgets workspace member T-bench Type: benchmark improvements labels Jan 10, 2023

adria0 force-pushed the feat/challenge-evm-circuit branch from e69e704 to 75149e6 Compare January 10, 2023 09:38

Revert: use halo2 from git

c755ee2

adria0 force-pushed the feat/challenge-evm-circuit branch from 75149e6 to c755ee2 Compare January 10, 2023 09:39

adria0 and others added 6 commits January 10, 2023 11:56

Merge branch 'main' of github.com:privacy-scaling-explorations/zkevm-…

df71e45

…circuits into feat/challenge-evm-circuit

Merge branch 'main' into feat/challenge-evm-circuit

cc01aab

Fix phase computation for inv

61710e8

fmt

248969b

Merge branch 'feat/challenge-evm-circuit' of github.com:privacy-scali…

55680a5

…ng-explorations/zkevm-circuits into feat/challenge-evm-circuit

Merge branch 'main' of github.com:privacy-scaling-explorations/zkevm-…

713107e

…circuits into feat/challenge-evm-circuit

han0110 reviewed Jan 11, 2023

View reviewed changes

@han0110 nitpicks

f4ab554

han0110 reviewed Jan 11, 2023

View reviewed changes

Comment thread zkevm-circuits/src/witness/block.rs Outdated

Re-add comment

693e4d5

han0110 approved these changes Jan 12, 2023

View reviewed changes

ChihChengLiang approved these changes Jan 12, 2023

View reviewed changes

andyguzmaneth added run-integration-tests trigger-integration-tests and removed run-integration-tests labels Jan 12, 2023

adria0 added 2 commits January 12, 2023 13:34

Add empty_hash_rlc, cleanup comments

93dc400

Merge branch 'main' of github.com:privacy-scaling-explorations/zkevm-…

92e9cd8

…circuits into feat/challenge-evm-circuit

adria0 merged commit 6d12683 into main Jan 12, 2023

KimiWu123 mentioned this pull request Jan 13, 2023

Refactor/callop call oog #1023

Merged

SuccinctPaul pushed a commit to SuccinctPaul/zkevm-circuits that referenced this pull request Jan 13, 2023

feat: challenge API in evm circuit (privacy-ethereum#1020)

58900d9

lispc mentioned this pull request Jan 16, 2023

Write extcodecopy circuit #720

Merged

adria0 deleted the feat/challenge-evm-circuit branch January 23, 2023 09:52

silathdiir mentioned this pull request Apr 27, 2023

refactor: add challenge randomness scroll-tech/mpt-circuit#36

Closed

noel2004 pushed a commit to noel2004/zkevm-circuits that referenced this pull request Nov 29, 2023

Refactor error-invalid-jump in bus-mapping. (privacy-ethereum#1020)

2470629

Conversation

adria0 commented Jan 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

han0110 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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

han0110 Jan 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ChihChengLiang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

han0110 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

han0110 left a comment

Choose a reason for hiding this comment

Uh oh!

ChihChengLiang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adria0 commented Jan 2, 2023 •

edited

Loading

han0110 Jan 9, 2023 •

edited

Loading