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

Circuit for ErrorInvalidOpcode state#1089

Merged
han0110 merged 13 commits into
privacy-ethereum:mainfrom
scroll-tech:execution/error-invalid-opcode
Feb 10, 2023
Merged

Circuit for ErrorInvalidOpcode state#1089
han0110 merged 13 commits into
privacy-ethereum:mainfrom
scroll-tech:execution/error-invalid-opcode

Conversation

@silathdiir
Copy link
Copy Markdown
Contributor

@silathdiir silathdiir commented Jan 19, 2023

Close #1138

Spec PR privacy-ethereum/zkevm-specs#362 has been merged.

Summary

  1. Add ErrorInvalidOpcodeGadget to handle invalid opcode error.

  2. Add invalid_opcodes function for returning the all invalid opcodes in OpcodeId struct.

  3. Fix to handle ErrorInvalidOpcode to return the all invalid opcodes in responsible_opcodes function. It should add the all invalid opcodes (with ErrorInvalidOpcode error state) into FixedTableTag::ResponsibleOpcode via this code. So the invalid opcode could be lookuped.

@github-actions 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 labels Jan 19, 2023
@silathdiir silathdiir marked this pull request as draft January 19, 2023 07:22
@silathdiir silathdiir marked this pull request as ready for review January 22, 2023 11:22
@silathdiir silathdiir marked this pull request as draft January 22, 2023 11:23
@silathdiir silathdiir force-pushed the execution/error-invalid-opcode branch from ae884cd to 221fc82 Compare January 24, 2023 11:29
@DreamWuGit DreamWuGit self-requested a review January 30, 2023 03:29
@github-actions github-actions Bot added the crate-eth-types Issues related to the eth-types workspace member label Feb 3, 2023
@silathdiir silathdiir force-pushed the execution/error-invalid-opcode branch from 627edcd to ebe7527 Compare February 3, 2023 08:20
@silathdiir silathdiir force-pushed the execution/error-invalid-opcode branch from ebe7527 to 491ce31 Compare February 6, 2023 11:13
@silathdiir silathdiir marked this pull request as ready for review February 6, 2023 11:17

// When it is an internal call, need to restore caller's state as finishing this
// call. Restore caller state to next StepState.
let restore_context = cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we prefer "not::expr" compared to "1.expr() - ". same thing, but more readable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replaced 1.expr() - with not::expr in 93ce388.

Copy link
Copy Markdown
Collaborator

@lispc lispc left a comment

Choose a reason for hiding this comment

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

i noticed you have created a unified "constrain_error_state" in spec side. So i think after this PR, we can create another PR do the similar thing in rust side?

@silathdiir
Copy link
Copy Markdown
Contributor Author

silathdiir commented Feb 6, 2023

i noticed you have created a unified "constrain_error_state" in spec side. So i think after this PR, we can create another PR do the similar thing in rust side?

Yes. Will discuss with @DreamWuGit if we could work on the issue of common error gadget #1059.

Discussed with @DreamWuGit , it will be refactored to common error gadget in another PR.

Comment thread zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs
@silathdiir silathdiir requested a review from DreamWuGit February 9, 2023 07:37
Comment thread bus-mapping/src/evm/opcodes.rs Outdated
Comment thread bus-mapping/src/evm/opcodes/error_invalid_opcode.rs Outdated
Comment thread zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs
Comment thread zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs Outdated
…iated_ops`. And Move `fn_gen_associated_ops` to the end of `gen_associated_ops` (after `get_step_err`).
@silathdiir silathdiir requested review from han0110 and removed request for DreamWuGit February 9, 2023 12:19
Copy link
Copy Markdown
Collaborator

@DreamWuGit DreamWuGit left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@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.

All LGTM now! But still need a rebase.

@han0110 han0110 merged commit b5cd9f1 into privacy-ethereum:main Feb 10, 2023
@silathdiir silathdiir deleted the execution/error-invalid-opcode branch February 10, 2023 01:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

crate-bus-mapping Issues related to the bus-mapping workspace member crate-eth-types Issues related to the eth-types workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Circuit for ErrorInvalidOpcode state

4 participants