core, triedb: remove destruct flag in state snapshot (#30752)

This pull request removes the destruct flag from the state snapshot to
simplify the code.

Previously, this flag indicated that an account was removed during a
state transition, making all associated storage slots inaccessible.
Because storage deletion can involve a large number of slots, the actual
deletion is deferred until the end of the process, where it is handled
in batches.

With the deprecation of self-destruct in the Cancun fork, storage
deletions are no longer expected. Historically, the largest storage
deletion event in Ethereum was around 15 megabytes—manageable in memory.

In this pull request, the single destruct flag is replaced by a set of
deletion markers for individual storage slots. Each deleted storage slot
will now appear in the Storage set with a nil value.

This change will simplify a lot logics, such as storage accessing,
storage flushing, storage iteration and so on.

Author: rjl493456442

core/state/snapshot/conversion.go

@@ -50,16 +50,6 @@ type (
 	leafCallbackFn func(db ethdb.KeyValueWriter, accountHash, codeHash common.Hash, stat *generateStats) (common.Hash, error)
 )
 
-// GenerateAccountTrieRoot takes an account iterator and reproduces the root hash.
-func GenerateAccountTrieRoot(it AccountIterator) (common.Hash, error) {
-	return generateTrieRoot(nil, "", it, common.Hash{}, stackTrieGenerate, nil, newGenerateStats(), true)
-}
-
-// GenerateStorageTrieRoot takes a storage iterator and reproduces the root hash.
-func GenerateStorageTrieRoot(account common.Hash, it StorageIterator) (common.Hash, error) {
-	return generateTrieRoot(nil, "", it, account, stackTrieGenerate, nil, newGenerateStats(), true)
-}
-
 // GenerateTrie takes the whole snapshot tree as the input, traverses all the
 // accounts as well as the corresponding storages and regenerate the whole state
 // (account trie + all storage tries).

core/state/snapshot/difflayer.go

@@ -30,6 +30,7 @@ import (
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/rlp"
 	bloomfilter "github.com/holiman/bloomfilter/v2"
+	"golang.org/x/exp/maps"
 )
 
 var (
@@ -73,23 +74,14 @@ var (
 	// bloom key for an account/slot. This is randomized at init(), so that the
 	// global population of nodes do not all display the exact same behaviour with
 	// regards to bloom content
-	bloomDestructHasherOffset = 0
-	bloomAccountHasherOffset  = 0
-	bloomStorageHasherOffset  = 0
+	bloomAccountHasherOffset = 0
+	bloomStorageHasherOffset = 0
 )
 
 func init() {
 	// Init the bloom offsets in the range [0:24] (requires 8 bytes)
-	bloomDestructHasherOffset = rand.Intn(25)
 	bloomAccountHasherOffset = rand.Intn(25)
 	bloomStorageHasherOffset = rand.Intn(25)
-
-	// The destruct and account blooms must be different, as the storage slots
-	// will check for destruction too for every bloom miss. It should not collide
-	// with modified accounts.
-	for bloomAccountHasherOffset == bloomDestructHasherOffset {
-		bloomAccountHasherOffset = rand.Intn(25)
-	}
 }
 
 // diffLayer represents a collection of modifications made to a state snapshot
@@ -106,29 +98,16 @@ type diffLayer struct {
 	root  common.Hash // Root hash to which this snapshot diff belongs to
 	stale atomic.Bool // Signals that the layer became stale (state progressed)
 
-	// destructSet is a very special helper marker. If an account is marked as
-	// deleted, then it's recorded in this set. However it's allowed that an account
-	// is included here but still available in other sets(e.g. storageData). The
-	// reason is the diff layer includes all the changes in a *block*. It can
-	// happen that in the tx_1, account A is self-destructed while in the tx_2
-	// it's recreated. But we still need this marker to indicate the "old" A is
-	// deleted, all data in other set belongs to the "new" A.
-	destructSet map[common.Hash]struct{}               // Keyed markers for deleted (and potentially) recreated accounts
-	accountList []common.Hash                          // List of account for iteration. If it exists, it's sorted, otherwise it's nil
 	accountData map[common.Hash][]byte                 // Keyed accounts for direct retrieval (nil means deleted)
-	storageList map[common.Hash][]common.Hash          // List of storage slots for iterated retrievals, one per account. Any existing lists are sorted if non-nil
 	storageData map[common.Hash]map[common.Hash][]byte // Keyed storage slots for direct retrieval. one per account (nil means deleted)
+	accountList []common.Hash                          // List of account for iteration. If it exists, it's sorted, otherwise it's nil
+	storageList map[common.Hash][]common.Hash          // List of storage slots for iterated retrievals, one per account. Any existing lists are sorted if non-nil
 
 	diffed *bloomfilter.Filter // Bloom filter tracking all the diffed items up to the disk layer
 
 	lock sync.RWMutex
 }
 
-// destructBloomHash is used to convert a destruct event into a 64 bit mini hash.
-func destructBloomHash(h common.Hash) uint64 {
-	return binary.BigEndian.Uint64(h[bloomDestructHasherOffset : bloomDestructHasherOffset+8])
-}
-
 // accountBloomHash is used to convert an account hash into a 64 bit mini hash.
 func accountBloomHash(h common.Hash) uint64 {
 	return binary.BigEndian.Uint64(h[bloomAccountHasherOffset : bloomAccountHasherOffset+8])
@@ -142,12 +121,11 @@ func storageBloomHash(h0, h1 common.Hash) uint64 {
 
 // newDiffLayer creates a new diff on top of an existing snapshot, whether that's a low
 // level persistent database or a hierarchical diff already.
-func newDiffLayer(parent snapshot, root common.Hash, destructs map[common.Hash]struct{}, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer {
+func newDiffLayer(parent snapshot, root common.Hash, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer {
 	// Create the new layer with some pre-allocated data segments
 	dl := &diffLayer{
 		parent:      parent,
 		root:        root,
-		destructSet: destructs,
 		accountData: accounts,
 		storageData: storage,
 		storageList: make(map[common.Hash][]common.Hash),
@@ -161,10 +139,7 @@ func newDiffLayer(parent snapshot, root common.Hash, destructs map[common.Hash]s
 		panic("unknown parent type")
 	}
 	// Sanity check that accounts or storage slots are never nil
-	for accountHash, blob := range accounts {
-		if blob == nil {
-			panic(fmt.Sprintf("account %#x nil", accountHash))
-		}
+	for _, blob := range accounts {
 		// Determine memory size and track the dirty writes
 		dl.memory += uint64(common.HashLength + len(blob))
 		snapshotDirtyAccountWriteMeter.Mark(int64(len(blob)))
@@ -179,7 +154,6 @@ func newDiffLayer(parent snapshot, root common.Hash, destructs map[common.Hash]s
 			snapshotDirtyStorageWriteMeter.Mark(int64(len(data)))
 		}
 	}
-	dl.memory += uint64(len(destructs) * common.HashLength)
 	return dl
 }
 
@@ -204,10 +178,6 @@ func (dl *diffLayer) rebloom(origin *diskLayer) {
 	} else {
 		dl.diffed, _ = bloomfilter.New(uint64(bloomSize), uint64(bloomFuncs))
 	}
-	// Iterate over all the accounts and storage slots and index them
-	for hash := range dl.destructSet {
-		dl.diffed.AddHash(destructBloomHash(hash))
-	}
 	for hash := range dl.accountData {
 		dl.diffed.AddHash(accountBloomHash(hash))
 	}
@@ -274,11 +244,8 @@ func (dl *diffLayer) AccountRLP(hash common.Hash) ([]byte, error) {
 	}
 	// Check the bloom filter first whether there's even a point in reaching into
 	// all the maps in all the layers below
-	hit := dl.diffed.ContainsHash(accountBloomHash(hash))
-	if !hit {
-		hit = dl.diffed.ContainsHash(destructBloomHash(hash))
-	}
 	var origin *diskLayer
+	hit := dl.diffed.ContainsHash(accountBloomHash(hash))
 	if !hit {
 		origin = dl.origin // extract origin while holding the lock
 	}
@@ -310,18 +277,14 @@ func (dl *diffLayer) accountRLP(hash common.Hash, depth int) ([]byte, error) {
 	if data, ok := dl.accountData[hash]; ok {
 		snapshotDirtyAccountHitMeter.Mark(1)
 		snapshotDirtyAccountHitDepthHist.Update(int64(depth))
-		snapshotDirtyAccountReadMeter.Mark(int64(len(data)))
+		if n := len(data); n > 0 {
+			snapshotDirtyAccountReadMeter.Mark(int64(n))
+		} else {
+			snapshotDirtyAccountInexMeter.Mark(1)
+		}
 		snapshotBloomAccountTrueHitMeter.Mark(1)
 		return data, nil
 	}
-	// If the account is known locally, but deleted, return it
-	if _, ok := dl.destructSet[hash]; ok {
-		snapshotDirtyAccountHitMeter.Mark(1)
-		snapshotDirtyAccountHitDepthHist.Update(int64(depth))
-		snapshotDirtyAccountInexMeter.Mark(1)
-		snapshotBloomAccountTrueHitMeter.Mark(1)
-		return nil, nil
-	}
 	// Account unknown to this diff, resolve from parent
 	if diff, ok := dl.parent.(*diffLayer); ok {
 		return diff.accountRLP(hash, depth+1)
@@ -345,11 +308,8 @@ func (dl *diffLayer) Storage(accountHash, storageHash common.Hash) ([]byte, erro
 		dl.lock.RUnlock()
 		return nil, ErrSnapshotStale
 	}
-	hit := dl.diffed.ContainsHash(storageBloomHash(accountHash, storageHash))
-	if !hit {
-		hit = dl.diffed.ContainsHash(destructBloomHash(accountHash))
-	}
 	var origin *diskLayer
+	hit := dl.diffed.ContainsHash(storageBloomHash(accountHash, storageHash))
 	if !hit {
 		origin = dl.origin // extract origin while holding the lock
 	}
@@ -391,14 +351,6 @@ func (dl *diffLayer) storage(accountHash, storageHash common.Hash, depth int) ([
 			return data, nil
 		}
 	}
-	// If the account is known locally, but deleted, return an empty slot
-	if _, ok := dl.destructSet[accountHash]; ok {
-		snapshotDirtyStorageHitMeter.Mark(1)
-		snapshotDirtyStorageHitDepthHist.Update(int64(depth))
-		snapshotDirtyStorageInexMeter.Mark(1)
-		snapshotBloomStorageTrueHitMeter.Mark(1)
-		return nil, nil
-	}
 	// Storage slot unknown to this diff, resolve from parent
 	if diff, ok := dl.parent.(*diffLayer); ok {
 		return diff.storage(accountHash, storageHash, depth+1)
@@ -410,8 +362,8 @@ func (dl *diffLayer) storage(accountHash, storageHash common.Hash, depth int) ([
 
 // Update creates a new layer on top of the existing snapshot diff tree with
 // the specified data items.
-func (dl *diffLayer) Update(blockRoot common.Hash, destructs map[common.Hash]struct{}, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer {
-	return newDiffLayer(dl, blockRoot, destructs, accounts, storage)
+func (dl *diffLayer) Update(blockRoot common.Hash, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer {
+	return newDiffLayer(dl, blockRoot, accounts, storage)
 }
 
 // flatten pushes all data from this point downwards, flattening everything into
@@ -436,12 +388,6 @@ func (dl *diffLayer) flatten() snapshot {
 	if parent.stale.Swap(true) {
 		panic("parent diff layer is stale") // we've flattened into the same parent from two children, boo
 	}
-	// Overwrite all the updated accounts blindly, merge the sorted list
-	for hash := range dl.destructSet {
-		parent.destructSet[hash] = struct{}{}
-		delete(parent.accountData, hash)
-		delete(parent.storageData, hash)
-	}
 	for hash, data := range dl.accountData {
 		parent.accountData[hash] = data
 	}
@@ -453,17 +399,13 @@ func (dl *diffLayer) flatten() snapshot {
 			continue
 		}
 		// Storage exists in both parent and child, merge the slots
-		comboData := parent.storageData[accountHash]
-		for storageHash, data := range storage {
-			comboData[storageHash] = data
-		}
+		maps.Copy(parent.storageData[accountHash], storage)
 	}
 	// Return the combo parent
 	return &diffLayer{
 		parent:      parent.parent,
 		origin:      parent.origin,
 		root:        dl.root,
-		destructSet: parent.destructSet,
 		accountData: parent.accountData,
 		storageData: parent.storageData,
 		storageList: make(map[common.Hash][]common.Hash),
@@ -489,15 +431,7 @@ func (dl *diffLayer) AccountList() []common.Hash {
 	dl.lock.Lock()
 	defer dl.lock.Unlock()
 
-	dl.accountList = make([]common.Hash, 0, len(dl.destructSet)+len(dl.accountData))
-	for hash := range dl.accountData {
-		dl.accountList = append(dl.accountList, hash)
-	}
-	for hash := range dl.destructSet {
-		if _, ok := dl.accountData[hash]; !ok {
-			dl.accountList = append(dl.accountList, hash)
-		}
-	}
+	dl.accountList = maps.Keys(dl.accountData)
 	slices.SortFunc(dl.accountList, common.Hash.Cmp)
 	dl.memory += uint64(len(dl.accountList) * common.HashLength)
 	return dl.accountList
@@ -512,32 +446,27 @@ func (dl *diffLayer) AccountList() []common.Hash {
 // not empty but the flag is true.
 //
 // Note, the returned slice is not a copy, so do not modify it.
-func (dl *diffLayer) StorageList(accountHash common.Hash) ([]common.Hash, bool) {
+func (dl *diffLayer) StorageList(accountHash common.Hash) []common.Hash {
 	dl.lock.RLock()
-	_, destructed := dl.destructSet[accountHash]
 	if _, ok := dl.storageData[accountHash]; !ok {
 		// Account not tracked by this layer
 		dl.lock.RUnlock()
-		return nil, destructed
+		return nil
 	}
 	// If an old list already exists, return it
 	if list, exist := dl.storageList[accountHash]; exist {
 		dl.lock.RUnlock()
-		return list, destructed // the cached list can't be nil
+		return list // the cached list can't be nil
 	}
 	dl.lock.RUnlock()
 
 	// No old sorted account list exists, generate a new one
 	dl.lock.Lock()
 	defer dl.lock.Unlock()
 
-	storageMap := dl.storageData[accountHash]
-	storageList := make([]common.Hash, 0, len(storageMap))
-	for k := range storageMap {
-		storageList = append(storageList, k)
-	}
+	storageList := maps.Keys(dl.storageData[accountHash])
 	slices.SortFunc(storageList, common.Hash.Cmp)
 	dl.storageList[accountHash] = storageList
 	dl.memory += uint64(len(dl.storageList)*common.HashLength + common.HashLength)
-	return storageList, destructed
+	return storageList
 }