ethereum / execution-specs

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

Implement EIP-1153 for Cancun #850

Closed gurukamath closed 5 months ago

gurukamath commented 8 months ago

What was wrong?

EIP-1153, which is slated for Cancun, is not currently implemented in the specs.

How was it fixed?

Implemented EIP-1153

Cute Animal Picture

3396657

codecov-commenter commented 8 months ago

Codecov Report

Attention: 2237 lines in your changes are missing coverage. Please review.

Comparison is base (673ccec) 69.91% compared to head (af90b66) 69.87%. Report is 53 commits behind head on forks/cancun.

Files Patch % Lines
src/ethereum/cancun/fork.py 0.00% 225 Missing :warning:
src/ethereum/cancun/vm/instructions/system.py 0.00% 208 Missing :warning:
src/ethereum/cancun/vm/instructions/__init__.py 0.00% 161 Missing :warning:
src/ethereum/cancun/state.py 0.00% 149 Missing :warning:
src/ethereum/cancun/vm/instructions/environment.py 0.00% 143 Missing :warning:
src/ethereum/cancun/vm/interpreter.py 0.00% 112 Missing :warning:
src/ethereum/cancun/vm/instructions/arithmetic.py 0.00% 109 Missing :warning:
src/ethereum/cancun/vm/gas.py 0.00% 97 Missing :warning:
src/ethereum/cancun/vm/instructions/stack.py 0.00% 94 Missing :warning:
src/ethereum/cancun/vm/__init__.py 0.00% 77 Missing :warning:
... and 55 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## forks/cancun #850 +/- ## ================================================ - Coverage 69.91% 69.87% -0.05% ================================================ Files 609 610 +1 Lines 33980 34338 +358 ================================================ + Hits 23758 23993 +235 - Misses 10222 10345 +123 ``` | [Flag](https://app.codecov.io/gh/ethereum/execution-specs/pull/850/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/850/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ethereum) | `69.87% <25.87%> (-0.05%)` | :arrow_down: | 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.

gurukamath commented 8 months ago

Rebased the EIP-1153 implementation on the forks/cancun branch

gurukamath commented 8 months ago

I think the basic architecture of this PR is wrong.

Firstly transient_state is not a State. State comes with a lot of extra baggage (disk backing, state root calculating, etc) which doesn't apply to transient storage.

You are right. Not sure how I missed that. Will update

Secondly, transient storage is strictly a transaction scoped rather that block scoped. It is "discarded" at the end of the transaction. It belongs in the Evm dataclass with all of the other transaction scoped ephemeral data.

Will look into this.

gurukamath commented 7 months ago

@petertdavies Updated the PR to include the TransientStorage class and include an attribute in the transaction scoped EVM class