erigontech / erigontech/erigon
execution/protocol: GetHashFn contains unreachable mutex protecting sequential-only code
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 465
Description
## Problem
`GetHashFn` in `execution/protocol/evm.go` creates a `sync.Mutex` and wraps every `getHeader` call in an unlock-during-IO pattern:
```go
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 design
- `exec3_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
1. Remove `hashLookupCacheLock` and all `Lock`/`Unlock` calls
2. Replace `lru.New[uint64, common.Hash](8192)` with `make(map[uint64]common.Hash, 256)`
3. Call `getHeader` directly without the IIFE unlock wrapper
4. Drop the `lru` import from this file if unused after the change
```go
// 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.
Contributor guide
Assessment
This issue has not been assessed yet.