chore(trie): Make `trie` Fork agnostic by kevaundray · Pull Request #2267 · ethereum/execution-specs

kevaundray · 2026-02-21T02:58:57Z

🗒️ Description

Bullet point 3 from #2207

This makes the Trie data structure and its methods fork agnostic.

There were some tox related errors that were surfacing due to Account not being fork agnostic for forks pre-Amsterdam. I've changed those imports for Account to point to the one defined in ethereum.state for simplicity (this is what I imagine the endgame solution tends towards, given the issue on making Account fork agnostic)

🔗 Related Issues or PRs

#2293

Related to #2154 -- we could also pull out Account in a separate PR

✅ Checklist

All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
```
uvx tox -e static
```
All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
All: Considered updating the online docs in the ./docs/ directory.
All: Set appropriate labels for the changes (only maintainers can apply labels).
Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.
Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

Cute Animal Picture

codecov · 2026-02-21T03:39:19Z

Codecov Report

❌ Patch coverage is 96.63866% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.91%. Comparing base (fa39bdc) to head (93ccb50).
⚠️ Report is 2 commits behind head on forks/amsterdam.

Files with missing lines	Patch %	Lines
src/ethereum/trie.py	33.33%	4 Missing ⚠️

Additional details and impacted files

@@                 Coverage Diff                 @@
##           forks/amsterdam    #2267      +/-   ##
===================================================
+ Coverage            86.00%   88.91%   +2.91%     
===================================================
  Files                  600      536      -64     
  Lines                39357    32516    -6841     
  Branches              3770     2797     -973     
===================================================
- Hits                 33850    28913    -4937     
+ Misses                4877     3022    -1855     
+ Partials               630      581      -49

