execution: implement EIP-2780 resource-based intrinsic transaction gas - #22053
Conversation
…/erigon into worktree-gd6-eip-2780
…devnet-6-fixtures
…/erigon into worktree-gd6-eip-2780
…devnet-6-fixtures
…/erigon into worktree-gd6-eip-2780
…devnet-6-fixtures
…/erigon into worktree-gd6-eip-2780
…devnet-6-fixtures
…devnet-6-fixtures
…/erigon into worktree-gd6-eip-2780
…devnet-6-fixtures
…devnet-6-fixtures
…/erigon into worktree-gd6-eip-2780
There was a problem hiding this comment.
Pull request overview
Implements EIP-2780 resource-based intrinsic transaction gas (for Amsterdam/glamsterdam rules) across intrinsic gas calculation, EVM execution-time top-level charges, txpool/shutter selection/validation, and RPC gas estimation—plus updates to tests and CI tolerances.
Changes:
- Extend intrinsic gas calculation inputs to support EIP-2780 (TX_BASE / recipient cold access / value costs, self-transfer exemptions) and plumb the new flags through execution and txn providers.
- Add EIP-2780 top-level frame charges in the EVM (NEW_ACCOUNT state gas for account-creating value transfers; delegated-recipient cold access).
- Update
eth_estimateGasto return exact gas used for code-less transfers and add regression tests / adjust engine-api tests for new gas accounting.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
txnprovider/txpool/pool.go |
Plumbs EIP-2780 inputs into intrinsic gas used for tx selection/validation. |
txnprovider/shutter/pool.go |
Plumbs EIP-2780 intrinsic gas inputs when selecting decrypted txns. |
tools/eest-spec-shards.yml |
Updates allowed failure thresholds for EEST shard runs. |
rpc/jsonrpc/eth_call.go |
Adjusts EstimateGas transfer short-circuit to return actual gas used (EIP-2780 aware). |
rpc/jsonrpc/eth_call_test.go |
Adds EIP-2780 estimateGas regression test + adjusts chain setup gas limits for fillers. |
execution/vm/evm.go |
Adds EIP-2780 top-level frame charging and threads adjusted gas into Run. |
execution/tests/testutil/transaction_test_util.go |
Updates intrinsic gas calc args in execution tests for EIP-2780. |
execution/protocol/txn_executor.go |
Updates intrinsic gas calc args for EIP-2780 in the executor path. |
execution/protocol/params/protocol.go |
Introduces EIP-2780 gas constants. |
execution/protocol/mdgas/intrinsic_gas.go |
Implements EIP-2780 intrinsic gas decomposition and floor cost changes. |
execution/protocol/mdgas/intrinsic_gas_test.go |
Adds intrinsic gas unit tests for EIP-2780 reference cases. |
execution/engineapi/engineapitester/transactor.go |
Switches simple transfer gas limit to RPC estimate (EIP-2780 compatible). |
execution/engineapi/engine_api_builder_test.go |
Updates engine API block gas overflow tests for account-creating transfers under Amsterdam. |
.github/workflows/test-hive-eest.yml |
Updates hive/eest workflow failure tolerance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yperbasis
left a comment
yperbasis
left a comment
There was a problem hiding this comment.
The intrinsic decomposition and the top-level runtime charges look right for the common paths (reference-table values check out), but there are a few spec divergences to fix:
-
EIP-7623 floor base (
mdgas/intrinsic_gas.go):FloorGasCostis seeded with a flatTxBaseEIP2780(12,000) for every tx shape. Per the EIP's interactions section, the floor base is the decomposed regular-gas intrinsic of the transaction (21,000 for a value transfer, 15,000 for a zero-value call, 23,000/24,756 for creation, 12,000 only for self-transfer). Undercharges floor-bound (data-heavy) txns by 3,000–12,756 and feeds both theGas() < FloorGasCostvalidity check and finaltxnGasUsed— consensus divergence. Reuse the base computed in theIsEIP2780branch for bothRegularGasandFloorGasCost.TestEIP2780IntrinsicGascurrently pins the flat value, but all its cases have empty calldata so the floor never binds — please add a floor-bound case. -
Delegated-recipient charge is always cold (
chargeTopLevelFrameGas): spec saysCOLD_ACCOUNT_ACCESSif cold,WARM_ACCESS(100) if warm (delegation target in the access list, == sender, etc.). Mirror the in-frame logic inoperations_acl.go(AddressInAccessList(dd)check beforeAddAddressToAccessList(dd)). -
Depth-0 defer reorder in
evm.callchanges failure accounting for all Amsterdam txns, not just 2780 paths. ZeroinggasUsed.Statebefore derivingRegularfixes the exceptional-halt receipt (previously under-reported), but on depth-0 REVERT the sender now pays the frame's reverted state gas as regular gas, where EIP-8037's refill rules say it's refunded — the old order got revert right and halt wrong, the new order the opposite. The two error kinds need distinguishing. Alsocreate()still has the old order, so CALL-tx vs CREATE-tx failure accounting is now inconsistent, and the comment above the defer still describes the old design. -
Pre-execution OOG doesn't revert 7702 delegations: on OOG in
chargeTopLevelFrameGaswe unwind only the snapshot taken insidecall(), but authorizations were applied earlier inTxnExecutor— the spec (test case 10) requires reverting those too ("all state changes are reverted — including any EIP-7702 delegations"). Suggests hoisting these runtime charges into the executor's pre-execution phase, where the auth application is in scope. -
Creation state charge vs spec: it stays intrinsic and unconditional (spec: runtime, only if the deployment target doesn't exist — see the
created_address.balance > 0reference row) and is part of theGas() < Regular+Statevalidity threshold (spec: validity is regular-intrinsic-only; underfunded txns are included and halt). If this is deliberate staging against the devnet-6 pin, a tracking note would help; otherwise it needs aligning. Same question for the worst-case per-auth state gas in validity, andPerAuthBaseCostEIP8037= 7,500 vs the EIP table's 7,816. -
Plumbing (first and third also flagged by Copilot): txpool never sets
IsSelfTransfer, so self-transfers are overcharged and valid txns with gas limit in [12k, 21k) get rejected —TxnSlot.Txn.GetTo()plus the senderID mapping make it computable, or comment the conservative choice; shutter ignoresGetSender()'s bool; andcase IsEIP2780now shadowscase IsAATxn, so under Amsterdam AA txns silently get 2780 (creation) pricing instead ofTxAAGas— make the precedence explicit.
Also missing: unit coverage for chargeTopLevelFrameGas itself (warm-target case, OOG-halt receipt, delegated self-transfer).
|
Follow-up to my review: I checked which EIP-2780 revision glamsterdam-devnet-6 actually pins (devnet page: ethereum/EIPs#11645 + ethereum/execution-specs#3017, fixtures Withdrawn as devnet-6 blockers — the PR matches the pinned EELS:
These three become follow-ups for whichever devnet picks up the Jun 25 / Jul 7 EIP revisions — worth a tracking issue so they don't get lost. Confirmed against the pinned fixtures — these still need fixing:
Point 5's validity-boundary concern is withdrawn (pinned EELS also includes intrinsic state gas in the validity check); point 6 (txpool/shutter/AA plumbing) is unaffected by the pin question. |
@yperbasis thanks. addressed applicable ones
|
|
Re-checked after 25dc0ec — the plumbing items are resolved: shutter now checks the Still outstanding — both are pinned devnet-6 fixture semantics (verified against EELS
Minor leftover: the |
taratorio
changed the title
## Problem
The `eest-spec-enginextests-benchmark-150m-{parallel,sequential}` jobs
intermittently kill their runner — the job log ends with `make: ***
Terminated` followed by `The runner has received a shutdown signal`, the
hosted-runner presentation of the VM exhausting memory. 5 of the last 54
`benchmark-150m` job instances died this way (e.g. [this run on
erigontech#22053](https://github.com/erigontech/erigon/actions/runs/29070572016/job/86291064304),
[an unrelated branch the same
morning](https://github.com/erigontech/erigon/actions/runs/29069679000),
[a sequential-variant
instance](https://github.com/erigontech/erigon/actions/runs/29067842538)),
always at 95–100% of the test phase, independent of the PR under test.
Profiling the 150m shard (locally, pinned to CI parallelism) shows peak
demand of **~16.8 GB against the runner's 16 GB**: a 9.9 GB evm process
footprint (live heap 4.9 GB, roughly doubled by default GOGC; dominated
by `opReturn` return-data buffers and `TemporalMemBatch` write-set
clones during the `test_unchunkified_bytecode` cases, which allocate
10.6–12.1 GB each) plus a 6.9 GB MDBX datadir sitting on the 8 GB
ramdisk — tmpfs bytes and process bytes compete for the same RAM. 1075
of the shard's 1077 tests share one `(fork, preAllocHash)` group, so a
single node's datadir grows for essentially the whole run and the peak
lands at the end. Whether a run survives comes down to GC timing and
randomized test order — hence the flakiness.
## Prior investigation
erigontech#22325 (closed in favour of this PR) profiled the same OOM and pinned
why the shard began flaking on Jul 7: the persistent code cache from
erigontech#22154 grew the benchmark datadir by ~2 GB (5.65 → 7.40 GB peak),
pushing the peak co-resident heap+datadir from ~12 GB to ~14+ GB — see
[the profiling
comment](erigontech#22325 (comment)).
The FCU-less CodeStore growth is being capped separately in erigontech#22335; with
the datadir off RAM, that growth no longer threatens the runner either
way — the two changes are complementary.
## Change
The ramdisk exists for shards that churn hundreds of short-lived
datadirs, where create/unlink journaling dominates. The benchmark shards
are the opposite shape (3 long-lived datadirs), so the ramdisk buys them
no wall time — and costs them the RAM that OOMs the runner.
- `tools/eest-spec-shards.yml`: new per-shard key `no-ramdisk: true`,
set on all 14 `enginextests-benchmark-*` shards. Opt-in-true on purpose:
a `false`-valued key would be invisible to both jq's `//` default and
GitHub expressions' loose `==` (`null == false` is true there).
- `.github/workflows/test-eest-spec.yml`: honors the key — skips
creating the tmpfs. Nothing else sets
`ERIGON_EXECUTION_TESTS_TMPDIR`/`TMPDIR` on Linux (the setup-erigon
TMPDIR override is Windows-gated), so these shards exercise the
env-var-unset path and their datadirs land in the runner's default temp
dir (`/tmp`, on the same root SSD as `$RUNNER_TEMP`).
- `tools/run-eest-spec-test.sh`: the same key also skips the local
(Darwin) auto-ramdisk for these shards — whose 2 GB default the 150m
datadir (~7 GB) could not fit anyway. Local runs likewise fall through
to the OS default temp dir.
Non-benchmark shards are unchanged.
## Wall-time evidence (A/B on identical runners)
[Dispatched run
29073818835](https://github.com/erigontech/erigon/actions/runs/29073818835)
— the 150m shards with datadirs on SSD, vs the five most recent green
ramdisk runs:
| | ramdisk (5 runs) | no ramdisk | delta |
|---|---|---|---|
| 150m-parallel, test phase | 16m40s – 17m06s | 17m09s | +1.8% vs mean |
| 150m-sequential, test phase | 11m14s – 13m06s | 13m22s | +8% vs mean,
within the baseline's own ±8% spread |
| 150m-parallel, job total | 20.3 – 22.5 min | 20.0 min | inside range |
| 150m-sequential, job total | 17.0 – 17.7 min | 17.3 min | inside range
|
Both A/B jobs passed, with ~7 GB more headroom at peak.
**Benchmark semantics note:** these are `--time` throughput shards, so
datadirs-on-disk puts SSD writeback inside the measured path — per-test
wall times (and any MGas/s derived from them) shift slightly at this
PR's boundary; the A/B above bounds it at ~+1.8% for the 150m-parallel
test phase. This is deliberate: production nodes run MDBX on SSD/NVMe
with the OS page cache, so the post-PR numbers are more representative
of real-world execution than tmpfs-backed ones. Treat pre-/post-PR
timings as different baselines when comparing historical job logs.
## Verification
- `actionlint`, `shellcheck`, `bash -n` clean; `make lint` clean.
- Matrix render (`yq -o=json` of the manifest) carries `no-ramdisk`
through to `matrix.*`; row parsing verified for benchmark, stable, and
race-regex shards.
- `make eest-spec-enginextests-benchmark-1m-sequential` run locally on
Darwin through the new path: no auto-ramdisk created, datadirs in the
default temp dir, all tests pass (1076/1076); a stable-shard control
still creates the ramdisk.
- On this PR's CI: every benchmark shard's `Create RAM disk` step is
skipped and its log prints the default-tmpdir routing, while stable
shards keep `tmpdir: /mnt/erigon-ramdisk`; the first CI Gate run had all
eest shards green including both 150m jobs.
- The no-ramdisk configuration was also validated end-to-end by the A/B
run above before this PR.
No Go code changes; this is CI/tooling configuration, so the TDD cycle
does not apply.
merge after #22023
closes #22027
ref https://eips.ethereum.org/EIPS/eip-2780