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

fix: fix accountCreated handling, add storageTrace, fix DELEGATECALL/STATICCALL caller state #42

Merged
merged 13 commits into from
Aug 11, 2022

Conversation

noel2004
Copy link
Member

@noel2004 noel2004 commented Aug 3, 2022

TODO:update zkevm-circuits

As discussed in the issue. Some inconsistecies are caused by unsuitable initialization of stateDB used by busmapping. This PR change the initialization by adding states being traced in storageTrace field of block results and this work should fix most of the inconsistency.

This PR also change the example traces by manually revert the byte order in proofs, because they have been unified into big-endian.

@noel2004
Copy link
Member Author

noel2004 commented Aug 4, 2022

I have test this PR on the skipped blocks in roller and all of them can passed the handl-block phase (so witness can be assigned and circuit be built), execpt for block 17, which assert with !callee_code_hash.is_zero().

@icemelon
Copy link
Member

icemelon commented Aug 4, 2022

@noel2004 I checked the transaction in the block 17 and it calls to the L2Messenger contract. The contract shouldn't have a zero codehash. So not sure why the assert failed for this block.

@lispc
Copy link
Collaborator

lispc commented Aug 4, 2022

You can Leave this error case to me. You can really proved some blocks in our dev server (i suggest only prove evm circuit. Don't prove agg)

@lispc
Copy link
Collaborator

lispc commented Aug 4, 2022

Suggest Set MOCK_PROVE env var to true to get better circuit error msg.

@lispc lispc mentioned this pull request Aug 4, 2022
@0xmountaintop 0xmountaintop changed the title Fix (partial) issue #36 fix: fix accountCreated handling, add storageTrace, fix DELEGATECALL/STATICCALL caller state Aug 5, 2022
@0xmountaintop
Copy link
Contributor

I will build and test without merging first.

BTW what dores issue.json mean?

  1. it's not block 17.
  2. issue inconsistency between state rebuilding #36? but there're 4 problems there.

types/src/eth.rs Outdated Show resolved Hide resolved
@noel2004
Copy link
Member Author

noel2004 commented Aug 5, 2022

I will build and test without merging first.

BTW what dores issue.json mean?

  1. it's not block 17.
  2. issue inconsistency between state rebuilding #36? but there're 4 problems there.

issue.json is being added accidentally. It would be prune

@noel2004
Copy link
Member Author

noel2004 commented Aug 5, 2022

Now the PR has passed with block 17 so it should be ready.

Work left:

  • lacking of serialize in storeTrace may cause roller fail for its store / reloading process.

@noel2004
Copy link
Member Author

Now the blockResult structure can be fully serialized / deserialized.

@scroll-dev scroll-dev merged commit d647dd3 into main Aug 11, 2022
@scroll-dev scroll-dev deleted the fix/36 branch August 11, 2022 14:38
cyphersnake pushed a commit to cyphersnake/scroll-prover that referenced this pull request Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants