Skip to content

refactor(test-benchmarks): ensure SSTORE N->N writes nonzero key/value#2255

Merged
LouisTsai-Csie merged 2 commits intoethereum:forks/amsterdamfrom
jochem-brouwer:tweak_storage_related_benchmarks
Feb 24, 2026
Merged

refactor(test-benchmarks): ensure SSTORE N->N writes nonzero key/value#2255
LouisTsai-Csie merged 2 commits intoethereum:forks/amsterdamfrom
jochem-brouwer:tweak_storage_related_benchmarks

Conversation

@jochem-brouwer
Copy link
Member

🗒️ Description

Tweaks benchmarks to fix expectations in repricings. CC @misilva73
Draft, will push more tweaks if more are found after analysis

🔗 Related Issues or PRs

N/A.

✅ Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    uvx tox -e static
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
  • Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.
  • Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

case StorageAction.WRITE_SAME_VALUE:
attack_block = Op.SSTORE(Op.PUSH0, Op.PUSH0)
case StorageAction.WRITE_NEW_VALUE:
attack_block = Op.SSTORE(Op.PUSH0, Op.GAS)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one was fine, no? It was writing a different value (Op.GAS) to the same slot (slot 0)

@jochem-brouwer jochem-brouwer force-pushed the tweak_storage_related_benchmarks branch from bee7413 to b686f8b Compare February 20, 2026 15:54
@misilva73
Copy link

To properly document this, test_storage_access_cold_benchmark also writes 0->0 in the WRITE_SAME_VALUE variant. However, in this case, since we are always writting to new accounts, I don't think we can easily fix it. Here is the code:

def test_storage_access_cold_benchmark(
    benchmark_test: BenchmarkTestFiller,
    storage_action: StorageAction,
) -> None:
    """
    Benchmark cold storage slot accesses using code generator.

    Each iteration accesses a different storage slot (incrementing key)
    to ensure cold access costs are measured.
    """
    if storage_action == StorageAction.READ:
        attack_block = Op.SLOAD(Op.GAS)
    elif storage_action == StorageAction.WRITE_SAME_VALUE:
        attack_block = Op.SSTORE(Op.GAS, Op.PUSH0)
    elif storage_action == StorageAction.WRITE_NEW_VALUE:
        attack_block = Op.SSTORE(Op.GAS, Op.GAS)

Copy link
Collaborator

@LouisTsai-Csie LouisTsai-Csie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @misilva73 @jochem-brouwer , what's the issue for these benchmark test, is there anything i could help? what benchmark combination do you need

The current changes looks good to me, i approved and wonder if you'd like to merge first

@LouisTsai-Csie LouisTsai-Csie added A-test-benchmark Area: execution_testing.benchmark and tests/benchmark C-refactor Category: refactor labels Feb 24, 2026
@LouisTsai-Csie LouisTsai-Csie changed the title fix(benchmarks): ensure sstore N->N writes nonzero key/value refactor(test-benchmarks): ensure SSTORE N->N writes nonzero key/value Feb 24, 2026
@LouisTsai-Csie LouisTsai-Csie marked this pull request as ready for review February 24, 2026 06:57
@codecov
Copy link

codecov bot commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.85%. Comparing base (e271298) to head (c2caf0e).
⚠️ Report is 30 commits behind head on forks/amsterdam.

Additional details and impacted files
@@                 Coverage Diff                 @@
##           forks/amsterdam    #2255      +/-   ##
===================================================
- Coverage            86.07%   85.85%   -0.23%     
===================================================
  Files                  599      599              
  Lines                39472    39428      -44     
  Branches              3780     3776       -4     
