Merge pull request #581 from OffchainLabs/Fix-RecentWasms-cache-bug-by-using-pointers-in-methods

core: Fix RecentWasms cache bug by using pointers in methods

Author: eljobe

core/state/statedb_arbitrum.go

@@ -330,8 +330,8 @@ func (s *StateDB) RecordEvictWasm(wasm EvictWasm) {
 	s.journal.entries = append(s.journal.entries, wasm)
 }
 
-func (s *StateDB) GetRecentWasms() RecentWasms {
-	return s.arbExtraData.recentWasms
+func (s *StateDB) GetRecentWasms() *RecentWasms {
+	return &s.arbExtraData.recentWasms
 }
 
 // Type for managing recent program access.
@@ -346,7 +346,7 @@ func NewRecentWasms() RecentWasms {
 }
 
 // Inserts a new item, returning true if already present.
-func (p RecentWasms) Insert(item common.Hash, retain uint16) bool {
+func (p *RecentWasms) Insert(item common.Hash, retain uint16) bool {
 	if p.cache == nil {
 		cache := lru.NewBasicLRU[common.Hash, struct{}](int(retain))
 		p.cache = &cache

core/state/statedb_arbitrum_test.go

@@ -0,0 +1,51 @@
+package state
+
+import (
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/types"
+)
+
+func TestRecentWasmsInsertAndCopy(t *testing.T) {
+	db := NewDatabaseForTesting()
+	state, err := New(types.EmptyRootHash, db)
+	if err != nil {
+		t.Fatalf("failed to create state: %v", err)
+	}
+
+	const retain = uint16(8)
+
+	hash1 := common.HexToHash("0x01")
+	hash2 := common.HexToHash("0x02")
+	hash3 := common.HexToHash("0x03")
+
+	if hit := state.GetRecentWasms().Insert(hash1, retain); hit {
+		t.Fatalf("first insert of hash1 should be a miss")
+	}
+
+	if hit := state.GetRecentWasms().Insert(hash1, retain); !hit {
+		t.Fatalf("second insert of hash1 should be a hit (cache not persisting)")
+	}
+
+	if hit := state.GetRecentWasms().Insert(hash2, retain); hit {
+		t.Fatalf("first insert of hash2 should be a miss")
+	}
+
+	copy := state.Copy()
+
+	if hit := copy.GetRecentWasms().Insert(hash1, retain); !hit {
+		t.Fatalf("copy: expected hit for hash1 present before copy")
+	}
+	if hit := copy.GetRecentWasms().Insert(hash2, retain); !hit {
+		t.Fatalf("copy: expected hit for hash2 present before copy")
+	}
+
+	if hit := copy.GetRecentWasms().Insert(hash3, retain); hit {
+		t.Fatalf("copy: first insert of hash3 should be a miss")
+	}
+
+	if hit := state.GetRecentWasms().Insert(hash3, retain); hit {
+		t.Fatalf("original: first insert of hash3 should be a miss (must be independent of copy)")
+	}
+}

core/state/statedb_hooked.go

@@ -321,7 +321,7 @@ func (s *hookedStateDB) RecordEvictWasm(wasm EvictWasm) {
 	s.inner.RecordEvictWasm(wasm)
 }
 
-func (s *hookedStateDB) GetRecentWasms() RecentWasms {
+func (s *hookedStateDB) GetRecentWasms() *RecentWasms {
 	return s.inner.GetRecentWasms()
 }
 

core/vm/interface.go

@@ -38,7 +38,7 @@ type StateDB interface {
 	ActivatedAsmMap(targets []rawdb.WasmTarget, moduleHash common.Hash) (asmMap map[rawdb.WasmTarget][]byte, missingTargets []rawdb.WasmTarget, err error)
 	RecordCacheWasm(wasm state.CacheWasm)
 	RecordEvictWasm(wasm state.EvictWasm)
-	GetRecentWasms() state.RecentWasms
+	GetRecentWasms() *state.RecentWasms
 
 	// Arbitrum: track stylus's memory footprint
 	GetStylusPages() (uint16, uint16)