feat(test-types): Fork-based State Commitment Property in Alloc

[marioevz]

Description

Attempt to respond to concern in #3251: teach Alloc to know which state-commitment scheme its state root is computed under, and let the fork decide that scheme. Today every fork commits with a Merkle-Patricia Trie (MPT); this change lets a future fork switch to a binary tree without touching the allocation-handling code — the correct root algorithm is selected per fork instead of being hard-wired to state_mpt.

This is behaviorally a no-op on mainnet forks: every fork returns StateCommitment.MPT, so all state roots are computed exactly as before. It only adds the plumbing (and the seeding) needed for a binary-tree fork to drop in later (or in work branches).

Core pieces

• StateCommitment enum (MPT / BINARY) added to base_types — a spec-free vocabulary type so both forks and test_types can reference it without new import edges.
• Fork.state_commitment() classmethod on BaseFork, defaulting to MPT. A binary-transition fork overrides it to BINARY; no spec import enters the forks package.
• Alloc carries its scheme. A private _state_commitment field (default MPT), a public state_commitment() getter, and migrate_state_commitment() to switch it (rejected once the alloc is FROZEN). state_root() / _materialize_state() now dispatch through a new _state_module() that maps the scheme → the spec implementation module; BINARY raises NotImplementedError until a state_bmt module exists.
• Backend-agnostic materialization. _materialize_state() is now typed against ethereum.state.PreState (which both state_mpt.State and the future binary State satisfy) and only touches the state through public module functions (State, set_account, set_storage, store_code) — it no longer pins state_mpt.State or pokes the private _code_store.
• Alloc.merge is commitment-aware. It now propagates a scheme instead of minting a default-MPT allocation: an explicit state_commitment= argument wins, otherwise the second (right-hand) allocation's scheme takes precedence. It also switched to model_copy(deep=True), which preserves the private scheme field.

Seeding the scheme (private attributes are not serialized)

Because _state_commitment is a private attribute, it is not restored by model_validate/model_dump_json and is dropped by JSON round-trips, so it must be (re)seeded from the fork wherever an allocation is constructed, loaded, or merged before a root is computed:

• shared/pre_alloc.Alloc.__init__ — seed at construction from fork.transitions_from().state_commitment().
• pre_alloc_groups.PreAllocGroupBuilder.model_post_init — seed on every load-from-file (covers PreAllocGroup and GroupPreAlloc via inheritance); add_test_alloc asserts both sides agree.
• execute/rpc/hive.py and execute/tests/test_execute_remote.py — seed the execute-path genesis allocation.
• ethereum_spec_tools/evm_tools/t8n — after the defensive model_copy(deep=True), seed self.alloc from t8n_data.fork (which is always a concrete, non-transition fork here), covering the JSON CLI path where the scheme is otherwise lost to the MPT default.

Enabling binary later (once a spec state_bmt module lands) is three steps, none in _materialize_state:

1. Import the binary state module in account_types.py.
2. Return it from _state_module() for StateCommitment.BINARY instead of raising.
3. Override Fork.state_commitment() → BINARY on the transition fork.

The transition-block state-root semantics at a commitment boundary remain to be defined in a follow-up.

cc @CPerezz

Related Issues or PRs

N/A.

Checklist

• Ran fast static checks to avoid CI fails, see Code Standards & Verifying Changes: just static
• PR title has the form <type>(<area>): <title>, where <type> and <area> come from an appropriate C-<type>, respectively A-<area>, label. The title should match the target squash commit message.

Cute Animal Picture

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.49%. Comparing base (9d6e6f8) to head (573c657).
⚠️ Report is 1 commits behind head on forks/amsterdam.

Additional details and impacted files

@@               Coverage Diff                @@
##           forks/amsterdam    #3279   +/-   ##
================================================
  Coverage            93.49%   93.49%           
================================================
  Files                  625      625           
  Lines                37032    37039    +7     
  Branches              3385     3392    +7     
================================================
+ Hits                 34623    34630    +7     
  Misses                1653     1653           
  Partials               756      756           

Flag	Coverage Δ
unittests	93.49% <ø> (+<0.01%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

[kevaundray]

Small note: in projects/binary-trie, its called state_pbt where pbt = partitioned binary tree

[kevaundray]

Haven't personally checked that this does not crash when we try to fill static tests -- I'm probably missing something, from a glance it seems that ci doesn't fill these

[kevaundray]

Related to this comment seems we don't need to remove this check?

[kevaundray]

Making the state commitment an explicit property of the fork makes sense to me :)

Left some small comments, but LGTM

[CPerezz]

I'm at the point in ethereum/go-ethereum#35436 where I can use it to try filling tests and reach consensus on the format etc.. Such that we can indeed be sure this works well with a real client.

[CPerezz]

With the help of Fable, I was able to find some loops that would break PBT. I instructed it to give a step-by-step guide on how we hit a critical correctness error:

PBT genesis roots for any contract with code are broken

• Step 1 — genesis roots are computed on a freshly-constructed alloc. The fill path calls state_root() directly on the test's pre-alloc, e.g. packages/testing/src/execution_testing/specs/blockchain.py:822 [head]:

state_root = pre_alloc.state_root()

Same shape at hive.py:156, pre_alloc_groups.py:78, execute_types.py:371 [head]. At that moment the alloc is in _Phase.CONSTRUCTION — nothing has touched it as a PreState yet.

