Skip to content

Commit

Permalink
core/state/snapshot: check difflayer staleness early (#27255) (#383)
Browse files Browse the repository at this point in the history
This PR adds a staleness-check to AccountRLP, before checking the bloom-filter and potentially going directly into the disklayer.

---------

Co-authored-by: Martin Holst Swende <martin@swende.se>
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
  • Loading branch information
3 people authored Nov 23, 2023
1 parent 84eaf51 commit f4a8cdb
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
12 changes: 11 additions & 1 deletion core/state/snapshot/difflayer.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,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))
Expand Down Expand Up @@ -358,6 +363,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))
Expand Down
4 changes: 4 additions & 0 deletions core/state/snapshot/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,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(),
Expand Down

0 comments on commit f4a8cdb

Please sign in to comment.