core, triedb/pathdb: address comments from marius

Author: rjl493456442

core/blockchain_repair_test.go

@@ -1999,14 +1999,16 @@ func testIssue23496(t *testing.T, scheme string) {
 	}
 	expHead := uint64(1)
 	if scheme == rawdb.PathScheme {
+		// The pathdb database makes sure that snapshot and trie are consistent,
+		// so only the last block is reverted in case of a crash.
 		expHead = uint64(3)
 	}
 	if head := chain.CurrentBlock(); head.Number.Uint64() != expHead {
 		t.Errorf("Head block mismatch: have %d, want %d", head.Number, expHead)
 	}
 	if scheme == rawdb.PathScheme {
-		// Reinsert B3-B4
-		if _, err := chain.InsertChain(blocks[2:]); err != nil {
+		// Reinsert B4
+		if _, err := chain.InsertChain(blocks[3:]); err != nil {
 			t.Fatalf("Failed to import canonical chain tail: %v", err)
 		}
 	} else {

core/blockchain_snapshot_test.go

@@ -569,11 +569,13 @@ func TestHighCommitCrashWithNewSnapshot(t *testing.T) {
 	//
 	// Expected head header    : C8
 	// Expected head fast block: C8
-	// Expected head block     : G
-	// Expected snapshot disk  : C4
+	// Expected head block     : G (Hash mode), C6 (Hash mode)
+	// Expected snapshot disk  : C4 (Hash mode)
 	for _, scheme := range []string{rawdb.HashScheme, rawdb.PathScheme} {
 		expHead := uint64(0)
 		if scheme == rawdb.PathScheme {
+			// The pathdb database makes sure that snapshot and trie are consistent,
+			// so only the last two blocks are reverted in case of a crash.
 			expHead = uint64(6)
 		}
 		test := &crashSnapshotTest{

core/state/database.go

@@ -175,27 +175,27 @@ func NewDatabaseForTesting() *CachingDB {
 func (db *CachingDB) Reader(stateRoot common.Hash) (Reader, error) {
 	var readers []StateReader
 
-	// Set up the state snapshot reader if available. This feature
-	// is optional and may be partially useful if it's not fully
-	// generated.
-	if db.snap != nil {
-		// If standalone state snapshot is available (hash scheme),
-		// then construct the legacy snap reader.
+	// Configure the state reader using the standalone snapshot in hash mode.
+	// This reader offers improved performance but is optional and only
+	// partially useful if the snapshot is not fully generated.
+	if db.TrieDB().Scheme() == rawdb.HashScheme && db.snap != nil {
 		snap := db.snap.Snapshot(stateRoot)
 		if snap != nil {
 			readers = append(readers, newFlatReader(snap))
 		}
-	} else {
-		// If standalone state snapshot is not available (path scheme
-		// or the state snapshot is explicitly disabled in hash mode),
-		// try to construct the state reader with database.
+	}
+	// Configure the state reader using the path database in path mode.
+	// This reader offers improved performance but is optional and only
+	// partially useful if the snapshot data in path database is not
+	// fully generated.
+	if db.TrieDB().Scheme() == rawdb.PathScheme {
 		reader, err := db.triedb.StateReader(stateRoot)
 		if err == nil {
-			readers = append(readers, newFlatReader(reader)) // state reader is optional
+			readers = append(readers, newFlatReader(reader))
 		}
 	}
-	// Set up the trie reader, which is expected to always be available
-	// as the gatekeeper unless the state is corrupted.
+	// Configure the trie reader, which is expected to be available as the
+	// gatekeeper unless the state is corrupted.
 	tr, err := newTrieReader(stateRoot, db.triedb, db.pointCache)
 	if err != nil {
 		return nil, err

triedb/pathdb/context.go

@@ -224,9 +224,9 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error {
 	return nil
 }
 
-// removeStorageLeft deletes all storage entries which are located after
+// removeRemainingStorage deletes all storage entries which are located after
 // the current iterator position.
-func (ctx *generatorContext) removeStorageLeft() uint64 {
+func (ctx *generatorContext) removeRemainingStorage() uint64 {
 	var (
 		count uint64
 		start = time.Now()

triedb/pathdb/generate.go

@@ -171,7 +171,7 @@ func (g *generator) progressMarker() []byte {
 // into two parts.
 func splitMarker(marker []byte) ([]byte, []byte) {
 	var accMarker []byte
-	if len(marker) > 0 { // []byte{} is the start, use nil for that
+	if len(marker) > 0 {
 		accMarker = marker[:common.HashLength]
 	}
 	return accMarker, marker
@@ -751,7 +751,7 @@ func (g *generator) generateAccounts(ctx *generatorContext, accMarker []byte) er
 		// Last step, cleanup the storages after the last account.
 		// All the left storages should be treated as dangling.
 		if origin == nil || exhausted {
-			g.stats.dangling += ctx.removeStorageLeft()
+			g.stats.dangling += ctx.removeRemainingStorage()
 			break
 		}
 	}