core/state: touch BAL on statedb cache (#34684)

The BAL reader tracker captures access list reads at the reader level.
When statedb has an account cached the BAL tracker is not informed of
the access. This is ok during the lifetime of a transaction because you
only need to record the access the first time. It is also ok during the
lifetime of a block because BAL reads are block-level (same as statedb
caches).

Where I think the issue can rise is in the miner. Namely when building a
block, if the miner picks up a tx which fails, it drops it and picks up
another tx to include. There might be some edge case here where the
failed tx which is not included poisons the cache and a future block
which is included omits an account because it wasn't aware of the
access.

core/state/reader_eip_7928.go

@@ -290,6 +290,14 @@ func (r *ReaderWithBlockLevelAccessList) CodeSize(addr common.Address, codeHash
 type StateReaderTracker interface {
 	GetStateAccessList() bal.StateAccesses
 	Clear()
+
+	// TouchAccount records an account access without performing a database read.
+	// This is used to ensure cache hits in the StateDB are still tracked for BAL.
+	TouchAccount(addr common.Address)
+
+	// TouchStorage records a storage slot access without performing a database read.
+	// This is used to ensure cache hits in the StateDB are still tracked for BAL.
+	TouchStorage(addr common.Address, slot common.Hash)
 }
 
 func NewReaderWithTracker(r Reader) Reader {
@@ -337,3 +345,20 @@ func (r *readerTracker) GetStateAccessList() bal.StateAccesses {
 func (r *readerTracker) Clear() {
 	r.access = make(bal.StateAccesses)
 }
+
+// TouchAccount records an account access without performing a database read.
+func (r *readerTracker) TouchAccount(addr common.Address) {
+	if _, exists := r.access[addr]; !exists {
+		r.access[addr] = make(bal.StorageAccessList)
+	}
+}
+
+// TouchStorage records a storage slot access without performing a database read.
+func (r *readerTracker) TouchStorage(addr common.Address, slot common.Hash) {
+	list, exists := r.access[addr]
+	if !exists {
+		list = make(bal.StorageAccessList)
+		r.access[addr] = list
+	}
+	list[slot] = struct{}{}
+}

core/state/reader_eip_7928_test.go

@@ -24,8 +24,10 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/tracing"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/internal/testrand"
+	"github.com/holiman/uint256"
 )
 
 type countingStateReader struct {
@@ -199,3 +201,65 @@ func TestReaderWithTracker(t *testing.T) {
 		}
 	}
 }
+
+// TestTrackerSurvivesStateDBCache verifies that the BAL reader tracker records
+// account and storage accesses even when the StateDB serves them from its
+// in-memory cache (stateObjects / originStorage). This is a regression test
+// for a bug where a reverted transaction left cached entries that subsequent
+// transactions read without hitting the reader, causing the BAL to be incomplete.
+func TestTrackerSurvivesStateDBCache(t *testing.T) {
+	var (
+		sdb            = NewDatabaseForTesting()
+		statedb, _     = New(types.EmptyRootHash, sdb)
+		addr           = common.HexToAddress("0xaaaa")
+		slot           = common.HexToHash("0x01")
+	)
+	// Set up committed state with one account that has a storage slot.
+	statedb.SetBalance(addr, uint256.NewInt(1e18), tracing.BalanceChangeUnspecified)
+	statedb.SetNonce(addr, 5, tracing.NonceChangeUnspecified)
+	statedb.SetState(addr, slot, common.HexToHash("0x42"))
+	root, _ := statedb.Commit(0, false, false)
+	sdb.TrieDB().Commit(root, false)
+
+	// Create a fresh StateDB with a reader tracker (as the miner does).
+	var (
+		reader, _ = sdb.Reader(root)
+		tracked   = NewReaderWithTracker(reader)
+		live, _   = NewWithReader(root, sdb, tracked)
+		tracker   = live.Reader().(StateReaderTracker)
+	)
+
+	// Simulate a failed transaction: read account and storage, then revert.
+	snap := live.Snapshot()
+	live.GetNonce(addr)
+	live.GetState(addr, slot)
+
+	reads := tracker.GetStateAccessList()
+	if _, ok := reads[addr]; !ok {
+		t.Fatal("addr should be tracked after first read")
+	}
+	if _, ok := reads[addr][slot]; !ok {
+		t.Fatal("slot should be tracked after first read")
+	}
+
+	tracker.Clear()
+	live.RevertToSnapshot(snap)
+
+	reads = tracker.GetStateAccessList()
+	if len(reads) != 0 {
+		t.Fatal("tracker should be empty after Clear")
+	}
+
+	// Simulate the next transaction reading the same account and slot.
+	// Both hit the stateObjects/originStorage caches.
+	live.GetNonce(addr)
+	live.GetState(addr, slot)
+
+	reads = tracker.GetStateAccessList()
+	if _, ok := reads[addr]; !ok {
+		t.Fatal("addr must be tracked on cache hit (account)")
+	}
+	if _, ok := reads[addr][slot]; !ok {
+		t.Fatal("slot must be tracked on cache hit (storage)")
+	}
+}

core/state/state_object.go

@@ -198,6 +198,11 @@ func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
 	}
 
 	if value, cached := s.originStorage[key]; cached {
+		// EIP-7928: record the access even on cache hit so the BAL
+		// tracker is aware of it.
+		if rt, ok := s.db.reader.(StateReaderTracker); ok {
+			rt.TouchStorage(s.address, key)
+		}
 		return value
 	}
 	// If the object was destructed in *this* block (and potentially resurrected),
@@ -213,12 +218,8 @@ func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
 		//
 		// The read operation is still essential for correctly building
 		// the block-level access list.
-		//
-		// TODO(rjl493456442) the reader interface can be extended with
-		// Touch, recording the read access without the actual disk load.
-		_, err := s.db.reader.Storage(s.address, key)
-		if err != nil {
-			s.db.setError(err)
+		if rt, ok := s.db.reader.(StateReaderTracker); ok {
+			rt.TouchStorage(s.address, key)
 		}
 		s.originStorage[key] = common.Hash{} // track the empty slot as origin value
 		return common.Hash{}

core/state/statedb.go

@@ -619,6 +619,13 @@ func (s *StateDB) deleteStateObject(addr common.Address) {
 func (s *StateDB) getStateObject(addr common.Address) *stateObject {
 	// Prefer live objects if any is available
 	if obj := s.stateObjects[addr]; obj != nil {
+		// EIP-7928: record the access even on cache hit so the BAL
+		// tracker is aware of it. Without this, a cache entry left
+		// behind by a reverted transaction hides the read from the
+		// tracker, producing an incomplete block access list.
+		if rt, ok := s.reader.(StateReaderTracker); ok {
+			rt.TouchAccount(addr)
+		}
 		return obj
 	}
 	// Short circuit if the account is already destructed in this block.