From 2052707b4369f9e5aa64e4a7b57a88bd5ea8e01c Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 16 May 2026 09:24:42 +0000 Subject: [PATCH 01/28] execution: explicitly set parallel/sequential blocktests/enginextests shards in ci --- .claude/skills/erigon-implement-eip/SKILL.md | 7 ++--- .claude/skills/erigon-test-all/SKILL.md | 5 ++-- tools/eest-spec-shards.json | 10 +++++-- tools/run-eest-spec-test.sh | 28 +++++++++++++------- 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/.claude/skills/erigon-implement-eip/SKILL.md b/.claude/skills/erigon-implement-eip/SKILL.md index a80b9d787a0..90a91c697fe 100644 --- a/.claude/skills/erigon-implement-eip/SKILL.md +++ b/.claude/skills/erigon-implement-eip/SKILL.md @@ -116,9 +116,10 @@ Run local tests using the `/erigon-test-all` skill. Analyse and fix any failures The most important tests when implementing a new EIP for the EL are the EEST spec test shards, exercised by the `cmd/evm` runners (`statetest`, `blocktest`, `enginextest`) via the Makefile targets: - `make eest-spec-statetests-stable` / `…-devnet` — state-tests against the stable/devnet EEST fixtures -- `make eest-spec-blocktests-stable` / `…-devnet` — blockchain-tests against the stable/devnet EEST fixtures. The devnet shard always runs under `ERIGON_EXEC3_PARALLEL=true` (the in-development hardfork requires it); the stable shard runs serial exec3. -- `make eest-spec-blocktests-stable-parallel` — same as `…-stable` but with `ERIGON_EXEC3_PARALLEL=true`; useful for catching parallel-only regressions on stable fixtures. -- `make eest-spec-enginextests-stable` — engine-x tests against the stable EEST fixtures (no devnet variant: the devnet tarball doesn't yet ship `blockchain_tests_engine_x/`). +- `make eest-spec-blocktests-stable-sequential` / `…-devnet` — blockchain-tests against the stable/devnet EEST fixtures. The devnet shard always runs under `ERIGON_EXEC3_PARALLEL=true` (the in-development hardfork requires it); the `…-sequential` shard pins `ERIGON_EXEC3_PARALLEL=false`. +- `make eest-spec-blocktests-stable-parallel` — same fixtures as `…-stable-sequential` but with `ERIGON_EXEC3_PARALLEL=true`; useful for catching parallel-only regressions on stable fixtures. +- `make eest-spec-enginextests-stable-sequential` — engine-x tests against the stable EEST fixtures with `ERIGON_EXEC3_PARALLEL=false`. No devnet variant: the devnet tarball doesn't yet ship `blockchain_tests_engine_x/`. +- `make eest-spec-enginextests-stable-parallel` — same fixtures as `…-stable-sequential` but with `ERIGON_EXEC3_PARALLEL=true`; useful for catching parallel-only regressions on engine-x stable fixtures. - `make eest-spec-enginextests-benchmark-1m` / `-5m` / `-10m` / `-30m` / `-60m` / `-100m` / `-150m` — engine-x tests against the per-gas-target benchmark fixtures, with `--time` per-test stats. - `make eest-spec-blocktests-stable-race-{pre-cancun,cancun,prague,osaka}` and `make eest-spec-blocktests-devnet-race-amsterdam` — race-detector variants split by fork. Each stable-race sub-shard also has a `-parallel` sibling (e.g. `…-race-cancun-parallel`) that exercises parallel exec3 under the race detector. The `blocktests-devnet-race-amsterdam` shard is always parallel (matches the non-race devnet behaviour). diff --git a/.claude/skills/erigon-test-all/SKILL.md b/.claude/skills/erigon-test-all/SKILL.md index 8afbeaa19b8..cf74c086d9e 100644 --- a/.claude/skills/erigon-test-all/SKILL.md +++ b/.claude/skills/erigon-test-all/SKILL.md @@ -15,9 +15,10 @@ To exercise the EEST suites locally, see `erigon-eest-spec` (or run a specific s ```bash make eest-spec-statetests-stable # state tests vs eest_stable fixtures -make eest-spec-blocktests-stable # blockchain tests vs eest_stable fixtures (serial exec3) +make eest-spec-blocktests-stable-sequential # blockchain tests vs eest_stable fixtures (ERIGON_EXEC3_PARALLEL=false) make eest-spec-blocktests-stable-parallel # same, but with ERIGON_EXEC3_PARALLEL=true -make eest-spec-enginextests-stable # engine-x tests vs eest_stable fixtures +make eest-spec-enginextests-stable-sequential # engine-x tests vs eest_stable (ERIGON_EXEC3_PARALLEL=false) +make eest-spec-enginextests-stable-parallel # same, but with ERIGON_EXEC3_PARALLEL=true make eest-spec-statetests-devnet # …vs eest_devnet fixtures make eest-spec-blocktests-devnet # devnet blocktests (always parallel exec3) make eest-spec-enginextests-benchmark-1m # engine-x benchmark fixtures @ 1M gas target diff --git a/tools/eest-spec-shards.json b/tools/eest-spec-shards.json index 53a5dbe9251..4436197472f 100644 --- a/tools/eest-spec-shards.json +++ b/tools/eest-spec-shards.json @@ -10,7 +10,7 @@ "max-allowed-failures": 5253 }, { - "shard": "blocktests-stable", + "shard": "blocktests-stable-sequential", "workers": 12, "max-allowed-failures": 0 }, @@ -27,10 +27,16 @@ "exec3-parallel": true }, { - "shard": "enginextests-stable", + "shard": "enginextests-stable-sequential", "workers": 8, "max-allowed-failures": 0 }, + { + "shard": "enginextests-stable-parallel", + "workers": 8, + "max-allowed-failures": 19, + "exec3-parallel": true + }, { "shard": "enginextests-benchmark-1m", "workers": 1, diff --git a/tools/run-eest-spec-test.sh b/tools/run-eest-spec-test.sh index 7199ead58e1..05b0ca523f3 100755 --- a/tools/run-eest-spec-test.sh +++ b/tools/run-eest-spec-test.sh @@ -7,9 +7,9 @@ # # statetests-stable state tests vs. eest_stable # statetests-devnet state tests vs. eest_devnet -# blocktests-stable blockchain tests vs. eest_stable +# blocktests-stable-sequential blockchain tests vs. eest_stable # blocktests-devnet blockchain tests vs. eest_devnet -# enginextests-stable engine-x tests vs. eest_stable +# enginextests-stable-sequential engine-x tests vs. eest_stable # enginextests-benchmark-{1m,5m,10m,30m,60m,100m,150m} # engine-x benchmark fixtures per # gas-target subdir; each value @@ -18,7 +18,7 @@ # benchmark fixtures # blocktests-stable-race-{pre-cancun,cancun,prague,osaka} # race-detector variant of -# blocktests-stable, split by +# blocktests-stable-sequential, split by # fork via the --run regex so # each sub-shard fits under ~30 # min. Caller (Makefile / CI) must @@ -28,8 +28,13 @@ # blocktests-devnet-race-amsterdam race-detector variant filtered # to the Amsterdam fork only. # *-parallel any of the above with "-parallel" -# appended sets ERIGON_EXEC3_PARALLEL=true -# from the manifest entry. +# appended runs with +# ERIGON_EXEC3_PARALLEL=true. Every +# other shard runs with +# ERIGON_EXEC3_PARALLEL=false so +# the runtime default in +# dbg.Exec3Parallel can flip without +# redefining the shards. # # Each shard maps to one cmd/evm subcommand running with --jsonout. Pass/fail # is decided here (not by the binary, which always exits 0): the shard fails @@ -71,14 +76,17 @@ if [[ -z "$budget_row" ]]; then exit 2 fi IFS=$'\t' read -r default_workers default_max exec3_parallel <<<"$budget_row" -if [[ "$exec3_parallel" == "true" ]]; then - export ERIGON_EXEC3_PARALLEL=true -fi +# Always set ERIGON_EXEC3_PARALLEL explicitly (true or false) so the shard's +# behaviour is pinned to the manifest, independent of whatever dbg.Exec3Parallel +# defaults to at runtime. If the default flips, the shards still run the mode +# they were defined for. +export ERIGON_EXEC3_PARALLEL="$exec3_parallel" -# Strip "-parallel" suffix for case-arm routing — the parallel variant has the -# same fixture path / regex as the non-parallel parent shard; only the +# Strip "-parallel" / "-sequential" suffix for case-arm routing — both variants +# share the same fixture path / regex as the parent shard; only the # ERIGON_EXEC3_PARALLEL env var differs. shard_route="${shard%-parallel}" +shard_route="${shard_route%-sequential}" # Per-shard structural config (cmd / fixture path / extra CLI flags). Match # against shard_route so "-parallel" variants reuse the same arm as their From ffbf5e5ed515ef76b8ba589dd99c5726763bf302 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 16 May 2026 09:41:20 +0000 Subject: [PATCH 02/28] execution: explicitly set parallel/sequential blocktests/enginextests shards in ci --- .claude/skills/erigon-ci/SKILL.md | 2 +- .claude/skills/erigon-implement-eip/SKILL.md | 4 ++-- .claude/skills/erigon-test-all/SKILL.md | 16 ++++++++------ .claude/skills/erigon-test-race/SKILL.md | 2 +- tools/eest-spec-shards.json | 22 ++++++++++---------- tools/run-eest-spec-test.sh | 4 ++-- 6 files changed, 27 insertions(+), 23 deletions(-) diff --git a/.claude/skills/erigon-ci/SKILL.md b/.claude/skills/erigon-ci/SKILL.md index 69b95034493..9addb34f114 100644 --- a/.claude/skills/erigon-ci/SKILL.md +++ b/.claude/skills/erigon-ci/SKILL.md @@ -22,7 +22,7 @@ Each test group has its own dedicated skill for drill-down on failures. Use thos | unit | `erigon-test-unit` | `make test-short` | ~5 min | Pre-push gate | | all | `erigon-test-all` | `GOGC=80 make test-all` | ~30 min | Before PR review | | race | `erigon-test-race` | `make test-all-race` | ~60 min | Concurrency changes | -| eest-spec | *(inline)* | `make eest-spec--` | varies | EEST state/blockchain/engine-x changes | +| eest-spec | *(inline)* | `make eest-spec--[-{sequential,parallel}]` | varies | EEST state/blockchain/engine-x changes (most shards split into `-sequential` / `-parallel` pairs that pin `ERIGON_EXEC3_PARALLEL`; see `tools/eest-spec-shards.json`) | | caplin spec | *(inline)* | `cd cl/spectest && make tests && make mainnet` | ~15 min | CL/consensus changes | | hive | `erigon-test-hive` | `make test-hive` | ~20 min | EL/CL interop changes | | rpc | `erigon-test-rpc` | *(requires synced DB)* | ~10 min | RPC API changes | diff --git a/.claude/skills/erigon-implement-eip/SKILL.md b/.claude/skills/erigon-implement-eip/SKILL.md index 90a91c697fe..7b5a1acab57 100644 --- a/.claude/skills/erigon-implement-eip/SKILL.md +++ b/.claude/skills/erigon-implement-eip/SKILL.md @@ -120,8 +120,8 @@ The most important tests when implementing a new EIP for the EL are the EEST spe - `make eest-spec-blocktests-stable-parallel` — same fixtures as `…-stable-sequential` but with `ERIGON_EXEC3_PARALLEL=true`; useful for catching parallel-only regressions on stable fixtures. - `make eest-spec-enginextests-stable-sequential` — engine-x tests against the stable EEST fixtures with `ERIGON_EXEC3_PARALLEL=false`. No devnet variant: the devnet tarball doesn't yet ship `blockchain_tests_engine_x/`. - `make eest-spec-enginextests-stable-parallel` — same fixtures as `…-stable-sequential` but with `ERIGON_EXEC3_PARALLEL=true`; useful for catching parallel-only regressions on engine-x stable fixtures. -- `make eest-spec-enginextests-benchmark-1m` / `-5m` / `-10m` / `-30m` / `-60m` / `-100m` / `-150m` — engine-x tests against the per-gas-target benchmark fixtures, with `--time` per-test stats. -- `make eest-spec-blocktests-stable-race-{pre-cancun,cancun,prague,osaka}` and `make eest-spec-blocktests-devnet-race-amsterdam` — race-detector variants split by fork. Each stable-race sub-shard also has a `-parallel` sibling (e.g. `…-race-cancun-parallel`) that exercises parallel exec3 under the race detector. The `blocktests-devnet-race-amsterdam` shard is always parallel (matches the non-race devnet behaviour). +- `make eest-spec-enginextests-benchmark-{1m,5m,10m,30m,60m,100m,150m}-{sequential,parallel}` — engine-x tests against the per-gas-target benchmark fixtures, with `--time` per-test stats. Each gas target has a `-sequential` (`ERIGON_EXEC3_PARALLEL=false`) and `-parallel` (`ERIGON_EXEC3_PARALLEL=true`) variant. +- `make eest-spec-blocktests-stable-race-{pre-cancun,cancun,prague,osaka}-{sequential,parallel}` and `make eest-spec-blocktests-devnet-race-amsterdam` — race-detector variants split by fork. Each stable-race sub-shard has a `-sequential` / `-parallel` pair; the `-parallel` siblings exercise parallel exec3 under the race detector. The `blocktests-devnet-race-amsterdam` shard is always parallel (matches the non-race devnet behaviour). The shard list / failure budgets / `exec3-parallel` flags are defined in `tools/eest-spec-shards.json` (single source of truth shared with the CI workflow and the local runner script). See `EEST_SPEC_SHARDS` / `EEST_SPEC_RACE_SHARDS` in the root `Makefile` for the partition into non-race vs race targets. diff --git a/.claude/skills/erigon-test-all/SKILL.md b/.claude/skills/erigon-test-all/SKILL.md index cf74c086d9e..19fec7f2cd9 100644 --- a/.claude/skills/erigon-test-all/SKILL.md +++ b/.claude/skills/erigon-test-all/SKILL.md @@ -21,14 +21,18 @@ make eest-spec-enginextests-stable-sequential # engine-x tests vs eest_stable (E make eest-spec-enginextests-stable-parallel # same, but with ERIGON_EXEC3_PARALLEL=true make eest-spec-statetests-devnet # …vs eest_devnet fixtures make eest-spec-blocktests-devnet # devnet blocktests (always parallel exec3) -make eest-spec-enginextests-benchmark-1m # engine-x benchmark fixtures @ 1M gas target +make eest-spec-enginextests-benchmark-1m-sequential + # engine-x benchmark fixtures @ 1M gas target # (with per-test --time stats); - # -5m/-10m/-30m/-60m/-100m/-150m variants too -make eest-spec-blocktests-stable-race-cancun # race-detector variant, sharded per fork: + # -5m/-10m/-30m/-60m/-100m/-150m variants too, + # each with a "-sequential" / "-parallel" pair +make eest-spec-blocktests-stable-race-cancun-sequential + # race-detector variant, sharded per fork: # -pre-cancun/-cancun/-prague/-osaka, plus - # eest-spec-blocktests-devnet-race-amsterdam - # each also has a "-parallel" sibling - # (e.g. ...-race-cancun-parallel) + # eest-spec-blocktests-devnet-race-amsterdam. + # Each stable-race sub-shard has a + # "-sequential" / "-parallel" pair + # (e.g. ...-race-cancun-{sequential,parallel}) ``` The shard list / failure budgets / `exec3-parallel` flags live in `tools/eest-spec-shards.json` (single source of truth for both this workflow and `tools/run-eest-spec-test.sh`). See `EEST_SPEC_SHARDS` / `EEST_SPEC_RACE_SHARDS` in the root `Makefile` for the partition into race vs non-race targets. diff --git a/.claude/skills/erigon-test-race/SKILL.md b/.claude/skills/erigon-test-race/SKILL.md index 711dfd6d96c..d6cbe8ee6b7 100644 --- a/.claude/skills/erigon-test-race/SKILL.md +++ b/.claude/skills/erigon-test-race/SKILL.md @@ -11,7 +11,7 @@ Runs the full test suite with Go's `-race` flag. Catches concurrency bugs that n `make test-all-race` no longer downloads any fixture tarballs. EEST spec tests (state/blockchain/engine-x) moved out of `go test ./...` and into the dedicated `eest-spec-*` Makefile targets driven by the **EEST spec tests** workflow (`test-eest-spec.yml`); the consensus spec test (`cl/spectest`) is skipped here via `ERIGON_SKIP_CL_SPECTEST=true` (set automatically by the Makefile) and runs only in `test-integration-caplin.yml`. -If you want race coverage on the EEST or consensus spec suites specifically, run them via their dedicated targets (those don't apply `-race` automatically — pass `GOFLAGS='-race'` or invoke `go test -race` against the relevant package directly). +If you want race coverage on the EEST blocktests, use the dedicated race shards — `make eest-spec-blocktests-stable-race-{pre-cancun,cancun,prague,osaka}-{sequential,parallel}` and `make eest-spec-blocktests-devnet-race-amsterdam`. These build a race-instrumented `evm.race` binary automatically (see `EEST_SPEC_RACE_SHARDS` in the root `Makefile`); the `-sequential` / `-parallel` split pins `ERIGON_EXEC3_PARALLEL` so race coverage hits both modes. For the consensus spec suite or other Go packages, pass `GOFLAGS='-race'` or invoke `go test -race` against the relevant package directly. Two side prerequisites still apply for tests `make test-all-race` does run: diff --git a/tools/eest-spec-shards.json b/tools/eest-spec-shards.json index 4436197472f..3b8fb27e248 100644 --- a/tools/eest-spec-shards.json +++ b/tools/eest-spec-shards.json @@ -38,7 +38,7 @@ "exec3-parallel": true }, { - "shard": "enginextests-benchmark-1m", + "shard": "enginextests-benchmark-1m-sequential", "workers": 1, "max-allowed-failures": 0 }, @@ -49,7 +49,7 @@ "exec3-parallel": true }, { - "shard": "enginextests-benchmark-5m", + "shard": "enginextests-benchmark-5m-sequential", "workers": 1, "max-allowed-failures": 0 }, @@ -60,7 +60,7 @@ "exec3-parallel": true }, { - "shard": "enginextests-benchmark-10m", + "shard": "enginextests-benchmark-10m-sequential", "workers": 1, "max-allowed-failures": 0 }, @@ -71,7 +71,7 @@ "exec3-parallel": true }, { - "shard": "enginextests-benchmark-30m", + "shard": "enginextests-benchmark-30m-sequential", "workers": 1, "max-allowed-failures": 0 }, @@ -82,7 +82,7 @@ "exec3-parallel": true }, { - "shard": "enginextests-benchmark-60m", + "shard": "enginextests-benchmark-60m-sequential", "workers": 1, "max-allowed-failures": 0 }, @@ -93,7 +93,7 @@ "exec3-parallel": true }, { - "shard": "enginextests-benchmark-100m", + "shard": "enginextests-benchmark-100m-sequential", "workers": 1, "max-allowed-failures": 0 }, @@ -104,7 +104,7 @@ "exec3-parallel": true }, { - "shard": "enginextests-benchmark-150m", + "shard": "enginextests-benchmark-150m-sequential", "workers": 1, "max-allowed-failures": 0 }, @@ -115,22 +115,22 @@ "exec3-parallel": true }, { - "shard": "blocktests-stable-race-pre-cancun", + "shard": "blocktests-stable-race-pre-cancun-sequential", "workers": 12, "max-allowed-failures": 0 }, { - "shard": "blocktests-stable-race-cancun", + "shard": "blocktests-stable-race-cancun-sequential", "workers": 12, "max-allowed-failures": 0 }, { - "shard": "blocktests-stable-race-prague", + "shard": "blocktests-stable-race-prague-sequential", "workers": 12, "max-allowed-failures": 0 }, { - "shard": "blocktests-stable-race-osaka", + "shard": "blocktests-stable-race-osaka-sequential", "workers": 12, "max-allowed-failures": 0 }, diff --git a/tools/run-eest-spec-test.sh b/tools/run-eest-spec-test.sh index 05b0ca523f3..d000be75474 100755 --- a/tools/run-eest-spec-test.sh +++ b/tools/run-eest-spec-test.sh @@ -10,13 +10,13 @@ # blocktests-stable-sequential blockchain tests vs. eest_stable # blocktests-devnet blockchain tests vs. eest_devnet # enginextests-stable-sequential engine-x tests vs. eest_stable -# enginextests-benchmark-{1m,5m,10m,30m,60m,100m,150m} +# enginextests-benchmark-{1m,5m,10m,30m,60m,100m,150m}-sequential # engine-x benchmark fixtures per # gas-target subdir; each value # maps to one for_osaka_at_M/ # directory under the engine_x # benchmark fixtures -# blocktests-stable-race-{pre-cancun,cancun,prague,osaka} +# blocktests-stable-race-{pre-cancun,cancun,prague,osaka}-sequential # race-detector variant of # blocktests-stable-sequential, split by # fork via the --run regex so From de2cfcab909946046a46b68c6b5ff5bf2ea79673 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 16 May 2026 11:55:43 +0000 Subject: [PATCH 03/28] execution: explicitly set parallel/sequential blocktests/enginextests shards in ci --- .claude/skills/erigon-ci/SKILL.md | 2 ++ .claude/skills/erigon-implement-eip/SKILL.md | 2 ++ .claude/skills/erigon-test-all/SKILL.md | 2 ++ .claude/skills/erigon-test-race/SKILL.md | 2 ++ tools/eest-spec-shards.json | 16 ++++++++-------- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/.claude/skills/erigon-ci/SKILL.md b/.claude/skills/erigon-ci/SKILL.md index 9addb34f114..6364d828c0a 100644 --- a/.claude/skills/erigon-ci/SKILL.md +++ b/.claude/skills/erigon-ci/SKILL.md @@ -30,6 +30,8 @@ Each test group has its own dedicated skill for drill-down on failures. Use thos `make test-all` no longer covers the EEST spec tests or the `cl/spectest` consensus spec test — run those via their dedicated targets above when relevant. +**Pitfall:** prefer `make eest-spec-` over `bash tools/run-eest-spec-test.sh `. The make target lists `evm` / `evm.race` as a prereq so stale binaries get rebuilt; the script invoked directly does not, and a stale binary against current fixtures will inflate failures or hide regressions. + ### Lint (run first — non-deterministic, may need multiple runs) ```bash make lint diff --git a/.claude/skills/erigon-implement-eip/SKILL.md b/.claude/skills/erigon-implement-eip/SKILL.md index 7b5a1acab57..8c5ac052763 100644 --- a/.claude/skills/erigon-implement-eip/SKILL.md +++ b/.claude/skills/erigon-implement-eip/SKILL.md @@ -125,6 +125,8 @@ The most important tests when implementing a new EIP for the EL are the EEST spe The shard list / failure budgets / `exec3-parallel` flags are defined in `tools/eest-spec-shards.json` (single source of truth shared with the CI workflow and the local runner script). See `EEST_SPEC_SHARDS` / `EEST_SPEC_RACE_SHARDS` in the root `Makefile` for the partition into non-race vs race targets. +**Pitfall: stale `evm` / `evm.race` binary.** When iterating on an EIP implementation, always invoke shards via `make eest-spec-` rather than `bash tools/run-eest-spec-test.sh ` — the make target lists `evm` (or `evm.race`) as a prereq and `go build` is cache-aware, so a fresh binary is built before each run. The script invoked directly **bypasses** that rebuild, so the runners exercise whatever `build/bin/evm{,.race}` happens to be on disk against current fixtures — silently inflating failures (e.g. devnet shards "regressing" by thousands of tests) or hiding regressions when comparing budgets before/after a change. + For an EIP on a hardfork under development, the **`-devnet`** shards are the primary signal; the `-stable` shards (and `-benchmark-*` shards, where applicable) are regression checks against prior hardforks. ### Where the test fixtures come from diff --git a/.claude/skills/erigon-test-all/SKILL.md b/.claude/skills/erigon-test-all/SKILL.md index 19fec7f2cd9..0d455a4b928 100644 --- a/.claude/skills/erigon-test-all/SKILL.md +++ b/.claude/skills/erigon-test-all/SKILL.md @@ -37,6 +37,8 @@ make eest-spec-blocktests-stable-race-cancun-sequential The shard list / failure budgets / `exec3-parallel` flags live in `tools/eest-spec-shards.json` (single source of truth for both this workflow and `tools/run-eest-spec-test.sh`). See `EEST_SPEC_SHARDS` / `EEST_SPEC_RACE_SHARDS` in the root `Makefile` for the partition into race vs non-race targets. +**Pitfall: stale `evm` / `evm.race` binary.** Always invoke shards via `make eest-spec-` — the Makefile lists `evm` (or `evm.race`) as a prereq and `go build` is cache-aware, so a stale binary gets rebuilt automatically. Calling `bash tools/run-eest-spec-test.sh ` directly **bypasses** the rebuild and silently exercises whatever `build/bin/evm{,.race}` happens to be on disk against current fixtures, inflating failures or hiding regressions. After pulling code, switching branches, or any time you suspect the binary is older than HEAD: `rm -f build/bin/evm build/bin/evm.race && make evm evm.race` before re-running. + Two side prerequisites still apply for tests `make test-all` does run: ```bash diff --git a/.claude/skills/erigon-test-race/SKILL.md b/.claude/skills/erigon-test-race/SKILL.md index d6cbe8ee6b7..a4e792a5c27 100644 --- a/.claude/skills/erigon-test-race/SKILL.md +++ b/.claude/skills/erigon-test-race/SKILL.md @@ -13,6 +13,8 @@ Runs the full test suite with Go's `-race` flag. Catches concurrency bugs that n If you want race coverage on the EEST blocktests, use the dedicated race shards — `make eest-spec-blocktests-stable-race-{pre-cancun,cancun,prague,osaka}-{sequential,parallel}` and `make eest-spec-blocktests-devnet-race-amsterdam`. These build a race-instrumented `evm.race` binary automatically (see `EEST_SPEC_RACE_SHARDS` in the root `Makefile`); the `-sequential` / `-parallel` split pins `ERIGON_EXEC3_PARALLEL` so race coverage hits both modes. For the consensus spec suite or other Go packages, pass `GOFLAGS='-race'` or invoke `go test -race` against the relevant package directly. +**Pitfall: stale `evm.race` binary.** `make eest-spec-` lists `evm.race` as a prereq and `go build` is cache-aware, so a stale binary gets rebuilt. Calling `bash tools/run-eest-spec-test.sh ` directly with `EVM_BIN=build/bin/evm.race` **bypasses** the rebuild and silently runs an old race-instrumented binary against current fixtures — race reports against code that no longer exists, missed races against code that does. After pulling or switching branches: `rm -f build/bin/evm.race && make evm.race` before re-running. + Two side prerequisites still apply for tests `make test-all-race` does run: ```bash diff --git a/tools/eest-spec-shards.json b/tools/eest-spec-shards.json index e9c4ce810c5..2fba107d86a 100644 --- a/tools/eest-spec-shards.json +++ b/tools/eest-spec-shards.json @@ -17,13 +17,13 @@ { "shard": "blocktests-stable-parallel", "workers": 12, - "max-allowed-failures": 20, + "max-allowed-failures": 2, "exec3-parallel": true }, { "shard": "blocktests-devnet", "workers": 12, - "max-allowed-failures": 25, + "max-allowed-failures": 5, "exec3-parallel": true }, { @@ -34,7 +34,7 @@ { "shard": "enginextests-stable-parallel", "workers": 8, - "max-allowed-failures": 19, + "max-allowed-failures": 11, "exec3-parallel": true }, { @@ -137,31 +137,31 @@ { "shard": "blocktests-stable-race-pre-cancun-parallel", "workers": 12, - "max-allowed-failures": 259, + "max-allowed-failures": 2, "exec3-parallel": true }, { "shard": "blocktests-stable-race-cancun-parallel", "workers": 12, - "max-allowed-failures": 140, + "max-allowed-failures": 0, "exec3-parallel": true }, { "shard": "blocktests-stable-race-prague-parallel", "workers": 12, - "max-allowed-failures": 136, + "max-allowed-failures": 0, "exec3-parallel": true }, { "shard": "blocktests-stable-race-osaka-parallel", "workers": 12, - "max-allowed-failures": 136, + "max-allowed-failures": 0, "exec3-parallel": true }, { "shard": "blocktests-devnet-race-amsterdam", "workers": 12, - "max-allowed-failures": 60, + "max-allowed-failures": 3, "exec3-parallel": true } ] From c593f3af604bf1ceaa0667884ff516faadb59e38 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 16 May 2026 13:26:23 +0000 Subject: [PATCH 04/28] log error --- tools/run-eest-spec-test.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/run-eest-spec-test.sh b/tools/run-eest-spec-test.sh index d000be75474..08d76a03b9d 100755 --- a/tools/run-eest-spec-test.sh +++ b/tools/run-eest-spec-test.sh @@ -185,7 +185,10 @@ if (( total == 0 )); then fi if (( failed > max )); then echo "ERROR: $failed failures exceed max-allowed $max" >&2 - jq -r '.[] | select(.pass == false) | " FAIL " + .name' "$result_file" >&2 + # Emit the cmd/evm `error` field per failing test, not just the name, + # so transient CI flakes are diagnosable from the job log alone (the + # raw JSON output is dropped after this script exits). + jq -r '.[] | select(.pass == false) | " FAIL " + .name + "\n error: " + (.error // "")' "$result_file" >&2 exit 1 fi exit 0 From 9d5fa7b30cec8914f1d06299e6dc6e95b4b6099c Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sun, 17 May 2026 15:49:43 +1000 Subject: [PATCH 05/28] EXEC3_PARALLEL=true by default --- common/dbg/experiments.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/dbg/experiments.go b/common/dbg/experiments.go index f19a996fb26..a413932b420 100644 --- a/common/dbg/experiments.go +++ b/common/dbg/experiments.go @@ -78,7 +78,7 @@ var ( CaplinSyncedDataMangerDeadlockDetection = EnvBool("CAPLIN_SYNCED_DATA_MANAGER_DEADLOCK_DETECTION", false) - Exec3Parallel = EnvBool("EXEC3_PARALLEL", false) + Exec3Parallel = EnvBool("EXEC3_PARALLEL", true) numWorkers = runtime.NumCPU() / 2 Exec3Workers = EnvInt("EXEC3_WORKERS", numWorkers) ExecTerseLoggerLevel = EnvInt("EXEC_TERSE_LOGGER_LEVEL", int(log.LvlWarn)) From 61fd1c5f65434eefc3cc9505c78a8a7f173f9fb5 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Mon, 18 May 2026 13:33:58 +1000 Subject: [PATCH 06/28] set all eest spec test shards to max-allowed-failures: 0 --- tools/eest-spec-shards.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/eest-spec-shards.json b/tools/eest-spec-shards.json index 2fba107d86a..1cdf00c980e 100644 --- a/tools/eest-spec-shards.json +++ b/tools/eest-spec-shards.json @@ -17,13 +17,13 @@ { "shard": "blocktests-stable-parallel", "workers": 12, - "max-allowed-failures": 2, + "max-allowed-failures": 0, "exec3-parallel": true }, { "shard": "blocktests-devnet", "workers": 12, - "max-allowed-failures": 5, + "max-allowed-failures": 0, "exec3-parallel": true }, { @@ -34,7 +34,7 @@ { "shard": "enginextests-stable-parallel", "workers": 8, - "max-allowed-failures": 11, + "max-allowed-failures": 0, "exec3-parallel": true }, { @@ -137,7 +137,7 @@ { "shard": "blocktests-stable-race-pre-cancun-parallel", "workers": 12, - "max-allowed-failures": 2, + "max-allowed-failures": 0, "exec3-parallel": true }, { @@ -161,7 +161,7 @@ { "shard": "blocktests-devnet-race-amsterdam", "workers": 12, - "max-allowed-failures": 3, + "max-allowed-failures": 0, "exec3-parallel": true } ] From 62bd0cf500767752608f006756b633c2ed86a00e Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Mon, 18 May 2026 09:58:34 +0000 Subject: [PATCH 07/28] cmd/evm: speedup statetest by setting up 1 mdbx env per file --- cmd/evm/staterunner.go | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/cmd/evm/staterunner.go b/cmd/evm/staterunner.go index 7707b200ae2..67a6834be4a 100644 --- a/cmd/evm/staterunner.go +++ b/cmd/evm/staterunner.go @@ -34,7 +34,6 @@ import ( "github.com/erigontech/erigon/common/dir" "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/db/datadir" - "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/kv/temporal/temporaltest" "github.com/erigontech/erigon/execution/tests/testutil" "github.com/erigontech/erigon/execution/tracing/tracers/logger" @@ -200,6 +199,20 @@ func runStateTest(ctx *cli.Context, cfg vm.Config, fname string) ([]testResult, emitStateRoot := ctx.Uint64(WorkersFlag.Name) == 1 results := make([]testResult, 0, len(stateTests)) + // One temp datadir + DB per file; per-subtest isolation comes from a + // fresh write tx that is always rolled back. Mirrors the Go-test pattern + // in execution/tests/state_test.go and avoids re-opening MDBX (and its + // aggregator) for every subtest, which dominated wall time on + // short-EVM-work subtests. + tmpDir, err := os.MkdirTemp("", "erigon-statetest-*") + if err != nil { + return nil, err + } + defer dir.RemoveAll(tmpDir) + dirs := datadir.New(tmpDir) + db := temporaltest.NewTestDB(nil, dirs) + defer db.Close() + for key, test := range stateTests { if !re.MatchString(key) { continue @@ -207,17 +220,14 @@ func runStateTest(ctx *cli.Context, cfg vm.Config, fname string) ([]testResult, for _, st := range test.Subtests() { result := &testResult{Name: key, Fork: st.Fork, Pass: true} - // Create fresh DB per subtest to avoid state pollution - tmpDir, err := os.MkdirTemp("", "erigon-statetest-*") - if err != nil { - result.Pass, result.Error = false, err.Error() - results = append(results, *result) - continue - } - dirs := datadir.New(tmpDir) - db := temporaltest.NewTestDB(nil, dirs) + func() { + tx, err := db.BeginTemporalRw(context.Background()) + if err != nil { + result.Pass, result.Error = false, err.Error() + return + } + defer tx.Rollback() - err = db.UpdateTemporal(context.Background(), func(tx kv.TemporalRwTx) error { statedb, root, err := test.Run(nil, tx, st, cfg, dirs) if err != nil { result.Pass, result.Error = false, err.Error() @@ -238,14 +248,7 @@ func runStateTest(ctx *cli.Context, cfg vm.Config, fname string) ([]testResult, }) result.Stats = &stats } - return nil - }) - if err != nil && result.Pass { - result.Pass, result.Error = false, err.Error() - } - - db.Close() - dir.RemoveAll(tmpDir) + }() results = append(results, *result) } From a0d6bb8d8d332cb6b982deafe777b854ee516f4b Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Mon, 18 May 2026 12:04:23 +0000 Subject: [PATCH 08/28] stylistic only do 1 sd.ComputeCommitment --- execution/tests/testutil/state_test_util.go | 45 ++++++++++----------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/execution/tests/testutil/state_test_util.go b/execution/tests/testutil/state_test_util.go index fa63b132187..017d5fa82f0 100644 --- a/execution/tests/testutil/state_test_util.go +++ b/execution/tests/testutil/state_test_util.go @@ -274,7 +274,7 @@ func (t *StateTest) Run(tb testing.TB, tx kv.TemporalRwTx, subtest StateSubtest, } // RunNoVerify runs a specific subtest and returns the statedb, post-state root and gas used. -func (t *StateTest) RunNoVerify(tb testing.TB, tx kv.TemporalRwTx, subtest StateSubtest, vmconfig vm.Config, dirs datadir.Dirs) (*state.IntraBlockState, common.Hash, uint64, error) { +func (t *StateTest) RunNoVerify(tb testing.TB, tx kv.TemporalRwTx, subtest StateSubtest, vmconfig vm.Config, dirs datadir.Dirs) (statedb *state.IntraBlockState, root common.Hash, gasUsed uint64, err error) { config, eips, err := GetChainConfig(subtest.Fork) if err != nil { return nil, common.Hash{}, 0, testforks.UnsupportedForkError{Name: subtest.Fork} @@ -300,20 +300,22 @@ func (t *StateTest) RunNoVerify(tb testing.TB, tx kv.TemporalRwTx, subtest State defer domains.Close() blockNum, txNum := readBlockNr, uint64(1) - // Pre-state root, computed once over the just-flushed pre-allocation. - // We return it for any early-return path so the test framework's - // `post.Root` check still has a real root to compare against when the tx - // was rejected before / during apply (the fixture's post.Root for those - // cases equals the pre-state root). - preRootBytes, err := domains.ComputeCommitment(context2.Background(), tx, true, blockNum, txNum, "", nil) - if err != nil { - return nil, common.Hash{}, 0, fmt.Errorf("pre-state ComputeCommitment: %w", err) - } - preRoot := common.BytesToHash(preRootBytes) + defer func() { + rootBytes, rootBytesErr := domains.ComputeCommitment(context2.Background(), tx, true, blockNum, txNum, "", nil) + if rootBytesErr != nil { + if err != nil { + err = fmt.Errorf("ComputeCommitment: %w: %w", rootBytesErr, err) + } else { + err = fmt.Errorf("ComputeCommitment: %w", rootBytesErr) + } + return + } + root = common.BytesToHash(rootBytes) + }() r := rpchelper.NewLatestStateReader(tx) w := rpchelper.NewLatestStateWriter(tx, domains, (*freezeblocks.BlockReader)(nil), writeBlockNr) - statedb := state.New(r) + statedb = state.New(r) var baseFee *uint256.Int if config.IsLondon(0) { @@ -363,16 +365,16 @@ func (t *StateTest) RunNoVerify(tb testing.TB, tx kv.TemporalRwTx, subtest State signer.SetMalleable(true) // allow Frontier/Homestead malleable signatures decodedTx, err := types.DecodeTransaction(post.Tx) if err != nil { - return statedb, preRoot, 0, err + return statedb, root, 0, err } msg, err = decodedTx.AsMessage(*signer, baseFee, chainRules) if err != nil { - return statedb, preRoot, 0, err + return statedb, root, 0, err } } else { msg, err = toMessage(t.Json.Tx, post, baseFee) if err != nil { - return statedb, preRoot, 0, err + return statedb, root, 0, err } } @@ -388,7 +390,6 @@ func (t *StateTest) RunNoVerify(tb testing.TB, tx kv.TemporalRwTx, subtest State gaspool := new(protocol.GasPool) gaspool.AddGas(block.GasLimit()).AddBlobGas(config.GetMaxBlobGasPerBlock(header.Time)) res, err := protocol.ApplyMessage(evm, msg, gaspool, true /* refunds */, false /* gasBailout */, nil /* engine */) - gasUsed := uint64(0) if res != nil { gasUsed = res.ReceiptGasUsed } @@ -400,7 +401,7 @@ func (t *StateTest) RunNoVerify(tb testing.TB, tx kv.TemporalRwTx, subtest State vmconfig.Tracer.OnTxEnd(&types.Receipt{GasUsed: gasUsed}, nil) } if err != nil { - return statedb, preRoot, gasUsed, err + return statedb, root, gasUsed, err } // Add 0-value mining reward. This only makes a difference in the cases @@ -410,17 +411,13 @@ func (t *StateTest) RunNoVerify(tb testing.TB, tx kv.TemporalRwTx, subtest State statedb.AddBalance(accounts.InternAddress(t.Json.Env.Coinbase), *uint256.NewInt(0), tracing.BalanceChangeUnspecified) if err = statedb.FinalizeTx(evm.ChainRules(), w); err != nil { - return nil, common.Hash{}, gasUsed, err + return nil, root, gasUsed, err } if err = statedb.CommitBlock(evm.ChainRules(), w); err != nil { - return nil, common.Hash{}, gasUsed, err + return nil, root, gasUsed, err } - rootBytes, err := domains.ComputeCommitment(context2.Background(), tx, true, blockNum, txNum, "", nil) - if err != nil { - return statedb, common.Hash{}, gasUsed, fmt.Errorf("ComputeCommitment: %w", err) - } - return statedb, common.BytesToHash(rootBytes), gasUsed, nil + return statedb, root, gasUsed, nil } func MakePreState(rules *chain.Rules, tx kv.TemporalRwTx, alloc types.GenesisAlloc, blockNr uint64) (*state.IntraBlockState, error) { From d12d29a4d6ddcd852c76172ec2320534d4119f21 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Tue, 19 May 2026 12:54:19 +1000 Subject: [PATCH 09/28] execution: eest spec shards 0 max-allowed-failures --- tools/eest-spec-shards.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/eest-spec-shards.json b/tools/eest-spec-shards.json index 2fba107d86a..1cdf00c980e 100644 --- a/tools/eest-spec-shards.json +++ b/tools/eest-spec-shards.json @@ -17,13 +17,13 @@ { "shard": "blocktests-stable-parallel", "workers": 12, - "max-allowed-failures": 2, + "max-allowed-failures": 0, "exec3-parallel": true }, { "shard": "blocktests-devnet", "workers": 12, - "max-allowed-failures": 5, + "max-allowed-failures": 0, "exec3-parallel": true }, { @@ -34,7 +34,7 @@ { "shard": "enginextests-stable-parallel", "workers": 8, - "max-allowed-failures": 11, + "max-allowed-failures": 0, "exec3-parallel": true }, { @@ -137,7 +137,7 @@ { "shard": "blocktests-stable-race-pre-cancun-parallel", "workers": 12, - "max-allowed-failures": 2, + "max-allowed-failures": 0, "exec3-parallel": true }, { @@ -161,7 +161,7 @@ { "shard": "blocktests-devnet-race-amsterdam", "workers": 12, - "max-allowed-failures": 3, + "max-allowed-failures": 0, "exec3-parallel": true } ] From 46963075def91e9b77302cbd970c4787f5cf204c Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Tue, 19 May 2026 03:22:50 +0000 Subject: [PATCH 10/28] tidy --- cmd/evm/staterunner.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/cmd/evm/staterunner.go b/cmd/evm/staterunner.go index 67a6834be4a..850fdeb8a16 100644 --- a/cmd/evm/staterunner.go +++ b/cmd/evm/staterunner.go @@ -199,11 +199,7 @@ func runStateTest(ctx *cli.Context, cfg vm.Config, fname string) ([]testResult, emitStateRoot := ctx.Uint64(WorkersFlag.Name) == 1 results := make([]testResult, 0, len(stateTests)) - // One temp datadir + DB per file; per-subtest isolation comes from a - // fresh write tx that is always rolled back. Mirrors the Go-test pattern - // in execution/tests/state_test.go and avoids re-opening MDBX (and its - // aggregator) for every subtest, which dominated wall time on - // short-EVM-work subtests. + // One temp datadir & DB per file; per-subtest isolation comes from tx rollback. tmpDir, err := os.MkdirTemp("", "erigon-statetest-*") if err != nil { return nil, err From 6aae39b4fe4ec3d3191d79d086b83bfe949f3b2b Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Tue, 19 May 2026 07:31:42 +0000 Subject: [PATCH 11/28] execution: implement 8037 changes for 7.2.0 fixtures --- execution/protocol/mdgas/md_gas.go | 41 +++++++++++++---- execution/protocol/txn_executor.go | 21 ++++----- execution/vm/evm.go | 71 ++++++++++++++++++------------ execution/vm/instructions.go | 23 +++------- execution/vm/interpreter.go | 59 +++++++++++-------------- test-fixtures.json | 6 +-- 6 files changed, 118 insertions(+), 103 deletions(-) diff --git a/execution/protocol/mdgas/md_gas.go b/execution/protocol/mdgas/md_gas.go index 11f42a6c02f..efc240c6297 100644 --- a/execution/protocol/mdgas/md_gas.go +++ b/execution/protocol/mdgas/md_gas.go @@ -23,7 +23,8 @@ import ( "github.com/erigontech/erigon/execution/protocol/params" ) -// MdGas represents multi-dimensional gas (regular + state). +// MdGas represents multi-dimensional gas (regular + state) for reservoirs +// and leftover balances. Both fields are non-negative by construction. type MdGas struct { Regular uint64 State uint64 @@ -35,15 +36,15 @@ type FullMdGas struct { Blob uint64 } -// MdGasUsage extends MdGas with EIP-8037 inline state-gas refund credit that -// hasn't yet been applied to any frame's reservoir. The credit walks up the -// call stack via this field (propagated by Run/Call/Create return values) until -// an ancestor with positive frame state usage absorbs it, or TxnExecutor reads -// it directly from the top-level Call/Create return on success. Revert paths -// drop it. +// MdGasUsage reports per-frame gas usage with a signed State component. +// State is the net state-gas charged minus inline state-gas refunds +// credited (per EIP-8037): it goes negative when refunds in a frame +// exceed its own charges — i.e. an SSTORE/CREATE clear refund inside +// a callee that matches a charge an ancestor made (sharing storage via +// CALLCODE/DELEGATECALL, or against a tx-level intrinsic state charge). type MdGasUsage struct { - MdGas - PendingStateGasCredit uint64 + Regular uint64 + State int64 } func NewFullMdGas(regular, state, blob uint64) FullMdGas { @@ -61,6 +62,28 @@ func (g MdGas) Total() uint64 { return g.Regular + g.State } +// PlusIntrinsic folds an intrinsic-gas MdGas into the frame-usage report, +// preserving signed semantics on State so a net state refund stays +// negative in the combined value. +func (u MdGasUsage) PlusIntrinsic(igas MdGas) MdGasUsage { + return MdGasUsage{ + Regular: u.Regular + igas.Regular, + State: u.State + int64(igas.State), + } +} + +// StateClamped returns max(0, State) as uint64. State is signed to model +// EIP-8037 inline refunds; for block accounting (block_state_gas_used) +// the spec uses max(0, tx_state_gas), so negative net state contributes 0 +// at the block level even though it does reduce the sender's tx_gas_used. +func (u MdGasUsage) StateClamped() uint64 { + return uint64(max(0, u.State)) +} + +func (u MdGasUsage) Total() uint64 { + return u.Regular + u.StateClamped() +} + type MdGasType uint8 const ( diff --git a/execution/protocol/txn_executor.go b/execution/protocol/txn_executor.go index 5f8a5909ad8..642b92ee92c 100644 --- a/execution/protocol/txn_executor.go +++ b/execution/protocol/txn_executor.go @@ -607,16 +607,10 @@ func (st *TxnExecutor) Execute(refunds bool, gasBailout bool) (result *evmtypes. refundQuotient = params.RefundQuotientEIP3529 } if rules.IsAmsterdam { - // EIP-8037 state-gas refund. Frame-local per the spec; the value - // is carried out via Call/Create's gasUsed.PendingStateGasCredit: - // - success: any SSTORE-clear credit that no frame absorbed. - // - error: the top frame's execution state-gas + (for CREATE) - // the intrinsic NEW_ACCOUNT state-gas. Set inside the depth==0 - // defers in evm.call / evm.create. - // State refunds bypass the EIP-3529 quotient cap (regular only). - st.blockStateGasUsed = imdGas.State + gasUsed.State - gasUsed.PendingStateGasCredit - st.blockRegularGasUsed = max(imdGas.Regular+gasUsed.Regular, intrinsicGasResult.FloorGasCost) - st.txnGasUsedB4Refunds = imdGas.Plus(gasUsed.MdGas).Total() - gasUsed.PendingStateGasCredit + combined := gasUsed.PlusIntrinsic(imdGas) + st.blockStateGasUsed = combined.StateClamped() + st.blockRegularGasUsed = max(combined.Regular, intrinsicGasResult.FloorGasCost) + st.txnGasUsedB4Refunds = combined.Total() refund := min(st.txnGasUsedB4Refunds/refundQuotient, st.state.GetRefund()) st.txnGasUsed = max(intrinsicGasResult.FloorGasCost, st.txnGasUsedB4Refunds-refund) } else if rules.IsPrague { @@ -632,9 +626,10 @@ func (st *TxnExecutor) Execute(refunds bool, gasBailout bool) (result *evmtypes. } st.refundGas() } else if rules.IsAmsterdam { - st.blockStateGasUsed = imdGas.State + gasUsed.State - st.blockRegularGasUsed = max(imdGas.Regular+gasUsed.Regular, intrinsicGasResult.FloorGasCost) - st.txnGasUsedB4Refunds = imdGas.Plus(gasUsed.MdGas).Total() + combined := gasUsed.PlusIntrinsic(imdGas) + st.blockStateGasUsed = combined.StateClamped() + st.blockRegularGasUsed = max(combined.Regular, intrinsicGasResult.FloorGasCost) + st.txnGasUsedB4Refunds = combined.Total() st.txnGasUsed = max(st.txnGasUsedB4Refunds, intrinsicGasResult.FloorGasCost) } else { // No-refund path: gasBailout (trace_call) or !refunds. diff --git a/execution/vm/evm.go b/execution/vm/evm.go index f18630a95fb..0e999a92237 100644 --- a/execution/vm/evm.go +++ b/execution/vm/evm.go @@ -159,10 +159,11 @@ func (evm *EVM) Cancelled() bool { return evm.abort.Load() } // state-gas reservoir restoration for child frames. // // At depth 0 there is no parent reservoir to restore to: TxnExecutor reads -// vmerr together with the returned gasUsed.State (and gasUsed.MdGas/intrinsic) -// to derive the receipt and block-state-gas accounting directly, so this -// function only needs the state-revert + regular-burn steps at the top level. -func (evm *EVM) handleFrameRevert(gas *mdgas.MdGas, err error, depth int, snapshot int, childStateConsumed uint64) { +// vmerr together with the returned gasUsed.State (signed net usage) and +// the intrinsic to derive the receipt and block-state-gas accounting +// directly, so this function only needs the state-revert + regular-burn +// steps at the top level. +func (evm *EVM) handleFrameRevert(gas *mdgas.MdGas, err error, depth int, snapshot int, childStateConsumed int64) { // 1. Revert state changes. evm.intraBlockState.RevertToSnapshot(snapshot, err) @@ -175,13 +176,16 @@ func (evm *EVM) handleFrameRevert(gas *mdgas.MdGas, err error, depth int, snapsh gas.Regular = 0 } - // 3. EIP-8037: at depth>0 restore the child's own state-gas charges - // (reservoir-sourced + spill) to the parent's reservoir. Regular gas: - // REVERT preserves it (step 2 didn't apply); exceptional halt burnt it in - // step 2. At depth==0 TxnExecutor handles the user-side refund directly - // from vmerr + gasUsed.State. + // 3. EIP-8037: at depth>0 restore the original child reservoir to the + // parent. Per the spec's incorporate_child_on_error invariant: + // parent.state_gas_left += child.state_gas_used + child.state_gas_left + // (the sum equals what the parent originally passed in, after any + // inline refunds the child credited are dropped along with reverted + // state). Regular gas: REVERT preserves it (step 2 didn't apply); + // exceptional halt burnt it in step 2. At depth==0 TxnExecutor handles + // the user-side refund directly from vmerr + gasUsed.State. if evm.chainRules.IsAmsterdam && depth > 0 { - gas.State += childStateConsumed + gas.State = uint64(int64(gas.State) + childStateConsumed) } } @@ -214,20 +218,26 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts // Derive gasUsed.Regular from the final leftOverGas at function exit, // uniformly across Run / precompile / no-code paths and after any // handleFrameRevert gas burn. gasUsed.State is set by Run's defer - // (frameStateUsed) and is 0 for precompile/no-code frames. + // (signed frameStateUsed = charges − inline refunds) and is 0 for + // precompile/no-code frames. Regular = (input − leftover) − state, + // computed in signed int64 so a negative net state correctly grows + // the regular component (refund credit lands in the reservoir, + // which leftover absorbs back). // - // At depth==0 on error, also route the frame's own execution state-gas - // out via PendingStateGasCredit so TxnExecutor subtracts it uniformly. - // (At depth>0, handleFrameRevert already restored child state to the - // parent's reservoir, and Run's defer dropped PendingStateGasCredit.) + // At depth==0 on error, the spec resets the top-frame's state_gas_used + // to 0 (the refund-on-failure invariant). Mirror that on the returned + // gasUsed.State so TxnExecutor's tx_state_gas computation is uniform + // across success and error paths. (At depth>0, handleFrameRevert + // already restored the child's reservoir to the parent.) inputTotal := gas.Total() defer func() { leftOverTotal := leftOverGas.Total() if leftOverTotal <= inputTotal { - gasUsed.Regular = (inputTotal - leftOverTotal) - gasUsed.State + delta := int64(inputTotal - leftOverTotal) + gasUsed.Regular = uint64(delta - gasUsed.State) } if depth == 0 && evm.chainRules.IsAmsterdam && err != nil { - gasUsed.PendingStateGasCredit = gasUsed.State + gasUsed.State = 0 } }() @@ -464,22 +474,29 @@ func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gasRem // Derive gasUsed.Regular from the final leftOverGas at function exit, // uniformly across all Create exit paths (Run, depth/balance/collision // errors, post-handleFrameRevert gas burn). gasUsed.State is set by Run's - // defer for the initcode frame and stays 0 on early-exit paths. + // defer for the initcode frame (signed net) and stays 0 on early-exit + // paths. Regular = (input − leftover) − state in signed int64 so a + // negative net state correctly grows the regular component. // - // At depth==0 on error, also route the frame's own execution state-gas - // PLUS the intrinsic NEW_ACCOUNT state-gas (paid by the tx but unused - // because the contract was never created) out via PendingStateGasCredit - // so TxnExecutor subtracts both uniformly. The Call counterpart does NOT - // add intrinsic AUTH state-gas — EIP-7702 auth side effects persist even - // on call failure. + // At depth==0 on error, the spec resets the top-frame's state_gas_used + // to 0 AND adds STATE_BYTES_PER_NEW_ACCOUNT * COST_PER_STATE_BYTE to + // state_refund (the contract was never created, so the intrinsic + // NEW_ACCOUNT state-gas is returned). Mirror both by setting + // gasUsed.State = −StateGasNewAccount so TxnExecutor's + // tx_state_gas = intrinsic_state + gasUsed.State naturally yields the + // "intrinsic refunded" outcome (= 0 for a CREATE tx, since its + // intrinsic_state == StateGasNewAccount). The Call counterpart does NOT + // refund intrinsic AUTH state-gas — EIP-7702 auth side effects persist + // even on call failure. inputTotal := gasRemaining.Total() defer func() { leftOverTotal := leftOverGas.Total() if leftOverTotal <= inputTotal { - gasUsed.Regular = (inputTotal - leftOverTotal) - gasUsed.State + delta := int64(inputTotal - leftOverTotal) + gasUsed.Regular = uint64(delta - gasUsed.State) } if depth == 0 && evm.chainRules.IsAmsterdam && err != nil { - gasUsed.PendingStateGasCredit = gasUsed.State + params.StateGasNewAccount + gasUsed.State = -int64(params.StateGasNewAccount) } }() @@ -621,7 +638,7 @@ func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gasRem evm.intraBlockState.SetCode(address, ret) // EIP-8037: post-Run code-deposit state charge counts toward this // frame's state-gas usage. - gasUsed.State += stateGas + gasUsed.State += int64(stateGas) } else { if evm.chainRules.IsAmsterdam { // Code deposit failed: per EIP-8037 the failure cost is diff --git a/execution/vm/instructions.go b/execution/vm/instructions.go index f8905644e85..6b09754f7d2 100644 --- a/execution/vm/instructions.go +++ b/execution/vm/instructions.go @@ -1072,13 +1072,12 @@ func execCreate(pc uint64, evm *EVM, scope *CallContext, value uint256.Int, inpu // added childUsed.State back to gas.State). scope.creditStateGasRefund(params.StateGasNewAccount) } else { - // EIP-8037: child success — fold child's state-gas usage into our - // frame and absorb any unapplied refund credit (SSTORE 0→x→0 inside - // the child) into this frame's reservoir/pending. + // EIP-8037: child success — fold child's net state-gas usage + // (signed: charges − inline refunds the child credited) into + // our frame. The child's reservoir leftover (which holds any + // refunded gas) has already been merged via restoreChildGas + // above. scope.frameStateUsed += childUsed.State - if childUsed.PendingStateGasCredit > 0 { - scope.creditStateGasRefund(childUsed.PendingStateGasCredit) - } } } @@ -1145,9 +1144,6 @@ func opCall(pc uint64, evm *EVM, scope *CallContext) (uint64, []byte, error) { scope.restoreChildGas(returnGas, evm.config.Tracer) if evm.chainRules.IsAmsterdam && err == nil { scope.frameStateUsed += childUsed.State - if childUsed.PendingStateGasCredit > 0 { - scope.creditStateGasRefund(childUsed.PendingStateGasCredit) - } } evm.returnData = ret return pc, ret, nil @@ -1196,9 +1192,6 @@ func opCallCode(pc uint64, evm *EVM, scope *CallContext) (uint64, []byte, error) scope.restoreChildGas(returnGas, evm.config.Tracer) if evm.chainRules.IsAmsterdam && err == nil { scope.frameStateUsed += childUsed.State - if childUsed.PendingStateGasCredit > 0 { - scope.creditStateGasRefund(childUsed.PendingStateGasCredit) - } } evm.returnData = ret return pc, ret, nil @@ -1243,9 +1236,6 @@ func opDelegateCall(pc uint64, evm *EVM, scope *CallContext) (uint64, []byte, er scope.restoreChildGas(returnGas, evm.config.Tracer) if evm.chainRules.IsAmsterdam && err == nil { scope.frameStateUsed += childUsed.State - if childUsed.PendingStateGasCredit > 0 { - scope.creditStateGasRefund(childUsed.PendingStateGasCredit) - } } evm.returnData = ret return pc, ret, nil @@ -1289,9 +1279,6 @@ func opStaticCall(pc uint64, evm *EVM, scope *CallContext) (uint64, []byte, erro scope.restoreChildGas(returnGas, evm.config.Tracer) if evm.chainRules.IsAmsterdam && err == nil { scope.frameStateUsed += childUsed.State - if childUsed.PendingStateGasCredit > 0 { - scope.creditStateGasRefund(childUsed.PendingStateGasCredit) - } } evm.returnData = ret return pc, ret, nil diff --git a/execution/vm/interpreter.go b/execution/vm/interpreter.go index d87639a29fb..8d9a45e3fa9 100644 --- a/execution/vm/interpreter.go +++ b/execution/vm/interpreter.go @@ -59,17 +59,17 @@ func (vmConfig *Config) HasEip3860(rules *chain.Rules) bool { type CallContext struct { gas uint64 stateGas uint64 - // EIP-8037 per-frame state-gas tracker. Increments on every state-gas - // charge (including reservoir→regular spill), decrements on inline refund - // applied to this frame. Returned via gasUsed.State at frame exit. - frameStateUsed uint64 - // EIP-8037 inline state-gas refund credit that couldn't be applied to this - // frame's own usage (creditStateGasRefund excess). Propagates to the caller - // on successful return so an ancestor with own usage can absorb it; on - // revert the frame returns 0 so the credit is dropped. - refundCreditPending uint64 - input []byte - Memory Memory + // EIP-8037 per-frame net state-gas tracker (signed). Increments on every + // state-gas charge (including reservoir→regular spill), decrements on + // inline refund (SSTORE clear, CREATE collision/revert) regardless of + // prior frame usage — the refund credits the local reservoir + // (`stateGas`) immediately and the matching charge it cancels may live + // in an ancestor sharing storage (CALLCODE/DELEGATECALL) or in the + // tx-level intrinsic, so `frameStateUsed` can go negative. Returned via + // gasUsed.State at frame exit. + frameStateUsed int64 + input []byte + Memory Memory // Opcode-scoped key/address intern cache. cacheGen is incremented once per // opcode dispatch in the interpreter loop; cachedKeyGen/cachedAddrGen hold @@ -129,7 +129,6 @@ func getCallContext(contract Contract, input []byte, gas mdgas.MdGas) *CallConte ctx.gas = gas.Regular ctx.stateGas = gas.State ctx.frameStateUsed = 0 - ctx.refundCreditPending = 0 ctx.input = input ctx.Contract = contract return ctx @@ -140,7 +139,6 @@ func (c *CallContext) put() { c.Stack.Reset() c.cacheGen = 0 c.frameStateUsed = 0 - c.refundCreditPending = 0 // Use sentinel values so that a peek call before the first cacheGen++ is // always a miss rather than returning a stale handle from a prior use. c.cachedKeyGen = ^uint64(0) @@ -168,25 +166,23 @@ func (c *CallContext) useMdGas(gas uint64, t mdgas.MdGasType, tracer *tracing.Ho c.gas = remaining.Regular c.stateGas = remaining.State if t == mdgas.StateGas { - c.frameStateUsed += gas + c.frameStateUsed += int64(gas) } } return ok } -// creditStateGasRefund applies an inline state-gas refund, clamped to this -// frame's own state-gas usage. The applied portion grows the frame's reservoir -// (so subsequent state ops can draw from it) and decrements frameStateUsed. -// Any unapplied excess accumulates in refundCreditPending and propagates to the -// caller on successful return; on revert the credit is dropped. +// creditStateGasRefund applies an inline state-gas refund per EIP-8037: +// the full amount credits the local frame's reservoir immediately and the +// frame's net state-gas usage drops by the same amount (going negative +// when the matching charge sits in an ancestor or the tx-level intrinsic). +// On a successful return the refund flows to the caller via the leftover +// reservoir + signed frameStateUsed; on revert the parent's reservoir is +// restored by handleFrameRevert so the refund is dropped along with the +// reverted state changes. func (c *CallContext) creditStateGasRefund(amount uint64) { - applied := amount - if applied > c.frameStateUsed { - applied = c.frameStateUsed - } - c.stateGas += applied - c.frameStateUsed -= applied - c.refundCreditPending += amount - applied + c.stateGas += amount + c.frameStateUsed -= int64(amount) } func useGas(initial uint64, gas uint64, tracer *tracing.Hooks, reason tracing.GasChangeReason) (remaining uint64, ok bool) { @@ -426,15 +422,12 @@ func (evm *EVM) Run(contract Contract, gas mdgas.MdGas, input []byte, readOnly b tracer.OnFault(pcCopy, byte(op), gasCopy, cost, callContext, evm.depth, VMErrorFromErr(err)) } } - // EIP-8037: snapshot gasUsed and pending credit before callContext.put() - // clears them. Pending credit propagates only on success (revert drops it). - // gasUsed.Regular is derived uniformly by evm.call/evm.create's defer from - // the final leftOverGas (covers precompile/no-code paths and + // EIP-8037: snapshot the frame's net state-gas usage (charges minus + // inline refunds, signed) before callContext.put() clears it. + // gasUsed.Regular is derived uniformly by evm.call/evm.create's defer + // from the final leftOverGas (covers precompile/no-code paths and // handleFrameRevert gas burn). gasUsed.State = callContext.frameStateUsed - if err == nil { - gasUsed.PendingStateGasCredit = callContext.refundCreditPending - } // this function must execute _after_: the `CaptureState` needs the stacks before callContext.put() if restoreReadonly { diff --git a/test-fixtures.json b/test-fixtures.json index 85f3409749a..fe2a5cf9116 100644 --- a/test-fixtures.json +++ b/test-fixtures.json @@ -5,9 +5,9 @@ "size": 1025048557 }, "eest_devnet": { - "url": "https://github.com/ethereum/execution-specs/releases/download/tests-bal%40v7.1.1/fixtures_bal.tar.gz", - "sha256": "181946ec22da60bc5483a8ecb2e7f7dba02ae9c30183e7c02882a9d6a49f6eed", - "size": 609842087, + "url": "https://github.com/ethereum/execution-specs/releases/download/tests-bal%40v7.2.0/fixtures_bal.tar.gz", + "sha256": "fc1d9ae174cdd5db789068839999e6f83666cc79f7dac36e973d7616d9a2e2cf", + "size": 611743632, "branch": "devnets/bal/7" }, "eest_stable": { From aade5c09049991691759bce203c25f619bab678a Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Tue, 19 May 2026 09:59:14 +0000 Subject: [PATCH 12/28] tidy --- execution/protocol/mdgas/md_gas.go | 30 ++++----- execution/vm/evm.go | 101 +++++++++++++++++------------ execution/vm/interpreter.go | 4 +- 3 files changed, 77 insertions(+), 58 deletions(-) diff --git a/execution/protocol/mdgas/md_gas.go b/execution/protocol/mdgas/md_gas.go index efc240c6297..a544153fbe7 100644 --- a/execution/protocol/mdgas/md_gas.go +++ b/execution/protocol/mdgas/md_gas.go @@ -30,6 +30,21 @@ type MdGas struct { State uint64 } +func (g MdGas) Plus(other MdGas) MdGas { + return MdGas{ + Regular: g.Regular + other.Regular, + State: g.State + other.State, + } +} + +func (g MdGas) Total() uint64 { + return g.Regular + g.State +} + +func NewFullMdGas(regular, state, blob uint64) FullMdGas { + return FullMdGas{MdGas: MdGas{Regular: regular, State: state}, Blob: blob} +} + // FullMdGas extends MdGas with blob gas. type FullMdGas struct { MdGas @@ -47,21 +62,6 @@ type MdGasUsage struct { State int64 } -func NewFullMdGas(regular, state, blob uint64) FullMdGas { - return FullMdGas{MdGas: MdGas{Regular: regular, State: state}, Blob: blob} -} - -func (g MdGas) Plus(other MdGas) MdGas { - return MdGas{ - Regular: g.Regular + other.Regular, - State: g.State + other.State, - } -} - -func (g MdGas) Total() uint64 { - return g.Regular + g.State -} - // PlusIntrinsic folds an intrinsic-gas MdGas into the frame-usage report, // preserving signed semantics on State so a net state refund stays // negative in the combined value. diff --git a/execution/vm/evm.go b/execution/vm/evm.go index 0e999a92237..230b40cd05e 100644 --- a/execution/vm/evm.go +++ b/execution/vm/evm.go @@ -158,12 +158,11 @@ func (evm *EVM) Cancelled() bool { return evm.abort.Load() } // state revert, regular gas burning on exceptional halt, and EIP-8037 // state-gas reservoir restoration for child frames. // -// At depth 0 there is no parent reservoir to restore to: TxnExecutor reads -// vmerr together with the returned gasUsed.State (signed net usage) and -// the intrinsic to derive the receipt and block-state-gas accounting -// directly, so this function only needs the state-revert + regular-burn -// steps at the top level. -func (evm *EVM) handleFrameRevert(gas *mdgas.MdGas, err error, depth int, snapshot int, childStateConsumed int64) { +// At depth 0 there is no parent reservoir to restore to: the evm.call / +// evm.create depth==0 defers fold the error semantics into gasUsed.State +// (reset to 0 for CALL, -StateGasNewAccount for CREATE) so TxnExecutor +// can read gasUsed.State directly without checking the error. +func (evm *EVM) handleFrameRevert(gasRemaining *mdgas.MdGas, err error, depth int, snapshot int, stateGasUsed int64) { // 1. Revert state changes. evm.intraBlockState.RevertToSnapshot(snapshot, err) @@ -171,9 +170,9 @@ func (evm *EVM) handleFrameRevert(gas *mdgas.MdGas, err error, depth int, snapsh // 2. On exceptional halt (not REVERT), burn remaining regular gas. if err != ErrExecutionReverted { if evm.config.Tracer != nil && evm.config.Tracer.OnGasChange != nil { - evm.config.Tracer.OnGasChange(gas.Regular, 0, tracing.GasChangeCallFailedExecution) + evm.config.Tracer.OnGasChange(gasRemaining.Regular, 0, tracing.GasChangeCallFailedExecution) } - gas.Regular = 0 + gasRemaining.Regular = 0 } // 3. EIP-8037: at depth>0 restore the original child reservoir to the @@ -182,10 +181,27 @@ func (evm *EVM) handleFrameRevert(gas *mdgas.MdGas, err error, depth int, snapsh // (the sum equals what the parent originally passed in, after any // inline refunds the child credited are dropped along with reverted // state). Regular gas: REVERT preserves it (step 2 didn't apply); - // exceptional halt burnt it in step 2. At depth==0 TxnExecutor handles - // the user-side refund directly from vmerr + gasUsed.State. + // exceptional halt burnt it in step 2. At depth==0 the user-side + // refund flows through gasUsed.State (see func doc). + // + // The sum must be >= 0. `gasRemaining.State + stateGasUsed` starts at + // the parent-passed-in reservoir R (≥ 0) and is monotonically + // non-decreasing: charges within the reservoir and inline refunds + // both leave the sum unchanged (charge: `stateGas -= a; + // frameStateUsed += a`; refund: `stateGas += a; frameStateUsed -= + // a`), and charges that spill into regular gas only grow it + // (stateGas clamps at 0 while frameStateUsed gets the full amount). + // So when stateGasUsed is negative, `gasRemaining.State >= |stateGasUsed|` + // and the cast doesn't underflow. If the sum does go negative, an + // EIP-8037 accounting invariant is broken upstream — panic rather + // than silently wrapping and corrupting the parent's reservoir. if evm.chainRules.IsAmsterdam && depth > 0 { - gas.State = uint64(int64(gas.State) + childStateConsumed) + restored := int64(gasRemaining.State) + stateGasUsed + if restored < 0 { + panic(fmt.Sprintf("EIP-8037 invariant violated: state_gas_left (%d) + child state_gas_used (%d) = %d < 0", + gasRemaining.State, stateGasUsed, restored)) + } + gasRemaining.State = uint64(restored) } } @@ -208,14 +224,15 @@ func (evm *EVM) SetPrecompiles(precompiles PrecompiledContracts) { evm.precompiles = precompiles } -func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts.Address, addr accounts.Address, input []byte, gas mdgas.MdGas, value uint256.Int, bailout bool) (ret []byte, leftOverGas mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { +func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts.Address, addr accounts.Address, input []byte, gas mdgas.MdGas, value uint256.Int, bailout bool) (ret []byte, gasRemaining mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { if evm.abort.Load() { - return ret, leftOverGas, gasUsed, nil + return nil, mdgas.MdGas{}, mdgas.MdGasUsage{}, nil } depth := evm.depth + gasRemaining = gas - // Derive gasUsed.Regular from the final leftOverGas at function exit, + // Derive gasUsed.Regular from the final gasRemaining at function exit, // uniformly across Run / precompile / no-code paths and after any // handleFrameRevert gas burn. gasUsed.State is set by Run's defer // (signed frameStateUsed = charges − inline refunds) and is 0 for @@ -231,7 +248,7 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts // already restored the child's reservoir to the parent.) inputTotal := gas.Total() defer func() { - leftOverTotal := leftOverGas.Total() + leftOverTotal := gasRemaining.Total() if leftOverTotal <= inputTotal { delta := int64(inputTotal - leftOverTotal) gasUsed.Regular = uint64(delta - gasUsed.State) @@ -245,7 +262,7 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts if (dbg.TraceTransactionIO && !dbg.TraceInstructions) && (evm.intraBlockState.Trace() || dbg.TraceAccount(caller.Handle())) { fmt.Printf("%d (%d.%d) %s: %x %x\n", evm.intraBlockState.BlockNumber(), version.TxIndex, version.Incarnation, typ, addr, input) defer func() { - fmt.Printf("%d (%d.%d) RETURN (%s): %x: %x, %d, %v\n", evm.intraBlockState.BlockNumber(), version.TxIndex, version.Incarnation, typ, addr, ret, leftOverGas, err) + fmt.Printf("%d (%d.%d) RETURN (%s): %x: %x, %d, %v\n", evm.intraBlockState.BlockNumber(), version.TxIndex, version.Incarnation, typ, addr, ret, gasRemaining, err) }() } @@ -262,7 +279,7 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts if evm.Config().Tracer != nil { evm.captureBegin(depth, typ, caller, addr, isPrecompile, input, gas, value, code) defer func(startGas mdgas.MdGas) { - evm.captureEnd(depth, typ, startGas, leftOverGas, ret, err) + evm.captureEnd(depth, typ, startGas, gasRemaining, ret, err) }(gas) } @@ -270,11 +287,11 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts evm.intraBlockState.MarkAddressAccess(addr, false) if evm.config.NoRecursion && depth > 0 { - return nil, gas, mdgas.MdGasUsage{}, nil + return nil, gasRemaining, mdgas.MdGasUsage{}, nil } // Fail if we're trying to execute above the call depth limit if depth > int(params.CallCreateDepth) { - return nil, gas, mdgas.MdGasUsage{}, ErrDepth + return nil, gasRemaining, mdgas.MdGasUsage{}, ErrDepth } syscall := isSystemCall(caller) @@ -287,7 +304,7 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts return nil, mdgas.MdGas{}, mdgas.MdGasUsage{}, err } if !canTransfer && !bailout { - return nil, gas, mdgas.MdGasUsage{}, ErrInsufficientBalance + return nil, gasRemaining, mdgas.MdGasUsage{}, ErrInsufficientBalance } } } @@ -307,7 +324,7 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts // beacon-root syscall's "no-op when not deployed" semantics at // the fork-transition block, before the contract is deployed. if !isPrecompile && evm.chainRules.IsSpuriousDragon && value.IsZero() { - return nil, gas, mdgas.MdGasUsage{}, nil + return nil, gasRemaining, mdgas.MdGasUsage{}, nil } evm.intraBlockState.CreateAccount(addr, false) } @@ -345,7 +362,7 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts // It is allowed to call precompiles, even via delegatecall if isPrecompile { - ret, gas.Regular, err = RunPrecompiledContract(p, input, gas.Regular, evm.Config().Tracer) + ret, gasRemaining.Regular, err = RunPrecompiledContract(p, input, gasRemaining.Regular, evm.Config().Tracer) } else if len(code) == 0 { // If the account has no code, we can abort here // The depth-check is already done, and precompiles handled above @@ -388,23 +405,23 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts if typ == STATICCALL { readOnly = true } - ret, gas, gasUsed, err = evm.Run(contract, gas, input, readOnly) + ret, gasRemaining, gasUsed, err = evm.Run(contract, gasRemaining, input, readOnly) } // When an error was returned by the EVM or when setting the creation code // above we revert to the snapshot and consume any gas remaining. Additionally // when we're in Homestead this also counts for code storage gas errors. if err != nil || evm.config.RestoreState { - evm.handleFrameRevert(&gas, err, depth, snapshot, gasUsed.State) + evm.handleFrameRevert(&gasRemaining, err, depth, snapshot, gasUsed.State) } - return ret, gas, gasUsed, err + return ret, gasRemaining, gasUsed, err } // Call executes the contract associated with the addr with the given input as // parameters. It also handles any necessary value transfer required and takes // the necessary steps to create accounts and reverses the state in case of an // execution error or failed value transfer. -func (evm *EVM) Call(caller accounts.Address, addr accounts.Address, input []byte, gas mdgas.MdGas, value uint256.Int, bailout bool) (ret []byte, leftOverGas mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { +func (evm *EVM) Call(caller accounts.Address, addr accounts.Address, input []byte, gas mdgas.MdGas, value uint256.Int, bailout bool) (ret []byte, gasRemaining mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { return evm.call(CALL, caller, caller, addr, input, gas, value, bailout) } @@ -415,7 +432,7 @@ func (evm *EVM) Call(caller accounts.Address, addr accounts.Address, input []byt // // CallCode differs from Call in the sense that it executes the given address' // code with the caller as context. -func (evm *EVM) CallCode(caller accounts.Address, addr accounts.Address, input []byte, gas mdgas.MdGas, value uint256.Int) (ret []byte, leftOverGas mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { +func (evm *EVM) CallCode(caller accounts.Address, addr accounts.Address, input []byte, gas mdgas.MdGas, value uint256.Int) (ret []byte, gasRemaining mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { return evm.call(CALLCODE, caller, caller, addr, input, gas, value, false) } @@ -424,7 +441,7 @@ func (evm *EVM) CallCode(caller accounts.Address, addr accounts.Address, input [ // // DelegateCall differs from CallCode in the sense that it executes the given address' // code with the caller as context and the caller is set to the caller of the caller. -func (evm *EVM) DelegateCall(caller accounts.Address, callerAddress accounts.Address, addr accounts.Address, input []byte, value uint256.Int, gas mdgas.MdGas) (ret []byte, leftOverGas mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { +func (evm *EVM) DelegateCall(caller accounts.Address, callerAddress accounts.Address, addr accounts.Address, input []byte, value uint256.Int, gas mdgas.MdGas) (ret []byte, gasRemaining mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { return evm.call(DELEGATECALL, caller, callerAddress, addr, input, gas, value, false) } @@ -432,7 +449,7 @@ func (evm *EVM) DelegateCall(caller accounts.Address, callerAddress accounts.Add // as parameters while disallowing any modifications to the state during the call. // Opcodes that attempt to perform such modifications will result in exceptions // instead of performing the modifications. -func (evm *EVM) StaticCall(caller accounts.Address, addr accounts.Address, input []byte, gas mdgas.MdGas) (ret []byte, leftOverGas mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { +func (evm *EVM) StaticCall(caller accounts.Address, addr accounts.Address, input []byte, gas mdgas.MdGas) (ret []byte, gasRemaining mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { return evm.call(STATICCALL, caller, caller, addr, input, gas, uint256.Int{}, false) } @@ -457,21 +474,23 @@ func (evm *EVM) OverlayCreate(caller accounts.Address, codeAndHash *codeAndHash, } // create creates a new contract using code as deployment code. -func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gasRemaining mdgas.MdGas, value uint256.Int, address accounts.Address, typ OpCode, incrementNonce bool, bailout bool) (ret []byte, createAddress accounts.Address, leftOverGas mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { +func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gas mdgas.MdGas, value uint256.Int, address accounts.Address, typ OpCode, incrementNonce bool, bailout bool) (ret []byte, createAddress accounts.Address, gasRemaining mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { + gasRemaining = gas + if dbg.TraceTransactionIO && (evm.intraBlockState.Trace() || dbg.TraceAccount(caller.Handle())) { defer func() { version := evm.intraBlockState.Version() if err != nil { fmt.Printf("%d (%d.%d) Create Contract: %x, err=%s\n", evm.intraBlockState.BlockNumber(), version.TxIndex, version.Incarnation, createAddress, err) } else { - fmt.Printf("%d (%d.%d) Create Contract: %x, gas=%d\n", evm.intraBlockState.BlockNumber(), version.TxIndex, version.Incarnation, createAddress, leftOverGas) + fmt.Printf("%d (%d.%d) Create Contract: %x, gas=%d\n", evm.intraBlockState.BlockNumber(), version.TxIndex, version.Incarnation, createAddress, gasRemaining) } }() } depth := evm.depth - // Derive gasUsed.Regular from the final leftOverGas at function exit, + // Derive gasUsed.Regular from the final gasRemaining at function exit, // uniformly across all Create exit paths (Run, depth/balance/collision // errors, post-handleFrameRevert gas burn). gasUsed.State is set by Run's // defer for the initcode frame (signed net) and stays 0 on early-exit @@ -488,9 +507,9 @@ func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gasRem // intrinsic_state == StateGasNewAccount). The Call counterpart does NOT // refund intrinsic AUTH state-gas — EIP-7702 auth side effects persist // even on call failure. - inputTotal := gasRemaining.Total() + inputTotal := gas.Total() defer func() { - leftOverTotal := leftOverGas.Total() + leftOverTotal := gasRemaining.Total() if leftOverTotal <= inputTotal { delta := int64(inputTotal - leftOverTotal) gasUsed.Regular = uint64(delta - gasUsed.State) @@ -501,10 +520,10 @@ func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gasRem }() if evm.Config().Tracer != nil { - evm.captureBegin(depth, typ, caller, address, false, codeAndHash.code, gasRemaining, value, nil) + evm.captureBegin(depth, typ, caller, address, false, codeAndHash.code, gas, value, nil) defer func(startGas mdgas.MdGas) { - evm.captureEnd(depth, typ, startGas, leftOverGas, ret, err) - }(gasRemaining) + evm.captureEnd(depth, typ, startGas, gasRemaining, ret, err) + }(gas) } // Depth check execution. Fail if we're trying to execute above the @@ -668,7 +687,7 @@ func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gasRem // If salt is non-nil, CREATE2 addressing is used (keccak256(0xff ++ msg.sender ++ salt ++ keccak256(init_code))[12:]); // otherwise the usual sender-and-nonce-hash is used (CREATE). // DESCRIBED: docs/programmers_guide/guide.md#nonce -func (evm *EVM) Create(caller accounts.Address, code []byte, gasRemaining mdgas.MdGas, endowment uint256.Int, salt *uint256.Int, bailout bool) (ret []byte, contractAddr accounts.Address, leftOverGas mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { +func (evm *EVM) Create(caller accounts.Address, code []byte, gas mdgas.MdGas, endowment uint256.Int, salt *uint256.Int, bailout bool) (ret []byte, contractAddr accounts.Address, gasRemaining mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { ch := &codeAndHash{code: code} op := CREATE if salt != nil { @@ -682,13 +701,13 @@ func (evm *EVM) Create(caller accounts.Address, code []byte, gasRemaining mdgas. } contractAddr = accounts.InternAddress(types.CreateAddress(caller.Value(), nonce)) } - return evm.create(caller, ch, gasRemaining, endowment, contractAddr, op, true /* incrementNonce */, bailout) + return evm.create(caller, ch, gas, endowment, contractAddr, op, true /* incrementNonce */, bailout) } // SysCreate is a special (system) contract creation methods for genesis constructors. // Unlike the normal Create & Create2, it doesn't increment caller's nonce. -func (evm *EVM) SysCreate(caller accounts.Address, code []byte, gas mdgas.MdGas, endowment uint256.Int, contractAddr accounts.Address) (ret []byte, leftOverGas mdgas.MdGas, err error) { - ret, _, leftOverGas, _, err = evm.create(caller, &codeAndHash{code: code}, gas, endowment, contractAddr, CREATE, false /* incrementNonce */, false) +func (evm *EVM) SysCreate(caller accounts.Address, code []byte, gas mdgas.MdGas, endowment uint256.Int, contractAddr accounts.Address) (ret []byte, gasRemaining mdgas.MdGas, err error) { + ret, _, gasRemaining, _, err = evm.create(caller, &codeAndHash{code: code}, gas, endowment, contractAddr, CREATE, false /* incrementNonce */, false) return } diff --git a/execution/vm/interpreter.go b/execution/vm/interpreter.go index 8d9a45e3fa9..5f6597d1432 100644 --- a/execution/vm/interpreter.go +++ b/execution/vm/interpreter.go @@ -373,7 +373,7 @@ func jumpTable(chainRules *chain.Rules, cfg Config) *JumpTable { // It's important to note that any errors returned by the interpreter should be // considered a revert-and-consume-all-gas operation except for // ErrExecutionReverted which means revert-and-keep-gas-left. -func (evm *EVM) Run(contract Contract, gas mdgas.MdGas, input []byte, readOnly bool) (ret []byte, leftOver mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { +func (evm *EVM) Run(contract Contract, gas mdgas.MdGas, input []byte, readOnly bool) (ret []byte, gasRemaining mdgas.MdGas, gasUsed mdgas.MdGasUsage, err error) { // Don't bother with the execution if there's no code. if len(contract.Code) == 0 { return nil, gas, mdgas.MdGasUsage{}, nil @@ -425,7 +425,7 @@ func (evm *EVM) Run(contract Contract, gas mdgas.MdGas, input []byte, readOnly b // EIP-8037: snapshot the frame's net state-gas usage (charges minus // inline refunds, signed) before callContext.put() clears it. // gasUsed.Regular is derived uniformly by evm.call/evm.create's defer - // from the final leftOverGas (covers precompile/no-code paths and + // from the final gasRemaining (covers precompile/no-code paths and // handleFrameRevert gas burn). gasUsed.State = callContext.frameStateUsed // this function must execute _after_: the `CaptureState` needs the stacks before From 8a02d62a9f15e39810982c29922183ccea0838cd Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Tue, 19 May 2026 10:11:23 +0000 Subject: [PATCH 13/28] tidy --- execution/vm/evm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/execution/vm/evm.go b/execution/vm/evm.go index 230b40cd05e..f3ab7ad8e64 100644 --- a/execution/vm/evm.go +++ b/execution/vm/evm.go @@ -405,7 +405,7 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts if typ == STATICCALL { readOnly = true } - ret, gasRemaining, gasUsed, err = evm.Run(contract, gasRemaining, input, readOnly) + ret, gasRemaining, gasUsed, err = evm.Run(contract, gas, input, readOnly) } // When an error was returned by the EVM or when setting the creation code // above we revert to the snapshot and consume any gas remaining. Additionally @@ -612,7 +612,7 @@ func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gas md return nil, address, gasRemaining, mdgas.MdGasUsage{}, nil } - ret, gasRemaining, gasUsed, err = evm.Run(contract, gasRemaining, nil, false) + ret, gasRemaining, gasUsed, err = evm.Run(contract, gas, nil, false) // EIP-170: Contract code size limit if err == nil { From 0392801bf4c0200319ec1af3112c31b777ded742 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Tue, 19 May 2026 10:18:26 +0000 Subject: [PATCH 14/28] tidy --- execution/vm/evm.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/execution/vm/evm.go b/execution/vm/evm.go index f3ab7ad8e64..2985c84e6f7 100644 --- a/execution/vm/evm.go +++ b/execution/vm/evm.go @@ -248,9 +248,9 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts // already restored the child's reservoir to the parent.) inputTotal := gas.Total() defer func() { - leftOverTotal := gasRemaining.Total() - if leftOverTotal <= inputTotal { - delta := int64(inputTotal - leftOverTotal) + gasRemainingTotal := gasRemaining.Total() + if gasRemainingTotal <= inputTotal { + delta := int64(inputTotal - gasRemainingTotal) gasUsed.Regular = uint64(delta - gasUsed.State) } if depth == 0 && evm.chainRules.IsAmsterdam && err != nil { @@ -509,9 +509,9 @@ func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gas md // even on call failure. inputTotal := gas.Total() defer func() { - leftOverTotal := gasRemaining.Total() - if leftOverTotal <= inputTotal { - delta := int64(inputTotal - leftOverTotal) + gasRemainingTotal := gasRemaining.Total() + if gasRemainingTotal <= inputTotal { + delta := int64(inputTotal - gasRemainingTotal) gasUsed.Regular = uint64(delta - gasUsed.State) } if depth == 0 && evm.chainRules.IsAmsterdam && err != nil { From dd41157e16250440a5bb86e2b86228ad57b16f14 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Tue, 19 May 2026 10:40:58 +0000 Subject: [PATCH 15/28] fix for deriveFrameRegularGasUsed --- execution/vm/evm.go | 26 ++++++---- execution/vm/evm_test.go | 102 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 10 deletions(-) create mode 100644 execution/vm/evm_test.go diff --git a/execution/vm/evm.go b/execution/vm/evm.go index 2985c84e6f7..b8936bd7e3a 100644 --- a/execution/vm/evm.go +++ b/execution/vm/evm.go @@ -205,6 +205,20 @@ func (evm *EVM) handleFrameRevert(gasRemaining *mdgas.MdGas, err error, depth in } } +// deriveFrameRegularGasUsed derives the regular-gas component of a frame's +// gasUsed from the total gas it received (inputTotal), the total left over +// at exit (gasRemainingTotal), and the frame's signed net state-gas usage. +// +// Regular = (input − leftover) − state, with leftover potentially exceeding +// input when state refunds (creditStateGasRefund, which grows the local +// reservoir) outweighed the regular gas spent. The arithmetic stays in +// int64 so a refund-heavy frame's negative delta cancels against an equally +// negative stateGasUsed to yield the correct positive regular-ops count. +func deriveFrameRegularGasUsed(inputTotal, gasRemainingTotal uint64, stateGasUsed int64) uint64 { + delta := int64(inputTotal) - int64(gasRemainingTotal) + return uint64(delta - stateGasUsed) +} + // CallGasTemp returns the callGasTemp for the EVM func (evm *EVM) CallGasTemp() uint64 { return evm.callGasTemp @@ -248,11 +262,7 @@ func (evm *EVM) call(typ OpCode, caller accounts.Address, callerAddress accounts // already restored the child's reservoir to the parent.) inputTotal := gas.Total() defer func() { - gasRemainingTotal := gasRemaining.Total() - if gasRemainingTotal <= inputTotal { - delta := int64(inputTotal - gasRemainingTotal) - gasUsed.Regular = uint64(delta - gasUsed.State) - } + gasUsed.Regular = deriveFrameRegularGasUsed(inputTotal, gasRemaining.Total(), gasUsed.State) if depth == 0 && evm.chainRules.IsAmsterdam && err != nil { gasUsed.State = 0 } @@ -509,11 +519,7 @@ func (evm *EVM) create(caller accounts.Address, codeAndHash *codeAndHash, gas md // even on call failure. inputTotal := gas.Total() defer func() { - gasRemainingTotal := gasRemaining.Total() - if gasRemainingTotal <= inputTotal { - delta := int64(inputTotal - gasRemainingTotal) - gasUsed.Regular = uint64(delta - gasUsed.State) - } + gasUsed.Regular = deriveFrameRegularGasUsed(inputTotal, gasRemaining.Total(), gasUsed.State) if depth == 0 && evm.chainRules.IsAmsterdam && err != nil { gasUsed.State = -int64(params.StateGasNewAccount) } diff --git a/execution/vm/evm_test.go b/execution/vm/evm_test.go new file mode 100644 index 00000000000..6a8bd9532e4 --- /dev/null +++ b/execution/vm/evm_test.go @@ -0,0 +1,102 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package vm + +import "testing" + +// TestDeriveFrameRegularGasUsed covers the EIP-8037 cases where the formula +// Regular = (inputTotal − gasRemainingTotal) − stateGasUsed must hold, +// including the refund-heavy case where stateGas grows above the input +// because inline state-gas refunds (creditStateGasRefund) credit the +// frame's local reservoir. +func TestDeriveFrameRegularGasUsed(t *testing.T) { + t.Parallel() + + const sgps = 64 * 1530 // params.StateGasPerStorageSet + + cases := []struct { + name string + inputTotal uint64 + gasRemainingTotal uint64 + stateGasUsed int64 + want uint64 + }{ + { + // Plain frame: 50 regular ops, 30 state charge from a 100-unit + // reservoir, no spillover. + // input = 1000 R + 100 S = 1100 + // leftover = 950 R + 70 S = 1020 + // stateGasUsed = 30 + name: "charges_only_no_spillover", + inputTotal: 1100, + gasRemainingTotal: 1020, + stateGasUsed: 30, + want: 50, + }, + { + // Spillover: 50 regular ops + state charge 200 against a 100-unit + // reservoir, 100 spills into regular gas. + // input = 1000 R + 100 S = 1100 + // leftover = 850 R + 0 S = 850 + // stateGasUsed = 200 (full charge, regardless of spillover) + // Want 50 (RegularUsedDirect only — the spilled portion is + // already represented in stateGasUsed). + name: "with_spillover", + inputTotal: 1100, + gasRemainingTotal: 850, + stateGasUsed: 200, + want: 50, + }, + { + // Refunds exceed the frame's own charges — typical for a + // DELEGATECALL/CALLCODE callee that clears a slot an ancestor + // set. The refund (sgps) credits the local reservoir directly, + // pushing gasRemainingTotal above inputTotal. + // input = 100 R + 50 S = 150 + // refund sgps grows leftover state to 50 + sgps + // 10 regular ops → leftover regular = 90 + // stateGasUsed = -sgps + // Want 10 (the regular ops). A guarded uint64 subtraction would + // see gasRemainingTotal > inputTotal and (wrongly) return 0. + name: "refunds_exceed_charges_intermediate_frame", + inputTotal: 150, + gasRemainingTotal: 90 + 50 + sgps, + stateGasUsed: -sgps, + want: 10, + }, + { + // Pure refund frame, no regular work. Verifies signed cancellation + // across the entire delta. + name: "refunds_only_no_regular_ops", + inputTotal: 150, + gasRemainingTotal: 150 + sgps, + stateGasUsed: -sgps, + want: 0, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := deriveFrameRegularGasUsed(tc.inputTotal, tc.gasRemainingTotal, tc.stateGasUsed) + if got != tc.want { + t.Fatalf("deriveFrameRegularGasUsed(input=%d, leftover=%d, state=%d) = %d, want %d", + tc.inputTotal, tc.gasRemainingTotal, tc.stateGasUsed, got, tc.want) + } + }) + } +} From 459a754a5e20c565f44fdc2a57a7ba6bd129d3ea Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Tue, 19 May 2026 12:53:25 +0000 Subject: [PATCH 16/28] fix panic: EIP-8037 invariant violated --- execution/vm/evm.go | 54 +++++++++++++++++++++++----------------- execution/vm/evm_test.go | 16 +++++++++++- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/execution/vm/evm.go b/execution/vm/evm.go index b8936bd7e3a..3a6679c178c 100644 --- a/execution/vm/evm.go +++ b/execution/vm/evm.go @@ -184,24 +184,34 @@ func (evm *EVM) handleFrameRevert(gasRemaining *mdgas.MdGas, err error, depth in // exceptional halt burnt it in step 2. At depth==0 the user-side // refund flows through gasUsed.State (see func doc). // - // The sum must be >= 0. `gasRemaining.State + stateGasUsed` starts at - // the parent-passed-in reservoir R (≥ 0) and is monotonically - // non-decreasing: charges within the reservoir and inline refunds - // both leave the sum unchanged (charge: `stateGas -= a; - // frameStateUsed += a`; refund: `stateGas += a; frameStateUsed -= - // a`), and charges that spill into regular gas only grow it - // (stateGas clamps at 0 while frameStateUsed gets the full amount). - // So when stateGasUsed is negative, `gasRemaining.State >= |stateGasUsed|` - // and the cast doesn't underflow. If the sum does go negative, an - // EIP-8037 accounting invariant is broken upstream — panic rather - // than silently wrapping and corrupting the parent's reservoir. if evm.chainRules.IsAmsterdam && depth > 0 { - restored := int64(gasRemaining.State) + stateGasUsed - if restored < 0 { - panic(fmt.Sprintf("EIP-8037 invariant violated: state_gas_left (%d) + child state_gas_used (%d) = %d < 0", - gasRemaining.State, stateGasUsed, restored)) + // Invariant: gasRemaining.State + stateGasUsed >= 0. useMdGas + // (charge: stateGas -= a, frameStateUsed += a) and + // creditStateGasRefund (refund: stateGas += a, frameStateUsed -= a) + // are the only mutators and update both sides in lock-step, so + // the sum can only grow (via spillover charges) from the parent's + // reservoir R ≥ 0. A negative sum here means some new code path + // mutated stateGas or frameStateUsed without the paired update, + // or the EIP-8037 accounting was reimplemented incorrectly. Keep + // the panic: silently wrapping the parent's reservoir on this + // path would corrupt block-level gas accounting downstream and be + // painful to track back here. + // + // The comparison stays in uint64 (`uint64(-stateGasUsed) > + // gasRemaining.State`) rather than promoting both sides to int64. + // gasRemaining.State is a reservoir balance — unsigned by design + // and free to use the full uint64 range — so an int64 cast would + // flip any value above 2^63 to negative and fire the panic + // spuriously. Comparing the magnitudes directly in uint64 keeps + // the check correct at any gas size. + if stateGasUsed < 0 && uint64(-stateGasUsed) > gasRemaining.State { + panic(fmt.Sprintf("EIP-8037 invariant violated: gasRemaining.State (%d) + stateGasUsed (%d) < 0", + gasRemaining.State, stateGasUsed)) } - gasRemaining.State = uint64(restored) + // uint64(negative int64) = 2^64 − |stateGasUsed|; the += then + // overflows and wraps into the correct subtraction for negative + // stateGasUsed. + gasRemaining.State += uint64(stateGasUsed) } } @@ -209,14 +219,12 @@ func (evm *EVM) handleFrameRevert(gasRemaining *mdgas.MdGas, err error, depth in // gasUsed from the total gas it received (inputTotal), the total left over // at exit (gasRemainingTotal), and the frame's signed net state-gas usage. // -// Regular = (input − leftover) − state, with leftover potentially exceeding -// input when state refunds (creditStateGasRefund, which grows the local -// reservoir) outweighed the regular gas spent. The arithmetic stays in -// int64 so a refund-heavy frame's negative delta cancels against an equally -// negative stateGasUsed to yield the correct positive regular-ops count. +// Regular = (input − leftover) − state. Computed in uint64 modular +// arithmetic: a negative stateGasUsed becomes a large uint64 via the cast, +// and subtracting it wraps mod 2^64 into the correct positive sum. Safe at +// any gas magnitude. func deriveFrameRegularGasUsed(inputTotal, gasRemainingTotal uint64, stateGasUsed int64) uint64 { - delta := int64(inputTotal) - int64(gasRemainingTotal) - return uint64(delta - stateGasUsed) + return inputTotal - gasRemainingTotal - uint64(stateGasUsed) } // CallGasTemp returns the callGasTemp for the EVM diff --git a/execution/vm/evm_test.go b/execution/vm/evm_test.go index 6a8bd9532e4..b8431f1dd30 100644 --- a/execution/vm/evm_test.go +++ b/execution/vm/evm_test.go @@ -16,7 +16,10 @@ package vm -import "testing" +import ( + "math" + "testing" +) // TestDeriveFrameRegularGasUsed covers the EIP-8037 cases where the formula // Regular = (inputTotal − gasRemainingTotal) − stateGasUsed must hold, @@ -87,6 +90,17 @@ func TestDeriveFrameRegularGasUsed(t *testing.T) { stateGasUsed: -sgps, want: 0, }, + { + // Gas totals are unsigned reservoir balances and are free to + // occupy the full uint64 range. The helper must stay correct + // when those totals sit above 2^63 — a naive int64 promotion + // would flip the sign and break the arithmetic. + name: "huge_gas_totals_uint64_safe", + inputTotal: math.MaxUint64, + gasRemainingTotal: math.MaxUint64 - 12345, + stateGasUsed: 300, + want: 12345 - 300, + }, } for _, tc := range cases { From c109d8c31874cd036543432ae00a2717d49898fa Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Wed, 20 May 2026 03:55:49 +0000 Subject: [PATCH 17/28] avoid panic, log loudly --- execution/vm/evm.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/execution/vm/evm.go b/execution/vm/evm.go index 3a6679c178c..7b4ae7cd7e9 100644 --- a/execution/vm/evm.go +++ b/execution/vm/evm.go @@ -30,6 +30,7 @@ import ( "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/crypto" "github.com/erigontech/erigon/common/dbg" + "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/execution/chain" "github.com/erigontech/erigon/execution/protocol/mdgas" "github.com/erigontech/erigon/execution/protocol/params" @@ -192,21 +193,23 @@ func (evm *EVM) handleFrameRevert(gasRemaining *mdgas.MdGas, err error, depth in // the sum can only grow (via spillover charges) from the parent's // reservoir R ≥ 0. A negative sum here means some new code path // mutated stateGas or frameStateUsed without the paired update, - // or the EIP-8037 accounting was reimplemented incorrectly. Keep - // the panic: silently wrapping the parent's reservoir on this - // path would corrupt block-level gas accounting downstream and be - // painful to track back here. + // or the EIP-8037 accounting was reimplemented incorrectly. We + // surface this loudly via a log + clamp; the corrupted accounting + // will manifest downstream as wrong execution / gas accounting / + // state root. // // The comparison stays in uint64 (`uint64(-stateGasUsed) > // gasRemaining.State`) rather than promoting both sides to int64. // gasRemaining.State is a reservoir balance — unsigned by design // and free to use the full uint64 range — so an int64 cast would - // flip any value above 2^63 to negative and fire the panic - // spuriously. Comparing the magnitudes directly in uint64 keeps - // the check correct at any gas size. + // flip any value above 2^63 to negative and break the comparison. + // Comparing the magnitudes directly in uint64 keeps the check + // correct at any gas size. if stateGasUsed < 0 && uint64(-stateGasUsed) > gasRemaining.State { - panic(fmt.Sprintf("EIP-8037 invariant violated: gasRemaining.State (%d) + stateGasUsed (%d) < 0", - gasRemaining.State, stateGasUsed)) + log.Error("EIP-8037 invariant violated; clamping parent reservoir to 0", + "gasRemainingState", gasRemaining.State, "stateGasUsed", stateGasUsed, "depth", depth) + gasRemaining.State = 0 + return } // uint64(negative int64) = 2^64 − |stateGasUsed|; the += then // overflows and wraps into the correct subtraction for negative From d85e3905ba562da029819c0723fa176939cda45d Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Wed, 20 May 2026 04:33:50 +0000 Subject: [PATCH 18/28] execution,cl: add TargetGasLimit to PayloadAttributesV4 --- cl/beacon/handler/block_production.go | 2 ++ cl/phase1/stages/forkchoice.go | 2 ++ execution/builder/create_block.go | 8 +++++++- execution/builder/parameters.go | 1 + execution/engineapi/engine_server.go | 5 +++++ execution/engineapi/engine_types/jsonrpc.go | 1 + execution/engineapi/engine_types/ssz.go | 11 +++++++++-- execution/engineapi/engineapitester/mock_cl.go | 4 ++++ execution/engineapi/sszrest_test.go | 4 ++++ execution/engineapi/testing_api.go | 1 + execution/execmodule/chainreader/chain_reader.go | 1 + 11 files changed, 37 insertions(+), 3 deletions(-) diff --git a/cl/beacon/handler/block_production.go b/cl/beacon/handler/block_production.go index 73ecd76710a..4b0bb5540d0 100644 --- a/cl/beacon/handler/block_production.go +++ b/cl/beacon/handler/block_production.go @@ -817,6 +817,8 @@ func (a *ApiHandler) produceBeaconBody( if stateVersion.AfterOrEqual(clparams.GloasVersion) { sn := hexutil.Uint64(targetSlot) attrs.SlotNumber = &sn + tgl := hexutil.Uint64(a.beaconChainCfg.DefaultBuilderGasLimit) + attrs.TargetGasLimit = &tgl } idBytes, err := a.engine.ForkChoiceUpdate( ctx, diff --git a/cl/phase1/stages/forkchoice.go b/cl/phase1/stages/forkchoice.go index d4f0d0168dd..5276232f18d 100644 --- a/cl/phase1/stages/forkchoice.go +++ b/cl/phase1/stages/forkchoice.go @@ -274,6 +274,8 @@ func emitNextPaylodAttributesEvent(cfg *Cfg, headSlot uint64, headRoot common.Ha if cfg.beaconCfg.GetCurrentStateVersion(epoch).AfterOrEqual(clparams.GloasVersion) { sn := hexutil.Uint64(nextSlot) payloadAttributes.SlotNumber = &sn + tgl := hexutil.Uint64(cfg.beaconCfg.DefaultBuilderGasLimit) + payloadAttributes.TargetGasLimit = &tgl } e := &beaconevents.PayloadAttributesData{ Version: cfg.beaconCfg.GetCurrentStateVersion(epoch).String(), diff --git a/execution/builder/create_block.go b/execution/builder/create_block.go index 1d7587f2449..b4f2d0e071b 100644 --- a/execution/builder/create_block.go +++ b/execution/builder/create_block.go @@ -164,7 +164,13 @@ func createBlock(ctx context.Context, sd *execctx.SharedDomains, tx kv.TemporalT uncles: mapset.NewSet[common.Hash](), } - header := MakeEmptyHeader(parent, cfg.chainConfig, timestamp, cfg.builder.BuilderConfig.GasLimit) + targetGasLimit := cfg.builder.BuilderConfig.GasLimit + if cfg.blockBuilderParameters != nil && cfg.blockBuilderParameters.TargetGasLimit != nil { + // PayloadAttributesV4: CL-supplied target gas limit takes precedence over the + // static --miner.gaslimit so the EL follows engine_forkchoiceUpdatedV4. + targetGasLimit = cfg.blockBuilderParameters.TargetGasLimit + } + header := MakeEmptyHeader(parent, cfg.chainConfig, timestamp, targetGasLimit) if err := misc.VerifyGaslimit(parent.GasLimit, header.GasLimit); err != nil { logger.Warn("Failed to verify gas limit given by the validator, defaulting to parent gas limit", "err", err) header.GasLimit = parent.GasLimit diff --git a/execution/builder/parameters.go b/execution/builder/parameters.go index c06344a42a8..a9c5025737b 100644 --- a/execution/builder/parameters.go +++ b/execution/builder/parameters.go @@ -33,6 +33,7 @@ type Parameters struct { Withdrawals []*types.Withdrawal // added in Shapella (EIP-4895) ParentBeaconBlockRoot *common.Hash // added in Dencun (EIP-4788) SlotNumber *uint64 // added in Amsterdam (EIP-7843) + TargetGasLimit *uint64 // added in Amsterdam PayloadAttributesV4 // CustomTxnProvider overrides the block's transaction source when non-nil. // nil → use the injected TxnProvider (normal mempool path) CustomTxnProvider txnprovider.TxnProvider diff --git a/execution/engineapi/engine_server.go b/execution/engineapi/engine_server.go index d61dfa12f44..8a70f987f5b 100644 --- a/execution/engineapi/engine_server.go +++ b/execution/engineapi/engine_server.go @@ -231,6 +231,10 @@ func (s *EngineServer) validatePayloadAttributesPostFCU(version clparams.StateVe if version >= clparams.GloasVersion && payloadAttributes.SlotNumber == nil { return &engine_helpers.InvalidPayloadAttributesErr // SlotNumber required for Glamsterdam (EIP-7843) } + // TODO: enable once tests catch up with glamsterdam-devnet-4 spec + // if version >= clparams.GloasVersion && payloadAttributes.TargetGasLimit == nil { + // return &engine_helpers.InvalidPayloadAttributesErr // TargetGasLimit required for V4 attrs + // } return nil } @@ -795,6 +799,7 @@ func (s *EngineServer) forkchoiceUpdated(ctx context.Context, forkchoiceState *e PrevRandao: payloadAttributes.PrevRandao, SuggestedFeeRecipient: payloadAttributes.SuggestedFeeRecipient, SlotNumber: (*uint64)(payloadAttributes.SlotNumber), + TargetGasLimit: (*uint64)(payloadAttributes.TargetGasLimit), } if version >= clparams.CapellaVersion { diff --git a/execution/engineapi/engine_types/jsonrpc.go b/execution/engineapi/engine_types/jsonrpc.go index 44b44389e73..0e011bc576c 100644 --- a/execution/engineapi/engine_types/jsonrpc.go +++ b/execution/engineapi/engine_types/jsonrpc.go @@ -70,6 +70,7 @@ type PayloadAttributes struct { Withdrawals []*types.Withdrawal `json:"withdrawals"` ParentBeaconBlockRoot *common.Hash `json:"parentBeaconBlockRoot"` SlotNumber *hexutil.Uint64 `json:"slotNumber"` + TargetGasLimit *hexutil.Uint64 `json:"targetGasLimit"` SSZVersion clparams.StateVersion `json:"-"` } diff --git a/execution/engineapi/engine_types/ssz.go b/execution/engineapi/engine_types/ssz.go index 9c3935a54a1..dc1d179602f 100644 --- a/execution/engineapi/engine_types/ssz.go +++ b/execution/engineapi/engine_types/ssz.go @@ -302,6 +302,10 @@ func (a *PayloadAttributes) EncodeSSZ(dst []byte) ([]byte, error) { if a.SlotNumber != nil { slot = uint64(*a.SlotNumber) } + var targetGasLimit uint64 + if a.TargetGasLimit != nil { + targetGasLimit = uint64(*a.TargetGasLimit) + } switch version { case clparams.BellatrixVersion: return ssz2.MarshalSSZ(dst, uint64(a.Timestamp), a.PrevRandao[:], a.SuggestedFeeRecipient[:]) @@ -310,7 +314,7 @@ func (a *PayloadAttributes) EncodeSSZ(dst []byte) ([]byte, error) { case clparams.DenebVersion: return ssz2.MarshalSSZ(dst, uint64(a.Timestamp), a.PrevRandao[:], a.SuggestedFeeRecipient[:], withdrawals, root[:]) default: - return ssz2.MarshalSSZ(dst, uint64(a.Timestamp), a.PrevRandao[:], a.SuggestedFeeRecipient[:], withdrawals, root[:], slot) + return ssz2.MarshalSSZ(dst, uint64(a.Timestamp), a.PrevRandao[:], a.SuggestedFeeRecipient[:], withdrawals, root[:], slot, targetGasLimit) } } @@ -320,6 +324,7 @@ func (a *PayloadAttributes) DecodeSSZ(buf []byte, version int) error { var timestamp uint64 var root common.Hash var slot uint64 + var targetGasLimit uint64 switch a.SSZVersion { case clparams.BellatrixVersion: if err := ssz2.UnmarshalSSZ(buf, version, ×tamp, a.PrevRandao[:], a.SuggestedFeeRecipient[:]); err != nil { @@ -337,13 +342,15 @@ func (a *PayloadAttributes) DecodeSSZ(buf []byte, version int) error { a.Withdrawals = withdrawalsFromList(withdrawals) a.ParentBeaconBlockRoot = &root default: - if err := ssz2.UnmarshalSSZ(buf, version, ×tamp, a.PrevRandao[:], a.SuggestedFeeRecipient[:], withdrawals, root[:], &slot); err != nil { + if err := ssz2.UnmarshalSSZ(buf, version, ×tamp, a.PrevRandao[:], a.SuggestedFeeRecipient[:], withdrawals, root[:], &slot, &targetGasLimit); err != nil { return err } a.Withdrawals = withdrawalsFromList(withdrawals) a.ParentBeaconBlockRoot = &root slotNumber := hexutil.Uint64(slot) a.SlotNumber = &slotNumber + tgl := hexutil.Uint64(targetGasLimit) + a.TargetGasLimit = &tgl } a.Timestamp = hexutil.Uint64(timestamp) return nil diff --git a/execution/engineapi/engineapitester/mock_cl.go b/execution/engineapi/engineapitester/mock_cl.go index 68ff01112f9..18394d673e9 100644 --- a/execution/engineapi/engineapitester/mock_cl.go +++ b/execution/engineapi/engineapitester/mock_cl.go @@ -55,6 +55,7 @@ type MockCl struct { engineApiClient *engineapi.JsonRpcClient suggestedFeeRecipient common.Address genesis common.Hash + genesisGasLimit uint64 state *MockClState blockListener *shutter.BlockListener chainConfig *chain.Config @@ -71,6 +72,7 @@ func NewMockCl(ctx context.Context, logger log.Logger, elClient *engineapi.JsonR blockListener: shutter.NewBlockListener(logger, stateChangesClient), suggestedFeeRecipient: genesis.Coinbase(), genesis: genesis.Hash(), + genesisGasLimit: genesis.GasLimit(), chainConfig: chainConfig, state: &MockClState{ ParentElBlock: genesis.Hash(), @@ -152,6 +154,8 @@ func (cl *MockCl) BuildNewPayload(ctx context.Context, opts ...BlockBuildingOpti } if cl.chainConfig.AmsterdamTime != nil { payloadAttributes.SlotNumber = (*hexutil.Uint64)(&slotNumber) + targetGasLimit := hexutil.Uint64(cl.genesisGasLimit) + payloadAttributes.TargetGasLimit = &targetGasLimit } cl.logger.Debug("[mock-cl] building block", "timestamp", timestamp) // start the block building process diff --git a/execution/engineapi/sszrest_test.go b/execution/engineapi/sszrest_test.go index ee2635975d6..54e7360e4de 100644 --- a/execution/engineapi/sszrest_test.go +++ b/execution/engineapi/sszrest_test.go @@ -247,11 +247,13 @@ func TestSSZRESTNewPayloadV5UsesGloasPayloadSchema(t *testing.T) { func TestSSZRESTForkchoiceV4UsesGloasPayloadAttributesSchema(t *testing.T) { slotNumber := hexutil.Uint64(456) + targetGasLimit := hexutil.Uint64(36000000) attrs := &engine_types.PayloadAttributes{ Timestamp: 1, SuggestedFeeRecipient: common.HexToAddress("0x1234"), Withdrawals: nil, SlotNumber: &slotNumber, + TargetGasLimit: &targetGasLimit, SSZVersion: clparams.GloasVersion, } state := engine_types.ForkChoiceState{} @@ -266,6 +268,8 @@ func TestSSZRESTForkchoiceV4UsesGloasPayloadAttributesSchema(t *testing.T) { require.NotNil(t, engineAttrs) require.NotNil(t, engineAttrs.SlotNumber) require.Equal(t, hexutil.Uint64(456), *engineAttrs.SlotNumber) + require.NotNil(t, engineAttrs.TargetGasLimit) + require.Equal(t, hexutil.Uint64(36000000), *engineAttrs.TargetGasLimit) } func TestExchangeCapabilitiesAdvertisesJSONRPCAndSSZREST(t *testing.T) { diff --git a/execution/engineapi/testing_api.go b/execution/engineapi/testing_api.go index 9db875e3603..906d9ed0b9b 100644 --- a/execution/engineapi/testing_api.go +++ b/execution/engineapi/testing_api.go @@ -206,6 +206,7 @@ func (t *testingImpl) BuildBlockV1( PrevRandao: payloadAttributes.PrevRandao, SuggestedFeeRecipient: payloadAttributes.SuggestedFeeRecipient, SlotNumber: (*uint64)(payloadAttributes.SlotNumber), + TargetGasLimit: (*uint64)(payloadAttributes.TargetGasLimit), CustomTxnProvider: customProvider, } if version >= clparams.CapellaVersion { diff --git a/execution/execmodule/chainreader/chain_reader.go b/execution/execmodule/chainreader/chain_reader.go index 64ac83b3771..9068ebd0dd7 100644 --- a/execution/execmodule/chainreader/chain_reader.go +++ b/execution/execmodule/chainreader/chain_reader.go @@ -336,6 +336,7 @@ func (c ChainReaderWriterEth1) AssembleBlock(baseHash common.Hash, attributes *e SuggestedFeeRecipient: attributes.SuggestedFeeRecipient, Withdrawals: attributes.Withdrawals, SlotNumber: (*uint64)(attributes.SlotNumber), + TargetGasLimit: (*uint64)(attributes.TargetGasLimit), ParentBeaconBlockRoot: attributes.ParentBeaconBlockRoot, } result, err := c.executionModule.AssembleBlock(context.Background(), params) From ae711931d0a06fdb9cb0391ce57c403d0aeedf60 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Wed, 20 May 2026 04:52:18 +0000 Subject: [PATCH 19/28] add test for engineapi --- .../engineapi/engine_api_builder_test.go | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/execution/engineapi/engine_api_builder_test.go b/execution/engineapi/engine_api_builder_test.go index b2ea4375ad2..21b7ae62c44 100644 --- a/execution/engineapi/engine_api_builder_test.go +++ b/execution/engineapi/engine_api_builder_test.go @@ -38,6 +38,7 @@ import ( "github.com/erigontech/erigon/execution/protocol/params" "github.com/erigontech/erigon/execution/state/contracts" "github.com/erigontech/erigon/execution/types" + "github.com/erigontech/erigon/node/ethconfig" "github.com/erigontech/erigon/rpc" ) @@ -264,6 +265,57 @@ func TestEngineApiBlockGasOverflowSpillsToNextBlock(t *testing.T) { }) } +// TestEngineApiV4TargetGasLimitOverridesMinerGasLimit checks that a CL-supplied +// targetGasLimit in PayloadAttributesV4 (engine_forkchoiceUpdatedV4) overrides +// the EL's static --miner.gaslimit when building a block — and that the +// resulting block respects the CL target as a cap. +// +// Setup picks numbers so the two values produce distinguishable block contents: +// - parent gas limit = 42_000 (room for two 21K-gas transfers) +// - static --miner.gaslimit = 21_000 (would cap the block at one transfer) +// - CL targetGasLimit = 42_000 (room for two transfers) +// +// Three transfers are submitted; only two must fit. If the static target won, +// the block would gas-limit at ~41_960 and contain a single transfer. +// See https://github.com/ethereum/execution-apis/pull/796. +func TestEngineApiV4TargetGasLimitOverridesMinerGasLimit(t *testing.T) { + ctx := t.Context() + logger := testlog.Logger(t, log.LvlDebug) + const targetGasLimit uint64 = 42_000 + const minerGasLimit uint64 = 21_000 + genesis, coinbaseKey, err := engineapitester.DefaultEngineApiTesterGenesis() + require.NoError(t, err) + genesis.GasLimit = targetGasLimit + eat, err := engineapitester.InitialiseEngineApiTester(ctx, engineapitester.EngineApiTesterInitArgs{ + Logger: logger, + DataDir: t.TempDir(), + Genesis: genesis, + CoinbaseKey: coinbaseKey, + EthConfigTweaker: func(config *ethconfig.Config) { + gl := minerGasLimit + config.Builder.GasLimit = &gl + }, + }) + require.NoError(t, err) + t.Cleanup(func() { + err := eat.Close() + require.NoError(t, err) + }) + eat.Run(t, func(ctx context.Context, t *testing.T, eat engineapitester.EngineApiTester) { + receiver := common.HexToAddress("0x42") + // Submit 3 transfers; only 2 should fit under the CL-supplied 42K cap. + for i := 0; i < 3; i++ { + _, err := eat.Transactor.SubmitSimpleTransfer(eat.CoinbaseKey, receiver, big.NewInt(int64(i+1))) + require.NoError(t, err) + } + payload, err := eat.MockCl.BuildCanonicalBlock(ctx) + require.NoError(t, err) + // Block gas limit follows the CL target — not the EL's --miner.gaslimit. + require.Equal(t, hexutil.Uint64(targetGasLimit), payload.ExecutionPayload.GasLimit) + require.Len(t, payload.ExecutionPayload.Transactions, 2) + }) +} + func TestEngineApiSequentialNonceAdvancement(t *testing.T) { ctx := t.Context() logger := testlog.Logger(t, log.LvlDebug) From 1169cb49e8636ccffe1c051bf86cf585c868df48 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Wed, 20 May 2026 13:42:20 +0000 Subject: [PATCH 20/28] execution: pass hive eest tests for bal-devnet-7 --- .github/workflows/test-hive-eest.yml | 13 +++-- execution/protocol/gaspool.go | 78 +++++++++++++++++++++++++- execution/protocol/txn_executor.go | 9 +-- execution/stagedsync/exec3_parallel.go | 8 ++- 4 files changed, 90 insertions(+), 18 deletions(-) diff --git a/.github/workflows/test-hive-eest.yml b/.github/workflows/test-hive-eest.yml index 1632359f721..0be0e5f4315 100644 --- a/.github/workflows/test-hive-eest.yml +++ b/.github/workflows/test-hive-eest.yml @@ -56,17 +56,20 @@ jobs: shard: rlp pool-size: 0 fixtures-tarball: eest_stable - # Glamsterdam devnet: BAL EIPs against devnet fixtures - sim: consume-engine - sim-limit: ".*(8024|7708|7778|7843|7928|7954|8037).*" + sim-limit: ".*(7708|7778|7843|7928|7954|7976|7981|8024|8037).*" shard: glamsterdam-devnet pool-size: 24 fixtures-tarball: eest_devnet extra-hive-flags: "--sim.loglevel=3 --client.checktimelimit=300s" erigon-extra-flags: "--experimental.bal" - # test_block_regular_gas_limit: error classification mismatch (GAS_USED_OVERFLOW vs GAS_ALLOWANCE_EXCEEDED) - # transitioning to bal-devnet-4 - max-failures: 579 + # EIP-7928 test_invalid_{pre,post}_fork_block_*_bal_hash_field + # expect -32602 InvalidParams (wrong test expectations), but the + # requests are well-formed V4/V5 newPayload — the only divergence + # is a blockHash computed over a wrong-fork-schema header. Spec + # convention is InvalidStatus + INVALID_BLOCK_HASH, which is what + # erigon returns. + max-failures: 2 steps: - name: Clean docker system run: | diff --git a/execution/protocol/gaspool.go b/execution/protocol/gaspool.go index 19a8c84a77f..5b5b944b79e 100644 --- a/execution/protocol/gaspool.go +++ b/execution/protocol/gaspool.go @@ -23,6 +23,11 @@ import ( "fmt" "math" "sync" + + "github.com/erigontech/erigon/execution/chain" + "github.com/erigontech/erigon/execution/protocol/mdgas" + "github.com/erigontech/erigon/execution/protocol/params" + "github.com/erigontech/erigon/execution/types" ) // GasPool tracks block-level gas availability across the two EIP-8037 dimensions. @@ -169,9 +174,8 @@ func (gp *GasPool) SubBlobGas(amount uint64) error { // CheckBlockGasInclusion verifies that the supplied per-dimension gas // values fit in the remaining EIP-8037 reservoirs. Callers compute the -// dimension contributions for their context: pre-execution uses -// min(MaxTxnGasLimit, tx.gas) and tx.gas; post-execution uses the -// realised BlockRegularGasUsed and BlockStateGasUsed. +// dimension contributions via InclusionContributions for pre-execution +// inclusion checks. func CheckBlockGasInclusion(gp *GasPool, regularGas, stateGas uint64) error { if gp == nil { return nil @@ -185,6 +189,74 @@ func CheckBlockGasInclusion(gp *GasPool, regularGas, stateGas uint64) error { return nil } +// InclusionContributions returns the per-dimension gas contributions for +// the EIP-8037 block-pool inclusion check. Use this from call sites that +// don't already have the intrinsic gas computed; otherwise prefer +// InclusionContributionsWithIgas to avoid the second IntrinsicGas pass. +// Returns ErrGasUintOverflow if intrinsic gas computation overflows uint64. +func InclusionContributions(txn types.Transaction, rules *chain.Rules) (uint64, uint64, error) { + if txn == nil { + return 0, 0, nil + } + gas := txn.GetGasLimit() + if !rules.IsAmsterdam { + return gas, 0, nil + } + + accessList := txn.GetAccessList() + intrinsic, overflow := mdgas.IntrinsicGas(mdgas.IntrinsicGasCalcArgs{ + Data: txn.GetData(), + AuthorizationsLen: uint64(len(txn.GetAuthorizations())), + AccessListLen: uint64(len(accessList)), + StorageKeysLen: uint64(accessList.StorageKeys()), + IsContractCreation: txn.GetTo() == nil, + IsEIP2: rules.IsHomestead, + IsEIP2028: rules.IsIstanbul, + IsEIP3860: rules.IsShanghai, + IsEIP7623: rules.IsPrague, + IsEIP7976: rules.IsAmsterdam, + IsEIP7981: rules.IsAmsterdam, + IsEIP8037: rules.IsAmsterdam, + }) + if overflow { + return 0, 0, ErrGasUintOverflow + } + regular, state := InclusionContributionsWithIgas(gas, intrinsic, rules.IsAmsterdam) + return regular, state, nil +} + +// InclusionContributionsWithIgas returns the per-dimension gas contributions +// for the EIP-8037 block-pool inclusion check, given the tx's declared +// gas_limit and the precomputed intrinsic gas result. +// +// Pre-Amsterdam: only the regular dimension is exercised; state is 0. +// Amsterdam onwards (EIP-8037), per the spec the inclusion check is: +// +// regular_contribution = min(MaxTxnGasLimit, tx.gas - intrinsic.state) +// state_contribution = tx.gas - intrinsic.regular +// +// Subtracting the matching intrinsic dimension from tx.gas before checking +// against the opposite reservoir is what lets a tx with a high gas_limit +// (e.g. a creation paying intrinsic.state up front, or an EIP-7702 tx +// paying per-auth state cost) still be includable when the un-subtracted +// gas_limit would over-budget the opposite dimension. +func InclusionContributionsWithIgas(gas uint64, intrinsic mdgas.IntrinsicGasCalcResult, isAmsterdam bool) (uint64, uint64) { + if !isAmsterdam { + return gas, 0 + } + var regularContribution, stateContribution uint64 + if gas >= intrinsic.StateGas { + regularContribution = gas - intrinsic.StateGas + } + if regularContribution > params.MaxTxnGasLimit { + regularContribution = params.MaxTxnGasLimit + } + if gas >= intrinsic.RegularGas { + stateContribution = gas - intrinsic.RegularGas + } + return regularContribution, stateContribution +} + // BlobGas returns the blob gas remaining. func (gp *GasPool) BlobGas() uint64 { if gp == nil { diff --git a/execution/protocol/txn_executor.go b/execution/protocol/txn_executor.go index 642b92ee92c..eeb9b6d253d 100644 --- a/execution/protocol/txn_executor.go +++ b/execution/protocol/txn_executor.go @@ -303,14 +303,7 @@ func (st *TxnExecutor) preCheck(gasBailout bool, intrinsicGasResult mdgas.Intrin } } - regularContribution := st.msg.Gas() - var stateContribution uint64 - if rules.IsAmsterdam { - stateContribution = regularContribution - if regularContribution > params.MaxTxnGasLimit { - regularContribution = params.MaxTxnGasLimit - } - } + regularContribution, stateContribution := InclusionContributionsWithIgas(st.msg.Gas(), intrinsicGasResult, rules.IsAmsterdam) if err := CheckBlockGasInclusion(st.gp, regularContribution, stateContribution); err != nil { return err } diff --git a/execution/stagedsync/exec3_parallel.go b/execution/stagedsync/exec3_parallel.go index 3160516402e..88ce930f74d 100644 --- a/execution/stagedsync/exec3_parallel.go +++ b/execution/stagedsync/exec3_parallel.go @@ -2664,8 +2664,12 @@ func (be *blockExecutor) nextResult(ctx context.Context, pe *parallelExecutor, r txTask := be.tasks[tx].Task if txn := txTask.Tx(); txn != nil { - if err := protocol.CheckBlockGasInclusion(be.gasPool, txResult.ExecutionResult.BlockRegularGasUsed, txResult.ExecutionResult.BlockStateGasUsed); err != nil { - return nil, fmt.Errorf("%w, block=%d txIdx=%d: %w", rules.ErrInvalidBlock, be.blockNum, txVersion.TxIndex, err) + regularContribution, stateContribution, err := protocol.InclusionContributions(txn, txTask.Rules()) + if err != nil { + return be.invalidBlockResult(fmt.Errorf("%w, block=%d txIdx=%d: %w", rules.ErrInvalidBlock, be.blockNum, txVersion.TxIndex, err)), nil + } + if err := protocol.CheckBlockGasInclusion(be.gasPool, regularContribution, stateContribution); err != nil { + return be.invalidBlockResult(fmt.Errorf("%w: block gas used overflow at block=%d txIdx=%d: %w", rules.ErrInvalidBlock, be.blockNum, txVersion.TxIndex, err)), nil } } From 399e85893295ddf6dc7e49661cd31abca0a633bf Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Thu, 21 May 2026 08:27:36 +0000 Subject: [PATCH 21/28] execution: fix BALs for selfdestrcut beneficiaries --- .../bal_selfdestruct_systemaddress_test.go | 249 ++++++++++++++++++ execution/state/intra_block_state.go | 35 --- execution/vm/operations_acl.go | 28 +- 3 files changed, 252 insertions(+), 60 deletions(-) create mode 100644 execution/execmodule/bal_selfdestruct_systemaddress_test.go diff --git a/execution/execmodule/bal_selfdestruct_systemaddress_test.go b/execution/execmodule/bal_selfdestruct_systemaddress_test.go new file mode 100644 index 00000000000..3c1271d973c --- /dev/null +++ b/execution/execmodule/bal_selfdestruct_systemaddress_test.go @@ -0,0 +1,249 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package execmodule_test + +import ( + "math/big" + "testing" + + "github.com/holiman/uint256" + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common" + "github.com/erigontech/erigon/common/crypto" + "github.com/erigontech/erigon/execution/chain" + "github.com/erigontech/erigon/execution/execmodule/execmoduletester" + "github.com/erigontech/erigon/execution/protocol/params" + "github.com/erigontech/erigon/execution/tests/blockgen" + "github.com/erigontech/erigon/execution/types" +) + +// TestBALIncludesSystemAddressOnSelfdestructToItWithZeroBalance asserts +// the EIP-7928 rule that a SELFDESTRUCT records the beneficiary as a +// state access independently of value transfer, combined with the +// SystemAddress carve-out exception ("MUST NOT be included unless it +// experiences state access itself"). A SELFDESTRUCT to SystemAddress +// is itself such an access, so the entry MUST appear in the BAL even +// when contract balance is zero and no value is transferred. +// +// Init code: PUSH20 SystemAddress; SELFDESTRUCT (0x73 || 20 bytes || 0xff) +func TestBALIncludesSystemAddressOnSelfdestructToItWithZeroBalance(t *testing.T) { + t.Parallel() + + privKey, err := crypto.GenerateKey() + require.NoError(t, err) + senderAddr := crypto.PubkeyToAddress(privKey.PublicKey) + + genesis := &types.Genesis{ + Config: chain.AllProtocolChanges, // AmsterdamTime = 0, EIP-7928 active + Alloc: types.GenesisAlloc{ + senderAddr: {Balance: new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil)}, + }, + } + m := execmoduletester.New(t, + execmoduletester.WithGenesisSpec(genesis), + execmoduletester.WithKey(privKey), + ) + + require.True(t, m.ChainConfig.IsAmsterdam(0), "Amsterdam must be active at genesis time for EIP-7928") + + // Init code: PUSH20 SystemAddress; SELFDESTRUCT. + // Bytecode: 0x73 || 20-byte SystemAddress || 0xff + initCode := append([]byte{0x73}, + append(params.SystemAddress.Value().Bytes(), 0xff)..., + ) + + baseFee := m.Genesis.BaseFee().Uint64() + gasPrice := baseFee * 2 + signer := types.LatestSignerForChainID(m.ChainConfig.ChainID) + + chainPack, err := blockgen.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, 1, func(i int, gen *blockgen.BlockGen) { + nonce := gen.TxNonce(senderAddr) + tx, txErr := types.SignTx( + types.NewContractCreation(nonce, uint256.NewInt(0), 1_000_000, uint256.NewInt(gasPrice), initCode), + *signer, + privKey, + ) + require.NoError(t, txErr) + gen.AddTx(tx) + }) + require.NoError(t, err) + + // The CREATE tx must succeed: the init code runs SELFDESTRUCT before + // returning any deploy bytes, so the receipt is a successful CREATE + // with no deployed code at the contract address. + require.Len(t, chainPack.Receipts[0], 1) + receipt := chainPack.Receipts[0][0] + require.Equal(t, types.ReceiptStatusSuccessful, receipt.Status, + "SELFDESTRUCT-in-init-code CREATE tx should succeed") + + // Sanity: the block header carries the BAL hash now that Amsterdam is + // active. The BlockAccessLists payload is the RLP encoding that hashes + // to BlockAccessListHash. + require.NotNil(t, chainPack.Headers[0].BlockAccessListHash, + "Amsterdam block header must carry BlockAccessListHash") + require.NotEmpty(t, chainPack.BlockAccessLists[0], + "Amsterdam block must produce a non-empty BAL payload") + + bal, err := types.DecodeBlockAccessListBytes(chainPack.BlockAccessLists[0]) + require.NoError(t, err) + + sysAddr := params.SystemAddress + var sysEntry *types.AccountChanges + for _, ac := range bal { + if ac.Address == sysAddr { + sysEntry = ac + break + } + } + + require.NotNil(t, sysEntry, + "BAL must include a SystemAddress entry: EIP-7928 records SELFDESTRUCT as an access on the beneficiary even when no value is transferred, and the SystemAddress carve-out exception fires because the SELFDESTRUCT is the access itself.") + + // All changesets are empty: the SELFDESTRUCT is the only access on + // SystemAddress in this block; no balance/nonce/code/storage state + // actually changes on the beneficiary. + require.Empty(t, sysEntry.StorageChanges, "SystemAddress entry should have no storage changes") + require.Empty(t, sysEntry.StorageReads, "SystemAddress entry should have no storage reads") + require.Empty(t, sysEntry.BalanceChanges, "SystemAddress entry should have no balance changes (zero transfer)") + require.Empty(t, sysEntry.NonceChanges, "SystemAddress entry should have no nonce changes") + require.Empty(t, sysEntry.CodeChanges, "SystemAddress entry should have no code changes") +} + +// TestBALIncludesSystemAddressOnSelfdestructToItWithNonZeroBalance is a +// guard for the non-zero balance variant: when the destructing contract +// has a non-zero balance, the BAL records a balance change on the +// SystemAddress beneficiary in addition to the access already recorded +// by the SELFDESTRUCT itself. +func TestBALIncludesSystemAddressOnSelfdestructToItWithNonZeroBalance(t *testing.T) { + t.Parallel() + + privKey, err := crypto.GenerateKey() + require.NoError(t, err) + senderAddr := crypto.PubkeyToAddress(privKey.PublicKey) + + genesis := &types.Genesis{ + Config: chain.AllProtocolChanges, + Alloc: types.GenesisAlloc{ + senderAddr: {Balance: new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil)}, + }, + } + m := execmoduletester.New(t, + execmoduletester.WithGenesisSpec(genesis), + execmoduletester.WithKey(privKey), + ) + + // Same init code as the zero-balance test — but this time the CREATE + // tx sends 1 wei to the new contract so its balance is non-zero at + // SELFDESTRUCT time. + initCode := append([]byte{0x73}, + append(params.SystemAddress.Value().Bytes(), 0xff)..., + ) + + baseFee := m.Genesis.BaseFee().Uint64() + gasPrice := baseFee * 2 + signer := types.LatestSignerForChainID(m.ChainConfig.ChainID) + + chainPack, err := blockgen.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, 1, func(i int, gen *blockgen.BlockGen) { + nonce := gen.TxNonce(senderAddr) + tx, txErr := types.SignTx( + types.NewContractCreation(nonce, uint256.NewInt(1), 1_000_000, uint256.NewInt(gasPrice), initCode), + *signer, + privKey, + ) + require.NoError(t, txErr) + gen.AddTx(tx) + }) + require.NoError(t, err) + require.Equal(t, types.ReceiptStatusSuccessful, chainPack.Receipts[0][0].Status) + + bal, err := types.DecodeBlockAccessListBytes(chainPack.BlockAccessLists[0]) + require.NoError(t, err) + + var sysEntry *types.AccountChanges + for _, ac := range bal { + if ac.Address == params.SystemAddress { + sysEntry = ac + break + } + } + require.NotNil(t, sysEntry, "BAL must include SystemAddress entry on non-zero-balance SELFDESTRUCT to it") +} + +// TestBALIncludesOrdinaryBeneficiaryOnSelfdestructWithZeroBalance is a +// guard for non-SystemAddress beneficiaries: SELFDESTRUCT with zero +// contract balance to an ordinary EOA must still record the beneficiary +// in the BAL. The SystemAddress carve-out filter does not apply here, +// so the entry's inclusion follows directly from the general EIP-7928 +// rule for SELFDESTRUCT beneficiaries. +func TestBALIncludesOrdinaryBeneficiaryOnSelfdestructWithZeroBalance(t *testing.T) { + t.Parallel() + + privKey, err := crypto.GenerateKey() + require.NoError(t, err) + senderAddr := crypto.PubkeyToAddress(privKey.PublicKey) + + // An arbitrary non-SystemAddress beneficiary EOA. Pick something + // distinct from the sender / coinbase / system contracts so we can + // tell it apart in the BAL. + beneficiary := common.HexToAddress("0x00000000000000000000000000000000000000aa") + + genesis := &types.Genesis{ + Config: chain.AllProtocolChanges, + Alloc: types.GenesisAlloc{ + senderAddr: {Balance: new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil)}, + }, + } + m := execmoduletester.New(t, + execmoduletester.WithGenesisSpec(genesis), + execmoduletester.WithKey(privKey), + ) + + // Init code: PUSH20 ; SELFDESTRUCT. + initCode := append([]byte{0x73}, + append(beneficiary.Bytes(), 0xff)..., + ) + + baseFee := m.Genesis.BaseFee().Uint64() + gasPrice := baseFee * 2 + signer := types.LatestSignerForChainID(m.ChainConfig.ChainID) + + chainPack, err := blockgen.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, 1, func(i int, gen *blockgen.BlockGen) { + nonce := gen.TxNonce(senderAddr) + tx, txErr := types.SignTx( + types.NewContractCreation(nonce, uint256.NewInt(0), 1_000_000, uint256.NewInt(gasPrice), initCode), + *signer, + privKey, + ) + require.NoError(t, txErr) + gen.AddTx(tx) + }) + require.NoError(t, err) + require.Equal(t, types.ReceiptStatusSuccessful, chainPack.Receipts[0][0].Status) + + bal, err := types.DecodeBlockAccessListBytes(chainPack.BlockAccessLists[0]) + require.NoError(t, err) + + var found bool + for _, ac := range bal { + if ac.Address.Value() == beneficiary { + found = true + break + } + } + require.True(t, found, "ordinary EOA beneficiary must appear in BAL on SELFDESTRUCT-to-it") +} diff --git a/execution/state/intra_block_state.go b/execution/state/intra_block_state.go index a69f79687d6..394becdf992 100644 --- a/execution/state/intra_block_state.go +++ b/execution/state/intra_block_state.go @@ -2429,41 +2429,6 @@ func (sdb *IntraBlockState) MarkReadsInternal(addr accounts.Address) { } } -// SnapshotVersionedReadKeys returns the current set of read keys for addr. -// Used with MarkNewReadsInternal to mark only reads added after the snapshot, -// preserving pre-existing legitimate reads. -func (sdb *IntraBlockState) SnapshotVersionedReadKeys(addr accounts.Address) map[AccountKey]struct{} { - if sdb.versionedReads == nil { - return nil - } - reads := sdb.versionedReads[addr] - if len(reads) == 0 { - return nil - } - snapshot := make(map[AccountKey]struct{}, len(reads)) - for k := range reads { - snapshot[k] = struct{}{} - } - return snapshot -} - -// MarkNewReadsInternal marks as internal only the reads for addr that were -// added after the given snapshot. Use this when gas-calculation-only reads -// were recorded on top of earlier legitimate reads — the legitimate ones -// must remain non-internal so they appear in the block access list. -func (sdb *IntraBlockState) MarkNewReadsInternal(addr accounts.Address, before map[AccountKey]struct{}) { - if sdb.versionedReads == nil { - return - } - for key, vr := range sdb.versionedReads[addr] { - if _, existed := before[key]; existed { - continue - } - vr.internal = true - sdb.versionedReads[addr][key] = vr - } -} - // AccessedAddresses returns and resets the set of addresses touched during the current transaction. func (sdb *IntraBlockState) AccessedAddresses() AccessSet { if len(sdb.addressAccess) == 0 { diff --git a/execution/vm/operations_acl.go b/execution/vm/operations_acl.go index 9b603fd749d..d7eb2e801f9 100644 --- a/execution/vm/operations_acl.go +++ b/execution/vm/operations_acl.go @@ -260,12 +260,6 @@ func makeSelfdestructGasFn(refundsEnabled bool) gasFunc { evm.IntraBlockState().AddAddressToAccessList(address) } - // Snapshot the beneficiary's existing reads before Empty() — so we can - // later mark only the reads newly recorded by the gas calc as internal, - // preserving any pre-existing legitimate reads (e.g. the tx sender's - // fee-deduction balance read when the sender is the SELFDESTRUCT - // beneficiary). - beneficiaryReadsBefore := evm.IntraBlockState().SnapshotVersionedReadKeys(address) // if empty and transfers value empty, err := evm.IntraBlockState().Empty(address) if err != nil { @@ -275,25 +269,9 @@ func makeSelfdestructGasFn(refundsEnabled bool) gasFunc { if err != nil { return mdgas.MdGas{}, err } - // Record the beneficiary address access for BAL tracking when the - // contract has non-zero balance. A zero-balance SELFDESTRUCT does - // not transfer value, so the beneficiary should not appear in the - // block access list on that basis alone. - // (The read-only path is unreachable here — see the early-return - // above.) - if !balance.IsZero() { - evm.IntraBlockState().MarkAddressAccess(address, false) - } - // When the destructed contract has zero balance, no value is - // transferred to the beneficiary. Mark the reads the Empty() gas-calc - // call above added for this beneficiary as internal so the - // beneficiary does not appear in the BAL purely because of that - // gas-calc access. Pre-existing reads are preserved. Skip when - // beneficiary == self so the contract's own legitimate reads are - // not affected. - if balance.IsZero() && address != callContext.Address() { - evm.IntraBlockState().MarkNewReadsInternal(address, beneficiaryReadsBefore) - } + // Per EIP-7928, SELFDESTRUCT is a state access on the beneficiary + // independently of any value transfer, so record it unconditionally. + evm.IntraBlockState().MarkAddressAccess(address, false) if empty && !balance.IsZero() { if evm.chainRules.IsAmsterdam { gas.State = params.StateGasNewAccount From 9539be8164846d4931f315d755f91a06517bfd4c Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Tue, 2 Jun 2026 06:08:30 +0000 Subject: [PATCH 22/28] execution/state: exclude no-op storage writes from the block access list --- execution/engineapi/engine_api_bal_test.go | 79 ++++++++++++++++++++++ execution/state/versionedio.go | 47 ++++++------- execution/state/versionedio_test.go | 43 ++++++++++++ 3 files changed, 142 insertions(+), 27 deletions(-) diff --git a/execution/engineapi/engine_api_bal_test.go b/execution/engineapi/engine_api_bal_test.go index eebff51a06a..eae417fb79d 100644 --- a/execution/engineapi/engine_api_bal_test.go +++ b/execution/engineapi/engine_api_bal_test.go @@ -310,6 +310,85 @@ func TestEngineApiBALStorageWrites(t *testing.T) { }) } +// TestEngineApiBALStorageNoOpWriteOmitted exercises the EIP-7928 no-op rule +// end-to-end through the parallel executor: a write storing the value a slot +// already holds must not appear in storage_changes. The contract does +// SSTORE(0,2);SSTORE(0,1) so the final write is recorded even though it equals +// the slot's prior value — a plain same-value SSTORE would be skipped before +// reaching the access list. Called twice in one block, the second call is a +// no-op whose write must be excluded. +func TestEngineApiBALStorageNoOpWriteOmitted(t *testing.T) { + if !dbg.Exec3Parallel { + t.Skip("requires parallel exec") + } + ctx := t.Context() + logger := testlog.Logger(t, log.LvlDebug) + eat, err := engineapitester.DefaultEngineApiTester(ctx, logger, t.TempDir()) + require.NoError(t, err) + t.Cleanup(func() { + err := eat.Close() + require.NoError(t, err) + }) + eat.Run(t, func(ctx context.Context, t *testing.T, eat engineapitester.EngineApiTester) { + signer := types.LatestSignerForChainID(eat.ChainConfig.ChainID) + coinbaseAddr := crypto.PubkeyToAddress(eat.CoinbaseKey.PublicKey) + gasPrice, err := eat.RpcApiClient.GasPrice() + require.NoError(t, err) + gasPriceU256, overflow := uint256.FromBig(gasPrice) + require.False(t, overflow) + + // init: 600b600c600039600b6000f3 -> return the 11-byte runtime + // code: 6002600055 6001600055 00 -> SSTORE(0,2); SSTORE(0,1); STOP + initCode := common.FromHex("600b600c600039600b6000f36002600055600160005500") + + signTx := func(nonce uint64, to *common.Address, data []byte, gas uint64) types.Transaction { + tx, err := types.SignTx(&types.LegacyTx{ + CommonTx: types.CommonTx{Nonce: nonce, GasLimit: gas, To: to, Value: uint256.Int{}, Data: data}, + GasPrice: *gasPriceU256, + }, *signer, eat.CoinbaseKey) + require.NoError(t, err) + return tx + } + + // Block 1: deploy the contract. + deployNonce, err := eat.RpcApiClient.GetTransactionCount(coinbaseAddr, rpc.PendingBlock) + require.NoError(t, err) + deployTx := signTx(deployNonce.Uint64(), nil, initCode, 1_000_000) + _, err = eat.RpcApiClient.SendTransaction(deployTx) + require.NoError(t, err) + _, err = eat.MockCl.BuildCanonicalBlock(ctx) + require.NoError(t, err) + + contractAddr := types.CreateAddress(coinbaseAddr, deployNonce.Uint64()) + code, err := eat.RpcApiClient.GetCode(contractAddr, rpc.LatestBlock) + require.NoError(t, err) + require.NotEmpty(t, code, "contract must be deployed") + + // Block 2: two calls to the contract in the same block. Same sender, so + // they execute in nonce order — fully deterministic, no scheduling race. + callNonce, err := eat.RpcApiClient.GetTransactionCount(coinbaseAddr, rpc.PendingBlock) + require.NoError(t, err) + firstCall := signTx(callNonce.Uint64(), &contractAddr, nil, 1_000_000) + secondCall := signTx(callNonce.Uint64()+1, &contractAddr, nil, 1_000_000) + _, err = eat.RpcApiClient.SendTransaction(firstCall) + require.NoError(t, err) + _, err = eat.RpcApiClient.SendTransaction(secondCall) + require.NoError(t, err) + + payload, err := eat.MockCl.BuildCanonicalBlock(ctx) + require.NoError(t, err, "BAL divergence: a repeated same-value SSTORE was recorded as a storage_change") + require.NoError(t, eat.TxnInclusionVerifier.VerifyTxnsInclusion(ctx, payload.ExecutionPayload, firstCall.Hash(), secondCall.Hash())) + + bal := decodeAndValidateBAL(t, payload) + cc := findAccountChanges(bal, accounts.InternAddress(contractAddr)) + require.NotNilf(t, cc, "contract must appear in BAL\n%s", bal.DebugString()) + require.Lenf(t, cc.StorageChanges, 1, "contract writes exactly one slot\n%s", bal.DebugString()) + require.Lenf(t, cc.StorageChanges[0].Changes, 1, + "slot 0 must record only the first call's real change (0->1); the second call leaves the value unchanged, so the no-op write must not appear in storage_changes\n%s", + bal.DebugString()) + }) +} + func TestEngineApiBALMultiTxBlock(t *testing.T) { if !dbg.Exec3Parallel { t.Skip("requires parallel exec") diff --git a/execution/state/versionedio.go b/execution/state/versionedio.go index bec33519696..27b23436056 100644 --- a/execution/state/versionedio.go +++ b/execution/state/versionedio.go @@ -1589,17 +1589,9 @@ func ensureAccountState(accounts map[accounts.Address]*accountState, addr accoun func (account *accountState) updateWrite(vw *VersionedWrite, accessIndex uint32) { switch vw.Path { case StoragePath: - // Skip intra-tx net-zero storage writes: if this is the first write - // to the slot (no prior tx wrote to it) and the written value equals - // the original read value, it's a no-op that should remain as a read. - if !hasStorageWrite(account.changes, vw.Key) { - if val, ok := vw.Val.(uint256.Int); ok { - if origVal, wasRead := account.storageReadValues[vw.Key]; wasRead && val.Eq(&origVal) { - return - } - } + if val, ok := vw.Val.(uint256.Int); ok { + account.addStorageUpdate(vw.Key, accessIndex, val) } - addStorageUpdate(account.changes, vw, accessIndex) case BalancePath: val, ok := vw.Val.(uint256.Int) if !ok { @@ -1714,28 +1706,29 @@ func (account *accountState) updateRead(vr *VersionedRead) { } } -func addStorageUpdate(ac *types.AccountChanges, vw *VersionedWrite, txIndex uint32) { - // If we already recorded a read for this slot, drop it because a write takes precedence. - removeStorageRead(ac, vw.Key) - - if ac.StorageChanges == nil { - ac.StorageChanges = []*types.SlotChanges{{ - Slot: vw.Key, - Changes: []*types.StorageChange{{Index: txIndex, Value: vw.Val.(uint256.Int)}}, - }} - return - } - +// addStorageUpdate records a value-changing storage write in a single pass over +// the slot's changes, applying the EIP-7928 no-op filter: a write whose value +// equals the slot's current value — the most recent earlier write this block, +// or the pre-block read value for the first write — is not a change and is +// omitted (the slot stays a read). +func (account *accountState) addStorageUpdate(slot accounts.StorageKey, accessIndex uint32, val uint256.Int) { + ac := account.changes for _, slotChange := range ac.StorageChanges { - if slotChange.Slot == vw.Key { - slotChange.Changes = append(slotChange.Changes, &types.StorageChange{Index: txIndex, Value: vw.Val.(uint256.Int)}) + if slotChange.Slot == slot { + if n := len(slotChange.Changes); n > 0 && val.Eq(&slotChange.Changes[n-1].Value) { + return + } + slotChange.Changes = append(slotChange.Changes, &types.StorageChange{Index: accessIndex, Value: val}) return } } - + if origVal, wasRead := account.storageReadValues[slot]; wasRead && val.Eq(&origVal) { + return + } + removeStorageRead(ac, slot) // a real write supersedes any recorded read ac.StorageChanges = append(ac.StorageChanges, &types.SlotChanges{ - Slot: vw.Key, - Changes: []*types.StorageChange{{Index: txIndex, Value: vw.Val.(uint256.Int)}}, + Slot: slot, + Changes: []*types.StorageChange{{Index: accessIndex, Value: val}}, }) } diff --git a/execution/state/versionedio_test.go b/execution/state/versionedio_test.go index 07ca706db4a..54fde81b187 100644 --- a/execution/state/versionedio_test.go +++ b/execution/state/versionedio_test.go @@ -431,6 +431,49 @@ func TestVersionedIO_PostWriteBalanceReadDoesNotPoisonInitialBalance(t *testing. require.True(t, found, "burn target address must appear in BAL") } +// TestVersionedIO_StorageNoOpWriteAfterChangeOmittedFromBAL verifies the +// EIP-7928 rule that, for a slot written multiple times in a block, a write +// storing the value an earlier write already set is a no-op and must be +// excluded from storage_changes — only value-changing writes are recorded. +func TestVersionedIO_StorageNoOpWriteAfterChangeOmittedFromBAL(t *testing.T) { + t.Parallel() + + addr := accounts.InternAddress(common.HexToAddress("0xc0ffee")) + slot := accounts.InternKey(common.HexToHash("0x01")) + valA := *uint256.NewInt(0x111) + valB := *uint256.NewInt(0x222) + + io := NewVersionedIO(4) + write := func(txIndex int, v uint256.Int) { + io.RecordWrites(Version{TxIndex: txIndex}, VersionedWrites{ + &VersionedWrite{Address: addr, Path: StoragePath, Key: slot, Version: Version{TxIndex: txIndex}, Val: v}, + }) + } + write(0, valA) // slot: 0 -> A (real change) + write(1, valB) // slot: A -> B (real change) + write(2, valB) // slot: B -> B (no-op) + write(3, valB) // slot: B -> B (no-op) + + bal := io.AsBlockAccessList() + + found := false + for _, ac := range bal { + if ac.Address != addr { + continue + } + found = true + require.Lenf(t, ac.StorageChanges, 1, "one slot expected\n%s", bal.DebugString()) + changes := ac.StorageChanges[0].Changes + require.Lenf(t, changes, 2, + "no-op repeated-value writes (idx 3,4) must be excluded from storage_changes\n%s", bal.DebugString()) + require.Equal(t, uint32(1), changes[0].Index) + require.True(t, changes[0].Value.Eq(&valA)) + require.Equal(t, uint32(2), changes[1].Index) + require.True(t, changes[1].Value.Eq(&valB)) + } + require.True(t, found, "contract must appear in BAL") +} + // fallthroughStateReader returns a fixed account and a fixed storage value, so // a test can observe whether a versionedRead fell through to the underlying // state (vs. returning a version-map value or aborting). From 64cd414ff57dd5d255ecd28bf31c552c3c69e988 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 6 Jun 2026 02:59:13 +0000 Subject: [PATCH 23/28] execution: rationalise extremely verbose val logging --- execution/engineapi/engine_server.go | 2 - execution/stagedsync/bal_create.go | 118 ++++++++----------------- execution/stagedsync/exec3_parallel.go | 2 +- 3 files changed, 39 insertions(+), 83 deletions(-) diff --git a/execution/engineapi/engine_server.go b/execution/engineapi/engine_server.go index 54cabdaca3b..e25ff7fe90e 100644 --- a/execution/engineapi/engine_server.go +++ b/execution/engineapi/engine_server.go @@ -388,8 +388,6 @@ func (s *EngineServer) newPayload(ctx context.Context, req *engine_types.Executi } } - log.Debug(fmt.Sprintf("bal from header: %s", blockAccessList.DebugString())) - if (!s.config.IsCancun(header.Time) && version >= clparams.DenebVersion) || (s.config.IsCancun(header.Time) && version < clparams.DenebVersion) || (!s.config.IsPrague(header.Time) && version >= clparams.ElectraVersion) || diff --git a/execution/stagedsync/bal_create.go b/execution/stagedsync/bal_create.go index e0dad3d75f2..351613bd264 100644 --- a/execution/stagedsync/bal_create.go +++ b/execution/stagedsync/bal_create.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" + "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/dbg" "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/db/kv" @@ -14,40 +15,31 @@ import ( "github.com/erigontech/erigon/execution/types" ) -func CreateBAL(blockNum uint64, txIO *state.VersionedIO, dataDir string) types.BlockAccessList { +func CreateBAL(blockNum uint64, txIO *state.VersionedIO, dataDir string, logger log.Logger) types.BlockAccessList { bal := txIO.AsBlockAccessList() if dbg.TraceBlockAccessLists { - writeBALToFile(bal, blockNum, dataDir) + writeBALToFile(bal, dataDir, fmt.Sprintf("computed_bal_%d.txt", blockNum), logger) } return bal } -// writeBALToFile writes the Block Access List to a text file for debugging/analysis -func writeBALToFile(bal types.BlockAccessList, blockNum uint64, dataDir string) { +// writeBALToFile dumps the Block Access List to /bal/ for debugging/analysis. +func writeBALToFile(bal types.BlockAccessList, dataDir string, name string, logger log.Logger) { if dataDir == "" { return } - balDir := filepath.Join(dataDir, "bal") - if err := os.MkdirAll(balDir, 0755); err != nil { - log.Warn("Failed to create BAL directory", "dir", balDir, "error", err) + if err := os.MkdirAll(balDir, 0o755); err != nil { + logger.Warn("failed to create BAL debug directory", "dir", balDir, "err", err) return } - - filename := filepath.Join(balDir, fmt.Sprintf("bal_block_%d.txt", blockNum)) - - file, err := os.Create(filename) - if err != nil { - log.Warn("Failed to create BAL file", "blockNum", blockNum, "error", err) - return + path := filepath.Join(balDir, name) + if err := os.WriteFile(path, []byte(bal.DebugString()), 0o644); err != nil { + logger.Warn("failed to write BAL debug file", "path", path, "err", err) } - defer file.Close() - - fmt.Fprintf(file, "Block Access List for Block %d\n", blockNum) - bal.DebugPrint(file) } -func ProcessBAL(tx kv.TemporalRwTx, h *types.Header, vio *state.VersionedIO, isEIP7928 bool, experimental bool, dataDir string) error { +func ProcessBAL(tx kv.TemporalRwTx, h *types.Header, vio *state.VersionedIO, isEIP7928 bool, experimental bool, dataDir string, logger log.Logger) error { if !isEIP7928 && !experimental { return nil } @@ -56,97 +48,63 @@ func ProcessBAL(tx kv.TemporalRwTx, h *types.Header, vio *state.VersionedIO, isE } blockNum := h.Number.Uint64() blockHash := h.Hash() - bal := CreateBAL(blockNum, vio, dataDir) - err := bal.Validate() + computedBlockBal := CreateBAL(blockNum, vio, dataDir, logger) + err := computedBlockBal.Validate() if err != nil { return fmt.Errorf("block %d: invalid computed block access list: %w", blockNum, err) } - log.Debug("bal", "blockNum", blockNum, "hash", bal.Hash()) if !isEIP7928 { return nil } // EIP-7928 size bound is only consensus-binding once Amsterdam activates. - if err := bal.ValidateMaxItems(h.GasLimit); err != nil { + if err := computedBlockBal.ValidateMaxItems(h.GasLimit); err != nil { return fmt.Errorf("%w, block %d: %w", rules.ErrInvalidBlock, blockNum, err) } if h.BlockAccessListHash == nil { return fmt.Errorf("block %d: EIP-7928 active but BlockAccessListHash is nil in header", blockNum) } - headerBALHash := *h.BlockAccessListHash - dbBALBytes, err := rawdb.ReadBlockAccessListBytes(tx, blockHash, blockNum) + blockBalHash := *h.BlockAccessListHash + blockBalBytes, err := rawdb.ReadBlockAccessListBytes(tx, blockHash, blockNum) if err != nil { return fmt.Errorf("block %d: read stored block access list: %w", blockNum, err) } // A stored BAL sidecar may be absent — eth/71 backfill is best-effort and // never blocks stage progress — so cross-check it only when present. - if dbBALBytes != nil { - dbBAL, err := types.DecodeBlockAccessListBytes(dbBALBytes) + var blockBal types.BlockAccessList + if blockBalBytes != nil { + blockBal, err = types.DecodeBlockAccessListBytes(blockBalBytes) if err != nil { return fmt.Errorf("block %d: read stored block access list: %w", blockNum, err) } - if err = dbBAL.Validate(); err != nil { + if err = blockBal.Validate(); err != nil { return fmt.Errorf("block %d: db block access list is invalid: %w", blockNum, err) } - if err = dbBAL.ValidateMaxItems(h.GasLimit); err != nil { + if err = blockBal.ValidateMaxItems(h.GasLimit); err != nil { return fmt.Errorf("block %d: stored block access list exceeds max items: %w", blockNum, err) } - - if headerBALHash != dbBAL.Hash() { - log.Info(fmt.Sprintf("bal from block: %s", dbBAL.DebugString())) - return fmt.Errorf("block %d: invalid block access list, hash mismatch: got %s expected %s", blockNum, dbBAL.Hash(), headerBALHash) + if blockBalHash != blockBal.Hash() { + reportBALMismatch(blockNum, blockHash, blockBal, blockBalHash, computedBlockBal, dataDir, logger) + return fmt.Errorf("block %d: invalid block access list, hash mismatch: got %s expected %s", blockNum, blockBal.Hash(), blockBalHash) } } // Always validate computed BAL against header. The BalancePath cross-check // in VersionMap.validateRead ensures deterministic parallel execution even // without a stored BAL body (HasBAL=false), so the computed BAL is accurate. - if headerBALHash != bal.Hash() { - // Dump full BAL DebugStrings to stderr via the logger so container - // stdout captures (kurtosis/docker log collection) preserve the diff - // even when the on-disk dumpDir below is unreachable (e.g. ephemeral - // container, CI runner without artifact capture of /data). - // Each DebugString is one multi-line value tagged with block metadata - // so operators can diff computed vs stored without hunting for files. - computedDebug := bal.DebugString() - var storedDebug string - if dbBALBytes != nil { - dbBAL2, decErr := types.DecodeBlockAccessListBytes(dbBALBytes) - if decErr != nil { - log.Warn("failed to decode stored BAL for debug dump", "err", decErr) - } else if dbBAL2 != nil { - storedDebug = dbBAL2.DebugString() - } - } - log.Error("BAL mismatch: computed", "block", blockNum, "hash", bal.Hash(), "headerHash", headerBALHash, "bal", computedDebug) - if storedDebug != "" { - log.Error("BAL mismatch: stored (from sidecar)", "block", blockNum, "hash", headerBALHash, "bal", storedDebug) - } - - dumpDir := "" - if dataDir != "" { - balDir := filepath.Join(dataDir, "bal") - if err := os.MkdirAll(balDir, 0o755); err != nil { - log.Warn("failed to create BAL debug directory", "dir", balDir, "err", err) - } else { - computedPath := filepath.Join(balDir, fmt.Sprintf("computed_bal_%d.txt", blockNum)) - if err := os.WriteFile(computedPath, []byte(computedDebug), 0o644); err != nil { - log.Warn("failed to write computed BAL debug file", "path", computedPath, "err", err) - } else { - dumpDir = balDir - } - if storedDebug != "" { - storedPath := filepath.Join(balDir, fmt.Sprintf("stored_bal_%d.txt", blockNum)) - if err := os.WriteFile(storedPath, []byte(storedDebug), 0o644); err != nil { - log.Warn("failed to write stored BAL debug file", "path", storedPath, "err", err) - } - } - } - } - if dumpDir != "" { - return fmt.Errorf("%w, block=%d (hash=%s): block access list mismatch: got %s expected %s; debug dumps in %s", - rules.ErrInvalidBlock, blockNum, blockHash, bal.Hash(), headerBALHash, dumpDir) - } + computedBlockBalHash := computedBlockBal.Hash() + if blockBalHash != computedBlockBalHash { + reportBALMismatch(blockNum, blockHash, blockBal, blockBalHash, computedBlockBal, dataDir, logger) return fmt.Errorf("%w, block=%d (hash=%s): block access list mismatch: got %s expected %s", - rules.ErrInvalidBlock, blockNum, blockHash, bal.Hash(), headerBALHash) + rules.ErrInvalidBlock, blockNum, blockHash, computedBlockBalHash, blockBalHash) } return nil } + +// reportBALMismatch logs a BAL hash mismatch and dumps the block's BAL and the +// computed one under /bal for offline diffing. +func reportBALMismatch(blockNum uint64, blockHash common.Hash, blockBal types.BlockAccessList, blockBalHash common.Hash, computedBlockBal types.BlockAccessList, dataDir string, logger log.Logger) { + logger.Error("BAL mismatch", "blockNum", blockNum, "blockHash", blockHash, "blockBalHash", blockBalHash, "computedBlockBalHash", computedBlockBal.Hash()) + writeBALToFile(computedBlockBal, dataDir, fmt.Sprintf("computed_bal_%d.txt", blockNum), logger) + if blockBal != nil { + writeBALToFile(blockBal, dataDir, fmt.Sprintf("block_bal_%d.txt", blockNum), logger) + } +} diff --git a/execution/stagedsync/exec3_parallel.go b/execution/stagedsync/exec3_parallel.go index f3315029ad9..e76808508a0 100644 --- a/execution/stagedsync/exec3_parallel.go +++ b/execution/stagedsync/exec3_parallel.go @@ -606,7 +606,7 @@ func (pe *parallelExecutor) execImpl(ctx context.Context, execStage *StageState, } if pe.cfg.chainConfig.IsAmsterdam(applyResult.BlockTime) || pe.cfg.experimentalBAL { - err = ProcessBAL(rwTx, lastHeader, applyResult.TxIO, pe.cfg.chainConfig.IsAmsterdam(applyResult.BlockTime), pe.cfg.experimentalBAL, pe.cfg.dirs.DataDir) + err = ProcessBAL(rwTx, lastHeader, applyResult.TxIO, pe.cfg.chainConfig.IsAmsterdam(applyResult.BlockTime), pe.cfg.experimentalBAL, pe.cfg.dirs.DataDir, pe.logger) if err != nil { return err } From 9b20bf015a7208b9474d567562fad5c2451d502c Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 6 Jun 2026 06:13:37 +0000 Subject: [PATCH 24/28] execution: fix sporadic bal mismatches due to phantom accesses in system txns --- execution/state/intra_block_state.go | 2 ++ execution/state/parallel_fixes_test.go | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/execution/state/intra_block_state.go b/execution/state/intra_block_state.go index ff87c527974..84ef58055fa 100644 --- a/execution/state/intra_block_state.go +++ b/execution/state/intra_block_state.go @@ -372,6 +372,8 @@ func (sdb *IntraBlockState) Reset() { // references are dropped, while the next execution lazily allocates fresh ones. sdb.versionedReads = nil sdb.versionedWrites = nil + sdb.recordAccess = false + sdb.addressAccess = nil sdb.accountReadDuration = 0 sdb.accountReadCount = 0 sdb.storageReadDuration = 0 diff --git a/execution/state/parallel_fixes_test.go b/execution/state/parallel_fixes_test.go index 98ba9b353da..437e8a42662 100644 --- a/execution/state/parallel_fixes_test.go +++ b/execution/state/parallel_fixes_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/erigontech/erigon/execution/chain" "github.com/erigontech/erigon/execution/commitment" "github.com/erigontech/erigon/execution/types/accounts" ) @@ -170,6 +171,26 @@ func TestAccessListResetInIBSReset(t *testing.T) { assert.False(t, ibs.AddressInAccessList(testAddr), "Address should be cold after Reset") } +// TestAddressAccessResetInIBSReset verifies that IBS.Reset() clears BAL +// address-access recording. An aborted incarnation never harvests +// AccessedAddresses, and only regular txs call Prepare (which re-inits +// recording) — a worker next assigned a system block-start/block-end +// transaction never calls Prepare, so it would harvest the leftovers into +// its own block's access list as phantom entries. +func TestAddressAccessResetInIBSReset(t *testing.T) { + ibs := New(nil) + sender := accounts.InternAddress([20]byte{0x01}) + coinbase := accounts.InternAddress([20]byte{0x02}) + leaked := accounts.InternAddress([20]byte{0x42}) + // Prepare enables access recording at tx start. + require.NoError(t, ibs.Prepare(&chain.Rules{}, sender, coinbase, accounts.NilAddress, nil, nil, nil)) + ibs.MarkAddressAccess(leaked, false) + // Tx aborts: AccessedAddresses is never harvested. The worker resets + // the shared IBS before the next task. + ibs.Reset() + assert.Empty(t, ibs.AccessedAddresses(), "no recorded accesses should survive Reset") +} + // TestTransientStorageResetInIBSReset verifies that IBS.Reset() clears // transient storage (EIP-1153). func TestTransientStorageResetInIBSReset(t *testing.T) { From b25c82078dc0773e0716468ca9a5df3789facfe0 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 6 Jun 2026 06:49:50 +0000 Subject: [PATCH 25/28] wip --- execution/execmodule/exec_module_test.go | 111 ++++++++++++++++++ .../execmoduletester/exec_module_tester.go | 42 ++++--- execution/stagedsync/committer.go | 99 +++++++++++++++- execution/stagedsync/exec3.go | 20 ++++ execution/stagedsync/exec3_parallel.go | 33 +++--- .../exec3_parallel_robustness_test.go | 81 +++++++++++++ 6 files changed, 353 insertions(+), 33 deletions(-) diff --git a/execution/execmodule/exec_module_test.go b/execution/execmodule/exec_module_test.go index b931977c151..4f6d21ca95e 100644 --- a/execution/execmodule/exec_module_test.go +++ b/execution/execmodule/exec_module_test.go @@ -39,6 +39,7 @@ import ( "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/kv/rawdbv3" "github.com/erigontech/erigon/db/rawdb" + "github.com/erigontech/erigon/db/state/changeset" "github.com/erigontech/erigon/execution/builder" "github.com/erigontech/erigon/execution/chain" "github.com/erigontech/erigon/execution/commitment/commitmentdb" @@ -2050,3 +2051,113 @@ func TestInsertBlocksWithBatchedFCU_BadBlockRecovery_Foreground(t *testing.T) { func TestInsertBlocksWithBatchedFCU_BadBlockRecovery_Background(t *testing.T) { runBatchedFCUBadBlockRecovery(t, true) } + +// transferGen returns a deterministic per-block tx generator: identical +// inputs produce identical blocks, which lets tests build forks that share +// a prefix with the canonical chain (requires a pre-Cancun config — Cancun+ +// blocks get a random ParentBeaconBlockRoot in blockgen). +func transferGen(t *testing.T, key *ecdsa.PrivateKey, to common.Address, amount uint64) func(int, *blockgen.BlockGen) { + return func(i int, b *blockgen.BlockGen) { + tx, err := types.SignTx( + types.NewTransaction(uint64(i), to, uint256.NewInt(amount), 50000, uint256.NewInt(1), nil), + *types.LatestSignerForChainID(nil), + key, + ) + require.NoError(t, err) + b.AddTx(tx) + } +} + +// Regression for #21650: a batch longer than MaxReorgDepth must still produce +// changesets for the last MaxReorgDepth blocks, otherwise no reorg of any +// depth is possible after the batch. +func TestLargeBatchExecGeneratesChangesetsForReorgWindow(t *testing.T) { + ctx := t.Context() + privKey, err := crypto.GenerateKey() + require.NoError(t, err) + senderAddr := crypto.PubkeyToAddress(privKey.PublicKey) + m := execmoduletester.New(t, + execmoduletester.WithKey(privKey), + execmoduletester.WithoutAlwaysGenerateChangesets(), + ) + + maxReorgDepth := m.Cfg().Sync.MaxReorgDepth + chainLen := int(maxReorgDepth) + 14 + + chainPack, err := blockgen.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, chainLen, + transferGen(t, privKey, senderAddr, 1_000)) + require.NoError(t, err) + + insRes, err := insertBlocks(ctx, m.ExecModule, chainPack.Blocks) + require.NoError(t, err) + require.Equal(t, execmodule.ExecutionStatusSuccess, insRes) + fcuRes, err := updateForkChoice(ctx, m.ExecModule, chainPack.TopBlock.Header()) + require.NoError(t, err) + require.Equal(t, execmodule.ExecutionStatusSuccess, fcuRes.Status) + + require.NoError(t, m.DB.ViewTemporal(ctx, func(tx kv.TemporalTx) error { + lowest, err := changeset.ReadLowestUnwindableBlock(tx) + require.NoError(t, err) + require.Equal(t, uint64(chainLen)-maxReorgDepth, lowest, + "changesets must cover the last MaxReorgDepth blocks of the batch") + return nil + })) +} + +// Reproduces #21650 end-to-end: after a single batch execution longer than +// MaxReorgDepth, an FCU onto a fork branching a few blocks below the tip must +// unwind and re-execute instead of failing with ReorgTooDeep. +func TestUpdateForkChoiceShallowReorgAfterLargeBatchExec(t *testing.T) { + ctx := t.Context() + privKey, err := crypto.GenerateKey() + require.NoError(t, err) + senderAddr := crypto.PubkeyToAddress(privKey.PublicKey) + m := execmoduletester.New(t, + execmoduletester.WithKey(privKey), + execmoduletester.WithoutAlwaysGenerateChangesets(), + ) + + maxReorgDepth := m.Cfg().Sync.MaxReorgDepth + chainLen := int(maxReorgDepth) + 14 + const reorgDepth = 4 + divergeFrom := chainLen - reorgDepth + + canonical, err := blockgen.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, chainLen, + transferGen(t, privKey, senderAddr, 1_000)) + require.NoError(t, err) + fork, err := blockgen.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, chainLen, + func(i int, b *blockgen.BlockGen) { + amount := uint64(1_000) + if i >= divergeFrom { + amount = 2_000 + } + transferGen(t, privKey, senderAddr, amount)(i, b) + }) + require.NoError(t, err) + require.Equal(t, canonical.Blocks[divergeFrom-1].Hash(), fork.Blocks[divergeFrom-1].Hash()) + require.NotEqual(t, canonical.Blocks[divergeFrom].Hash(), fork.Blocks[divergeFrom].Hash()) + + insRes, err := insertBlocks(ctx, m.ExecModule, canonical.Blocks) + require.NoError(t, err) + require.Equal(t, execmodule.ExecutionStatusSuccess, insRes) + fcuRes, err := updateForkChoice(ctx, m.ExecModule, canonical.TopBlock.Header()) + require.NoError(t, err) + require.Equal(t, execmodule.ExecutionStatusSuccess, fcuRes.Status) + + insRes, err = insertBlocks(ctx, m.ExecModule, fork.Blocks[divergeFrom:]) + require.NoError(t, err) + require.Equal(t, execmodule.ExecutionStatusSuccess, insRes) + fcuRes, err = updateForkChoice(ctx, m.ExecModule, fork.TopBlock.Header()) + require.NoError(t, err) + require.Equal(t, execmodule.ExecutionStatusSuccess, fcuRes.Status, + "shallow reorg of %d blocks after a %d-block batch must succeed; status=%s validationError=%q", + reorgDepth, chainLen, fcuRes.Status, fcuRes.ValidationError) + + require.NoError(t, m.DB.ViewTemporal(ctx, func(tx kv.TemporalTx) error { + execProg, err := stages.GetStageProgress(tx, stages.Execution) + require.NoError(t, err) + require.Equal(t, uint64(chainLen), execProg) + require.Equal(t, fork.TopBlock.Hash(), rawdb.ReadHeadBlockHash(tx)) + return nil + })) +} diff --git a/execution/execmodule/execmoduletester/exec_module_tester.go b/execution/execmodule/execmoduletester/exec_module_tester.go index 395936eab38..f279158fba9 100644 --- a/execution/execmodule/execmoduletester/exec_module_tester.go +++ b/execution/execmodule/execmoduletester/exec_module_tester.go @@ -333,6 +333,14 @@ func WithFcuBackgroundCommit() Option { } } +// WithoutAlwaysGenerateChangesets reverts to the production default where +// changesets are generated only for blocks within MaxReorgDepth of the batch end. +func WithoutAlwaysGenerateChangesets() Option { + return func(opts *options) { + opts.alwaysGenerateChangesets = false + } +} + func WithFcuBackgroundPrune() Option { return func(opts *options) { opts.fcuBackgroundPrune = true @@ -340,27 +348,29 @@ func WithFcuBackgroundPrune() Option { } type options struct { - stepSize *uint64 - experimentalBAL bool - genesis *types.Genesis - chainConfig *chain.Config - key *ecdsa.PrivateKey - engine rules.Engine - pruneMode *prune.Mode - withTxPool bool - enableDomains []kv.Domain - fcuBackgroundCommit bool - fcuBackgroundPrune bool + stepSize *uint64 + experimentalBAL bool + genesis *types.Genesis + chainConfig *chain.Config + key *ecdsa.PrivateKey + engine rules.Engine + pruneMode *prune.Mode + withTxPool bool + enableDomains []kv.Domain + fcuBackgroundCommit bool + fcuBackgroundPrune bool + alwaysGenerateChangesets bool } func applyOptions(opts []Option) options { defaultKey, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") defaultPruneMode := prune.MockMode opt := options{ - key: defaultKey, - pruneMode: &defaultPruneMode, - chainConfig: chain.TestChainBerlinConfig, - experimentalBAL: false, + key: defaultKey, + pruneMode: &defaultPruneMode, + chainConfig: chain.TestChainBerlinConfig, + experimentalBAL: false, + alwaysGenerateChangesets: true, } for _, o := range opts { o(&opt) @@ -420,7 +430,7 @@ func New(tb testing.TB, opts ...Option) *ExecModuleTester { cfg.Sync.BodyDownloadTimeoutSeconds = 10 cfg.TxPool.Disable = !withTxPool cfg.Dirs = dirs - cfg.AlwaysGenerateChangesets = true + cfg.AlwaysGenerateChangesets = opt.alwaysGenerateChangesets cfg.PersistReceiptsCacheV2 = true cfg.ChaosMonkey = false cfg.Snapshot.ChainName = gspec.Config.ChainName diff --git a/execution/stagedsync/committer.go b/execution/stagedsync/committer.go index 70f1a59ed34..6729524e77d 100644 --- a/execution/stagedsync/committer.go +++ b/execution/stagedsync/committer.go @@ -109,6 +109,13 @@ type commitmentCalculator struct { // LAST block's changeset, which is wrong for per-block unwind. forcePerBlockCompute bool + // perBlockFrom is the batch's changeset window start: blocks >= it + // compute per-block (their changesets must record per-block branch + // deltas); blocks below it accumulate in batch mode. The last + // pre-window block triggers a transition compute (computeTransition) + // so no pre-window branch deltas leak into a window block's changeset. + perBlockFrom uint64 + wg sync.WaitGroup done chan struct{} } @@ -120,6 +127,7 @@ func newCommitmentCalculator( logPrefix string, logger log.Logger, forcePerBlockCompute bool, + perBlockFrom uint64, in chan applyResult, out chan commitmentResult, ) (*commitmentCalculator, error) { @@ -162,6 +170,7 @@ func newCommitmentCalculator( in: in, out: out, forcePerBlockCompute: forcePerBlockCompute, + perBlockFrom: perBlockFrom, done: make(chan struct{}), }, nil } @@ -250,8 +259,9 @@ func (cc *commitmentCalculator) handleMessage(ctx context.Context, msg applyResu // serial's gate (exec3_serial.go around the `if !dbg.BatchCommitments // || shouldGenerateChangesets || ...` check) — per-block compute is // required when changesets must record per-block branch deltas - // (reorg support, KeepExecutionProofs). - if !dbg.BatchCommitments || cc.forcePerBlockCompute { + // (reorg support, KeepExecutionProofs). Blocks from the changeset + // window (perBlockFrom) onward compute per-block for the same reason. + if !dbg.BatchCommitments || cc.forcePerBlockCompute || r.BlockNum >= cc.perBlockFrom { if cc.lastComputedBlock == 0 && r.isPartial { // First block is partial (resumed mid-block). // Compute it (like serial does) to save trie state, then @@ -261,6 +271,18 @@ func (cc *commitmentCalculator) handleMessage(ctx context.Context, msg applyResu } else { cc.computeAndCheck(ctx, r) } + if r.BlockNum+1 == cc.perBlockFrom { + // Pre-window per-block computes (BatchCommitments off) defer + // branch writes too — flush the boundary block's pending update + // outside any changeset before the first window block's compute + // routes into its saved CS. + cc.flushPendingUpdatesWithoutChangeset(ctx, r) + } + } else if r.BlockNum+1 == cc.perBlockFrom { + // Last pre-window block: fold everything accumulated in batch + // mode now, so the first window block's per-block compute (and + // changeset) covers only its own deltas. + cc.computeTransition(ctx, r) } // In BatchCommitments mode (without forcePerBlockCompute): just // accumulate — compute only on explicit commitComputeRequest from @@ -450,6 +472,79 @@ func (cc *commitmentCalculator) computeAndCheck(ctx context.Context, br *blockRe } } +// flushPendingUpdatesWithoutChangeset eagerly applies the pending deferred +// update under a nil accumulator — a pre-window block's branch deltas must +// not pend into the first window block's changeset-routed compute. +func (cc *commitmentCalculator) flushPendingUpdatesWithoutChangeset(ctx context.Context, br *blockResult) { + cc.doms.LockChangesetAccumulator() + prev := cc.doms.GetChangesetAccumulatorLocked() + cc.doms.SetChangesetAccumulatorLocked(nil) + err := cc.doms.FlushPendingUpdatesLocked(ctx, cc.roTx) + cc.doms.SetChangesetAccumulatorLocked(prev) + cc.doms.UnlockChangesetAccumulator() + if err != nil { + cc.publish(ctx, commitmentResult{ + blockNum: br.BlockNum, + txNum: br.lastTxNum, + err: fmt.Errorf("commitmentCalculator: %w", err), + }) + } +} + +// computeTransition folds all batch-mode blocks at the last pre-window block +// so the first window block's compute (and changeset) covers only its own +// deltas. Branch writes and the deferred-update flush run under a nil +// accumulator — pre-window deltas must not land in any block's changeset. +func (cc *commitmentCalculator) computeTransition(ctx context.Context, br *blockResult) { + if err := cc.state.LazyLoadErr(); err != nil { + cc.publish(ctx, commitmentResult{ + blockNum: br.BlockNum, + txNum: br.lastTxNum, + err: fmt.Errorf("commitmentCalculator: lazy-load failed: %w", err), + }) + return + } + cc.state.FlushToUpdates(cc.updates) + cc.state.ResetBlockFlags() + + sdCtx := cc.doms.GetCommitmentContext() + sdCtx.SetUpdates(cc.updates) + cc.updates = cc.updates.NewEmpty() + + cc.asOfReader.txNum = br.lastTxNum + 1 + sdCtx.SetStateReader(cc.asOfReader) + + cc.doms.LockChangesetAccumulator() + prev := cc.doms.GetChangesetAccumulatorLocked() + cc.doms.SetChangesetAccumulatorLocked(nil) + rh, err := cc.doms.ComputeCommitmentLocked(ctx, cc.roTx, true, br.BlockNum, br.lastTxNum, cc.logPrefix, nil) + if err == nil { + err = cc.doms.FlushPendingUpdatesLocked(ctx, cc.roTx) + } + cc.doms.SetChangesetAccumulatorLocked(prev) + cc.doms.UnlockChangesetAccumulator() + if err != nil { + cc.publish(ctx, commitmentResult{ + blockNum: br.BlockNum, + txNum: br.lastTxNum, + err: fmt.Errorf("commitmentCalculator: %w", err), + }) + return + } + + cc.lastComputedBlock = br.BlockNum + cc.hasComputed = true + + if !bytes.Equal(rh, br.StateRoot[:]) { + cc.publish(ctx, commitmentResult{ + blockNum: br.BlockNum, + txNum: br.lastTxNum, + rootHash: rh, + err: fmt.Errorf("%w: block %d root %x expected %x", ErrWrongTrieRoot, br.BlockNum, rh, br.StateRoot), + }) + } +} + func (cc *commitmentCalculator) publish(ctx context.Context, r commitmentResult) { select { case cc.out <- r: diff --git a/execution/stagedsync/exec3.go b/execution/stagedsync/exec3.go index 425adcd13b2..810b9ae9bb7 100644 --- a/execution/stagedsync/exec3.go +++ b/execution/stagedsync/exec3.go @@ -21,6 +21,7 @@ import ( "context" "errors" "fmt" + "math" "os" "sync" "sync/atomic" @@ -834,3 +835,22 @@ func shouldGenerateChangeSets(cfg ExecuteBlockCfg, blockNum, maxBlockNum uint64) // so the node can handle reorgs at the tip. return blockNum+cfg.syncCfg.MaxReorgDepth >= maxBlockNum } + +// changesetWindowStart returns the first block in [startBlockNum, maxBlockNum] +// for which shouldGenerateChangeSets is true, or math.MaxUint64 when there is +// none. Parallel exec gates per-block changeset capture and the commitment +// calculator's per-block mode on this boundary. +func changesetWindowStart(alwaysGenerateChangesets bool, maxReorgDepth uint64, frozenBlocks uint64, startBlockNum uint64, maxBlockNum uint64) uint64 { + if alwaysGenerateChangesets { + return startBlockNum + } + windowStart := startBlockNum + if maxBlockNum > maxReorgDepth { + windowStart = max(windowStart, maxBlockNum-maxReorgDepth) + } + windowStart = max(windowStart, frozenBlocks) + if windowStart > maxBlockNum { + return math.MaxUint64 + } + return windowStart +} diff --git a/execution/stagedsync/exec3_parallel.go b/execution/stagedsync/exec3_parallel.go index f3315029ad9..2c7a0d011e1 100644 --- a/execution/stagedsync/exec3_parallel.go +++ b/execution/stagedsync/exec3_parallel.go @@ -112,8 +112,11 @@ type parallelExecutor struct { // goroutine and avoids the data race between SetChangesetAccumulator // (apply loop) and ApplyStateWrites (exec loop, via SysCallContract for // block-end system calls) on SharedDomains.mem. - shouldGenerateChangesets bool - currentChangeSet *changeset.StateChangeSet + // changesetWindowStart is the first block of the batch that must capture + // a changeset (see changesetWindowStart in exec3.go); blocks below it run + // without an accumulator. + changesetWindowStart uint64 + currentChangeSet *changeset.StateChangeSet // currentChangeSetBlock is the block number currentChangeSet belongs to // (0 == none). Tracked so ensureChangesetAccumulator can be a no-op when the // accumulator is already installed for the block whose writes are about to @@ -130,7 +133,7 @@ type parallelExecutor struct { // SetChangesetAccumulator, which must be single-writer (see the comment on // currentChangeSet). func (pe *parallelExecutor) ensureChangesetAccumulator(blockNum uint64) { - if !pe.shouldGenerateChangesets || blockNum == 0 || blockNum > pe.maxBlockNum { + if blockNum < pe.changesetWindowStart || blockNum == 0 || blockNum > pe.maxBlockNum { return } if pe.currentChangeSet != nil && pe.currentChangeSetBlock == blockNum { @@ -256,18 +259,18 @@ func (pe *parallelExecutor) execImpl(ctx context.Context, execStage *StageState, // exec loop owns all subsequent SetChangesetAccumulator transitions // (per-block save/clear/install) so apply-loop and exec-loop sd.mem // writes never race on SharedDomains.mem. - pe.shouldGenerateChangesets = shouldGenerateChangeSets(pe.cfg, startBlockNum, maxBlockNum) + pe.changesetWindowStart = changesetWindowStart(pe.cfg.syncCfg.AlwaysGenerateChangesets, + pe.cfg.syncCfg.MaxReorgDepth, pe.cfg.blockReader.FrozenBlocks(), startBlockNum, maxBlockNum) pe.ensureChangesetAccumulator(startBlockNum) - // Start the commitment calculator. forcePerBlockCompute mirrors serial's - // per-block gate (exec3_serial.go: `if !dbg.BatchCommitments || - // shouldGenerateChangesets || KeepExecutionProofs`). When changesets - // must be generated (for unwind/reorg) the calculator must compute - // per-block — otherwise batch-mode dedupes branch updates across the - // batch and flushes them all into the last block's changeset, which - // fails on subsequent reorgs. - forcePerBlockCompute := pe.shouldGenerateChangesets || pe.cfg.syncCfg.KeepExecutionProofs - calculator, err := newCommitmentCalculator(executorContext, pe.rs.Domains(), pe.cfg.db, pe.logPrefix, pe.logger, forcePerBlockCompute, commitResults, rootResults) + // Start the commitment calculator. It mirrors serial's per-block gate + // (exec3_serial.go: `if !dbg.BatchCommitments || shouldGenerateChangesets + // || KeepExecutionProofs`): blocks from the changeset window onward must + // compute per-block — otherwise batch-mode dedupes branch updates across + // the batch and flushes them all into one block's changeset, which fails + // on subsequent reorgs. + forcePerBlockCompute := pe.cfg.syncCfg.KeepExecutionProofs + calculator, err := newCommitmentCalculator(executorContext, pe.rs.Domains(), pe.cfg.db, pe.logPrefix, pe.logger, forcePerBlockCompute, pe.changesetWindowStart, commitResults, rootResults) if err != nil { return nil, nil, err } @@ -310,7 +313,7 @@ func (pe *parallelExecutor) execImpl(ctx context.Context, execStage *StageState, } defer applyRoTx.Rollback() - // pe.shouldGenerateChangesets and pe.currentChangeSet were set up + // pe.changesetWindowStart and pe.currentChangeSet were set up // before pe.run/executeBlocks launched their goroutines (above the // calculator.Start call). Per-block accumulator save/clear/install // transitions are driven from the exec loop's blockResult handler. @@ -978,7 +981,7 @@ func (pe *parallelExecutor) execLoop(ctx context.Context) (err error) { // blockResult, so that the commitment calculator (which consumes // blockResults on a separate goroutine) can find this block's // saved changeset via GetChangesetByBlockNum at compute time. - // In per-block compute mode (shouldGenerateChangesets), the + // In per-block compute mode (changeset window), the // calculator switches the accumulator to this saved CS for the // duration of ComputeCommitment (committer.go:computeWithBlockAccumulator) // so branch writes land in block N's CS rather than whatever the diff --git a/execution/stagedsync/exec3_parallel_robustness_test.go b/execution/stagedsync/exec3_parallel_robustness_test.go index 60767de06cb..72c47b94357 100644 --- a/execution/stagedsync/exec3_parallel_robustness_test.go +++ b/execution/stagedsync/exec3_parallel_robustness_test.go @@ -12,6 +12,7 @@ package stagedsync import ( "context" "errors" + "math" "strings" "sync" "sync/atomic" @@ -22,6 +23,86 @@ import ( "github.com/erigontech/erigon/execution/protocol/rules" ) +// TestChangesetWindowStart covers the pure helper that decides where the +// changeset window of a batch begins. Regression for #21650: parallel exec +// used to evaluate the window once at startBlockNum, so any batch longer +// than MaxReorgDepth produced no changesets at all and the node could not +// reorg afterwards. +func TestChangesetWindowStart(t *testing.T) { + cases := []struct { + name string + alwaysGenerateChangesets bool + maxReorgDepth uint64 + frozenBlocks uint64 + startBlockNum uint64 + maxBlockNum uint64 + want uint64 + }{ + { + name: "big batch: window covers the last maxReorgDepth blocks", + maxReorgDepth: 96, + startBlockNum: 1, + maxBlockNum: 1000, + want: 904, + }, + { + name: "small batch: whole batch in window", + maxReorgDepth: 96, + startBlockNum: 950, + maxBlockNum: 1000, + want: 950, + }, + { + name: "batch end below maxReorgDepth: window from batch start", + maxReorgDepth: 96, + startBlockNum: 1, + maxBlockNum: 96, + want: 1, + }, + { + name: "alwaysGenerateChangesets overrides depth and frozen gates", + alwaysGenerateChangesets: true, + maxReorgDepth: 96, + frozenBlocks: 2000, + startBlockNum: 1, + maxBlockNum: 1000, + want: 1, + }, + { + name: "frozen blocks push the window up", + maxReorgDepth: 96, + frozenBlocks: 950, + startBlockNum: 1, + maxBlockNum: 1000, + want: 950, + }, + { + name: "fully frozen batch has no window", + maxReorgDepth: 96, + frozenBlocks: 2000, + startBlockNum: 1, + maxBlockNum: 1000, + want: math.MaxUint64, + }, + { + name: "incident shape (#21650): batch to 6137 must cover 6134..6137", + maxReorgDepth: 96, + startBlockNum: 5138, + maxBlockNum: 6137, + want: 6041, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := changesetWindowStart(tc.alwaysGenerateChangesets, tc.maxReorgDepth, tc.frozenBlocks, tc.startBlockNum, tc.maxBlockNum) + if got != tc.want { + t.Fatalf("changesetWindowStart got %d, want %d", got, tc.want) + } + }) + } +} + // TestApplyLoopMissingBlocks covers the pure completeness-check helper. // Every entry asserts a single invariant — see the comment on each case. func TestApplyLoopMissingBlocks(t *testing.T) { From 6d7626b8cd4968b6019bbfa492da0cdd4ae629ef Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 6 Jun 2026 07:41:26 +0000 Subject: [PATCH 26/28] match always-generate-changesets default in exec module tests --- execution/execmodule/exec_module_test.go | 10 ++------- .../execmoduletester/exec_module_tester.go | 21 ++++++++++--------- execution/tests/blockchain_test.go | 9 +++++--- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/execution/execmodule/exec_module_test.go b/execution/execmodule/exec_module_test.go index 4f6d21ca95e..efdd8c4b084 100644 --- a/execution/execmodule/exec_module_test.go +++ b/execution/execmodule/exec_module_test.go @@ -2076,10 +2076,7 @@ func TestLargeBatchExecGeneratesChangesetsForReorgWindow(t *testing.T) { privKey, err := crypto.GenerateKey() require.NoError(t, err) senderAddr := crypto.PubkeyToAddress(privKey.PublicKey) - m := execmoduletester.New(t, - execmoduletester.WithKey(privKey), - execmoduletester.WithoutAlwaysGenerateChangesets(), - ) + m := execmoduletester.New(t, execmoduletester.WithKey(privKey)) maxReorgDepth := m.Cfg().Sync.MaxReorgDepth chainLen := int(maxReorgDepth) + 14 @@ -2112,10 +2109,7 @@ func TestUpdateForkChoiceShallowReorgAfterLargeBatchExec(t *testing.T) { privKey, err := crypto.GenerateKey() require.NoError(t, err) senderAddr := crypto.PubkeyToAddress(privKey.PublicKey) - m := execmoduletester.New(t, - execmoduletester.WithKey(privKey), - execmoduletester.WithoutAlwaysGenerateChangesets(), - ) + m := execmoduletester.New(t, execmoduletester.WithKey(privKey)) maxReorgDepth := m.Cfg().Sync.MaxReorgDepth chainLen := int(maxReorgDepth) + 14 diff --git a/execution/execmodule/execmoduletester/exec_module_tester.go b/execution/execmodule/execmoduletester/exec_module_tester.go index f279158fba9..eab983676fd 100644 --- a/execution/execmodule/execmoduletester/exec_module_tester.go +++ b/execution/execmodule/execmoduletester/exec_module_tester.go @@ -333,11 +333,11 @@ func WithFcuBackgroundCommit() Option { } } -// WithoutAlwaysGenerateChangesets reverts to the production default where -// changesets are generated only for blocks within MaxReorgDepth of the batch end. -func WithoutAlwaysGenerateChangesets() Option { +// WithAlwaysGenerateChangesets mirrors --experimental.always-generate-changesets, +// for tests that reorg deeper than MaxReorgDepth. +func WithAlwaysGenerateChangesets() Option { return func(opts *options) { - opts.alwaysGenerateChangesets = false + opts.alwaysGenerateChangesets = true } } @@ -366,11 +366,10 @@ func applyOptions(opts []Option) options { defaultKey, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") defaultPruneMode := prune.MockMode opt := options{ - key: defaultKey, - pruneMode: &defaultPruneMode, - chainConfig: chain.TestChainBerlinConfig, - experimentalBAL: false, - alwaysGenerateChangesets: true, + key: defaultKey, + pruneMode: &defaultPruneMode, + chainConfig: chain.TestChainBerlinConfig, + experimentalBAL: false, } for _, o := range opts { o(&opt) @@ -430,7 +429,9 @@ func New(tb testing.TB, opts ...Option) *ExecModuleTester { cfg.Sync.BodyDownloadTimeoutSeconds = 10 cfg.TxPool.Disable = !withTxPool cfg.Dirs = dirs - cfg.AlwaysGenerateChangesets = opt.alwaysGenerateChangesets + if opt.alwaysGenerateChangesets { + cfg.AlwaysGenerateChangesets = true + } cfg.PersistReceiptsCacheV2 = true cfg.ChaosMonkey = false cfg.Snapshot.ChainName = gspec.Config.ChainName diff --git a/execution/tests/blockchain_test.go b/execution/tests/blockchain_test.go index 55eed7ffe33..ea539ba3719 100644 --- a/execution/tests/blockchain_test.go +++ b/execution/tests/blockchain_test.go @@ -1230,7 +1230,9 @@ func TestLargeReorgTrieGC(t *testing.T) { t.Parallel() // Generate the original common chain segment and the two competing forks - m, m2 := execmoduletester.New(t), execmoduletester.New(t) + // The competitor fork reorgs ~2*triesInMemory blocks deep — beyond + // MaxReorgDepth, so the inserting node needs changesets for every block. + m, m2 := execmoduletester.New(t), execmoduletester.New(t, execmoduletester.WithAlwaysGenerateChangesets()) shared, err := blockgen.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, 64, func(i int, b *blockgen.BlockGen) { b.SetCoinbase(common.Address{1}) @@ -1317,8 +1319,9 @@ func TestLowDiffLongChain(t *testing.T) { t.Fatalf("generate fork: %v", err) } - // Import the canonical chain - m2 := execmoduletester.New(t) + // Import the canonical chain. The fork branches at block 11 — far beyond + // MaxReorgDepth — so the inserting node needs changesets for every block. + m2 := execmoduletester.New(t, execmoduletester.WithAlwaysGenerateChangesets()) if err := m2.InsertChain(chain); err != nil { t.Fatalf("failed to insert into chain: %v", err) From e37f82c7dbeac208751d8d71d471da1a1effe5be Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 6 Jun 2026 07:49:16 +0000 Subject: [PATCH 27/28] explicit alwaysproducechangesets --- execution/execmodule/exec_module_test.go | 10 ++++++++-- .../execmoduletester/exec_module_tester.go | 16 +++++++++------- execution/tests/blockchain_test.go | 4 ++-- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/execution/execmodule/exec_module_test.go b/execution/execmodule/exec_module_test.go index efdd8c4b084..9003950cfcc 100644 --- a/execution/execmodule/exec_module_test.go +++ b/execution/execmodule/exec_module_test.go @@ -2076,7 +2076,10 @@ func TestLargeBatchExecGeneratesChangesetsForReorgWindow(t *testing.T) { privKey, err := crypto.GenerateKey() require.NoError(t, err) senderAddr := crypto.PubkeyToAddress(privKey.PublicKey) - m := execmoduletester.New(t, execmoduletester.WithKey(privKey)) + m := execmoduletester.New(t, + execmoduletester.WithKey(privKey), + execmoduletester.WithAlwaysGenerateChangesets(false), + ) maxReorgDepth := m.Cfg().Sync.MaxReorgDepth chainLen := int(maxReorgDepth) + 14 @@ -2109,7 +2112,10 @@ func TestUpdateForkChoiceShallowReorgAfterLargeBatchExec(t *testing.T) { privKey, err := crypto.GenerateKey() require.NoError(t, err) senderAddr := crypto.PubkeyToAddress(privKey.PublicKey) - m := execmoduletester.New(t, execmoduletester.WithKey(privKey)) + m := execmoduletester.New(t, + execmoduletester.WithKey(privKey), + execmoduletester.WithAlwaysGenerateChangesets(false), + ) maxReorgDepth := m.Cfg().Sync.MaxReorgDepth chainLen := int(maxReorgDepth) + 14 diff --git a/execution/execmodule/execmoduletester/exec_module_tester.go b/execution/execmodule/execmoduletester/exec_module_tester.go index eab983676fd..78405e904f7 100644 --- a/execution/execmodule/execmoduletester/exec_module_tester.go +++ b/execution/execmodule/execmoduletester/exec_module_tester.go @@ -333,11 +333,13 @@ func WithFcuBackgroundCommit() Option { } } -// WithAlwaysGenerateChangesets mirrors --experimental.always-generate-changesets, -// for tests that reorg deeper than MaxReorgDepth. -func WithAlwaysGenerateChangesets() Option { +// WithAlwaysGenerateChangesets pins --experimental.always-generate-changesets +// regardless of the tester default: true for tests that reorg deeper than +// MaxReorgDepth, false for tests that rely on the windowed-changesets +// production behaviour. +func WithAlwaysGenerateChangesets(v bool) Option { return func(opts *options) { - opts.alwaysGenerateChangesets = true + opts.alwaysGenerateChangesets = &v } } @@ -359,7 +361,7 @@ type options struct { enableDomains []kv.Domain fcuBackgroundCommit bool fcuBackgroundPrune bool - alwaysGenerateChangesets bool + alwaysGenerateChangesets *bool } func applyOptions(opts []Option) options { @@ -429,8 +431,8 @@ func New(tb testing.TB, opts ...Option) *ExecModuleTester { cfg.Sync.BodyDownloadTimeoutSeconds = 10 cfg.TxPool.Disable = !withTxPool cfg.Dirs = dirs - if opt.alwaysGenerateChangesets { - cfg.AlwaysGenerateChangesets = true + if opt.alwaysGenerateChangesets != nil { + cfg.AlwaysGenerateChangesets = *opt.alwaysGenerateChangesets } cfg.PersistReceiptsCacheV2 = true cfg.ChaosMonkey = false diff --git a/execution/tests/blockchain_test.go b/execution/tests/blockchain_test.go index ea539ba3719..54b39983c8b 100644 --- a/execution/tests/blockchain_test.go +++ b/execution/tests/blockchain_test.go @@ -1232,7 +1232,7 @@ func TestLargeReorgTrieGC(t *testing.T) { // The competitor fork reorgs ~2*triesInMemory blocks deep — beyond // MaxReorgDepth, so the inserting node needs changesets for every block. - m, m2 := execmoduletester.New(t), execmoduletester.New(t, execmoduletester.WithAlwaysGenerateChangesets()) + m, m2 := execmoduletester.New(t), execmoduletester.New(t, execmoduletester.WithAlwaysGenerateChangesets(true)) shared, err := blockgen.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, 64, func(i int, b *blockgen.BlockGen) { b.SetCoinbase(common.Address{1}) @@ -1321,7 +1321,7 @@ func TestLowDiffLongChain(t *testing.T) { // Import the canonical chain. The fork branches at block 11 — far beyond // MaxReorgDepth — so the inserting node needs changesets for every block. - m2 := execmoduletester.New(t, execmoduletester.WithAlwaysGenerateChangesets()) + m2 := execmoduletester.New(t, execmoduletester.WithAlwaysGenerateChangesets(true)) if err := m2.InsertChain(chain); err != nil { t.Fatalf("failed to insert into chain: %v", err) From 3978b6834bdbe92a57d53c999d86442a01c37c44 Mon Sep 17 00:00:00 2001 From: taratorio <94537774+taratorio@users.noreply.github.com> Date: Sat, 6 Jun 2026 08:36:01 +0000 Subject: [PATCH 28/28] tidy --- execution/execmodule/exec_module_test.go | 12 ++++++------ .../stagedsync/exec3_parallel_robustness_test.go | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/execution/execmodule/exec_module_test.go b/execution/execmodule/exec_module_test.go index 9003950cfcc..69680c1f708 100644 --- a/execution/execmodule/exec_module_test.go +++ b/execution/execmodule/exec_module_test.go @@ -2068,9 +2068,9 @@ func transferGen(t *testing.T, key *ecdsa.PrivateKey, to common.Address, amount } } -// Regression for #21650: a batch longer than MaxReorgDepth must still produce -// changesets for the last MaxReorgDepth blocks, otherwise no reorg of any -// depth is possible after the batch. +// A batch longer than MaxReorgDepth must still produce changesets for the +// last MaxReorgDepth blocks, otherwise no reorg of any depth is possible +// after the batch. func TestLargeBatchExecGeneratesChangesetsForReorgWindow(t *testing.T) { ctx := t.Context() privKey, err := crypto.GenerateKey() @@ -2104,9 +2104,9 @@ func TestLargeBatchExecGeneratesChangesetsForReorgWindow(t *testing.T) { })) } -// Reproduces #21650 end-to-end: after a single batch execution longer than -// MaxReorgDepth, an FCU onto a fork branching a few blocks below the tip must -// unwind and re-execute instead of failing with ReorgTooDeep. +// After a single batch execution longer than MaxReorgDepth, an FCU onto a +// fork branching a few blocks below the tip must unwind and re-execute +// instead of failing with ReorgTooDeep. func TestUpdateForkChoiceShallowReorgAfterLargeBatchExec(t *testing.T) { ctx := t.Context() privKey, err := crypto.GenerateKey() diff --git a/execution/stagedsync/exec3_parallel_robustness_test.go b/execution/stagedsync/exec3_parallel_robustness_test.go index 72c47b94357..b6cffcb7405 100644 --- a/execution/stagedsync/exec3_parallel_robustness_test.go +++ b/execution/stagedsync/exec3_parallel_robustness_test.go @@ -24,10 +24,10 @@ import ( ) // TestChangesetWindowStart covers the pure helper that decides where the -// changeset window of a batch begins. Regression for #21650: parallel exec -// used to evaluate the window once at startBlockNum, so any batch longer -// than MaxReorgDepth produced no changesets at all and the node could not -// reorg afterwards. +// changeset window of a batch begins. Evaluating the window once at +// startBlockNum (instead of per block) would leave any batch longer than +// MaxReorgDepth without changesets, making the node unable to reorg +// afterwards. func TestChangesetWindowStart(t *testing.T) { cases := []struct { name string @@ -85,7 +85,7 @@ func TestChangesetWindowStart(t *testing.T) { want: math.MaxUint64, }, { - name: "incident shape (#21650): batch to 6137 must cover 6134..6137", + name: "long catch-up batch keeps a shallow reorg below its tip unwindable", maxReorgDepth: 96, startBlockNum: 5138, maxBlockNum: 6137,