Flag	Coverage Δ
unittests	`88.91% <96.63%> (+2.91%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kevaundray · 2026-02-21T17:24:41Z

The issue with EMPTY_ACCOUNT, also applies to Account -- have removed those re-exports from fork_types since Account and EMPTY_ACCOUNT doesn't change across forks.

We now import from ethereum.state for previous forks

kevaundray · 2026-02-21T18:42:19Z

-        with (fork_dir / "trie.py").open("r") as f:
-            assert (
-                "from ethereum.forks.paris import trie as previous_trie"
-                in f.read()
-            )


trie.py is no longer fork dependent, so I removed this check in new-fork tool

kevaundray · 2026-02-21T18:43:00Z

@@ -322,18 +322,18 @@ def create_ether(self) -> Any:

    @property
    def root(self) -> Any:
-        """Root function of the fork."""
-        return self._module("trie").root
+        """Root function."""
+        return trie_root

    @property
    def copy_trie(self) -> Any:
-        """copy_trie function of the fork."""
-        return self._module("trie").copy_trie
+        """copy_trie function."""
+        return copy_trie

    @property
    def trie_get(self) -> Any:
-        """trie_get function of the fork."""
-        return self._module("trie").trie_get
+        """trie_get function."""
+        return trie_get


These are no longer dependent on the fork, left them here to not increase the diff further in this PR.

kevaundray · 2026-02-21T18:57:39Z

I think the rough shape of this is now visible, so would say the experiment is done.

CC @SamWilsn and @gurukamath for input when you get back next week.

Kind of a PITA that it touches so many files, but this was somewhat inevitable

gurukamath

Largely looks good to me. Thanks for taking this on :). Just left a couple of smaller comments.

gurukamath · 2026-02-25T09:15:43Z

        return keccak256(encoded)


-def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes:


Suggested change

def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes:

def encode_node(node: Optional[Extended], storage_root: Optional[Bytes] = None) -> Bytes:

And then handle the None case explicitly in the function body to raise the relevant AssertionError. The V typevar also becomes V = TypeVar("V", bound=Optional[Extended]) in that case.

For context, we do use the broad Any type in some tooling related code but try to avoid it in the specs code itself.

This is more just a comment from a quick fly-by. @gurukamath any reason to not use Extended | None instead of importing Optional just to use Optional[Extended]? It seems to be the preferred usage for Python >= 3.10. Of course, we can make our own decisions on that, but just curious.

gurukamath · 2026-02-25T09:35:55Z

+V = TypeVar("V")
+
+
+def encode_account(raw_account_data: Account, storage_root: Bytes) -> Bytes:


Perhaps, this belongs in ethereum.state since that is also where we have defined Account

…agnostic-trie # Conflicts: # src/ethereum/forks/amsterdam/fork.py # src/ethereum/forks/amsterdam/fork_types.py # src/ethereum/forks/amsterdam/state.py

SamWilsn

This makes sense. I guess maybe it might be reasonable to rename it to mp_trie.py eventually if we need to differentiate between trie implementations.

My internal reasoning for accepting this hoist is that the diffs between the MP trie and any other trie would be extremely messy and not particularly useful.

SamWilsn · 2026-02-25T20:29:31Z



-def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes:
+def encode_node(node: Any, storage_root: Optional[Bytes] = None) -> Bytes:


No Any please.

SamWilsn · 2026-02-25T20:32:35Z


 Address = Bytes20
 Root = Hash32
 Account_ = Any  # noqa N806


If there's only one Account now, I don't think we need this Any any longer.

SamWilsn · 2026-02-25T20:33:05Z

-    )
    state_mod = cast(Any, import_module("ethereum.forks." + fork + ".state"))
-    Account = types_mod.Account  # noqa N806
+    Account = Account_cls  # noqa: N806


What is the purpose of this line?

kevaundray · 2026-02-25T21:00:05Z

I think this is semi-blocked on backporting Account to previous forks because Amsterdam has Account with code hash while other forks have Account with code and a part of making the Trie fork agnostic is that the Account is also fork agnostic.

We could have each fork define an encode_account method and Account class for now, but I think its probably better to backport first

kevaundray · 2026-03-02T21:03:55Z

superceded by #2381

kevaundray added 3 commits February 21, 2026 01:20

migrate

eb044c1

fix proper import

5a3546f

fix amsterdam: re-export EMPTY_ACCOUNT and Account

8acd6c6

kevaundray added 2 commits February 21, 2026 16:53

remove assertion since trie.py is fork agnostic

9ca38de

don't re-export EMPTY_ACCOUNT, Account

3f12fc9

kevaundray force-pushed the kw/fork-agnostic-trie branch from 3dd17e4 to 3f12fc9 Compare February 21, 2026 17:23

kevaundray commented Feb 21, 2026

View reviewed changes

Comment thread src/ethereum/forks/amsterdam/trie.py Outdated

remove trie.py and update forks

e510758

kevaundray commented Feb 21, 2026

View reviewed changes

Comment thread src/ethereum_spec_tools/evm_tools/loaders/fork_loader.py

kevaundray added 4 commits February 21, 2026 18:00

update comment

9802729

fix forkloader

5d45ff8

fix test_genesis import

8aaee41

fix relative reference

a171093

kevaundray commented Feb 21, 2026

View reviewed changes

kevaundray changed the title ~~chore(do not merge): Experiment with fork agnostic Trie~~ chore(trie): Make trie Fork agnostic Feb 21, 2026

kevaundray marked this pull request as ready for review February 21, 2026 18:53

kevaundray force-pushed the kw/fork-agnostic-trie branch from cde3285 to 9a6bf98 Compare February 22, 2026 03:08

remove more outdated imports

93ccb50

kevaundray force-pushed the kw/fork-agnostic-trie branch from 9a6bf98 to 93ccb50 Compare February 22, 2026 14:49

marioevz requested a review from SamWilsn February 24, 2026 23:21

marioevz assigned SamWilsn Feb 24, 2026

gurukamath mentioned this pull request Feb 25, 2026

Changes to be back-ported from Amsterdam #2245

Open

9 tasks

gurukamath reviewed Feb 25, 2026

View reviewed changes

kevaundray added 2 commits February 25, 2026 12:14

Merge remote-tracking branch 'upstream/forks/amsterdam' into kw/fork-…

00060e7

…agnostic-trie # Conflicts: # src/ethereum/forks/amsterdam/fork.py # src/ethereum/forks/amsterdam/fork_types.py # src/ethereum/forks/amsterdam/state.py

lint

3b76fc6

SamWilsn reviewed Feb 25, 2026

View reviewed changes

gurukamath mentioned this pull request Mar 2, 2026

refactor(specs): make State fork agnostic #2381

Merged

2 tasks

kevaundray closed this Mar 2, 2026

		return keccak256(encoded)


		def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes:

	def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes:
	def encode_node(node: Optional[Extended], storage_root: Optional[Bytes] = None) -> Bytes:

		V = TypeVar("V")


		def encode_account(raw_account_data: Account, storage_root: Bytes) -> Bytes:



		def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes:
		def encode_node(node: Any, storage_root: Optional[Bytes] = None) -> Bytes:

Conversation

kevaundray commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗒️ Description

🔗 Related Issues or PRs

✅ Checklist

Cute Animal Picture

Uh oh!

codecov Bot commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kevaundray commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevaundray Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

kevaundray Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevaundray commented Feb 21, 2026

Uh oh!

gurukamath left a comment

Choose a reason for hiding this comment

Uh oh!

gurukamath Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

fselmo Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gurukamath Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

SamWilsn left a comment

Choose a reason for hiding this comment

Uh oh!

SamWilsn Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

SamWilsn Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

SamWilsn Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

kevaundray commented Feb 25, 2026

Uh oh!

kevaundray commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kevaundray commented Feb 21, 2026 •

edited

Loading

codecov Bot commented Feb 21, 2026 •

edited

Loading

kevaundray commented Feb 21, 2026 •

edited

Loading

kevaundray Feb 21, 2026 •

edited

Loading

fselmo Feb 25, 2026 •

edited

Loading