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

Add dummy_gen_create_ops to avoid call stack empty panic#454

Merged
CPerezz merged 4 commits into
privacy-ethereum:mainfrom
han0110:feature/dummy-gen-create-ops
May 16, 2022
Merged

Add dummy_gen_create_ops to avoid call stack empty panic#454
CPerezz merged 4 commits into
privacy-ethereum:mainfrom
han0110:feature/dummy-gen-create-ops

Conversation

@han0110
Copy link
Copy Markdown
Contributor

@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 review from a team and 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
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.

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

Comment thread bus-mapping/src/evm/opcodes.rs
Comment thread bus-mapping/src/evm/opcodes.rs
Comment thread bus-mapping/src/evm/opcodes.rs
Comment thread bus-mapping/src/evm/opcodes/sload.rs Outdated
Comment thread bus-mapping/src/state_db.rs
@han0110 han0110 force-pushed the feature/dummy-gen-create-ops branch from c10b991 to 1536b64 Compare May 15, 2022 14:03
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.

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
Copy Markdown
Contributor

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
Copy Markdown
Contributor 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-ethereum:main May 16, 2022
@han0110 han0110 deleted the feature/dummy-gen-create-ops branch May 16, 2022 23:59
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.

3 participants