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.