ethereum / execution-specs

Specification for the Execution Layer. Tracking network upgrades.
Creative Commons Zero v1.0 Universal
831 stars 234 forks source link

Support `ethereum-spec-evm statetest` #907

Closed SamWilsn closed 5 months ago

SamWilsn commented 6 months ago

What was wrong?

We don't have fuzzy eels.

How was it fixed?

Implemented the statetest command for ethereum-spec-evm.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

SamWilsn commented 6 months ago

Note to self: comment on https://github.com/holiman/goevmlab/pull/131 when this is merged.

holiman commented 6 months ago

In https://github.com/ethereum/execution-specs/pull/907/commits/88d195e7392119ce54e174740617c83edc62fc8e, you made it output the result to stderr even if we're not tracing. Was that in response to my comment here: https://github.com/holiman/goevmlab/pull/131#issuecomment-2009148751 ?

When in "skip trace" mode, (a.k.a speedMode), we still read from stderr. But goevmlab does not supply the --json switch, so the { "stateroot": ... is not output on stderr. There is json output on stdout, but we're not looking at it, though.

So the copyUntilEnd just doesn't get anything. The easiest fix is if you make it always output the stateroot on stderr, which is how I fixed geth here https://github.com/ethereum/go-ethereum/pull/29290

If so, while it does improve things, it's still a bit wonky. Because you output the full json struct on stderr. So, in order to read it, goevmlab needs to know not to expect jsonl, but to instead expect multiline output. Which means either parsing json or just trying to parse out the stateroot from the blob. This is what I do for nethermind: https://github.com/holiman/goevmlab/blob/master/evms/nethermind.go#L143

A much (from my PoV) simpler solution is if eels instead just emits jsonl, that is:

{ "stateRoot": "0x..." } 

So, my wishlist would be: same output as --json, but just do not generate the per-opcode lines of output.

SamWilsn commented 6 months ago

@holiman I totally misunderstood what you meant :sweat_smile: This makes a lot more sense.

holiman commented 6 months ago

If you rebase this on top of https://github.com/ethereum/execution-specs/pull/912, that'd be great

holiman commented 6 months ago

Please rebase on top of #915 ?

codecov-commenter commented 5 months ago

Codecov Report

Attention: Patch coverage is 79.31034% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 74.86%. Comparing base (bf47143) to head (a908534). Report is 126 commits behind head on forks/cancun.

Files Patch % Lines
src/ethereum/arrow_glacier/vm/exceptions.py 0.00% 4 Missing :warning:
src/ethereum/dao_fork/vm/exceptions.py 0.00% 4 Missing :warning:
src/ethereum/gray_glacier/vm/exceptions.py 0.00% 4 Missing :warning:
src/ethereum/muir_glacier/vm/exceptions.py 0.00% 4 Missing :warning:
src/ethereum/base_types.py 92.68% 3 Missing :warning:
src/ethereum/paris/vm/exceptions.py 50.00% 2 Missing :warning:
...m/arrow_glacier/vm/precompiled_contracts/modexp.py 0.00% 1 Missing :warning:
...um/gray_glacier/vm/precompiled_contracts/modexp.py 0.00% 1 Missing :warning:
...um/muir_glacier/vm/precompiled_contracts/modexp.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## forks/cancun #907 +/- ## ================================================ + Coverage 69.96% 74.86% +4.89% ================================================ Files 610 643 +33 Lines 34295 35223 +928 ================================================ + Hits 23993 26368 +2375 + Misses 10302 8855 -1447 ``` | [Flag](https://app.codecov.io/gh/ethereum/execution-specs/pull/907/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ethereum) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/ethereum/execution-specs/pull/907/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ethereum) | `74.86% <79.31%> (+4.89%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ethereum#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.