Skip to content

Glamsterdam devnet 8 specific commits - #12719

Open
flcl42 wants to merge 3 commits into
masterfrom
glamsterdam-devnet-8-consensus
Open

Glamsterdam devnet 8 specific commits#12719
flcl42 wants to merge 3 commits into
masterfrom
glamsterdam-devnet-8-consensus

Conversation

@flcl42

@flcl42 flcl42 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Resolves #12599

Changes

  • Update Glamsterdam devnet tests to v8.1.0
  • feat(eip-2780): fold EIP-7708 transfer log cost into TX_VALUE_COST
  • Apply final EIP-8038 state-access repricing

flcl42 and others added 3 commits August 6, 2026 12:57
Update Glamsterdam devnet fixtures to v8.1.0
…12713)

* feat(eip-2780): fold EIP-7708 transfer log cost into TX_VALUE_COST

Per the glamsterdam devnet-8 EIP-2780 update, the EIP-7708 transfer-log
cost is folded into TX_VALUE_COST instead of being charged separately:

- TxValueCostEip2780: 4244 -> 6000 (absorbs the 1756 transfer-log cost)
- Remove the now-redundant TransferLogEip2780 constant (intrinsic-only)
- Contract creation no longer charges the transfer log on a value
  endowment; the recipient balance write is covered by CREATE_ACCESS, so
  create-with-value and create-with-zero-value have identical intrinsic
  cost.

A plain value transfer's total is unchanged (re-labeled); create-with-value
drops by the folded 1756.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(eip-2780): pin Amsterdam create value-independence; fix comment accuracy

Address review on #12713:

- Clarify that the recipient balance write is covered by the create charge
  (CREATE_ACCESS under EIP-8038, with 8037/legacy fallbacks otherwise),
  rather than implying CREATE_ACCESS is unconditional.
- Parameterize Eip2780_intrinsic_gas_for_create_is_value_independent over
  EIP-8038 on/off so the devnet-8 reference row (zero-value Amsterdam
  create) is pinned directly, and the asserted create charge matches the
  active config.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @flcl42's task in 13m 32s —— View job


Code review

  • Gather context (diff of the 3 PR commits)
  • Review production changes (Eip8038Constants, GasCostOf, EthereumGasPolicy, IntrinsicGasCalculator)
  • Cross-check every updated test expectation against the new constants
  • Check CI signal (this PR + the source PR Apply final EIP-8038 state-access repricing #12633)
  • Post findings

Summary

Three devnet-8 commits: EEST fixture bump to v8.1.0, the EIP-2780 TX_VALUE_COST fold, and the final EIP-8038 repricing (COLD_STORAGE_ACCESS 3000→2100, ACCOUNT_WRITE 8000→9000, access-list entries → cold − warm, CREATE_ACCESSACCOUNT_WRITE + COLD_ACCOUNT_ACCESS).

I verified the arithmetic of essentially every updated expectation, and it holds up well:

  • TxValueCostEip2780 = 6000 is exactly 4244 + 1756, so non-create value transfers are unchanged — the fold is a pure refactor there, and the 20600 / 14600 test names still hold.
  • The Eip7981Tests tie retune is exact: with 115 zero bytes + 1 address + 2 keys, standard = 15000 + 460 + (6900 + 16×336) = 27736 and floor = 15000 + (460+336)×16 = 27736. A genuine tie, and it needed retuning because the old 50-byte/1-address case stops tying at addressCost = 2900.
  • Every Eip8037* delta reconciles: −900 (cold storage), +1000 (CREATE_ACCESS / ACCOUNT_WRITE), +864 (SClear refund), −984 (63/64 of the +1000 CALL_VALUE bump), and the ×4/5 scaling where the EIP-3529 refund cap binds.
  • PerAuthBaseExecution correctly stays at 7816 (ColdAccountAccess unchanged), and no dangling references to the removed TransferLogEip2780 remain.

Findings are inline; nothing blocks on correctness of the implementation — the open questions are all "does this match the devnet-8 spec text", which the code can't answer on its own.

# Severity Finding
1 Medium Create-with-value: the EIP-7708 transfer log is now unpriced, and the code comment justifies the change with the wrong component
2 Medium ColdStorageAccess silently reprices SClearEip8038 12480 → 11616 via an undocumented 4800/5000 ratio that disagrees with the EIP-3529 structural derivation
3 Medium The v8.1.0 fixture bump is not green — see below
4 Low Access-list cold − warm departs from EIP-2930's cold − 2×warm; no EIP citation
5 Low Duplicated rationale in Eip2780ExtraGas

3. The fixture bump is not green (Medium)

CI on this PR is currently useless — nearly every nethtest shard failed with Internal Server Error occurred while resolving "actions/checkout@v6" etc., i.e. a GitHub Actions infra outage, not test failures. So I went to the run for #12633, which carries the same tree (glamsterdam-devnet-8, run 31107564873):

  • txTest: 15 failures, all forks (Frontier through Amsterdam), all test_tx_nonce_overflow: Expected 'TransactionException.NONCE_OVERFLOW' but got 'NonceTooHigh: Nonce exceeds max nonce'. This is an exception-mapping gap surfaced by the v8.1.0 bump, not a gas issue — but it reproduces on all three variants (checked, flat, no-intrinsics) and should be fixed or explicitly tracked as a known-failure before the branch is promoted.
  • engineTest: 1 failure in each of 4 shards (15664/15665 passing) plus zkevmTest failing. I couldn't extract the individual case names from the artifacts in this sandbox, so these need a look — if any of them is an Amsterdam gas vector it's directly relevant to findings 1 and 2.

Good news: Nethermind.Merge.Plugin.Test, Nethermind.Merge.AuRa.Test and Nethermind.Evm.Test all passed on that run, which confirms the updated engine-module hashes are correct (including the eip8037Enabled: true cases whose expectations are unchanged — worth a mental note that those cases appear insensitive to this repricing, so the repriced paths are covered only by the eip8037Enabled: false cases there).

Note on test design

Eip8038IntrinsicGasTests expresses its expectations in terms of the constants under test, so it can't catch a wrong constant — Eip8038ConstantsTests.Derived_parameters_match_the_eip8038_derivations (which pins 2900 / 2000 / 11616 / 12000 as literals) is the only real guard. That's fine as-is, but it means the EEST fixtures are doing all the real validation, which is why finding 3 matters.

Verdict

The implementation is internally consistent and the test updates are correct. Blocking items are the three Medium findings — two need a spec citation (cheap to resolve), and one needs the v8.1.0 failures triaged. DO NOT MERGE is already on the PR, so this is guidance for promoting the devnet branch rather than for merging to master.
· branch glamsterdam-devnet-8-consensus

Comment thread src/Nethermind/Nethermind.Evm/GasPolicy/EthereumGasPolicy.cs
Comment thread src/Nethermind/Nethermind.Core/Eip8038Constants.cs
Comment thread src/Nethermind/Nethermind.Core/Eip8038Constants.cs
Comment thread src/Nethermind/Nethermind.Evm/GasPolicy/EthereumGasPolicy.cs
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

EVM Opcode Benchmark Diff

Aggregated runs: base=3, pr=3
Noisy rerun opcodes: EXTCODESIZE, JUMPDEST

No significant regressions or improvements detected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement glamsterdam-devnet-8

2 participants