erigontech / erigontech/erigon

cmd/evm statetest runs on the serial IntraBlockState, so state tests cannot catch versioned-path defects

Open
#23,679 4 comments 1 reaction 0 assignees View on GitHub
QA
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

## Who found this

@chfast, while reviewing #23672. He wrote 45 EEST fixtures targeting the `Empty()` bug fixed in that PR, and noticed something odd: **all 45 pass as state tests on unfixed `main`, and fail as blockchain tests.** That is what led to this issue.

## What the problem is

State tests and block execution do not run on the same state implementation.

`cmd/evm statetest` builds its state with `state.New()`, which is the **serial** `IntraBlockState`. Block execution uses the **versioned** one, through the parallel executor. So if a bug lives only on the versioned path, no state test can see it — no matter how many we run.

The single line is [`execution/tests/testutil/state_test_util.go#L318`](https://github.com/erigontech/erigon/blob/67da19c18e77b24b58ed4deb7f1c8f2596d65217/execution/tests/testutil/state_test_util.go#L318), inside `RunNoVerify`, which serves both `cmd/evm statetest` and the Go `TestState` suites:

```go
statedb = state.New(r)
```

I checked how far this reaches: `grep NewWithVersionMap execution/tests/` returns **no hits at all**. Nothing in that tree builds a versioned `IntraBlockState`. Blockchain tests catch versioned-path bugs only because they run the real staged-sync pipeline, not because the test harness itself knows about the versioned path.

## Why it matters

The bug in #23672 is a good example of what this costs. It is a live consensus divergence on forks that are already deployed:

- `EXTCODEHASH` returns the hash of empty code instead of `0`, breaking EIP-1052, from ConstantinopleFix onwards.
- `CALL` and `SELFDESTRUCT` both skip a 25000 gas charge, from SpuriousDragon onwards.

It sat in `main` while a very large state-test suite ran against it the whole time, including the 108593 legacy state tests. Only a blockchain test could ever have caught it.

To be clear, this is **not** a CI misconfiguration. As @chfast notes, `blocktests-*` already runs at `max-allowed-failures: 0` and would have caught this the day a covering fixture existed. There simply was not one. The real problem is that a green state-test run looks like broad coverage, when it says nothing about the execution path the client actually uses for blocks.

## I tried the obvious fix — it is not enough

Swapping the constructor is genuinely one line. Note `noMaterialize` is not needed: `Empty()` and friends branch on `versionMap != nil`, so a version map alone puts the harness on the versioned path while keeping the stateObject cache.

```go
- statedb = state.New(r)
+ statedb = state.NewWithVersionMap(r, state.NewVersionMap(nil))
```

It compiles and the suite runs, but two corner cases then fail with post-state-root mismatches:

```
--- FAIL: TestStateCornerCases/CallNonExistingAccount.json/Prague/0
post state root mismatch: got 5150b713…, want 5044a37d…
--- FAIL: TestStateCornerCases/SingletonStorageCell_UpdateKindPropagate_AllTheWayUpToRoot.json/Osaka/0
post state root mismatch: got 0821a23a…, want 55a777fb…
```

The likely reason is that the harness would then execute on the versioned path but still write out through the serial one. `FinalizeTx` iterates `sdb.stateObjects`, whereas exec3 derives its writes from `versionedWrites` via `FinalizedWrites` and `Normalize`, which is where the EIP-161 empty-account rules get applied. `CallNonExistingAccount` is a value transfer that creates a new account, so it lands squarely on that difference.

To be clear about what I have and have not shown: I have **not** established whether those two failures are harness gaps or genuine versioned-path bugs. That distinction matters and should be settled before anyone reads them either way — either the harness needs the exec3 write path, or these are two more real defects. Both are worth knowing.

So the work is in the write-out plumbing, not the constructor.

## Possible ways forward

1. Give `RunNoVerify` a versioned mode with the matching write path, and run the corpus in both. Best coverage; the two failures above are the first thing to resolve.
2. If that is too invasive, document the limitation clearly, so nobody reads a green state-test run as evidence about versioned-path behaviour.
3. Either way, prefer blockchain tests when writing regression coverage for state-model bugs. @chfast's 45 fixtures are the right shape for this.

Related: #23670, #23672, #23673.

Contributor guide

Open the contributing guide

Research direction

Start in execution/tests/testutil/state_test_util.go at RunNoVerify, then compare the serial state construction and write-out with the versioned path described in the issue. Run TestStateCornerCases, including CallNonExistingAccount and SingletonStorageCell_UpdateKindPropagate_AllTheWayUpToRoot, and use the 45 fixtures to validate behavior. Done means the harness and its write path consistently test the intended versioned execution behavior, or the limitation and failing cases are documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
blockchain, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.