core: drop and tighten comments per PR feedback

core/blockchain.go

@@ -652,10 +652,8 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block *
 	writeTime := time.Since(writeStart)
 	var stats ExecuteStats
 
-	// Counts: write counts come from the BAL state transition; read counts
+	// AccountLoaded/StorageLoaded come from the BAL access list (deduplicated);
-	// for accounts/storage come from the BAL access list (deduplicated);
+	// per-StateDB sums would over-count addresses touched by multiple phases.
-	// code-load counts come from a deduplicated address set tracked across
-	// all phase StateDBs by the parallel processor.
 	stats.StateCounts = res.Counts
 	stats.StateCounts.Add(stateTransition.WriteCounts())
 	if al := block.AccessList(); al != nil {
@@ -665,17 +663,10 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block *
 	stats.StateCounts.CodeLoaded = res.CodeLoaded
 	stats.StateCounts.CodeLoadBytes = res.CodeLoadBytes
 
-	// Time durations under parallel execution use wall-clock semantics.
+	stats.Execution = res.ExecTime
-	// Per-tx duration sums (CPU-time) are intentionally not plumbed: they
-	// would conflict with mgas/sec accounting against TotalTime.
-	stats.Execution = res.ExecTime // wall-clock parallel execution
 	stats.ExecWall = res.ExecTime
 	stats.PostProcess = res.PostProcessTime
 
-	// Map BALStateTransitionMetrics (already wall-clock-correct) onto schema
-	// fields used by logSlow's StateHashMs computation. The sum
-	// AccountUpdate+StateUpdate+StateHash is the parallel state-root compute
-	// time, matching reportBALMetrics's stateRootComputeTimer.
 	if m := res.StateTransitionMetrics; m != nil {
 		stats.AccountHashes = m.AccountUpdate + m.StateUpdate + m.StateHash
 		stats.AccountCommits = m.AccountCommits
@@ -683,10 +674,7 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block *
 		stats.DatabaseCommit = m.TrieDBCommits
 		stats.Prefetch = m.StatePrefetch
 	}
-	// Sum read times across per-tx execution, BAL state-transition, and
+	// Sum-of-CPU-time across per-tx, BAL state-transition, and prefetcher paths.
-	// prefetcher async fetches. Sum-of-CPU-time, not wall-clock. No
-	// WaitPrefetch needed: state is already committed, so the prefetcher
-	// (bounded by BAL contents) has drained.
 	var prefetchAccountReads, prefetchStorageReads time.Duration
 	if pr, ok := prefetchReader.(interface {
 		PrefetchReadTimes() (time.Duration, time.Duration)
@@ -698,7 +686,6 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block *
 	stats.StorageReads = res.Reads.Storage + prefetchStorageReads + balStorageReads
 	stats.CodeReads = res.Reads.Code
 
-	// Cache stats from the shared prefetch reader (accumulates centrally).
 	if r, ok := prefetchReader.(state.ReaderStater); ok {
 		stats.StateReadCacheStats = r.GetStats()
 	}

core/blockchain_stats.go

@@ -28,9 +28,7 @@ import (
 
 // ExecuteStats includes all the statistics of a block execution in details.
 type ExecuteStats struct {
-	// State read times. For BAL blocks these are sum-of-CPU-time across
+	// State read times
-	// per-tx, pre-tx, post-tx, BAL state-transition and prefetcher paths;
-	// can exceed TotalTime by design. Sequential blocks: wall-clock.
 	AccountReads   time.Duration // Time spent on the account reads
 	StorageReads   time.Duration // Time spent on the storage reads
 	AccountHashes  time.Duration // Time spent on the account trie hash
@@ -40,8 +38,7 @@ type ExecuteStats struct {
 	StorageCommits time.Duration // Time spent on the storage trie commit
 	CodeReads      time.Duration // Time spent on the contract code read
 
-	// Embedded state-mutation counts. Field promotion preserves access as
+	// State-mutation counts. StorageUpdated/StorageDeleted are int64
-	// s.AccountLoaded etc. Note StorageUpdated/StorageDeleted are int64 here
 	// (snapshot from atomic.Int64 on StateDB).
 	state.StateCounts
 
@@ -55,9 +52,7 @@ type ExecuteStats struct {
 	TotalTime       time.Duration // The total time spent on block execution
 	MgasPerSecond   float64       // The million gas processed per second
 
-	// BAL extension durations — set by processBlockWithAccessList for blocks
+	// BAL parallel-path durations, surfaced under slowBlockLog.BAL.
-	// processed via the parallel BAL path. Surfaced in the slow-block log's
-	// optional `bal` block.
 	ExecWall    time.Duration // Wall-clock parallel transaction execution
 	PostProcess time.Duration // Post-tx finalization (system contracts, requests)
 	Prefetch    time.Duration // BAL state prefetching
@@ -123,9 +118,7 @@ type slowBlockLog struct {
 	StateReads  slowBlockReads  `json:"state_reads"`
 	StateWrites slowBlockWrites `json:"state_writes"`
 	Cache       slowBlockCache  `json:"cache"`
-	// BAL is the parallel-execution extension. Present iff the block was
+	// BAL is set only for blocks processed via the parallel BAL path.
-	// processed via the BAL parallel path. Cross-client consumers can use its
-	// presence to distinguish parallel-executed blocks from sequential ones.
 	BAL *slowBlockBAL `json:"bal,omitempty"`
 }
 
@@ -187,31 +180,18 @@ type slowBlockCodeCacheEntry struct {
 	MissBytes int64   `json:"miss_bytes"`
 }
 
-// slowBlockBAL is the parallel-execution extension surfaced under the
+// slowBlockBAL holds parallel-execution timings that don't fit the sequential schema.
-// optional "bal" field of slowBlockLog. It carries timings that are
-// well-defined under parallel execution but don't fit the sequential schema.
 type slowBlockBAL struct {
-	// ExecWallMs is wall-clock parallel transaction execution.
 	ExecWallMs       float64 `json:"exec_wall_ms"`
-	// PostProcessMs is post-tx system contracts (withdrawals, consolidations, finalize).
 	PostProcessMs    float64 `json:"post_process_ms"`
-	// PrefetchMs is the BAL state prefetcher (alias of state_prefetch_ms).
 	PrefetchMs       float64 `json:"prefetch_ms"`
-	// StatePrefetchMs is async state-load time during state-root computation.
 	StatePrefetchMs  float64 `json:"state_prefetch_ms"`
-	// AccountUpdateMs is the account trie update phase.
 	AccountUpdateMs  float64 `json:"account_update_ms"`
-	// StateUpdateMs is the state trie update phase.
 	StateUpdateMs    float64 `json:"state_update_ms"`
-	// StateHashMs is state-root hash computation.
 	StateHashMs      float64 `json:"state_hash_ms"`
-	// AccountCommitMs is the account trie commit to disk.
 	AccountCommitMs  float64 `json:"account_commit_ms"`
-	// StorageCommitMs is the storage trie commit to disk.
 	StorageCommitMs  float64 `json:"storage_commit_ms"`
-	// TrieDBCommitMs is the trie database commit.
 	TrieDBCommitMs   float64 `json:"triedb_commit_ms"`
-	// SnapshotCommitMs is the state snapshot commit.
 	SnapshotCommitMs float64 `json:"snapshot_commit_ms"`
 }
 
@@ -221,9 +201,8 @@ func durationToMs(d time.Duration) float64 {
 	return float64(d.Nanoseconds()) / 1e6
 }
 
-// buildSlowBlockLog constructs the slow-block log JSON struct from execution
+// buildSlowBlockLog builds the slow-block JSON payload. Split out from logSlow
-// statistics. Pure function — no side effects, no logging — to make the JSON
+// so the JSON shape is directly testable.
-// shape directly testable.
 func buildSlowBlockLog(s *ExecuteStats, block *types.Block) slowBlockLog {
 	logEntry := slowBlockLog{
 		Level: "warn",
@@ -278,7 +257,6 @@ func buildSlowBlockLog(s *ExecuteStats, block *types.Block) slowBlockLog {
 			},
 		},
 	}
-	// Populate the parallel-execution extension only for BAL-processed blocks.
 	if m := s.balTransitionStats; m != nil {
 		logEntry.BAL = &slowBlockBAL{
 			ExecWallMs:       durationToMs(s.ExecWall),

core/parallel_state_processor.go

@@ -24,17 +24,14 @@ type ProcessResultWithMetrics struct {
 	// the time it took to execute all txs in the block
 	ExecTime        time.Duration
 	PostProcessTime time.Duration
-	// Counts is the per-StateDB sum of state-mutation counters across pre-tx,
+	// Counts sums state-mutation counters across pre-tx, per-tx and post-tx
-	// per-tx and post-tx phases. The caller may override AccountLoaded and
+	// StateDBs. AccountLoaded/StorageLoaded are NOT deduplicated here — the
-	// StorageLoaded with deduplicated counts derived from block.AccessList()
+	// caller overrides them from block.AccessList().
-	// (per-StateDB sums over-count addresses touched by multiple phases).
 	Counts state.StateCounts
-	// Reads is the sum of per-StateDB read times across pre-tx, per-tx and
+	// Reads sums per-StateDB read times (sum-of-CPU-time, not wall-clock).
-	// post-tx phases. Sum-of-CPU-time, not wall-clock.
 	Reads state.ReadDurations
-	// CodeLoaded is the deduplicated count of unique contract addresses whose
+	// CodeLoaded/CodeLoadBytes are deduplicated by contract address across
-	// code body was fetched during the block (across all phase StateDBs).
+	// all phase StateDBs.
-	// CodeLoadBytes is the sum of those code lengths.
 	CodeLoaded    int
 	CodeLoadBytes int
 }
@@ -184,9 +181,7 @@ func (p *ParallelStateProcessor) prepareExecResult(block *types.Block, tExecStar
 
 	tPostprocess := time.Since(tPostprocessStart)
 
-	// Fold post-tx statedb counts and reads into the aggregate. postTxState is
+	// Fold post-tx counts/reads in: postTxState is local and otherwise discarded.
-	// local and would otherwise be discarded; this captures system-contract
-	// activity (withdrawal queue, consolidation queue) and engine.Finalize.
 	aggCounts.Add(postTxState.SnapshotCounts())
 	aggReads.Add(postTxState.SnapshotReads())
 	for addr, l := range postTxState.SnapshotCodeLoads() {
@@ -230,14 +225,10 @@ type txExecResult struct {
 
 	stateReads bal.StateAccesses
 
-	// Per-tx state-mutation counts and read durations, snapshotted from the
+	// Per-tx counts/reads/code-loads, aggregated single-threaded in resultHandler.
-	// worker statedb just before send. Aggregated single-threaded in
-	// resultHandler.
 	counts    state.StateCounts
 	reads     state.ReadDurations
-	// codeLoads is addr→codeLen for contracts whose code body was fetched
+	codeLoads map[common.Address]int // addr → code len, deduped across phases
-	// in this tx. Deduped across all phases in resultHandler.
-	codeLoads map[common.Address]int
 }
 
 // resultHandler polls until all transactions have finished executing and the
@@ -251,13 +242,9 @@ func (p *ParallelStateProcessor) resultHandler(block *types.Block, preTxAccesses
 	var numTxComplete int
 
 	// Seed aggregates with the pre-tx contribution (BeaconRoot, ParentBlockHash).
-	// Per-tx fold below; post-tx fold in prepareExecResult.
 	accesses := preTxAccesses
 	aggCounts := preCounts
 	aggReads := preReads
-	// Dedup'd map of contract addresses whose code body was fetched by any
-	// phase StateDB. Address-keyed so multiple phases adding the same contract
-	// only count it once.
 	aggCodeLoads := make(map[common.Address]int)
 	for addr, l := range preCodeLoads {
 		aggCodeLoads[addr] = l
@@ -406,9 +393,7 @@ func (p *ParallelStateProcessor) processBlockPreTx(block *types.Block, statedb *
 	if !accessList.MutationsAt(0).Eq(mutations) {
 		return nil, state.StateCounts{}, state.ReadDurations{}, nil, fmt.Errorf("invalid block access list: mismatch between local/remote access list mutations at idx 0")
 	}
-	// Snapshot the pre-tx statedb's counts/reads/code-loads so system-contract
+	// Snapshot pre-tx counts/reads/code-loads: sdb is local and otherwise discarded.
-	// activity (BeaconRoot, ParentBlockHash) contributes to the aggregate;
-	// sdb is local and would otherwise be discarded.
 	return reads, sdb.SnapshotCounts(), sdb.SnapshotReads(), sdb.SnapshotCodeLoads(), nil
 }
 

core/state/bal_state_transition.go

@@ -41,18 +41,14 @@ type BALStateTransition struct {
 	tries     sync.Map //map[common.Address]Trie
 	deletions map[common.Address]struct{}
 
-	// Storage counters use atomic.Int64 because they're written from per-address
+	// Storage/read counters are atomic — written from per-address goroutines.
-	// goroutines. The others are written single-threaded inside IntermediateRoot's
+	// account/code counters are plain int — written single-threaded.
-	// serial mutation loop, so plain int matches StateCounts' int fields.
 	accountDeleted  int
 	accountUpdated  int
 	storageDeleted  atomic.Int64
 	storageUpdated  atomic.Int64
 	codeUpdated     int
 	codeUpdateBytes int
-
-	// Read-time accumulators for state-root recomputation reads. Atomic
-	// because s.reader.Account/Storage is called from per-address goroutines.
 	accountReadNS   atomic.Int64
 	storageReadNS   atomic.Int64
 
@@ -64,22 +60,16 @@ type BALStateTransition struct {
 	err error
 }
 
-// Metrics returns the cached commit/hash-phase timings. Read-time atomics
-// are exposed separately via ReadTimes; that decoupling avoids the
-// snapshot-staleness pitfall when commitAccount runs more reads after
-// Metrics is first called.
 func (s *BALStateTransition) Metrics() *BALStateTransitionMetrics {
 	return &s.metrics
 }
 
-// ReadTimes returns the current accumulated read times from atomic counters.
+// ReadTimes returns the accumulated state-read times.
-// Always live; safe to call at any point after IntermediateRoot/Commit work.
 func (s *BALStateTransition) ReadTimes() (account, storage time.Duration) {
 	return time.Duration(s.accountReadNS.Load()), time.Duration(s.storageReadNS.Load())
 }
 
-// WriteCounts returns the state-mutation counts tracked during the parallel
+// WriteCounts returns the state-mutation counts from the parallel state-root pass.
-// state-root computation.
 func (s *BALStateTransition) WriteCounts() StateCounts {
 	return StateCounts{
 		AccountUpdated:  s.accountUpdated,
@@ -523,11 +513,9 @@ func (s *BALStateTransition) IntermediateRoot(_ bool) common.Hash {
 		} else {
 			acct, code := s.updateAccount(mutatedAddr)
 
-			// Use len(code) > 0 (not code != nil) to match the non-BAL semantic
+			// Empty []byte is non-nil but means "no code install" in devnet-3
-			// at statedb.go (obj.dirtyCode && len(obj.code) > 0). In devnet-3
+			// BAL access lists; matches the obj.dirtyCode && len(obj.code) > 0
-			// BAL access lists, an empty []byte is non-nil but encodes "no code
+			// gate in statedb.go.
-			// install"; treating it as a code mutation would over-count and
-			// call UpdateContractCode with an empty payload.
 			if len(code) > 0 {
 				codeHash := crypto.Keccak256Hash(code)
 				acct.CodeHash = codeHash.Bytes()

core/state/reader.go

@@ -565,9 +565,7 @@ func (r *reader) GetStats() ReaderStats {
 	}
 }
 
-// PrefetchReadTimes returns the prefetcher's accumulated read times if the
+// PrefetchReadTimes forwards to the wrapped prefetcher, or returns zero.
-// underlying state reader exposes them (e.g. *prefetchStateReader). Returns
-// zero if the wrapped reader doesn't track these (sequential paths, tests).
 func (r *reader) PrefetchReadTimes() (account, storage time.Duration) {
 	if pr, ok := r.StateReader.(interface {
 		PrefetchReadTimes() (time.Duration, time.Duration)
@@ -577,8 +575,7 @@ func (r *reader) PrefetchReadTimes() (account, storage time.Duration) {
 	return 0, 0
 }
 
-// WaitPrefetch blocks until the wrapped prefetcher (if any) finishes its
+// WaitPrefetch blocks until the wrapped prefetcher drains; no-op otherwise.
-// task list. No-op for non-prefetch readers.
 func (r *reader) WaitPrefetch() {
 	if pr, ok := r.StateReader.(interface{ Wait() error }); ok {
 		_ = pr.Wait()

core/state/reader_eip_7928.go

@@ -89,8 +89,7 @@ type prefetchStateReader struct {
 	term      chan struct{}
 	closeOnce sync.Once
 
-	// Async-fetch read-time accumulators (atomic because process() runs
+	// Atomic — process() runs across N goroutines.
-	// across N goroutines).
 	accountReadNS atomic.Int64
 	storageReadNS atomic.Int64
 }
@@ -374,9 +373,8 @@ func (r *readerTracker) TouchStorage(addr common.Address, slot common.Hash) {
 	list[slot] = struct{}{}
 }
 
-// GetStateStats forwards stats from the wrapped *stateReaderWithStats so the
+// GetStateStats forwards stats from the wrapped reader; without this, BAL
-// (*reader).GetStateStats type assertion succeeds. Without this, account/
+// blocks would emit zero cache hit/miss counts.
-// storage cache hit/miss counts emit zero on BAL blocks.
 func (r *prefetchStateReader) GetStateStats() StateReaderStats {
 	if stater, ok := r.StateReader.(StateReaderStater); ok {
 		return stater.GetStateStats()
@@ -384,9 +382,8 @@ func (r *prefetchStateReader) GetStateStats() StateReaderStats {
 	return StateReaderStats{}
 }
 
-// PrefetchReadTimes returns the accumulated wall-time-of-each-call durations
+// PrefetchReadTimes returns sum-of-CPU-time across worker goroutines (not
-// for asynchronous account/storage prefetches. Sum-of-CPU-time across worker
+// wall-clock).
-// goroutines; not wall-clock total prefetch time.
 func (r *prefetchStateReader) PrefetchReadTimes() (account, storage time.Duration) {
 	return time.Duration(r.accountReadNS.Load()), time.Duration(r.storageReadNS.Load())
 }

core/state/reader_eip_7928_test.go

@@ -291,23 +291,17 @@ func TestPrefetchStateReaderForwardsStats(t *testing.T) {
 	}
 }
 
-// TestReaderForwardsPrefetchReadTimes locks down that the *reader aggregator
+// TestReaderForwardsPrefetchReadTimes locks down that *reader exposes the
-// (the type returned by ReaderEIP7928) exposes PrefetchReadTimes via the
+// inner prefetcher's read-time counters via PrefetchReadTimes.
-// inner *prefetchStateReader. Without the forwarding method on *reader,
-// callers that hold a Reader interface would not see the prefetcher's
-// accumulated read times even though the prefetcher tracks them.
 func TestReaderForwardsPrefetchReadTimes(t *testing.T) {
 	stub := newRefStateReader()
 	cached := newStateReaderWithCache(stub)
 	withStats := newStateReaderWithStats(cached)
 	prefetch := newPrefetchStateReaderInternal(withStats, nil, 1)
 
-	// Seed timer values directly on the prefetcher.
 	prefetch.accountReadNS.Store(123)
 	prefetch.storageReadNS.Store(456)
 
-	// Wrap in *reader the way ReaderEIP7928 does (with a nil code reader for
-	// brevity; PrefetchReadTimes only inspects the state side).
 	r := newReader(nil, prefetch)
 
 	a, s := r.PrefetchReadTimes()
@@ -319,8 +313,8 @@ func TestReaderForwardsPrefetchReadTimes(t *testing.T) {
 	}
 }
 
-// TestReaderPrefetchReadTimesNonPrefetch verifies the safe zero fallback when
+// TestReaderPrefetchReadTimesNonPrefetch verifies the zero fallback when the
-// the wrapped state reader doesn't expose PrefetchReadTimes (sequential path).
+// wrapped reader doesn't expose PrefetchReadTimes.
 func TestReaderPrefetchReadTimesNonPrefetch(t *testing.T) {
 	r := newReader(nil, newRefStateReader())
 	a, s := r.PrefetchReadTimes()

core/state/statedb.go

@@ -223,9 +223,8 @@ func (s *StateDB) WithReader(reader Reader) *StateDB {
 	return cpy
 }
 
-// ReadDurations groups the {Account, Storage, Code} state-read times that are
+// ReadDurations groups the {Account, Storage, Code} state-read times.
-// aggregated across pre-tx, per-tx and post-tx statedbs in the BAL parallel
+// Sum-of-CPU-time when aggregated across BAL phase statedbs.
-// path. Sum-of-CPU-time, not wall-clock.
 type ReadDurations struct {
 	Account time.Duration
 	Storage time.Duration
@@ -239,10 +238,8 @@ func (r *ReadDurations) Add(other ReadDurations) {
 	r.Code += other.Code
 }
 
-// StateCounts holds count-only statistics gathered during a block's state
+// StateCounts is a plain-int snapshot of state-mutation counters. Atomic
-// transition. Plain-int snapshot type, safe to copy through channels.
+// fields on StateDB are Load()'d at the SnapshotCounts boundary.
-// Atomic counters on StateDB are converted at the snapshot boundary in
-// SnapshotCounts. Read durations live in ReadDurations (separate type).
 type StateCounts struct {
 	AccountLoaded   int   // accounts retrieved from the database during the state transition
 	AccountUpdated  int   // accounts updated during the state transition
@@ -256,10 +253,7 @@ type StateCounts struct {
 	CodeUpdateBytes int   // total bytes of persisted code written
 }
 
-// Add merges other into c. Plain integer addition — no atomics here, since
+// Add merges other into c.
-// StateCounts is the snapshot type. The receiver is the only mutated party;
-// other is taken by value (the struct is small and value semantics matches
-// the snapshot thesis stated above).
 func (c *StateCounts) Add(other StateCounts) {
 	c.AccountLoaded += other.AccountLoaded
 	c.AccountUpdated += other.AccountUpdated
@@ -273,9 +267,7 @@ func (c *StateCounts) Add(other StateCounts) {
 	c.CodeUpdateBytes += other.CodeUpdateBytes
 }
 
-// SnapshotCounts returns a value-copy of the state-mutation counters as a
+// SnapshotCounts returns a plain-int copy of the state-mutation counters.
-// plain-int StateCounts. Atomic fields are read via Load(); the result is
-// safe to copy, pass through channels, and aggregate via StateCounts.Add.
 func (s *StateDB) SnapshotCounts() StateCounts {
 	return StateCounts{
 		AccountLoaded:   s.AccountLoaded,
@@ -291,8 +283,7 @@ func (s *StateDB) SnapshotCounts() StateCounts {
 	}
 }
 
-// SnapshotReads returns a value-copy of the {Account, Storage, Code} read
+// SnapshotReads returns the {Account, Storage, Code} read durations.
-// durations accumulated on this StateDB.
 func (s *StateDB) SnapshotReads() ReadDurations {
 	return ReadDurations{
 		Account: s.AccountReads,
@@ -301,9 +292,8 @@ func (s *StateDB) SnapshotReads() ReadDurations {
 	}
 }
 
-// SnapshotCodeLoads returns the addresses whose contract code body was
+// SnapshotCodeLoads returns addresses whose code body was fetched, mapped to
-// fetched during this StateDB's lifetime, mapped to byte length. Used by the
+// byte length. Used to deduplicate code-load events across BAL phase StateDBs.
-// BAL parallel pipeline to deduplicate code-load events across phase StateDBs.
 func (s *StateDB) SnapshotCodeLoads() map[common.Address]int {
 	if len(s.stateObjects) == 0 {
 		return nil