triedb/pathdb: polish code

triedb/pathdb/database.go

@@ -255,8 +255,9 @@ func New(diskdb ethdb.Database, config *Config, isVerkle bool) *Database {
 	// mandatory. This ensures that uncovered flat states are not accessed,
 	// even if background generation is not allowed. If permitted, the generation
 	// might be scheduled.
-	db.setStateGenerator()
-
+	if err := db.setStateGenerator(); err != nil {
+		log.Crit("Failed to setup the generator", "err", err)
+	}
 	fields := config.fields()
 	if db.isVerkle {
 		fields = append(fields, "verkle", true)
@@ -316,10 +317,13 @@ func (db *Database) repairHistory() error {
 
 // setStateGenerator loads the state generation progress marker and potentially
 // resume the state generation if it's permitted.
-func (db *Database) setStateGenerator() {
+func (db *Database) setStateGenerator() error {
 	// Load the state snapshot generation progress marker to prevent access
 	// to uncovered states.
-	generator, root := loadGenerator(db.diskdb)
+	generator, root, err := loadGenerator(db.diskdb, db.hasher)
+	if err != nil {
+		return err
+	}
 	if generator == nil {
 		// Initialize an empty generator to rebuild the state snapshot from scratch
 		generator = &journalGenerator{
@@ -330,7 +334,7 @@ func (db *Database) setStateGenerator() {
 	// The generator will be left as nil in disk layer for representing the whole
 	// state snapshot is available for accessing.
 	if generator.Done {
-		return
+		return nil
 	}
 	var origin uint64
 	if len(generator.Marker) >= 8 {
@@ -345,18 +349,24 @@ func (db *Database) setStateGenerator() {
 	}
 	dl := db.tree.bottom()
 
+	// Disable the background snapshot building in these circumstances:
+	// - the database is opened in read only mode
+	// - the snapshot build is explicitly disabled
+	// - the database is opened in verkle tree mode
+	noBuild := db.readOnly || db.config.SnapshotNoBuild || db.isVerkle
+
 	// Construct the generator and link it to the disk layer, ensuring that the
 	// generation progress is resolved to prevent accessing uncovered states
 	// regardless of whether background state snapshot generation is allowed.
-	noBuild := db.readOnly || db.config.SnapshotNoBuild || db.isVerkle
 	dl.setGenerator(newGenerator(db.diskdb, noBuild, generator.Marker, stats))
 
 	// Short circuit if the background generation is not permitted
 	if noBuild || db.waitSync {
-		return
+		return nil
 	}
 	stats.log("Starting snapshot generation", root, generator.Marker)
 	dl.generator.run(root)
+	return nil
 }
 
 // Update adds a new layer into the tree, if that can be linked to an existing

triedb/pathdb/disklayer.go

@@ -33,8 +33,13 @@ type diskLayer struct {
 	root common.Hash // Immutable, root hash to which this layer was made for
 	id   uint64      // Immutable, corresponding state id
 	db   *Database   // Path-based trie database
+
+	// These two caches must be maintained separately, because the key
+	// for the root node of the storage trie (accountHash) is identical
+	// to the key for the account data.
 	nodes  *fastcache.Cache // GC friendly memory cache of clean nodes
 	states *fastcache.Cache // GC friendly memory cache of clean states
+
 	buffer *buffer      // Dirty buffer to aggregate writes of nodes and states
 	stale  bool         // Signals that the layer became stale (state progressed)
 	lock   sync.RWMutex // Lock used to protect stale flag and genMarker

triedb/pathdb/flush.go

@@ -80,9 +80,10 @@ func writeStates(batch ethdb.Batch, genMarker []byte, accountData map[common.Has
 		slots    int
 	)
 	for addrHash, blob := range accountData {
-		// Skip any account not covered yet by the snapshot. The account
-		// at generation position (addrHash == genMarker[:common.HashLength])
-		// should be updated.
+		// Skip any account not yet covered by the snapshot. The account
+		// at the generation marker position (addrHash == genMarker[:common.HashLength])
+		// should still be updated, as it would be skipped in the next
+		// generation cycle.
 		if genMarker != nil && bytes.Compare(addrHash[:], genMarker) > 0 {
 			continue
 		}
@@ -107,9 +108,10 @@ func writeStates(batch ethdb.Batch, genMarker []byte, accountData map[common.Has
 		midAccount := genMarker != nil && bytes.Equal(addrHash[:], genMarker[:common.HashLength])
 
 		for storageHash, blob := range storages {
-			// Skip any slot not covered yet by the snapshot. The storage slot
-			// at generation position (addrHash == genMarker[:common.HashLength]
-			// and storageHash == genMarker[common.HashLength:]) should be updated.
+			// Skip any storage slot not yet covered by the snapshot. The storage slot
+			// at the generation marker position (addrHash == genMarker[:common.HashLength]
+			// and storageHash == genMarker[common.HashLength:]) should still be updated,
+			// as it would be skipped in the next generation cycle.
 			if midAccount && bytes.Compare(storageHash[:], genMarker[common.HashLength:]) > 0 {
 				continue
 			}

triedb/pathdb/journal.go

@@ -26,7 +26,6 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/rawdb"
 	"github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/ethdb"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/rlp"
@@ -106,36 +105,40 @@ type journalGenerator struct {
 }
 
 // loadGenerator loads the state generation progress marker from the database.
-func loadGenerator(db ethdb.KeyValueReader) (*journalGenerator, common.Hash) {
-	trieRoot := types.EmptyRootHash
-	if blob := rawdb.ReadAccountTrieNode(db, nil); len(blob) > 0 {
-		trieRoot = crypto.Keccak256Hash(blob)
+func loadGenerator(db ethdb.KeyValueReader, hash nodeHasher) (*journalGenerator, common.Hash, error) {
+	trieRoot, err := hash(rawdb.ReadAccountTrieNode(db, nil))
+	if err != nil {
+		return nil, common.Hash{}, err
 	}
 	// State generation progress marker is lost, rebuild it
 	blob := rawdb.ReadSnapshotGenerator(db)
 	if len(blob) == 0 {
 		log.Info("State snapshot generator is not found")
-		return nil, trieRoot
+		return nil, trieRoot, nil
 	}
 	// State generation progress marker is not compatible, rebuild it
 	var generator journalGenerator
 	if err := rlp.DecodeBytes(blob, &generator); err != nil {
 		log.Info("State snapshot generator is not compatible")
-		return nil, trieRoot
+		return nil, trieRoot, nil
 	}
-	// The state snapshot is inconsistent with the trie data and needs to be rebuilt.
-	// Note: The SnapshotRoot and SnapshotGenerator are always consistent with each
-	// other, no matter in the legacy state snapshot or the path database.
+	// The state snapshot is inconsistent with the trie data and must
+	// be rebuilt.
+	//
+	// Note: The SnapshotRoot and SnapshotGenerator are always consistent
+	// with each other, both in the legacy state snapshot and the path database.
+	// Therefore, if the SnapshotRoot does not match the trie root,
+	// the entire generator is considered stale and must be discarded.
 	stateRoot := rawdb.ReadSnapshotRoot(db)
 	if trieRoot != stateRoot {
-		log.Info("State snapshot is not consistent with trie", "trie", trieRoot, "state", stateRoot)
-		return nil, trieRoot
+		log.Info("State snapshot is not consistent", "trie", trieRoot, "state", stateRoot)
+		return nil, trieRoot, nil
 	}
 	// Slice null-ness is lost after rlp decoding, reset it back to empty
 	if !generator.Done && generator.Marker == nil {
 		generator.Marker = []byte{}
 	}
-	return &generator, trieRoot
+	return &generator, trieRoot, nil
 }
 
 // loadLayers loads a pre-existing state layer backed by a key-value store.