Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

fix evm circuit phase2 cells #1087

Merged
merged 5 commits into from
Jan 23, 2023
Merged

fix evm circuit phase2 cells #1087

merged 5 commits into from
Jan 23, 2023

Conversation

lispc
Copy link
Collaborator

@lispc lispc commented Jan 19, 2023

fix #1086

(these types of bugs can be detected with this modified MockProver scroll-tech/halo2@e409a69 )

@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Jan 19, 2023
Copy link
Collaborator

@han0110 han0110 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for fixing this.

Also thanks for the reference of MockProver fix, I'll try to resolve the inconsistent behavior between MockProver and real prover to avoid this kind of mistake to happen.

Copy link
Member

@adria0 adria0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -75,6 +75,10 @@ impl<F: Field> MemoryAddressGadget<F> {
memory_offset: Cell<F>,
memory_length: MemoryAddress<F>,
) -> Self {
debug_assert_eq!(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you add cell_column_index only for this check.
Could you add a comment about this assertion?

@adria0 adria0 merged commit 37c9d93 into main Jan 23, 2023
@lispc lispc deleted the fix-evm-phase branch January 27, 2023 04:06
SuccinctPaul pushed a commit to SuccinctPaul/zkevm-circuits that referenced this pull request Feb 15, 2024
…cy-scaling-explorations#1087)

* reproduce phase issues in sha256_circuit

* fix issues

* fmt and clippy

* fix gate issue

---------

Co-authored-by: kunxian xia <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-zkevm-circuits Issues related to the zkevm-circuits workspace member run-integration-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

evm: a lot of opcode gadgets need to be updated with CellType::StoragePhase2
4 participants