common/lru, core/vm: count key bytes against the precompile cache budget (#35526)
Some checks are pending
/ Linux Build (push) Waiting to run
/ Linux Build (arm) (push) Waiting to run
/ Keeper Build (push) Waiting to run
/ Windows Build (push) Waiting to run
/ Docker Image (push) Waiting to run

#35473 keys the precompile cache on the input, but SizeConstrainedCache
only counts the value’s bytes, so a cacheable empty output (e.g. a
failed ECRECOVER) is never evicted and grows the cache without bound.

Skipping empty outputs alone would still leave sha256/ripemd160 ~256x
over budget, and would only hold as long as every failing output happens
to be empty.
This commit is contained in:
0xSHKWON 2026-08-14 08:17:00 +09:00 committed by GitHub
parent aa1f2fcf51
commit 6bb0588ad8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 130 additions and 5 deletions

View file

@ -30,24 +30,49 @@ type blobType interface {
// is at capacity, and a new item is added, older items are evicted until the size
// constraint is met.
//
// By default only the value bytes count towards the size constraint. If the key
// carries a meaningful share of an entry's memory, pass a key-size function to
// NewSizeConstrainedCacheWithKeySize so it is accounted too.
//
// OBS: This cache assumes that items are content-addressed: keys are unique per content.
// In other words: two Add(..) with the same key K, will always have the same value V.
type SizeConstrainedCache[K comparable, V blobType] struct {
size uint64
maxSize uint64
keySize func(K) uint64
lru BasicLRU[K, V]
lock sync.Mutex
}
// NewSizeConstrainedCache creates a new size-constrained LRU cache.
// NewSizeConstrainedCache creates a new size-constrained LRU cache in which only
// the value bytes count towards the size constraint.
func NewSizeConstrainedCache[K comparable, V blobType](maxSize uint64) *SizeConstrainedCache[K, V] {
return NewSizeConstrainedCacheWithKeySize[K, V](maxSize, nil)
}
// NewSizeConstrainedCacheWithKeySize is like NewSizeConstrainedCache but also
// charges each entry's key against the size constraint, measured by keySize.
// This matters when the key, not the value, holds the bulk of an entry's memory.
// A nil keySize counts keys as free, identical to NewSizeConstrainedCache.
func NewSizeConstrainedCacheWithKeySize[K comparable, V blobType](maxSize uint64, keySize func(K) uint64) *SizeConstrainedCache[K, V] {
return &SizeConstrainedCache[K, V]{
size: 0,
maxSize: maxSize,
keySize: keySize,
lru: NewBasicLRU[K, V](math.MaxInt),
}
}
// entrySize reports the number of bytes an entry counts for against the size
// constraint: the value length, plus the key size when a measure was provided.
func (c *SizeConstrainedCache[K, V]) entrySize(key K, value V) uint64 {
n := uint64(len(value))
if c.keySize != nil {
n += c.keySize(key)
}
return n
}
// Add adds a value to the cache. Returns true if an eviction occurred.
// OBS: This cache assumes that items are content-addressed: keys are unique per content.
// In other words: two Add(..) with the same key K, will always have the same value V.
@ -59,15 +84,15 @@ func (c *SizeConstrainedCache[K, V]) Add(key K, value V) (evicted bool) {
// Unless it is already present, might need to evict something.
// OBS: If it is present, we still call Add internally to bump the recentness.
if !c.lru.Contains(key) {
targetSize := c.size + uint64(len(value))
targetSize := c.size + c.entrySize(key, value)
for targetSize > c.maxSize {
evicted = true
_, v, ok := c.lru.RemoveOldest()
k, v, ok := c.lru.RemoveOldest()
if !ok {
// list is now empty. Break
break
}
targetSize -= uint64(len(v))
targetSize -= c.entrySize(k, v)
}
c.size = targetSize
}

View file

@ -153,3 +153,54 @@ func TestSizeConstrainedCacheEmpties(t *testing.T) {
}
}
}
// TestSizeConstrainedCacheCountsKeys verifies that a cache constructed with a
// key-size function charges the key against the byte budget, so entries whose
// bytes live in the key (empty or tiny values) can no longer grow it without
// bound.
func TestSizeConstrainedCacheCountsKeys(t *testing.T) {
const (
maxSize = 64 * 1024
keyLen = 256
)
c := NewSizeConstrainedCacheWithKeySize[string, []byte](maxSize, func(k string) uint64 { return uint64(len(k)) })
// Add far more distinct keyLen-byte keys with EMPTY values than the budget
// can hold. Without key accounting the size would stay 0 and nothing would
// ever be evicted.
const n = 10000
for i := 0; i < n; i++ {
var key [keyLen]byte
binary.BigEndian.PutUint64(key[:8], uint64(i))
c.Add(string(key[:]), nil)
}
if c.size > maxSize {
t.Fatalf("reported size %d exceeds budget %d", c.size, maxSize)
}
if c.lru.Len() >= n {
t.Fatalf("no eviction happened: kept all %d empty-value entries", n)
}
// The earliest entry (all-zero key) must have been evicted.
var first [keyLen]byte
if _, ok := c.Get(string(first[:])); ok {
t.Fatal("oldest empty-value entry was never evicted")
}
}
// TestSizeConstrainedCacheKeysFreeByDefault pins the default behaviour: the
// plain constructor does not count keys, so existing callers are unaffected.
func TestSizeConstrainedCacheKeysFreeByDefault(t *testing.T) {
c := NewSizeConstrainedCache[string, []byte](1024)
const n = 1000
for i := 0; i < n; i++ {
var key [512]byte
binary.BigEndian.PutUint64(key[:8], uint64(i))
c.Add(string(key[:]), nil) // empty value -> size stays 0
}
if c.size != 0 {
t.Fatalf("expected size 0 with keys free, got %d", c.size)
}
if c.lru.Len() != n {
t.Fatalf("expected all %d entries retained, got %d", n, c.lru.Len())
}
}

View file

@ -149,7 +149,11 @@ func (c *PrecompileCache) store(scope precompileCacheScope, key []byte, output [
if results == nil {
c.data.mu.Lock()
if results = c.data.caches[scope]; results == nil {
results = lru.NewSizeConstrainedCache[string, []byte](maxCacheablePrecompileBytes)
// The key is the precompile input, which dwarfs the output (up to
// maxCacheablePrecompileInput against maxCacheablePrecompileOutput),
// so it must count towards the budget. Without it an empty output
// (e.g. a failed ECRECOVER) would let the cache grow without bound.
results = lru.NewSizeConstrainedCacheWithKeySize[string, []byte](maxCacheablePrecompileBytes, func(k string) uint64 { return uint64(len(k)) })
c.data.caches[scope] = results
}
c.data.mu.Unlock()

View file

@ -18,6 +18,7 @@ package vm
import (
"bytes"
"encoding/binary"
"encoding/hex"
"encoding/json"
"fmt"
@ -431,3 +432,47 @@ func BenchmarkPrecompileCacheSynthetic(b *testing.B) {
}
}
}
// TestPrecompileCacheBoundedByKeyBytes checks that the precompile result cache
// stays within its byte budget even when its entries carry empty outputs, such
// as a failed ECRECOVER. The cache key is the precompile input, so it must
// count towards the budget; otherwise a stream of distinct failing inputs would
// grow the cache without bound, since an empty output adds nothing to the size.
func TestPrecompileCacheBoundedByKeyBytes(t *testing.T) {
cache := NewPrecompileCache()
scope := precompileCacheScope{activePrecompiledContracts(params.Rules{}), common.BytesToAddress([]byte{0x1})}
ec := &ecrecover{}
// A failing signature runs to an empty output with no error, and is still
// eligible for caching. Keep its key as the eviction witness.
var first [128]byte
first[63] = 0xff // invalid v
out, err := ec.Run(first[:])
if err != nil || len(out) != 0 {
t.Fatalf("expected empty failed-ecrecover output, got len=%d err=%v", len(out), err)
}
firstKey, ok := precompileCacheKey(ec, first[:])
if !ok {
t.Fatal("failed ecrecover unexpectedly not cacheable")
}
cache.store(scope, firstKey, out)
// Store many more distinct failing inputs. A trailing sentinel byte keeps
// every normalized key a full 128 bytes, so their key bytes alone overrun
// the 1 MiB budget several times over.
const n = 20000
for i := 1; i < n; i++ {
var in [128]byte
in[63] = 0xff
in[127] = 0x01
binary.BigEndian.PutUint64(in[64:72], uint64(i))
key, ok := precompileCacheKey(ec, in[:])
if !ok {
t.Fatalf("input %d not cacheable", i)
}
cache.store(scope, key, nil)
}
if _, hit := cache.load(scope, firstKey); hit {
t.Fatal("cache is unbounded: oldest empty-output entry was never evicted")
}
}