refactor(specs): port state refactor to older forks#2750
Merged
gurukamath merged 8 commits intoMay 12, 2026
Conversation
9 tasks
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #2750 +/- ##
===================================================
+ Coverage 88.62% 90.01% +1.38%
===================================================
Files 577 539 -38
Lines 35659 32618 -3041
Branches 3490 3030 -460
===================================================
- Hits 31604 29361 -2243
+ Misses 3492 2699 -793
+ Partials 563 558 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Note to reviewers: This is just a backport of the changes we made during the state refactor. Therefore even though the number of files touched is large, most of the changes are merely mechanical. The one exception to that is the pre-Cancun of self-destructs. That requires us to track storage clears explicitly in |
SamWilsn
approved these changes
May 7, 2026
Contributor
SamWilsn
left a comment
SamWilsn
left a comment
There was a problem hiding this comment.
None of these are blockers, but some should probably be addressed eventually.
Comment on lines
+2
to
+18
| State Tracking for Block Execution. | ||
|
|
||
| Track state changes on top of a read-only ``PreState``. At block end, | ||
| accumulated diffs feed into | ||
| ``PreState.compute_state_root_and_trie_changes()``. | ||
|
|
||
| .. contents:: Table of Contents | ||
| :backlinks: none | ||
| :local: | ||
|
|
||
| Introduction | ||
| ------------ | ||
|
|
||
| Replace the mutable ``State`` class with lightweight state trackers that | ||
| record diffs. ``BlockState`` accumulates committed transaction | ||
| changes across a block. ``TransactionState`` tracks in-flight changes | ||
| within a single transaction and supports copy-on-write rollback. |
Contributor
There was a problem hiding this comment.
New files should be documented in markdown. There's a Claude skill for doing it, if that's your thing.
Apply the state tracker refactor (originally done for amsterdam) to the remaining post-merge forks: osaka, prague, cancun, shanghai, and paris. For each fork: - Delete fork-specific state.py and trie.py - Add fork-specific state_tracker.py with BlockState, TransactionState, and the associated read/write helpers - Update fork.py to wrap chain.state in a BlockState, thread TransactionState through check_transaction / process_transaction / system transactions / withdrawals, and apply the block diff via apply_changes_to_state at the end of state_transition - Update vm/__init__.py to use BlockState/TransactionState and import Trie from ethereum.merkle_patricia_trie - Update interpreter to use copy_tx_state/restore_tx_state in place of begin/commit/rollback_transaction - Update utils/message.py, vm/eoa_delegation.py, and vm/instructions/* to read state via evm.message.tx_env.state - Update blocks.py docstring cross-refs to the shared state / trie modules
Apply the state tracker refactor (originally done for amsterdam) to the remaining pre-merge forks: gray_glacier, arrow_glacier, london, berlin, muir_glacier, istanbul, constantinople, byzantium, spurious_dragon, tangerine_whistle, dao_fork, homestead, and frontier. For each fork: - Delete fork-specific state.py and trie.py - Add fork-specific state_tracker.py with BlockState, TransactionState, and the associated read/write helpers - Update fork.py to wrap chain.state in a BlockState, thread TransactionState through check_transaction / process_transaction / block-reward payout, and apply the block diff via apply_changes_to_state at the end of state_transition - Update vm/__init__.py to use BlockState/TransactionState and import Trie from ethereum.merkle_patricia_trie - Update interpreter to use copy_tx_state/restore_tx_state in place of begin/commit/rollback_transaction - Update utils/message.py and vm/instructions/* to read state via evm.message.tx_env.state - Update blocks.py docstring cross-refs to the shared state / trie modules For pre-merge forks, the state_tracker also exposes `create_ether` (used for paying ommer/miner rewards) and the pre-EIP-161 forks have `destroy_touched_empty_accounts` and `touch_account`. Pre-byzantium receipts include an intermediate state root, computed by committing the tx's writes to the block state and calling `pre_state.compute_state_root_and_trie_changes` on the accumulated block diff. The DAO fork irregular state transition now creates a throwaway BlockState/TransactionState, applies the moves, and flushes the diff back to `chain.state`.
Pre-EIP-6780 ``SELFDESTRUCT`` fully wipes an account's storage, and a later ``CREATE2`` at the same address must start from empty storage. After the state refactor, ``destroy_storage`` only removed tx-level writes, so reads still fell through to the pre_state's storage trie — ``CREATE2`` then saw ``account_has_storage`` return True and bailed without bumping the nonce. Track wiped addresses in a new ``storage_clears`` set on ``BlockState``, ``TransactionState``, and ``BlockDiff``. Reads short-circuit on it, and ``compute_state_root_and_trie_changes`` / ``apply_changes_to_state`` drop the corresponding storage tries before re-applying writes, so post-wipe writes begin from empty storage. Applies to Frontier through Shanghai; post-6780 forks leave the set empty.
The t8n tool kept a mix of direct imports from ``amsterdam`` and raw ``_module(...)`` lookups into each fork's ``state_tracker`` and ``block_access_lists`` modules, and ``ForkLoad`` still carried fallback shims for helpers (``State``, ``set_account``, ``copy_trie``, ``root``, etc.) that now live exclusively in the shared ``ethereum.state`` and ``ethereum.merkle_patricia_trie`` modules. Collapse all three: * Drop the fork-independent shims from ``ForkLoad`` and import the helpers directly at the t8n / fixture-loader call sites. * Expose ``BlockState``, ``TransactionState``, ``extract_block_diff``, ``incorporate_tx_into_block``, ``BlockAccessIndex``, ``BlockAccessListBuilder``, and ``validate_block_access_list_gas_limit`` as ``ForkLoad`` properties so t8n never has to hardcode ``amsterdam`` or reach into ``_module``. * Fix ``pay_block_rewards`` to wrap ``BlockState`` in a ``TransactionState`` via the fork's own state_tracker (the previous code passed the block-scoped state straight to ``create_ether``, which expects the tx-scoped wrapper). * Remove the now-dead ``has_block_state`` feature flag and its fallback branches — every fork has a ``state_tracker`` module.
Point the json_loader tests at the shared ``ethereum.state`` and ``ethereum.merkle_patricia_trie`` modules now that per-fork ``state`` and ``trie`` modules no longer exist, and switch to the State-method accessors (``state.get_storage(...)`` etc.) instead of the old module-level functions. Skip ``test_optimized_state`` wholesale: the optimized state integration still imports per-fork ``state`` modules and exposes a pre-refactor ``destroy_storage(state, addr)`` API, both removed by the refactor. Tracked by ethereum#2256 pending the optimized state redesign. Extend ``vulture_whitelist.py`` to cover the ``@add_item``-registered ``begin_transaction`` / ``commit_transaction`` / ``rollback_transaction`` patches in ``ethereum_optimized.state_db``.
The state refactor initially re-imported ``Account``, ``Address``, ``EMPTY_ACCOUNT``, and ``EMPTY_CODE_HASH`` from ``ethereum.state`` into each pre-merge fork's ``fork_types.py`` (plus a matching ``__all__``) so existing call sites like ``from .fork_types import Address`` could stay untouched. ``Root`` was similarly duplicated as ``Root = Hash32`` in every fork. ``docc`` does not follow ``__all__`` re-exports when resolving cross-references, which breaks ``just docs-spec`` on the first consumer that imports ``EMPTY_ACCOUNT`` through a fork's ``fork_types``. Drop the re-exports from all 13 pre-merge forks and import the shared symbols directly from ``ethereum.state`` at every call site — matching the shape that shanghai / paris already use. Each ``fork_types.py`` collapses to ``Bloom``, ``encode_account``, and the ``Account`` type import that ``encode_account`` needs. Also update ``tests/json_loader/test_optimized_state.py`` to import ``EMPTY_ACCOUNT`` from ``ethereum.state`` rather than through the dropped ``frontier.fork_types`` re-export.
Apply review suggestions from @SamWilsn: * ``ethereum.state.BlockDiff.storage_clears`` docstring switches to markdown style with a cross-ref to ``storage_changes``, so docc renders the link. * In every fork's ``state_tracker.get_account``, replace the ``isinstance(account, Account)`` guard with an explicit ``if account is None`` check. The ``isinstance`` was a leftover from when ``fork_types.Account`` was a separate class; now that every fork pulls ``Account`` directly from ``ethereum.state``, the ``is None`` form expresses the intent ("is there an account?") rather than the implementation ("is this an account?"). * Reintroduce the ``create_ether`` helper in every post-merge fork's state tracker (paris through bpo5). It was dropped during the post-merge state refactor because the PoS forks no longer ``pay_rewards`` to a miner, but it remains the natural helper for ``process_withdrawals``. Updates the withdrawal call sites in shanghai onwards (paris itself has no caller; ``create_ether`` is added there purely to keep sibling parity with london and shanghai). Also relocate ``create_ether`` in every pre-merge fork from its legacy spot in the trailing "Pre-EIP-161 cleanup" section to immediately after ``move_ether`` — they are structurally the same kind of balance mutator, and the new placement is consistent across every fork. * Rewrite the ``untracked_state`` comment in ``process_checked_system_transaction`` (prague through bpo5) to make the throwaway-scope and "always calls process_unchecked_system_transaction below" contracts explicit.
gurukamath
force-pushed
the
port/state-refactor
branch
from
3e69609 to
31d7c42
Compare
Contributor
Author
|
@SamWilsn I have incorporated the comments except the markdown one. That one can be handled in #671 Have also rebased the PR branch on the latest PS: I have also generated all the fixtures and used |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🗒️ Description
Finish the State refactor (started in #2381, #2207 and #2264) and make one change to state for the older pre-Cancun forks related to EIP-6780. This PR tackles the following changes:
🔗 Related Issues or PRs
#2381
#2207
#2264
#2256
#2245
✅ Checklist
just statictype(scope):.Cute Animal Picture