Skip to content

feat(tests): EIP-7928 catch per-tx no-op SSTORE after prior tx write#2946

Merged
danceratopz merged 3 commits into
ethereum:forks/amsterdamfrom
pk910:feat/bal-noop-sstore-after-prior-tx-write
Jun 2, 2026
Merged

feat(tests): EIP-7928 catch per-tx no-op SSTORE after prior tx write#2946
danceratopz merged 3 commits into
ethereum:forks/amsterdamfrom
pk910:feat/bal-noop-sstore-after-prior-tx-write

Conversation

@pk910
Copy link
Copy Markdown
Member

@pk910 pk910 commented Jun 2, 2026

🗒️ Description

Adds test_bal_intra_tx_round_trip_after_prior_tx_write to verify that a per-tx no-op SSTORE is classified as a read even when an earlier transaction in the same block already wrote to the slot.

Two txs call a contract that does
SSTORE(slot=1, 0xff); SSTORE(slot=1, 0x42); STOP. Tx 1 changes slot 1 from 0 to 0x42 (real change). Tx 2 round-trips 0x42 -> 0xff -> 0x42 (per-tx no-op). Per EIP-7928 §Storage, "MUST check the pre-transaction value" — and pre-transaction is per-tx, not per-block — so only tx 1 must appear in storage_changes.

Regression test for the bal-devnet-7 chain split (slot 96338 / EL block 94626). Reproduced live on a 2-node nethermind + erigon kurtosis devnet running the bal-devnet-7 images: erigon rejects with block access list mismatch and forks off.

🔗 Related Issues or PRs

N/A.

✅ Checklist

  • All: Ran fast static checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    just 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

    /\_/\
  =( o.o )=
    > ^ <

@pk910 pk910 force-pushed the feat/bal-noop-sstore-after-prior-tx-write branch 2 times, most recently from abea5a6 to 932b002 Compare June 2, 2026 01:41
Adds `test_bal_intra_tx_round_trip_after_prior_tx_write` to verify that
a per-tx no-op SSTORE is classified as a read even when an earlier
transaction in the same block already wrote to the slot.

Two txs call a contract that does
`SSTORE(slot=1, 0xff); SSTORE(slot=1, 0x42); STOP`.
Tx 1 changes slot 1 from 0 to 0x42 (real change). Tx 2 round-trips
0x42 -> 0xff -> 0x42 (per-tx no-op). Per EIP-7928 §Storage, "MUST
check the pre-transaction value" — and pre-transaction is per-tx, not
per-block — so only tx 1 must appear in storage_changes.

Regression test for the bal-devnet-7 chain split (slot 96338 /
EL block 94626). Reproduced live on a 2-node nethermind + erigon
kurtosis devnet running the bal-devnet-7 images: erigon rejects with
`block access list mismatch` and forks off.
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.50%. Comparing base (d964fd5) to head (461a321).

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #2946   +/-   ##
================================================
  Coverage            90.50%   90.50%           
================================================
  Files                  535      535           
  Lines                32407    32407           
  Branches              3011     3011           
================================================
  Hits                 29331    29331           
  Misses                2559     2559           
  Partials               517      517           
Flag Coverage Δ
unittests 90.50% <ø> (ø)

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.

Copy link
Copy Markdown
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

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

Thanks for finding and adding this test, @pk910!

Just a couple of minor nits below.

pk910 and others added 2 commits June 2, 2026 13:52
Copy link
Copy Markdown
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

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

Thanks!

@danceratopz danceratopz merged commit cb77cc5 into ethereum:forks/amsterdam Jun 2, 2026
18 checks passed
@danceratopz danceratopz added C-feat Category: an improvement or new feature A-tests Area: Consensus tests. labels Jun 2, 2026
Sahil-4555 pushed a commit to Sahil-4555/erigon that referenced this pull request Jun 2, 2026
…ist (erigontech#21574)

## Summary

Fixes an EIP-7928 block-access-list (BAL) bug where erigon recorded
spurious
`storage_changes` for no-op SSTOREs, producing a BAL hash that diverged
from the
block header. The node rejected otherwise-valid blocks with
`block access list mismatch` and forked off the canonical chain
(observed on
bal-devnet-7 at block 94626).

## Root cause

`(*VersionedIO).AsBlockAccessList` filtered no-op storage writes only
for the
**first** write to a slot; any later write was recorded unconditionally.
EIP-7928 requires comparing **each** write against the slot's value
immediately
before its block-access index — a write storing the value the slot
already holds
is a no-op and must stay a read, not appear in `storage_changes`.

So when a transaction wrote a slot the value an earlier transaction in
the same
block had already set, the parallel executor's BAL gained a spurious
`storage_change`, diverging from the canonical BAL.

## Fix

`addStorageUpdate` now compares every storage write against the slot's
most
recent recorded value (falling back to the pre-block read value for the
first
write) and skips no-ops, in a single pass.

## Testing

- **Unit** —
`TestVersionedIO_StorageNoOpWriteAfterChangeOmittedFromBAL`.
- **Integration (parallel exec)** —
`TestEngineApiBALStorageNoOpWriteOmitted`: a
contract doing `SSTORE(B); SSTORE(A)` called twice in one block (the
second
  call a per-tx no-op). Red without the fix, green with it.
- **EEST `blocktests-devnet` shard** (`devnets/bal/7`) — passes 82,941 /
0
  failures under parallel exec.
- **Upstream spec test** — the new test in ethereum/execution-specs#2946
(`test_bal_intra_tx_round_trip_after_prior_tx_write`), filled against
the
matching devnet spec and run via `evm blocktest`: **passes with the fix,
fails
  with `block access list mismatch` without it.**
- **Devnet** — a node previously stuck at block 94626 now validates it
and syncs
  to the network head.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-tests Area: Consensus tests. C-feat Category: an improvement or new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants