Problem
GetHashFn in execution/protocol/evm.go creates a sync.Mutex and wraps every getHeader call in an unlock-during-IO pattern:
hashLookupCache, _ := lru.New[uint64, common.Hash](8192)
hashLookupCacheLock := sync.Mutex{}
// ...inside the returned closure:
header, err := func() (*types.Header, error) {
hash, num := lastKnownHash, lastKnownNumber
hashLookupCacheLock.Unlock() // release during I/O
defer hashLookupCacheLock.Lock()
return getHeader(hash, num)
}()The mutex is unreachable in practice. All callers execute transactions sequentially:
ExecuteBlockEphemerally iterates block.Transactions() in a plain for-loop (block_exec.go:128)exec3_serial.go: single-threaded by designexec3_parallel.go: each worker goroutine owns its own closure instance — the closure is never shared across goroutines
The closure is created once per block and reused across all transactions in that block, so the walk-back state (lastKnownNumber, lastKnownHash) is shared intra-block but never concurrent.
Additionally, the LRU is sized at 8192 entries, but the EVM BLOCKHASH opcode can only look back 256 blocks (EIP-210). The cache will never hold more than 256 entries, making the extra capacity wasted overhead.
Impact
- Dead synchronization code that makes the function harder to understand and audit
- Future contributors may assume concurrency is possible here and add more synchronization, compounding complexity
sync.Mutex lock/unlock on every getHeader call adds noise to profiling and tracing- LRU eviction bookkeeping on a cache that cannot fill beyond 256 entries
Proposed Fix
- Remove
hashLookupCacheLock and all Lock/Unlock calls
- Replace
lru.New[uint64, common.Hash](8192) with make(map[uint64]common.Hash, 256)
- Call
getHeader directly without the IIFE unlock wrapper
- Drop the
lru import from this file if unused after the change
// After: simple sequential walk-back with a plain map cache
hashLookupCache := make(map[uint64]common.Hash, 256)
hashLookupCache[refNumber] = refHash
return func(n uint64) (common.Hash, error) {
if n > refNumber {
lastKnownNumber = refNumber
lastKnownHash = refHash
}
if hash, ok := hashLookupCache[n]; ok {
return hash, nil
}
for lastKnownNumber != n {
if n > lastKnownNumber {
lastKnownNumber = refNumber
lastKnownHash = refHash
}
header, err := getHeader(lastKnownHash, lastKnownNumber)
if err != nil || header == nil {
return common.Hash{}, nil
}
lastKnownHash = header.ParentHash
lastKnownNumber = header.Number.Uint64() - 1
hashLookupCache[lastKnownNumber] = lastKnownHash
}
return lastKnownHash, nil
}Result: identical behaviour, roughly half the current line count, no synchronization primitives.
Problem
GetHashFninexecution/protocol/evm.gocreates async.Mutexand wraps everygetHeadercall in an unlock-during-IO pattern:The mutex is unreachable in practice. All callers execute transactions sequentially:
ExecuteBlockEphemerallyiteratesblock.Transactions()in a plain for-loop (block_exec.go:128)exec3_serial.go: single-threaded by designexec3_parallel.go: each worker goroutine owns its own closure instance — the closure is never shared across goroutinesThe closure is created once per block and reused across all transactions in that block, so the walk-back state (
lastKnownNumber,lastKnownHash) is shared intra-block but never concurrent.Additionally, the LRU is sized at 8192 entries, but the EVM
BLOCKHASHopcode can only look back 256 blocks (EIP-210). The cache will never hold more than 256 entries, making the extra capacity wasted overhead.Impact
sync.Mutexlock/unlock on everygetHeadercall adds noise to profiling and tracingProposed Fix
hashLookupCacheLockand allLock/Unlockcallslru.New[uint64, common.Hash](8192)withmake(map[uint64]common.Hash, 256)getHeaderdirectly without the IIFE unlock wrapperlruimport from this file if unused after the changeResult: identical behaviour, roughly half the current line count, no synchronization primitives.