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

Implementation ExecutionState::STOP#419

Merged
han0110 merged 3 commits into
privacy-ethereum:mainfrom
han0110:feature/stop
Jul 4, 2022
Merged

Implementation ExecutionState::STOP#419
han0110 merged 3 commits into
privacy-ethereum:mainfrom
han0110:feature/stop

Conversation

@han0110
Copy link
Copy Markdown
Contributor

@han0110 han0110 commented Apr 2, 2022

This PR aims to implement ExecutionState::STOP. For spec, please refer to privacy-ethereum/zkevm-specs#100.

It also resolves #464 by removing the incomplete trace testing used in COPYTOMEMORY and COPYCODETOMEMORY because they should be covered by CALLDATACOPY and CODECOPY.

TODO

Broken tests

Currently some of tests using hand-crafted testing witness (instead of using CircuitInputBuilder) are failed by this PR, those unfixed include:

Opcode fetching out of range

@github-actions github-actions Bot added crate-bus-mapping Issues related to the bus-mapping workspace member crate-keccak Issues related to the keccak workspace member T-opcode Type: opcode-related and focused PR/Issue crate-zkevm-circuits Issues related to the zkevm-circuits workspace member labels Apr 2, 2022
@github-actions github-actions Bot removed crate-keccak Issues related to the keccak workspace member T-opcode Type: opcode-related and focused PR/Issue labels Apr 14, 2022
Copy link
Copy Markdown
Contributor

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

First review round. I'm missing review of:

  • zkevm-circuits/src/evm_circuit/step.rs
  • zkevm-circuits/src/evm_circuit/util/common_gadget.rs
  • zkevm-circuits/src/evm_circuit/witness.rs

Overall looks good! I've added minor comments and suggestions.

Comment thread bus-mapping/src/evm/opcodes.rs
Comment thread bus-mapping/src/evm/opcodes.rs
Comment thread bus-mapping/src/evm/opcodes/return.rs
Comment thread bus-mapping/src/evm/opcodes/stop.rs
Comment thread bus-mapping/src/evm/opcodes/stop.rs Outdated
Comment thread zkevm-circuits/src/evm_circuit/execution/return.rs Outdated
Comment thread zkevm-circuits/src/evm_circuit/execution/stop.rs Outdated
Comment thread zkevm-circuits/src/evm_circuit/execution/stop.rs Outdated
Comment thread zkevm-circuits/src/evm_circuit/execution/stop.rs Outdated
Comment thread zkevm-circuits/src/evm_circuit/execution/stop.rs
Copy link
Copy Markdown
Contributor

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

Review complete. LGTM!

@han0110 han0110 marked this pull request as ready for review July 1, 2022 15:12
Comment thread zkevm-circuits/src/evm_circuit/util/common_gadget.rs
Comment thread zkevm-circuits/src/evm_circuit/util/common_gadget.rs
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, only left some classifications!

@han0110 han0110 merged commit d59be68 into privacy-ethereum:main Jul 4, 2022
@han0110 han0110 deleted the feature/stop branch July 4, 2022 08:42
zemse pushed a commit to zemse/zkevm-circuits that referenced this pull request Mar 22, 2023
* Implement circuit for `ErrorOutOfGasSHA3` state.

* Add `MemoryExpandedAddressGadget` to support memory offset plus length greater than `0x1FFFFFFFE0`.
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-zkevm-circuits Issues related to the zkevm-circuits workspace member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable first and last step ExecutionState check

3 participants