From 407cf11930fc7f399e4962dcddef02741aa2fef2 Mon Sep 17 00:00:00 2001 From: Sina M <1591639+s1na@users.noreply.github.com> Date: Wed, 8 Apr 2026 19:20:58 +0200 Subject: [PATCH] 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 | 25 ++++++++++++ core/state/reader_eip_7928_test.go | 64 ++++++++++++++++++++++++++++++ core/state/state_object.go | 13 +++--- core/state/statedb.go | 7 ++++ 4 files changed, 103 insertions(+), 6 deletions(-) diff --git a/core/state/reader_eip_7928.go b/core/state/reader_eip_7928.go index cff36eba3c..2f6ee478a4 100644 --- a/core/state/reader_eip_7928.go +++ b/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{}{} +} diff --git a/core/state/reader_eip_7928_test.go b/core/state/reader_eip_7928_test.go index 86641f65b1..ef67a67444 100644 --- a/core/state/reader_eip_7928_test.go +++ b/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)") + } +} diff --git a/core/state/state_object.go b/core/state/state_object.go index 6ec2777684..b83d66d3d6 100644 --- a/core/state/state_object.go +++ b/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{} diff --git a/core/state/statedb.go b/core/state/statedb.go index c89d0502e4..b8081c149a 100644 --- a/core/state/statedb.go +++ b/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.