Forward prefetchStateReader.Wait() through *reader.WaitPrefetch and call
it before reading the read-time atomics. Eliminates the edge-case where
prefetcher goroutines outlast block execution + commit. For slow blocks
(the metric's target audience) this is a no-op; for fast blocks it
ensures the metric is complete rather than slightly under.
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).
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>
* 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
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.
Add persistent storage for Block Access Lists (BALs) in `core/rawdb/`.
This provides read/write/delete accessors for BALs in the active
key-value store.
---------
Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com>
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
`pool.signer.Sender(tx)` bypasses the sender cache used by types.Sender,
which can force an extra signature recovery for every promotable tx
(promotion runs frequently). Use `types.Sender(pool.signer, tx)` here to
keep sender derivation cached and consistent.
