Merge pull request #2155 from sysvm/fix-prunestate

cmd/geth/chaincmd.go

@@ -655,7 +655,7 @@ func dump(ctx *cli.Context) error {
 	if err != nil {
 		return err
 	}
-	triedb := utils.MakeTrieDatabase(ctx, db, true, false) // always enable preimage lookup
+	triedb := utils.MakeTrieDatabase(ctx, db, true, true) // always enable preimage lookup
 	defer triedb.Close()
 
 	state, err := state.New(root, state.NewDatabaseWithNodeDB(db, triedb), nil)

cmd/geth/genesis_test.go

@@ -176,12 +176,12 @@ func TestCustomBackend(t *testing.T) {
 		{ // Can't start pebble on top of leveldb
 			initArgs:   []string{"--db.engine", "leveldb"},
 			execArgs:   []string{"--db.engine", "pebble"},
-			execExpect: `Fatal: Could not open database: db.engine choice was pebble but found pre-existing leveldb database in specified data directory`,
+			execExpect: `Fatal: Failed to register the Ethereum service: db.engine choice was pebble but found pre-existing leveldb database in specified data directory`,
 		},
 		{ // Can't start leveldb on top of pebble
 			initArgs:   []string{"--db.engine", "pebble"},
 			execArgs:   []string{"--db.engine", "leveldb"},
-			execExpect: `Fatal: Could not open database: db.engine choice was leveldb but found pre-existing pebble database in specified data directory`,
+			execExpect: `Fatal: Failed to register the Ethereum service: db.engine choice was leveldb but found pre-existing pebble database in specified data directory`,
 		},
 		{ // Reject invalid backend choice
 			initArgs:   []string{"--db.engine", "mssql"},

cmd/utils/flags.go

@@ -1848,7 +1848,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
 		if cfg.SyncMode == downloader.FullSync {
 			cfg.PruneAncientData = ctx.Bool(PruneAncientDataFlag.Name)
 		} else {
-			log.Crit("pruneancient parameter didn't take effect for current syncmode")
+			log.Crit("pruneancient parameter can only be used with syncmode=full")
 		}
 	}
 	if gcmode := ctx.String(GCModeFlag.Name); gcmode != "full" && gcmode != "archive" {
@@ -1884,15 +1884,11 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
 	if ctx.IsSet(StateHistoryFlag.Name) {
 		cfg.StateHistory = ctx.Uint64(StateHistoryFlag.Name)
 	}
-	// Parse state scheme, abort the process if it's not compatible.
-	chaindb := tryMakeReadOnlyDatabase(ctx, stack)
-	scheme, err := ParseStateScheme(ctx, chaindb)
-	chaindb.Close()
+	scheme, err := compareCLIWithConfig(ctx)
 	if err != nil {
 		Fatalf("%v", err)
 	}
 	cfg.StateScheme = scheme
-
 	// Parse transaction history flag, if user is still using legacy config
 	// file with 'TxLookupLimit' configured, copy the value to 'TransactionHistory'.
 	if cfg.TransactionHistory == ethconfig.Defaults.TransactionHistory && cfg.TxLookupLimit != ethconfig.Defaults.TxLookupLimit {
@@ -2046,7 +2042,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
 				readonly = false
 			}
 			// Check if we have an already initialized chain and fall back to
-			// that if so. Otherwise we need to generate a new genesis spec.
+			// that if so. Otherwise, we need to generate a new genesis spec.
 			chaindb := MakeChainDatabase(ctx, stack, readonly, false)
 			if rawdb.ReadCanonicalHash(chaindb, 0) != (common.Hash{}) {
 				cfg.Genesis = nil // fallback to db content
@@ -2281,6 +2277,8 @@ func MakeChainDatabase(ctx *cli.Context, stack *node.Node, readonly, disableFree
 
 // tryMakeReadOnlyDatabase try to open the chain database in read-only mode,
 // or fallback to write mode if the database is not initialized.
+//
+//nolint:unused
 func tryMakeReadOnlyDatabase(ctx *cli.Context, stack *node.Node) ethdb.Database {
 	// If datadir doesn't exist we need to open db in write-mode
 	// so database engine can create files.
@@ -2355,7 +2353,11 @@ func MakeChain(ctx *cli.Context, stack *node.Node, readonly bool) (*core.BlockCh
 	if gcmode := ctx.String(GCModeFlag.Name); gcmode != "full" && gcmode != "archive" {
 		Fatalf("--%s must be either 'full' or 'archive'", GCModeFlag.Name)
 	}
-	scheme, err := ParseStateScheme(ctx, chainDb)
+	provided, err := compareCLIWithConfig(ctx)
+	if err != nil {
+		Fatalf("%v", err)
+	}
+	scheme, err := rawdb.ParseStateScheme(provided, chainDb)
 	if err != nil {
 		Fatalf("%v", err)
 	}
@@ -2418,52 +2420,16 @@ func MakeConsolePreloads(ctx *cli.Context) []string {
 	return preloads
 }
 
-// ParseStateScheme checks if the specified state scheme is compatible with
-// the stored state.
-//
-//   - If the provided scheme is none, use the scheme consistent with persistent
-//     state, or fallback to hash-based scheme if state is empty.
-//
-//   - If the provided scheme is hash, use hash-based scheme or error out if not
-//     compatible with persistent state scheme.
-//
-//   - If the provided scheme is path: use path-based scheme or error out if not
-//     compatible with persistent state scheme.
-func ParseStateScheme(ctx *cli.Context, disk ethdb.Database) (string, error) {
-	// If state scheme is not specified, use the scheme consistent
-	// with persistent state, or fallback to hash mode if database
-	// is empty.
-	provided, err := compareCLIWithConfig(ctx)
-	if err != nil {
-		log.Error("failed to compare CLI with config", "error", err)
-		return "", err
-	}
-
-	stored := rawdb.ReadStateScheme(disk)
-	if provided == "" {
-		if stored == "" {
-			// use default scheme for empty database, flip it when
-			// path mode is chosen as default
-			log.Info("State scheme set to default", "scheme", rawdb.HashScheme)
-			return rawdb.HashScheme, nil
-		}
-		log.Info("State scheme set to already existing disk db", "scheme", stored)
-		return stored, nil // reuse scheme of persistent scheme
-	}
-	// If state scheme is specified, ensure it's compatible with persistent state.
-	if stored == "" || provided == stored {
-		log.Info("State scheme set by user", "scheme", provided)
-		return provided, nil
-	}
-	return "", fmt.Errorf("incompatible state scheme, db stored: %s, user provided: %s", stored, provided)
-}
-
 // MakeTrieDatabase constructs a trie database based on the configured scheme.
 func MakeTrieDatabase(ctx *cli.Context, disk ethdb.Database, preimage bool, readOnly bool) *trie.Database {
 	config := &trie.Config{
 		Preimages: preimage,
 	}
-	scheme, err := ParseStateScheme(ctx, disk)
+	provided, err := compareCLIWithConfig(ctx)
+	if err != nil {
+		Fatalf("%v", err)
+	}
+	scheme, err := rawdb.ParseStateScheme(provided, disk)
 	if err != nil {
 		Fatalf("%v", err)
 	}

core/genesis.go

@@ -120,8 +120,8 @@ func (ga *GenesisAlloc) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
-// deriveHash computes the state root according to the genesis specification.
-func (ga *GenesisAlloc) deriveHash() (common.Hash, error) {
+// hash computes the state root according to the genesis specification.
+func (ga *GenesisAlloc) hash() (common.Hash, error) {
 	// Create an ephemeral in-memory database for computing hash,
 	// all the derived states will be discarded to not pollute disk.
 	db := state.NewDatabase(rawdb.NewMemoryDatabase())
@@ -142,9 +142,9 @@ func (ga *GenesisAlloc) deriveHash() (common.Hash, error) {
 	return root, err
 }
 
-// flush is very similar with deriveHash, but the main difference is
-// all the generated states will be persisted into the given database.
-// Also, the genesis state specification will be flushed as well.
+// flush is very similar with hash, but the main difference is all the generated
+// states will be persisted into the given database. Also, the genesis state
+// specification will be flushed as well.
 func (ga *GenesisAlloc) flush(db ethdb.Database, triedb *trie.Database, blockhash common.Hash) error {
 	trieConfig := triedb.Config()
 	if trieConfig != nil {
@@ -455,7 +455,7 @@ func (g *Genesis) configOrDefault(ghash common.Hash) *params.ChainConfig {
 
 // ToBlock returns the genesis block according to genesis specification.
 func (g *Genesis) ToBlock() *types.Block {
-	root, err := g.Alloc.deriveHash()
+	root, err := g.Alloc.hash()
 	if err != nil {
 		panic(err)
 	}

core/genesis_test.go

@@ -209,7 +209,7 @@ func TestReadWriteGenesisAlloc(t *testing.T) {
 			{1}: {Balance: big.NewInt(1), Storage: map[common.Hash]common.Hash{{1}: {1}}},
 			{2}: {Balance: big.NewInt(2), Storage: map[common.Hash]common.Hash{{2}: {2}}},
 		}
-		hash, _ = alloc.deriveHash()
+		hash, _ = alloc.hash()
 	)
 	blob, _ := json.Marshal(alloc)
 	rawdb.WriteGenesisStateSpec(db, hash, blob)

core/rawdb/accessors_trie.go

@@ -314,3 +314,38 @@ func ValidateStateScheme(stateScheme string) bool {
 	}
 	return false
 }
+
+// ParseStateScheme checks if the specified state scheme is compatible with
+// the stored state.
+//
+//   - If the provided scheme is none, use the scheme consistent with persistent
+//     state, or fallback to hash-based scheme if state is empty.
+//
+//   - If the provided scheme is hash, use hash-based scheme or error out if not
+//     compatible with persistent state scheme.
+//
+//   - If the provided scheme is path: use path-based scheme or error out if not
+//     compatible with persistent state scheme.
+func ParseStateScheme(provided string, disk ethdb.Database) (string, error) {
+	// If state scheme is not specified, use the scheme consistent
+	// with persistent state, or fallback to hash mode if database
+	// is empty.
+	stored := ReadStateScheme(disk)
+	if provided == "" {
+		if stored == "" {
+			// use default scheme for empty database, flip it when
+			// path mode is chosen as default
+			log.Info("State schema set to default", "scheme", "hash")
+			return HashScheme, nil
+		}
+		log.Info("State scheme set to already existing disk db", "scheme", stored)
+		return stored, nil // reuse scheme of persistent scheme
+	}
+	// If state scheme is specified, ensure it's compatible with
+	// persistent state.
+	if stored == "" || provided == stored {
+		log.Info("State scheme set by user", "scheme", provided)
+		return provided, nil
+	}
+	return "", fmt.Errorf("incompatible state scheme, stored: %s, user provided: %s", stored, provided)
+}

core/rawdb/database.go

@@ -219,25 +219,28 @@ func NewFreezerDb(db ethdb.KeyValueStore, frz, namespace string, readonly bool,
 
 // resolveChainFreezerDir is a helper function which resolves the absolute path
 // of chain freezer by considering backward compatibility.
+//
+// rules:
+// 1. in path mode, block data is stored in chain dir and state data is in state dir.
+// 2. in hash mode, block data is stored in chain dir or ancient dir(before big merge), no state dir.
 func resolveChainFreezerDir(ancient string) string {
 	// Check if the chain freezer is already present in the specified
 	// sub folder, if not then two possibilities:
 	// - chain freezer is not initialized
 	// - chain freezer exists in legacy location (root ancient folder)
-	freezer := path.Join(ancient, chainFreezerName)
-	if !common.FileExist(freezer) {
-		if !common.FileExist(ancient) {
-			// The entire ancient store is not initialized, still use the sub
-			// folder for initialization.
-		} else {
-			// Ancient root is already initialized, then we hold the assumption
-			// that chain freezer is also initialized and located in root folder.
-			// In this case fallback to legacy location.
-			freezer = ancient
-			log.Info("Found legacy ancient chain path", "location", ancient)
-		}
+	chain := path.Join(ancient, chainFreezerName)
+	state := path.Join(ancient, stateFreezerName)
+	if common.FileExist(chain) {
+		return chain
+	}
+	if common.FileExist(state) {
+		return chain
 	}
-	return freezer
+	if common.FileExist(ancient) {
+		log.Info("Found legacy ancient chain path", "location", ancient)
+		chain = ancient
+	}
+	return chain
 }
 
 // NewDatabaseWithFreezer creates a high level database on top of a given key-
@@ -264,6 +267,7 @@ func NewDatabaseWithFreezer(db ethdb.KeyValueStore, ancient string, namespace st
 			WriteAncientType(db, PruneFreezerType)
 		}
 		return &freezerdb{
+			ancientRoot:   ancient,
 			KeyValueStore: db,
 			AncientStore:  frdb,
 		}, nil
@@ -336,7 +340,7 @@ func NewDatabaseWithFreezer(db ethdb.KeyValueStore, ancient string, namespace st
 							break
 						}
 					}
-					// We are about to exit on error. Print database metdata beore exiting
+					// We are about to exit on error. Print database metdata before exiting
 					printChainMetadata(db)
 					return nil, fmt.Errorf("gap in the chain between ancients [0 - #%d] and leveldb [#%d - #%d] ",
 						frozen-1, number, head)
@@ -365,7 +369,7 @@ func NewDatabaseWithFreezer(db ethdb.KeyValueStore, ancient string, namespace st
 			// freezer.
 		}
 	}
-	// no prune ancinet start success
+	// no prune ancient start success
 	if !readonly {
 		WriteAncientType(db, EntireFreezerType)
 	}
@@ -516,6 +520,11 @@ func Open(o OpenOptions) (ethdb.Database, error) {
 	if err != nil {
 		return nil, err
 	}
+	if ReadAncientType(kvdb) == PruneFreezerType {
+		if !o.PruneAncientData {
+			log.Warn("Disk db is pruned")
+		}
+	}
 	if len(o.AncientsDirectory) == 0 {
 		return kvdb, nil
 	}
@@ -556,6 +565,7 @@ func (s *stat) Size() string {
 func (s *stat) Count() string {
 	return s.count.String()
 }
+
 func AncientInspect(db ethdb.Database) error {
 	offset := counter(ReadOffSetOfCurrentAncientFreezer(db))
 	// Get number of ancient rows inside the freezer.

core/rawdb/database_test.go

@@ -15,3 +15,80 @@
 // along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
 
 package rawdb
+
+import (
+	"fmt"
+	"os"
+	"testing"
+)
+
+const (
+	mockChainFreezerPath   = "/geth/chaindata/ancient/chain"
+	mockStateFreezerPath   = "/geth/chaindata/ancient/state"
+	mockAncientFreezerPath = "/geth/chaindata/ancient"
+)
+
+func Test_resolveChainFreezerDir(t *testing.T) {
+	tests := []struct {
+		name         string
+		fn           func(dir string) string
+		ancient      string
+		wantedResult string
+	}{
+		{
+			// chain dir is existent, so it should be returned.
+			name: "1",
+			fn: func(dir string) string {
+				path := fmt.Sprintf("%s%s", dir, mockChainFreezerPath)
+				if err := os.MkdirAll(path, 0700); err != nil {
+					t.Fatalf("Failed to mkdir all dirs, error: %v", err)
+				}
+				return fmt.Sprintf("%s%s", dir, mockAncientFreezerPath)
+			},
+			wantedResult: mockChainFreezerPath,
+		},
+		{
+			// chain dir is nonexistent and state dir is existent; so chain
+			// dir should be returned.
+			name: "2",
+			fn: func(dir string) string {
+				path := fmt.Sprintf("%s%s", dir, mockStateFreezerPath)
+				if err := os.MkdirAll(path, 0700); err != nil {
+					t.Fatalf("Failed to mkdir all dirs, error: %v", err)
+				}
+				return fmt.Sprintf("%s%s", dir, mockAncientFreezerPath)
+			},
+			wantedResult: mockChainFreezerPath,
+		},
+		{
+			// both chain dir and state dir are nonexistent, if ancient dir is
+			// existent, so ancient dir should be returned.
+			name: "3",
+			fn: func(dir string) string {
+				path := fmt.Sprintf("%s%s", dir, mockAncientFreezerPath)
+				if err := os.MkdirAll(path, 0700); err != nil {
+					t.Fatalf("Failed to mkdir all dirs, error: %v", err)
+				}
+				return path
+			},
+			wantedResult: mockAncientFreezerPath,
+		},
+		{
+			// ancient dir is nonexistent, so chain dir should be returned.
+			name: "4",
+			fn: func(dir string) string {
+				return fmt.Sprintf("%s%s", dir, mockAncientFreezerPath)
+			},
+			wantedResult: mockChainFreezerPath,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tempDir := t.TempDir()
+			got := resolveChainFreezerDir(tt.fn(tempDir))
+			if got != fmt.Sprintf("%s%s", tempDir, tt.wantedResult) {
+				t.Fatalf("resolveChainFreezerDir() = %s, wanted = %s", got, tt.wantedResult)
+			}
+		})
+	}
+}

core/rawdb/freezer.go

@@ -152,7 +152,7 @@ func NewFreezer(datadir string, namespace string, readonly bool, offset uint64,
 	}
 
 	// Some blocks in ancientDB may have already been frozen and been pruned, so adding the offset to
-	// reprensent the absolute number of blocks already frozen.
+	// represent the absolute number of blocks already frozen.
 	freezer.frozen.Add(offset)
 	freezer.tail.Add(offset)