The first Metrics() call inside calcAndVerifyRoot snapshots accountReadNS
and storageReadNS into the cached AccountReadTime/StorageReadTime fields.
But commitAccount (called from writeBlockWithState's CommitWithUpdate
path) increments storageReadNS *after* that snapshot, so reading
m.StorageReadTime later would silently drop those reads.
Re-call Metrics() before reading the read-time fields so the cache
reflects the post-commit atomics. Other metric fields (AccountUpdate,
AccountCommits, etc.) are written directly to s.metrics elsewhere and
remain untouched by Metrics().
Found via the metric-correctness audit.
The struct is 80 bytes (10 ints) — value semantics matches the type's
"snapshot, safe to pass by value" thesis stated in its doc comment, and
removes three unnecessary &-takings at call sites. No behavior change.
- Add a comment at the code-mutation gate explaining the deliberate
len(code) > 0 (vs code != nil) match against non-BAL semantics; in
devnet-3 BAL access lists, an empty []byte is non-nil but encodes
"no code install".
- Remove BALStateTransitionMetrics.OriginStorageLoadTime: declared but
never assigned anywhere in the tree. The actual state-transition
read time is captured by AccountReadTime/StorageReadTime added in
the prior commit.
Mirrors the nil-check already used in buildSlowBlockLog. The previous
unguarded access was safe today only because parallel_state_processor
short-circuits on error before the metrics path is reached, but the API
contract was fragile — a future caller could reach reportBALMetrics
without an established balTransitionStats and panic.
Without this, the inline interface assertion in processBlockWithAccessList
silently fell through (the prefetchReader returned by ReaderEIP7928 is a
*reader wrapper, not the inner *prefetchStateReader), causing the
prefetcher contribution to state_read_ms to drop to zero in production.
Mirrors the existing GetStateStats forwarding pattern. Adds a regression
test that asserts *reader exposes PrefetchReadTimes via the BAL chain,
plus a fallback test for non-prefetch readers.
Adds the StateCounts type that the BAL slow-block work depends on:
- core/state/state_counts.go: 10-field plain-int snapshot type with
Add merge primitive; isolates the live atomic mutation surface from
the value-typed aggregation pipeline.
- core/state/statedb.go: SnapshotCounts() method that converts the
StateDB's atomic counters to a plain StateCounts at the boundary.
- core/blockchain_stats.go: ExecuteStats embeds state.StateCounts;
adds ExecWall/PostProcess/Prefetch BAL extension fields, the
slowBlockBAL JSON struct + BAL field on slowBlockLog, and extracts
buildSlowBlockLog as a pure helper for direct testing.
Without this commit the bal-devnet-3 branch as committed in subsequent
commits would not build for a fresh clone (state.StateCounts undefined).
Wire up slotnum for `testing_buildBlockV1` for `bal-devnet-3` branch
We are experimenting testing block building through hive via EELS (PR
[here](https://github.com/ethereum/execution-specs/pull/2679)). This is
the only change needed to test against `bal-devnet-3` - missing slotnum.
The BAL reader tracker captures access list reads at the reader level.
When statedb has an account cached the BAL tracker is not informed of
the access. This is ok during the lifetime of a transaction because you
only need to record the access the first time. It is also ok during the
lifetime of a block because BAL reads are block-level (same as statedb
caches).
Where I think the issue can rise is in the miner. Namely when building a
block, if the miner picks up a tx which fails, it drops it and picks up
another tx to include. There might be some edge case here where the
failed tx which is not included poisons the cache and a future block
which is included omits an account because it wasn't aware of the
access.
To check whether a transaction can be applied, we validate that
`blockGasLimit > txGasLimit + (cumulativeRegularGasUsed +
cumulativeStateGasUsed)`. However, the check should only be applied to
the bottleneck resource, i.e. `blockGasLimit >
max(txRegularGasUsed+cumulativeRegularGasUsed, txStateGasUsed+
cumulativeStateGasUsed)`.
The changes here break multiple tests. I am trying to determine why.
---------
Co-authored-by: qu0b <stefan@starflinger.eu>
## Summary
- The BAL refactoring in 5808d212b removed the line `env.header.GasUsed
= env.gasPool.Used()` from `applyTransaction()`, causing all
geth-proposed blocks to have `GasUsed=0` in the header
- Every other client rejects these blocks during validation, so geth can
never successfully propose on the network
- Restores the single line that was present in upstream master
## Test plan
- [x] Verified line exists in upstream/master at miner/worker.go:409
- [x] Confirmed all 34 geth-proposed blocks on a 4-client devnet had
`GasUsed=0` via `debug_getBadBlocks` on besu
- [ ] Build local image and verify geth proposals are accepted in a
multi-client Kurtosis network
* add method on StateReaderTracker to clear the accumulated reads
* don't factor the BAL size into the payload size during construction in the miner
* simplify miner code for constructing payloads-with-BALs via the use of aformentioned StateReaderTracker clear method
* clean up the configuration of the BAL execution mode based on the preset flag specified
Adds support for returning `blockAccessListHash` in the RPC block header
response.
This change includes `blockAccessListHash` in `RPCMarshalHeader` when
`head.BlockAccessListHash` is not nil, aligning the RPC output with the
Header struct which already contains this field.
CopyHeader copies all pointer-typed header fields (WithdrawalsHash,
RequestsHash, SlotNumber, etc.) but was missing the deep copy for
BlockAccessListHash added by EIP-7928. This caused the BAL hash
to be silently shared between the original and the copy, leading
to potential data races and incorrect nil-checks on copied headers.