===================================================
- Hits                 33977    33851     -126     
- Misses                4862     4946      +84     
+ Partials               633      631       -2     
Flag Coverage Δ
unittests 85.85% <ø> (-0.23%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jochem-brouwer
Copy link
Member Author

This one can be merged!

@LouisTsai-Csie LouisTsai-Csie merged commit 6f80f72 into ethereum:forks/amsterdam Feb 24, 2026
14 checks passed
kevaundray added a commit to kevaundray/execution-specs that referenced this pull request Feb 28, 2026
commit a48c892
Author: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Date:   Fri Feb 27 17:39:02 2026 +0000

    introduce BlockDiff

commit a4a4b47
Merge: e801c45 5034aa2
Author: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Date:   Tue Feb 24 23:25:33 2026 +0000

    Merge remote-tracking branch 'upstream/forks/amsterdam' into kw/extract-stateless-functionality

commit e801c45
Author: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Date:   Tue Feb 24 23:14:11 2026 +0000

    add doc comments for ChainContext fields

commit de68f63
Author: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Date:   Tue Feb 24 22:57:06 2026 +0000

    initial commit

commit 5034aa2
Author: kevaundray <kevtheappdev@gmail.com>
Date:   Tue Feb 24 22:56:44 2026 +0000

    chore(ci): Skip diffs if not on default branch (ethereum#2304)

    * skip diffs

    * Update .github/workflows/gh-pages.yaml

    Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

    * revert: actionlint

    ---------

    Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

commit 7ca499e
Author: danceratopz <danceratopz@gmail.com>
Date:   Tue Feb 24 20:06:22 2026 +0100

    feat(test-fill,test-benchmark): organize fixtures into fork and gas-limit subdirs (ethereum#2134)

    * feat(test-benchmark): split gas benchmark outputs by subdir

    * docs(test-benchmark): document gas benchmark output layout

    * test(test-benchmark): verify gas benchmark subdir layout

    * feat(test-benchmark): update folder structure according to review

    * refactor(test-filler): move fixture_output to shared

    Move FixtureOutput from filler/ to shared/ and absorb fixture_naming.py.
    This prepares for future build-tool plugins that also generate fixtures.

    * feat(test-filler): organize fill fixture output by target fork

    Always include the gas-limit suffix in the fork output subdirectory,
    not just for benchmark tests. Non-benchmark (consensus) tests now also
    get a subdirectory like `for_prague_at_0120M/` using the environment
    default gas limit (respecting `--block-gas-limit`).

    The `_at_` separator makes the directory name unambiguous:
      for_{fork}_at_{gas}M   (e.g. for_prague_at_0120M)

    * docs(releases): document fixture output directory layout

    Add a "Fixture Output Directory Structure" section explaining the
    for_{fork}_at_{gas}M subdirectory convention, default gas limit, and
    how benchmark tests use --gas-benchmark-values.

    * feat(test-filler): drop gas limit from consensus fixture subdirs

    Consensus fixtures now use `for_{fork}/` (e.g. `for_prague/`) instead of
    `for_{fork}_at_{gas}M/`. Benchmark fixtures keep the gas limit suffix.

    * feat(test-filler): use fixture_subfolder marker for output subdirs

    Replace manual gas-benchmark detection in the filler with a composable
    pytest.mark.fixture_subfolder(level, prefix) marker:

    - Add level-0 marker (fork prefix) in fork parametrization.
    - Add level-1 marker in GasBenchmarkValues and OpcodeCountsConfig.
    - Resolve output subdirs from markers in filler instead of hard-coding.
    - Break circular import so collector can share the separator constant.
    - Register the marker in execute_fill alongside other shared markers.

    * feat(test-tests): add fixed-opcode-count subdirectory output test

    - Verify --fixed-opcode-count splits fixtures into per-count subdirs.
    - Mirror the existing gas-benchmark subdir test for opcode counts.

    * fix(test-fill): fix-up typehints for mypy

    * fix(test-fill): remove dead code and tighten types

    - Remove unused `should_auto_enable_all_formats` property.
    - Remove unused `format_gas_limit_subdir()` function.
    - Remove unreachable `FORK_SUBDIR_PREFIX` check in post-merge
      verification (format dir is always at `parts[0]`).
    - Tighten `resolve_fixture_subfolder` parameter from `list` to
      `list[pytest.Mark]`.
    - Clarify "benchmark" prefix stripping in collector with comment.

    * docs(test-fill): update docs for fixed-opcode-count fixture dir layout

    * fix: static tests

    ---------

    Co-authored-by: Mario Vega <marioevz@gmail.com>

commit 84025b7
Author: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
Date:   Tue Feb 24 14:05:19 2026 -0500

    chore: bump docc version (ethereum#2303)

commit 4fff5eb
Author: Mario Vega <marioevz@gmail.com>
Date:   Tue Feb 24 19:30:35 2026 +0100

    feat(test-execute): Support `testing_buildBlockV1` (ethereum#2295)

    * feat(test-rpc): Add `Testing` namespace

    * nit: transactions parameter

    * feat(test-execute): Support `testing_buildBlockV1`

    * docs

    * refactor(execute): Testing endpoint non-auth

    * review comments

    Co-authored-by: fselmo <fselmo2@gmail.com>

    ---------

    Co-authored-by: fselmo <fselmo2@gmail.com>

commit 7329095
Author: Carson <carson@ethereum.org>
Date:   Tue Feb 24 12:29:35 2026 -0500

    refactor(test): rename/align gas cost constants (ethereum#2094)

    - rename base gas cost
    - rename very low gas cost
    - rename memory gas cost
    - rename copy gas cost
    - rename low gas cost
    - rename medium gas cost
    - rename high gas cost
    - rename warm access gas cost
    - rename cold access gas cost
    - rename storage reset gas cost
    - rename storage clear gas cost
    - rename storage set gas cost
    - rename warm account gas cost
    - rename cold account gas cost
    - rename initcode word gas cost
    - rename code deposit gas cost
    - rename create gas cost
    - rename call stipen gas cost
    - rename selfdestruct gas cost
    - rename call value gas cost
    - rename new account gas cost
    - rename exp gas cost
    - rename exp per byte gas cost
    - rename keccak256 gas cost
    - rename keccak256 word gas cost
    - rename blockhash gas cost
    - rename log data gas cost
    - rename log gas cost
    - rename log topic gas cost
    - rename tx based gas cost
    - rename contract creation gas cost
    - rename access list addr gas cost
    - rename access list storage gas cost
    - rename floor calldata gas cost
    - rename token standard gas cost
    - rename authorize gas cost
    - rename authorize base cost
    - rename data zero gas cost
    - rename non-zero data gas cost
    - rename blob gas per block gas cost
    - rename blob min price gas cost
    - rename all precompile gas costs
    - (refactor) TX_ACCESS_LIST_ADDRESS_COST => GAS_TX_ACCESS_LIST_ADDRESS
    - (refactor) GAS_CODE_DEPOSIT => GAS_CODE_DEPOSIT_PER_BYTE
    - (refactor) remove unnecessary Uint cast for REFUND_STORAGE_CLEAR
    - (refactor) TX_ACCESS_LIST_STORAGE_KEY_COST => GAS_TX_ACCESS_LIST_STORAGE_KEY
    - (refactor) GAS_KECCAK256_WORD => GAS_KECCAK256_PER_WORD
    - (refactor) TX_BASE_COST => GAS_TX_BASE
    - (refactor) TX_CREATE_COST => GAS_TX_CREATE
    - (refactor) TX_DATA_STANDARD_TOKEN_COST => GAS_TX_DATA_TOKEN_STANDARD
    - (refactor) => TX_DATA_FLOOR_TOKEN_COST => GAS_TX_DATA_TOKEN_FLOOR
    - (refactor) GAS_INIT_CODE_WORD_COST => GAS_CODE_INIT_PER_WORD
    - (refactor) TX_DATA_COST_PER_ZERO => GAS_TX_DATA_PER_ZERO
    - (refactor) TX_DATA_COST_PER_NON_ZERO => GAS_TX_DATA_PER_NON_ZERO
    - (refactor) GAS_AUTH_PER_EMPTY_ACCOUNT_COST => GAS_AUTH_PER_EMPTY_ACCOUNT
    - (refactor) GAS_LOG_DATA => GAS_LOG_DATA_PER_BYTE
    - (refactor) GAS_PRECOMPILE_SHA256_WORD => GAS_PRECOMPILE_SHA256_PER_WORD
    - (refactor) GAS_PRECOMPILE_RIPEMD160_WORD => GAS_PRECOMPILE_RIPEMD160_PER_WORD
    - (refactor) GAS_PRECOMPILE_IDENTITY_WORD => GAS_PRECOMPILE_IDENTITY_PER_WORD
    - (refactor) REFUND_PER_AUTH_BASE_COST => REFUND_AUTH_PER_EXISTING_ACCOUNT

    ---------

    Co-authored-by: LouisTsai <q1030176@gmail.com>

commit cdf9add
Author: Paweł Bylica <pawel@hepcolgum.band>
Date:   Tue Feb 24 16:53:55 2026 +0100

    feat(tests): port EXTCODEHASH dynamicAccountOverwriteEmpty (ethereum#2032)

    Port the legacy static test:
    stExtCodeHash/dynamicAccountOverwriteEmpty_Paris
    for EIP-1052 EXTCODEHASH testing.

    The test verifies EXTCODEHASH correctly updates when an empty account is
    overwritten by CREATE2. Modified from original: target account has no
    storage to avoid EIP-7610 collision behavior.

commit 9bd111a
Author: felix <felix314159@users.noreply.github.com>
Date:   Tue Feb 24 14:20:18 2026 +0100

    fix: failing test in ci for 7981 (ethereum#2297)

commit 6f80f72
Author: Jochem Brouwer <jochembrouwer96@gmail.com>
Date:   Tue Feb 24 10:25:24 2026 +0100

    refactor(test-benchmarks): ensure `SSTORE` N->N writes nonzero key/value (ethereum#2255)

commit 04fb242
Author: Paweł Bylica <pawel@hepcolgum.band>
Date:   Mon Feb 23 23:24:31 2026 +0100

    feat(tests): port EXTCODEHASH codeless account with storage test (ethereum#2291)

    Port sub-case from dynamicAccountOverwriteEmpty_ParisFiller.yml:
    a codeless account with storage returns keccak256("") for EXTCODEHASH
    and 0 for EXTCODESIZE. Parametrized over three non-empty variants
    (balance only, nonce only, both).

commit 7add493
Author: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
Date:   Mon Feb 23 15:45:25 2026 -0500

    chore: expand docstring guide (ethereum#2195)

    Co-authored-by: danceratopz <danceratopz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-test-benchmark Area: execution_testing.benchmark and tests/benchmark C-refactor Category: refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants