execution/state: exclude no-op storage writes from the block access list

[taratorio]

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 feat(tests): EIP-7928 catch per-tx no-op SSTORE after prior tx write 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.

[Copilot]

Pull request overview

Fixes an EIP-7928 BAL bug where Erigon recorded spurious storage_changes for no-op SSTOREs that occurred after a prior write to the same slot in the same block. Previously, the no-op filter only applied to the first write to a slot; subsequent writes were always recorded, causing a divergent BAL hash and rejecting valid blocks (observed on bal-devnet-7 at block 94626).

Changes:

• Refactor addStorageUpdate into a method on accountState that, in a single pass, compares each storage write against the slot's most recent recorded value (or pre-block read for first write) and skips no-ops.
• Simplify updateWrite's storage branch by delegating the entire no-op check to addStorageUpdate.
• Add unit and integration tests covering repeated same-value writes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
execution/state/versionedio.go	Move no-op filter into addStorageUpdate, comparing each write against the slot's last recorded value.
execution/state/versionedio_test.go	Add unit test for repeated-value no-op write filtering across multiple txs.
execution/engineapi/engine_api_bal_test.go	Add end-to-end engine API test exercising the no-op filter via parallel executor.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yperbasis]

Approving.

The fix correctly implements the EIP-7928 no-op rule: each storage write is compared against the slot's value immediately before its block-access index — the most recent recorded change, or the pre-block read value for the first write — and same-value writes are skipped. The old code only applied that filter to the first write to a slot, so a later write of an already-set value (devnet-7 block 94626) produced a spurious storage_change. Folding the check into a single pass in addStorageUpdate is clean, and the comparison-against-last-change is sound because AsBlockAccessList accumulates writes in ascending block-access-index order.

One optional nit: without the fix a single node still builds the block as VALID (it accepts its own BAL), so the failure actually surfaces at the StorageChanges length assertion rather than at BuildCanonicalBlock — the "BAL divergence" message on that require.NoError is slightly misleading. Not blocking.