• Step 2 — state_root() materializes a spec-side state (account_types.py:311-313 → :381 [head]). The PR's _materialize_state() ends with (account_types.py:413-422 [head]):

                mod.set_storage(
                    state, addr,
                    Bytes32(int(key_hi).to_bytes(32, "big")),
                    U256(value_int),
                )
        for stored_code in self._code_store.values():
            mod.store_code(state, stored_code)
        return state

The per-account loop above it computes code_hash = spec_keccak256(code) without ever storing the bytes — code bytes only reach the spec state through that trailing loop over self._code_store.

• Step 3 — _code_store is empty on this path. It's only populated by _build_cache() (account_types.py:353-362 [head]), which only runs from _ensure_live() (:364-368), whose only callers are the PreState getters and compute_state_root (:431, :451, :467, :478, :491 [head]). state_root() and _materialize_state() never call it. So on the genesis path, the trailing loop iterates an empty dict.

• Step 4 — MPT doesn't care, PBT does. MPT commits accounts by code_hash only, so a missing code body is invisible. But a content-committing scheme must chunk the actual bytecode: src/ethereum/state_pbt.py:77 [branch] inside embed_flat_state:

        code = get_code(account.code_hash)

which resolves through State.get_code (state_pbt.py:131-139 [branch]):

        if code_hash == EMPTY_CODE_HASH:
            return b""
        return self._code_store[code_hash]   # ← KeyError: bytes were never stored

So the moment _state_module() returns state_pbt, every genesis containing a contract raises KeyError (or, in a scheme that tolerated it, would silently commit wrong chunk leaves).

[CPerezz]

execute_types.py:371 silently produces MPT roots by default.

genesis_fork, env, self.alloc.state_root()

self.alloc.state_root() runs with the default-MPT enum — nothing seeds this alloc. The fork is sitting in scope a few lines above (#3251 mentions exactly this).

The PR fixes the other execute-path site (hive.py:153-155) but not this one. Today it's harmless (mainnet forks are all MPT); the moment someone points execute --eth-config at a binary devnet, it produces an MPT root silently.

[CPerezz]

Pydantic private attrs are excluded from serialization, so the scheme evaporates at exactly the boundaries fixtures cross. Imagine for example the following:

a = Alloc({}); a.migrate_state_commitment(StateCommitment.BINARY)
b = Alloc.model_validate_json(a.model_dump_json())
# F2  round-trip: BINARY -> MPT                       ← silent
c = Alloc.merge(a, Alloc({}))
# F2b merge(BINARY, fresh-MPT) -> MPT                 ← "alloc_2 wins" (:244-245)

Every future entry point (a new loader, a new merge site) fails silently toward MPT, producing a root no client can reproduce. Related to #3246 's bug. Note the precedence rule is doing no real work today:hive.pymerges and then immediately re-seeds anyway.

Why not make the default None and raise in _state_module() when a root is computed on an unseeded alloc?? Maybe there's better ways. But this definitely needs some attention

[CPerezz]

Why did we get rid of model_dump?

merged[address] = merged_account calls _require_construction, so a LIVE left-hand alloc raises. My repro used an overlapping account, but by the code above the set happens for every non-empty alloc_2 entry, so it generalizes: merging a LIVE alloc with any non-empty right side raises.

• The unconditional migrate_state_commitment hits the FROZEN guard, so a FROZEN left-hand alloc raises even when both sides are MPT and no scheme change was asked for.

Did a repro with a couple agents and hit these:

the old model_dump() merge was phase-immune; no production caller triggers them today, so latent API regressions):

• F3a: Alloc.merge(frozen, x) raises RuntimeError: migrate_state_commitment not allowed: Alloc is FROZEN — the unconditional migrate at account_types.py:241-245 runs on the deep copy that inherited alloc_1's phase, even with no scheme change requested.
• F3b: Alloc.merge(live, non_empty) raises RuntimeError: __setitem__ not allowed: Alloc is in phase LIVE — merged[address] = … hits _require_construction; any PreState read (e.g. get_account_optional) flips an alloc to LIVE first.

Paste-ready tests:

import pytest
from execution_testing.base_types import Account, Address
from execution_testing.test_types.account_types import Alloc


def test_merge_frozen_alloc_raises():  # F3a — worked before #3279
    frozen = Alloc({})
    frozen.freeze()
    with pytest.raises(RuntimeError, match="FROZEN"):
        Alloc.merge(frozen, Alloc({}))


def test_merge_live_alloc_raises():  # F3b — worked before #3279
    addr = Address(0x1234)
    live = Alloc({addr: Account(balance=1)})
    live.get_account_optional(bytes(addr))  # any PreState read -> LIVE
    with pytest.raises(RuntimeError, match="LIVE"):
        Alloc.merge(live, Alloc({addr: Account(balance=2)}))

Not sure there are any callers to freeze in the repo. But still the API is there so we should handle this correctly.

[marioevz]

model_copy preserves the _state_commitment, so it's a way to avoid setting it again.

Plus, dumping to JSON to read it again in a function that is this widely used was a bad idea from the beginning.

I suggest we keep this as is, and if there's an use case to merging two frozen Allocs we can revisit the suggestion.

[spencer-tb]

LGTM from my side as it stands.

PR/commit: marioevz#9, if you want to add the --state-trie binary flag to the fill command :)

[marioevz]

execute_types.py:371

Yes, I saw this and thought that mainnet/testnets would never have PBT genesis roots, but I didn't think of devnets. It's an easy fix, so I'll apply it. Thanks!

[marioevz]

@kevaundray @CPerezz I've applied all the comments. Please take a look. Thanks!

[CPerezz]

LGTM! Great work!

Thanks for doing it that fast!