Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dummy_gen_create_ops to avoid call stack empty panic #454

Merged

Conversation

han0110
Copy link
Collaborator

@han0110 han0110 commented Apr 12, 2022

This PR aims to make CircuitInputBuilder work with trace containing CREATE* by dummy_gen_create_ops, which takes care of call stack and state db to enable the parsing.

It also does:

  1. Change the behavior of Memory::read_word to be able to read a word even the memory[offset..offset+32] is out of range (the same behavior of EVM), and we would want this behavior because the step of trace has the memory before expansion.
  2. Add storage access into list instead of only checking whether it has been accessed or not.
  3. Add dummy_gen_selfdestructed_op.

With #440 implemented, we should be able create partially verified proof of all kinds of trace (hopefully) instead of panic.

@han0110 han0110 requested a review from a team as a code owner April 12, 2022 13:29
@han0110 han0110 requested review from ed255 and removed request for a team April 12, 2022 13:29
@github-actions github-actions bot added 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 labels Apr 12, 2022
Copy link
Member

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

Overall looks good! Nevertheless, please take a look at how push_op_reversible already applies the write operations to the StateDB (and thus there's no need to do it before). For context, this change was done here https://github.com/appliedzkp/zkevm-circuits/pull/436/files#diff-a06662ad93069ff8d12d1029c92dd22ef395882417fc8daa8a292f582eded9deR812

bus-mapping/src/evm/opcodes.rs Show resolved Hide resolved
bus-mapping/src/evm/opcodes.rs Show resolved Hide resolved
bus-mapping/src/evm/opcodes.rs Show resolved Hide resolved
bus-mapping/src/evm/opcodes/sload.rs Outdated Show resolved Hide resolved
bus-mapping/src/state_db.rs Show resolved Hide resolved
@han0110 han0110 force-pushed the feature/dummy-gen-create-ops branch from c10b991 to 1536b64 Compare May 15, 2022 14:03
Copy link
Member

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

LGTM! :)

@CPerezz CPerezz force-pushed the feature/dummy-gen-create-ops branch from 8e26eda to d43c6d8 Compare May 16, 2022 15:09
@CPerezz
Copy link
Member

CPerezz commented May 16, 2022

Just rebased and I think this is ready to be merged!

Should we merge @han0110 ? Or is there anything left you want to do?

@han0110
Copy link
Collaborator Author

han0110 commented May 16, 2022

Nothing left to do, let's merge it after CI is finished. Thanks!

@CPerezz CPerezz merged commit 2693c32 into privacy-scaling-explorations:main May 16, 2022
@han0110 han0110 deleted the feature/dummy-gen-create-ops branch May 16, 2022 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

None yet

3 participants