From eb83e7c54021573eaceb14236af3a7a8c64f6027 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 16 May 2023 09:18:39 -0400 Subject: [PATCH] core/state/snapshot: check difflayer staleness early (#27255) This PR adds a staleness-check to AccountRLP, before checking the bloom-filter and potentially going directly into the disklayer. --------- Co-authored-by: rjl493456442 --- core/state/snapshot/difflayer.go | 12 +++++++++++- core/state/snapshot/snapshot_test.go | 4 ++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/core/state/snapshot/difflayer.go b/core/state/snapshot/difflayer.go index 4701acccd3..203d66f93d 100644 --- a/core/state/snapshot/difflayer.go +++ b/core/state/snapshot/difflayer.go @@ -292,9 +292,14 @@ func (dl *diffLayer) Account(hash common.Hash) (*Account, error) { // // Note the returned account is not a copy, please don't modify it. func (dl *diffLayer) AccountRLP(hash common.Hash) ([]byte, error) { + dl.lock.RLock() + // Check staleness before reaching further. + if dl.Stale() { + dl.lock.RUnlock() + return nil, ErrSnapshotStale + } // Check the bloom filter first whether there's even a point in reaching into // all the maps in all the layers below - dl.lock.RLock() hit := dl.diffed.Contains(accountBloomHasher(hash)) if !hit { hit = dl.diffed.Contains(destructBloomHasher(hash)) @@ -361,6 +366,11 @@ func (dl *diffLayer) Storage(accountHash, storageHash common.Hash) ([]byte, erro // Check the bloom filter first whether there's even a point in reaching into // all the maps in all the layers below dl.lock.RLock() + // Check staleness before reaching further. + if dl.Stale() { + dl.lock.RUnlock() + return nil, ErrSnapshotStale + } hit := dl.diffed.Contains(storageBloomHasher{accountHash, storageHash}) if !hit { hit = dl.diffed.Contains(destructBloomHasher(accountHash)) diff --git a/core/state/snapshot/snapshot_test.go b/core/state/snapshot/snapshot_test.go index 249da10aa6..038bd6df62 100644 --- a/core/state/snapshot/snapshot_test.go +++ b/core/state/snapshot/snapshot_test.go @@ -185,6 +185,10 @@ func TestDiskLayerExternalInvalidationPartialFlatten(t *testing.T) { // be returned with junk data. This version of the test retains the bottom diff // layer to check the usual mode of operation where the accumulator is retained. func TestDiffLayerExternalInvalidationPartialFlatten(t *testing.T) { + // Un-commenting this triggers the bloom set to be deterministic. The values below + // were used to trigger the flaw described in https://github.com/ethereum/go-ethereum/issues/27254. + // bloomDestructHasherOffset, bloomAccountHasherOffset, bloomStorageHasherOffset = 14, 24, 5 + // Create an empty base layer and a snapshot tree out of it base := &diskLayer{ diskdb: rawdb.NewMemoryDatabase(),