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

inconsistency between state rebuilding #36

Closed
lispc opened this issue Aug 1, 2022 · 6 comments
Closed

inconsistency between state rebuilding #36

lispc opened this issue Aug 1, 2022 · 6 comments
Assignees

Comments

@lispc
Copy link
Collaborator

lispc commented Aug 1, 2022

VS https://github.com/scroll-tech/zkevm-circuits/blob/3a48411dec61360e7a441bdf5abbf4b052f2d7ad/bus-mapping/src/circuit_input_builder/access.rs#L114

  1. if tx.to == null, trace the newly created contract. accountCreated? but be careful with this problem problems of 'AccountCreated' in trace go-ethereum#137
  2. DELEGATECALL / STATICCALL, need caller state?
  3. history block hashes is empty now. (upstream is also "TODO" ... , at least we should provide 1 prev block hash for pseudo randomness)
  4. some edge case handling codes may be missing here. Eg: 'create' fails?
@lispc
Copy link
Collaborator Author

lispc commented Aug 1, 2022

@noel2004
Copy link
Member

noel2004 commented Aug 1, 2022

So I would resolve following 4 tasks:

  • Handle issue 137 in l2geth: problems of 'AccountCreated' in trace go-ethereum#137
  • In geth, update the trace output for DELEGATECALL / STATICCALL, with caller state
  • Fix to make history block hashes is not empty now, for at least one history block
  • handle some edge case like 'create' fails

@noel2004
Copy link
Member

noel2004 commented Aug 1, 2022

I think task 1 can be resolved by "not an issue":

scroll-tech/go-ethereum#137 (comment)

@lispc
Copy link
Collaborator Author

lispc commented Aug 1, 2022

oh i think you should trace_proof AccountCreated in common rs or else that err block result json will fail?

@noel2004
Copy link
Member

noel2004 commented Aug 1, 2022

oh i think you should trace_proof AccountCreated in common rs or else that err block result json will fail?

Yes. trace_proof AccountCreated seems fix the issue.

However I am still not very confident about which field we should trace (accountCreated or accountAfter), I may spend some time to dive deeper in the code to understand that.

@lispc lispc mentioned this issue Aug 2, 2022
@noel2004
Copy link
Member

noel2004 commented Aug 3, 2022

For contract creation, there are basically three phase for the contract address's data (except for storageRoot):

Phase Nonce Balance CodeHash
1 Before creation (account node not exist) 0 (default) 0 (default) Hash{}
2 Before constructor execution 1 (after EIP 158) <value being transferred> Hash(nil)
3 After construction 1 <value after construction> Hash(code)

The "...before" state recorded inside execution_results and accountCreated fields in fact represent the state of phase 2.

There are would be issues to change them representing the state of phase 1:

  1. Storage module rely on this record to construct the corresponding writing action on account. Without the record, it would require the module to made more sophisticated "replay" action like what bustmapping has did instead of simply trace for reconstructing storage states.

  2. The state in phase 1 is just a duplication of the the last "...after" state or the state being stored in storageTrace.

Since the stateDB for busmapping should be initialized with the state before a block being handled. I prefer parsing the storageTrace field for these states to trace them from the execution_results for each transction, which has stored the real "before" states required in handling a block. It should also fix the issue that missing of caller in STATICCALL (for DELEGATECALL there should be no another caller?)

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

No branches or pull requests

3 participants