diff --git a/cmd/ronin/dbcmd.go b/cmd/ronin/dbcmd.go index 53a83b5031..1de4223294 100644 --- a/cmd/ronin/dbcmd.go +++ b/cmd/ronin/dbcmd.go @@ -248,60 +248,73 @@ WARNING: This is a low-level operation which may cause database corruption!`, func removeDB(ctx *cli.Context) error { stack, config := makeConfigNode(ctx) - // Remove the full node state database - path := stack.ResolvePath("chaindata") - if common.FileExist(path) { - confirmAndRemoveDB(path, "full node state database") - } else { - log.Info("Full node state database missing", "path", path) - } - // Remove the full node ancient database - path = config.Eth.DatabaseFreezer + // Resolve folder paths. + var ( + rootDir = stack.ResolvePath("chaindata") + ancientDir = config.Eth.DatabaseFreezer + ) switch { - case path == "": - path = filepath.Join(stack.ResolvePath("chaindata"), "ancient") - case !filepath.IsAbs(path): - path = config.Node.ResolvePath(path) - } - if common.FileExist(path) { - confirmAndRemoveDB(path, "full node ancient database") - } else { - log.Info("Full node ancient database missing", "path", path) - } - // Remove the light node database - path = stack.ResolvePath("lightchaindata") - if common.FileExist(path) { - confirmAndRemoveDB(path, "light node database") - } else { - log.Info("Light node database missing", "path", path) - } + case ancientDir == "": + ancientDir = filepath.Join(stack.ResolvePath("chaindata"), "ancient") + case !filepath.IsAbs(ancientDir): + ancientDir = config.Node.ResolvePath(ancientDir) + } + // Delete state data + statePaths := []string{rootDir, filepath.Join(ancientDir, rawdb.StateFreezerName)} + confirmAndRemoveDB(statePaths, "state data") + + // Delete ancient chain + chainPaths := []string{filepath.Join(ancientDir, rawdb.ChainFreezerName)} + confirmAndRemoveDB(chainPaths, "ancient chain") return nil } +// removeFolder deletes all files (not folders) inside the directory 'dir' (but +// not files in subfolders). +func removeFolder(dir string) { + filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + // If we're at the top level folder, recurse into + if path == dir { + return nil + } + // Delete all the files, but not subfolders + if !info.IsDir() { + os.Remove(path) + return nil + } + return filepath.SkipDir + }) +} + // confirmAndRemoveDB prompts the user for a last confirmation and removes the -// folder if accepted. -func confirmAndRemoveDB(database string, kind string) { - confirm, err := prompt.Stdin.PromptConfirm(fmt.Sprintf("Remove %s (%s)?", kind, database)) +// list of folders if accepted. +func confirmAndRemoveDB(paths []string, kind string) { + msg := fmt.Sprintf("Location(s) of '%s': \n", kind) + for _, path := range paths { + msg += fmt.Sprintf("\t- %s\n", path) + } + fmt.Println(msg) + + confirm, err := prompt.Stdin.PromptConfirm(fmt.Sprintf("Remove '%s'?", kind)) switch { case err != nil: utils.Fatalf("%v", err) case !confirm: - log.Info("Database deletion skipped", "path", database) + log.Info("Database deletion skipped", "kind", kind, "paths", paths) default: - start := time.Now() - filepath.Walk(database, func(path string, info os.FileInfo, err error) error { - // If we're at the top level folder, recurse into - if path == database { - return nil + var ( + deleted []string + start = time.Now() + ) + for _, path := range paths { + if common.FileExist(path) { + removeFolder(path) + deleted = append(deleted, path) + } else { + log.Info("Folder is not existent", "path", path) } - // Delete all the files, but not subfolders - if !info.IsDir() { - os.Remove(path) - return nil - } - return filepath.SkipDir - }) - log.Info("Database successfully deleted", "path", database, "elapsed", common.PrettyDuration(time.Since(start))) + } + log.Info("Database successfully deleted", "kind", kind, "paths", deleted, "elapsed", common.PrettyDuration(time.Since(start))) } } diff --git a/common/types.go b/common/types.go index 0f53406980..7779e800a7 100644 --- a/common/types.go +++ b/common/types.go @@ -49,6 +49,9 @@ const ( var ( hashT = reflect.TypeOf(Hash{}) addressT = reflect.TypeOf(Address{}) + + // MaxHash represents the maximum possible hash value. + MaxHash = HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff") ) // Hash represents the 32 byte Keccak256 hash of arbitrary data. diff --git a/core/rawdb/accessors_trie.go b/core/rawdb/accessors_trie.go index 14b3a96ba0..869991a388 100644 --- a/core/rawdb/accessors_trie.go +++ b/core/rawdb/accessors_trie.go @@ -294,6 +294,11 @@ func ReadStateScheme(db ethdb.Reader) string { if len(blob) != 0 { return PathScheme } + // The root node might be deleted during the initial snap sync, check + // the persistent state id then. + if id := ReadPersistentStateID(db); id != 0 { + return PathScheme + } // In a hash-based scheme, the genesis state is consistently stored // on the disk. To assess the scheme of the persistent state, it // suffices to inspect the scheme of the genesis state. diff --git a/core/rawdb/ancient_scheme.go b/core/rawdb/ancient_scheme.go index 0e6d4bea5a..5a173f4915 100644 --- a/core/rawdb/ancient_scheme.go +++ b/core/rawdb/ancient_scheme.go @@ -73,16 +73,16 @@ var chainFreezerNoSnappy = map[string]bool{ // The list of identifiers of ancient stores. It can split more in the futures. var ( - chainFreezerName = "chain" // the folder name of chain segment ancient store. - stateFreezerName = "state" // the folder name of reverse diff ancient store. + ChainFreezerName = "chain" // the folder name of chain segment ancient store. + StateFreezerName = "state" // the folder name of reverse diff ancient store. ) // freezers the collections of all builtin freezers. -var freezers = []string{chainFreezerName, stateFreezerName} +var freezers = []string{ChainFreezerName, StateFreezerName} // NewStateFreezer initializes the freezer for state history. func NewStateFreezer(ancientDir string, readOnly bool) (*ResettableFreezer, error) { return NewResettableFreezer( - filepath.Join(ancientDir, stateFreezerName), namespace, readOnly, + filepath.Join(ancientDir, StateFreezerName), namespace, readOnly, stateHistoryTableSize, stateFreezerNoSnappy) } diff --git a/core/rawdb/ancient_utils.go b/core/rawdb/ancient_utils.go index fd76e5348a..fa2e0e4678 100644 --- a/core/rawdb/ancient_utils.go +++ b/core/rawdb/ancient_utils.go @@ -81,14 +81,14 @@ func inspectFreezers(db ethdb.Database) ([]freezerInfo, error) { var infos []freezerInfo for _, freezer := range freezers { switch freezer { - case chainFreezerName: - info, err := inspect(chainFreezerName, chainFreezerNoSnappy, db) + case ChainFreezerName: + info, err := inspect(ChainFreezerName, chainFreezerNoSnappy, db) if err != nil { return nil, err } infos = append(infos, info) - case stateFreezerName: + case StateFreezerName: datadir, err := db.AncientDatadir() if err != nil { return nil, err @@ -99,7 +99,7 @@ func inspectFreezers(db ethdb.Database) ([]freezerInfo, error) { } defer f.Close() - info, err := inspect(stateFreezerName, stateFreezerNoSnappy, f) + info, err := inspect(StateFreezerName, stateFreezerNoSnappy, f) if err != nil { return nil, err } @@ -123,9 +123,9 @@ func InspectFreezerTable(ancient string, freezerName string, tableName string, s ) switch freezerName { - case chainFreezerName: + case ChainFreezerName: path, tables = resolveChainFreezerDir(ancient), chainFreezerNoSnappy - case stateFreezerName: + case StateFreezerName: path, tables = filepath.Join(ancient, freezerName), stateFreezerNoSnappy default: return fmt.Errorf("unknown freezer, supported ones: %v", freezers) diff --git a/core/rawdb/database.go b/core/rawdb/database.go index 9ad0287d59..6bf5366ea2 100644 --- a/core/rawdb/database.go +++ b/core/rawdb/database.go @@ -173,7 +173,7 @@ func resolveChainFreezerDir(ancient string) string { // - chain freezer is not initialized // - it's legacy location, chain freezer is present in the root ancient folder - freezer := path.Join(ancient, chainFreezerName) + freezer := path.Join(ancient, ChainFreezerName) if !common.FileExist(freezer) { if !common.FileExist(ancient) { // The entire ancient store is not initialized, still use the sub diff --git a/core/state/snapshot/conversion.go b/core/state/snapshot/conversion.go index b567579525..03f6466f49 100644 --- a/core/state/snapshot/conversion.go +++ b/core/state/snapshot/conversion.go @@ -363,22 +363,16 @@ func generateTrieRoot(db ethdb.KeyValueWriter, scheme string, it Iterator, accou func stackTrieGenerate(db ethdb.KeyValueWriter, scheme string, owner common.Hash, in chan trieKV, out chan common.Hash) { - var nodeWriter trie.NodeWriteFunc + options := trie.NewStackTrieOptions() // Implement nodeWriter in case db is existed otherwise let it be nil. if db != nil { - nodeWriter = func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { rawdb.WriteTrieNode(db, owner, path, hash, blob, scheme) - } + }) } - t := trie.NewStackTrieWithOwner(nodeWriter, owner) + t := trie.NewStackTrie(options) for leaf := range in { t.TryUpdate(leaf.key[:], leaf.value) } - var root common.Hash - if db == nil { - root = t.Hash() - } else { - root, _ = t.Commit() - } - out <- root + out <- t.Commit() } diff --git a/eth/protocols/snap/metrics.go b/eth/protocols/snap/metrics.go index a8ea143b54..ffc9a6a5f3 100644 --- a/eth/protocols/snap/metrics.go +++ b/eth/protocols/snap/metrics.go @@ -8,4 +8,32 @@ var ( IngressRegistrationErrorMeter = metrics.NewRegisteredMeter(ingressRegistrationErrorName, nil) EgressRegistrationErrorMeter = metrics.NewRegisteredMeter(egressRegistrationErrorName, nil) + + // deletionGauge is the metric to track how many trie node deletions + // are performed in total during the sync process. + deletionGauge = metrics.NewRegisteredGauge("eth/protocols/snap/sync/delete", nil) + + // lookupGauge is the metric to track how many trie node lookups are + // performed to determine if node needs to be deleted. + lookupGauge = metrics.NewRegisteredGauge("eth/protocols/snap/sync/lookup", nil) + + // boundaryAccountNodesGauge is the metric to track how many boundary trie + // nodes in account trie are met. + boundaryAccountNodesGauge = metrics.NewRegisteredGauge("eth/protocols/snap/sync/boundary/account", nil) + + // boundaryAccountNodesGauge is the metric to track how many boundary trie + // nodes in storage tries are met. + boundaryStorageNodesGauge = metrics.NewRegisteredGauge("eth/protocols/snap/sync/boundary/storage", nil) + + // smallStorageGauge is the metric to track how many storages are small enough + // to retrieved in one or two request. + smallStorageGauge = metrics.NewRegisteredGauge("eth/protocols/snap/sync/storage/small", nil) + + // largeStorageGauge is the metric to track how many storages are large enough + // to retrieved concurrently. + largeStorageGauge = metrics.NewRegisteredGauge("eth/protocols/snap/sync/storage/large", nil) + + // skipStorageHealingGauge is the metric to track how many storages are retrieved + // in multiple requests but healing is not necessary. + skipStorageHealingGauge = metrics.NewRegisteredGauge("eth/protocols/snap/sync/storage/noheal", nil) ) diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index 538d6f0b5a..e1d9406f21 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -699,6 +699,19 @@ func (s *Syncer) Sync(root common.Hash, cancel chan struct{}) error { } } +// cleanPath is used to remove the dangling nodes in the stackTrie. +func (s *Syncer) cleanPath(batch ethdb.Batch, owner common.Hash, path []byte) { + if owner == (common.Hash{}) && rawdb.ExistsAccountTrieNode(s.db, path) { + rawdb.DeleteAccountTrieNode(batch, path) + deletionGauge.Inc(1) + } + if owner != (common.Hash{}) && rawdb.ExistsStorageTrieNode(s.db, owner, path) { + rawdb.DeleteStorageTrieNode(batch, owner, path) + deletionGauge.Inc(1) + } + lookupGauge.Inc(1) +} + // loadSyncStatus retrieves a previously aborted sync status from the database, // or generates a fresh one if none is available. func (s *Syncer) loadSyncStatus() { @@ -721,9 +734,22 @@ func (s *Syncer) loadSyncStatus() { s.accountBytes += common.StorageSize(len(key) + len(value)) }, } - task.genTrie = trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { - rawdb.WriteTrieNode(task.genBatch, owner, path, hash, val, s.scheme) + options := trie.NewStackTrieOptions() + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(task.genBatch, common.Hash{}, path, hash, blob, s.scheme) }) + if s.scheme == rawdb.PathScheme { + // Configure the dangling node cleaner and also filter out boundary nodes + // only in the context of the path scheme. Deletion is forbidden in the + // hash scheme, as it can disrupt state completeness. + options = options.WithCleaner(func(path []byte) { + s.cleanPath(task.genBatch, common.Hash{}, path) + }) + // Skip the left boundary if it's not the first range. + // Skip the right boundary if it's not the last range. + options = options.WithSkipBoundary(task.Next != (common.Hash{}), task.Last != common.MaxHash, boundaryAccountNodesGauge) + } + task.genTrie = trie.NewStackTrie(options) for accountHash, subtasks := range task.SubTasks { for _, subtask := range subtasks { @@ -735,9 +761,24 @@ func (s *Syncer) loadSyncStatus() { s.storageBytes += common.StorageSize(len(key) + len(value)) }, } - subtask.genTrie = trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { - rawdb.WriteTrieNode(subtask.genBatch, owner, path, hash, val, s.scheme) - }, accountHash) + owner := accountHash // local assignment for stacktrie writer closure + options := trie.NewStackTrieOptions() + + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(subtask.genBatch, owner, path, hash, blob, s.scheme) + }) + if s.scheme == rawdb.PathScheme { + // Configure the dangling node cleaner and also filter out boundary nodes + // only in the context of the path scheme. Deletion is forbidden in the + // hash scheme, as it can disrupt state completeness. + options = options.WithCleaner(func(path []byte) { + s.cleanPath(subtask.genBatch, owner, path) + }) + // Skip the left boundary if it's not the first range. + // Skip the right boundary if it's not the last range. + options = options.WithSkipBoundary(subtask.Next != common.Hash{}, subtask.Last != common.MaxHash, boundaryStorageNodesGauge) + } + subtask.genTrie = trie.NewStackTrie(options) } } } @@ -786,14 +827,27 @@ func (s *Syncer) loadSyncStatus() { s.accountBytes += common.StorageSize(len(key) + len(value)) }, } + options := trie.NewStackTrieOptions() + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(batch, common.Hash{}, path, hash, blob, s.scheme) + }) + if s.scheme == rawdb.PathScheme { + // Configure the dangling node cleaner and also filter out boundary nodes + // only in the context of the path scheme. Deletion is forbidden in the + // hash scheme, as it can disrupt state completeness. + options = options.WithCleaner(func(path []byte) { + s.cleanPath(batch, common.Hash{}, path) + }) + // Skip the left boundary if it's not the first range. + // Skip the right boundary if it's not the last range. + options = options.WithSkipBoundary(next != common.Hash{}, last != common.MaxHash, boundaryAccountNodesGauge) + } s.tasks = append(s.tasks, &accountTask{ Next: next, Last: last, SubTasks: make(map[common.Hash][]*storageTask), genBatch: batch, - genTrie: trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { - rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme) - }), + genTrie: trie.NewStackTrie(options), }) log.Debug("Created account sync task", "from", next, "last", last) next = common.BigToHash(new(big.Int).Add(last.Big(), common.Big1)) @@ -1930,6 +1984,7 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { if res.subTask == nil && res.mainTask.needState[j] && (i < len(res.hashes)-1 || !res.cont) { res.mainTask.needState[j] = false res.mainTask.pend-- + smallStorageGauge.Inc(1) } // If the last contract was chunked, mark it as needing healing // to avoid writing it out to disk prematurely. @@ -1965,7 +2020,11 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { log.Debug("Chunked large contract", "initiators", len(keys), "tail", lastKey, "chunks", chunks) } r := newHashRange(lastKey, chunks) - + if chunks == 1 { + smallStorageGauge.Inc(1) + } else { + largeStorageGauge.Inc(1) + } // Our first task is the one that was just filled by this response. batch := ethdb.HookedBatch{ Batch: s.db.NewBatch(), @@ -1973,14 +2032,25 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { s.storageBytes += common.StorageSize(len(key) + len(value)) }, } + owner := account // local assignment for stacktrie writer closure + options := trie.NewStackTrieOptions() + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(batch, owner, path, hash, blob, s.scheme) + }) + if s.scheme == rawdb.PathScheme { + options = options.WithCleaner(func(path []byte) { + s.cleanPath(batch, owner, path) + }) + // Keep the left boundary as it's the first range. + // Skip the right boundary if it's not the last range. + options = options.WithSkipBoundary(false, r.End() != common.MaxHash, boundaryStorageNodesGauge) + } tasks = append(tasks, &storageTask{ Next: common.Hash{}, Last: r.End(), root: acc.Root, genBatch: batch, - genTrie: trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { - rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme) - }, account), + genTrie: trie.NewStackTrie(options), }) for r.Next() { batch := ethdb.HookedBatch{ @@ -1989,14 +2059,27 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { s.storageBytes += common.StorageSize(len(key) + len(value)) }, } + options := trie.NewStackTrieOptions() + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(batch, owner, path, hash, blob, s.scheme) + }) + if s.scheme == rawdb.PathScheme { + // Configure the dangling node cleaner and also filter out boundary nodes + // only in the context of the path scheme. Deletion is forbidden in the + // hash scheme, as it can disrupt state completeness. + options = options.WithCleaner(func(path []byte) { + s.cleanPath(batch, owner, path) + }) + // Skip the left boundary as it's not the first range + // Skip the right boundary if it's not the last range. + options = options.WithSkipBoundary(true, r.End() != common.MaxHash, boundaryStorageNodesGauge) + } tasks = append(tasks, &storageTask{ Next: r.Start(), Last: r.End(), root: acc.Root, genBatch: batch, - genTrie: trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { - rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme) - }, account), + genTrie: trie.NewStackTrie(options), }) } for _, task := range tasks { @@ -2041,9 +2124,23 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { slots += len(res.hashes[i]) if i < len(res.hashes)-1 || res.subTask == nil { - tr := trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { - rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme) - }, account) + // no need to make local reassignment of account: this closure does not outlive the loop + options := trie.NewStackTrieOptions() + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(batch, account, path, hash, blob, s.scheme) + }) + if s.scheme == rawdb.PathScheme { + // Configure the dangling node cleaner only in the context of the + // path scheme. Deletion is forbidden in the hash scheme, as it can + // disrupt state completeness. + // + // Notably, boundary nodes can be also kept because the whole storage + // trie is complete. + options = options.WithCleaner(func(path []byte) { + s.cleanPath(batch, account, path) + }) + } + tr := trie.NewStackTrie(options) for j := 0; j < len(res.hashes[i]); j++ { tr.Update(res.hashes[i][j][:], res.slots[i][j]) } @@ -2065,18 +2162,25 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { // Large contracts could have generated new trie nodes, flush them to disk if res.subTask != nil { if res.subTask.done { - if root, err := res.subTask.genTrie.Commit(); err != nil { - log.Error("Failed to commit stack slots", "err", err) - } else if root == res.subTask.root { - // If the chunk's root is an overflown but full delivery, clear the heal request + root := res.subTask.genTrie.Commit() + if err := res.subTask.genBatch.Write(); err != nil { + log.Error("Failed to persist stack slots", "err", err) + } + res.subTask.genBatch.Reset() + + // If the chunk's root is an overflown but full delivery, + // clear the heal request. + accountHash := res.accounts[len(res.accounts)-1] + if root == res.subTask.root && rawdb.HasStorageTrieNode(s.db, accountHash, nil, root) { for i, account := range res.mainTask.res.hashes { - if account == res.accounts[len(res.accounts)-1] { + if account == accountHash { res.mainTask.needHeal[i] = false + skipStorageHealingGauge.Inc(1) } } } } - if res.subTask.genBatch.ValueSize() > ethdb.IdealBatchSize || res.subTask.done { + if res.subTask.genBatch.ValueSize() > ethdb.IdealBatchSize { if err := res.subTask.genBatch.Write(); err != nil { log.Error("Failed to persist stack slots", "err", err) } @@ -2283,9 +2387,7 @@ func (s *Syncer) forwardAccountTask(task *accountTask) { // flush after finalizing task.done. It's fine even if we crash and lose this // write as it will only cause more data to be downloaded during heal. if task.done { - if _, err := task.genTrie.Commit(); err != nil { - log.Error("Failed to commit stack account", "err", err) - } + task.genTrie.Commit() } if task.genBatch.ValueSize() > ethdb.IdealBatchSize || task.done { if err := task.genBatch.Write(); err != nil { diff --git a/ethdb/dbtest/testsuite.go b/ethdb/dbtest/testsuite.go index a2b7003c27..30cef82bec 100644 --- a/ethdb/dbtest/testsuite.go +++ b/ethdb/dbtest/testsuite.go @@ -272,9 +272,13 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { b.Put([]byte("5"), nil) b.Delete([]byte("1")) b.Put([]byte("6"), nil) - b.Delete([]byte("3")) + + b.Delete([]byte("3")) // delete then put b.Put([]byte("3"), nil) + b.Put([]byte("7"), nil) // put then delete + b.Delete([]byte("7")) + if err := b.Write(); err != nil { t.Fatal(err) } diff --git a/tests/fuzzers/stacktrie/trie_fuzzer.go b/tests/fuzzers/stacktrie/trie_fuzzer.go index 9ade6f2b2f..0e291d7b9d 100644 --- a/tests/fuzzers/stacktrie/trie_fuzzer.go +++ b/tests/fuzzers/stacktrie/trie_fuzzer.go @@ -153,9 +153,10 @@ func (f *fuzzer) fuzz() int { trieA = trie.NewEmpty(dbA) spongeB = &spongeDb{sponge: sha3.NewLegacyKeccak256()} dbB = trie.NewDatabase(rawdb.NewDatabase(spongeB), nil) - trieB = trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { - rawdb.WriteTrieNode(spongeB, owner, path, hash, blob, dbB.Scheme()) + options = trie.NewStackTrieOptions().WithWriter(func(path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(spongeB, common.Hash{}, path, hash, blob, dbB.Scheme()) }) + trieB = trie.NewStackTrie(options) vals kvs useful bool maxElements = 10000 @@ -203,9 +204,7 @@ func (f *fuzzer) fuzz() int { trieB.Update(kv.k, kv.v) } rootB := trieB.Hash() - if _, err := trieB.Commit(); err != nil { - panic(err) - } + trieB.Commit() if rootA != rootB { panic(fmt.Sprintf("roots differ: (trie) %x != %x (stacktrie)", rootA, rootB)) } @@ -217,22 +216,20 @@ func (f *fuzzer) fuzz() int { // Ensure all the nodes are persisted correctly // Need tracked deleted nodes. var ( - nodeset = make(map[string][]byte) // path -> blob - trieC = trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { + nodeset = make(map[string][]byte) // path -> blob + optionsC = trie.NewStackTrieOptions().WithWriter(func(path []byte, hash common.Hash, blob []byte) { if crypto.Keccak256Hash(blob) != hash { panic("invalid node blob") } - if owner != (common.Hash{}) { - panic("invalid node owner") - } nodeset[string(path)] = common.CopyBytes(blob) }) + trieC = trie.NewStackTrie(optionsC) checked int ) for _, kv := range vals { trieC.Update(kv.k, kv.v) } - rootC, _ := trieC.Commit() + rootC := trieC.Commit() if rootA != rootC { panic(fmt.Sprintf("roots differ: (trie) %x != %x (stacktrie)", rootA, rootC)) } diff --git a/trie/iterator_test.go b/trie/iterator_test.go index 240aa25284..f173e3d45b 100644 --- a/trie/iterator_test.go +++ b/trie/iterator_test.go @@ -86,6 +86,10 @@ type kv struct { t bool } +func (k *kv) cmp(other *kv) int { + return bytes.Compare(k.k, other.k) +} + func TestIteratorLargeData(t *testing.T) { trie := NewEmpty(NewDatabase(rawdb.NewMemoryDatabase(), nil)) vals := make(map[string]*kv) diff --git a/trie/proof.go b/trie/proof.go index 52673e19b3..437cf3d00b 100644 --- a/trie/proof.go +++ b/trie/proof.go @@ -502,7 +502,7 @@ func VerifyRangeProof(rootHash common.Hash, firstKey []byte, lastKey []byte, key if proof == nil { tr := NewStackTrie(nil) for index, key := range keys { - tr.TryUpdate(key, values[index]) + tr.Update(key, values[index]) } if have, want := tr.Hash(), rootHash; have != want { return false, fmt.Errorf("invalid proof, want hash %x, got %x", want, have) diff --git a/trie/stacktrie.go b/trie/stacktrie.go index 9fcc0831ee..8a18a6f86f 100644 --- a/trie/stacktrie.go +++ b/trie/stacktrie.go @@ -17,179 +17,145 @@ package trie import ( - "bufio" "bytes" - "encoding/gob" - "errors" "fmt" - "io" "sync" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/metrics" ) -var ErrCommitDisabled = errors.New("no database for committing") +var ( + stPool = sync.Pool{New: func() any { return new(stNode) }} + _ = types.TrieHasher((*StackTrie)(nil)) // Ensure StackTrie implements the TrieHasher interface +) + +// StackTrieOptions contains the configured options for manipulating the stackTrie. +type StackTrieOptions struct { + Writer func(path []byte, hash common.Hash, blob []byte) // The function to commit the dirty nodes + Cleaner func(path []byte) // The function to clean up dangling nodes -var stPool = sync.Pool{ - New: func() interface{} { - return NewStackTrie(nil) - }, + SkipLeftBoundary bool // Flag whether the nodes on the left boundary are skipped for committing + SkipRightBoundary bool // Flag whether the nodes on the right boundary are skipped for committing + boundaryGauge metrics.Gauge // Gauge to track how many boundary nodes are met } -// NodeWriteFunc is used to provide all information of a dirty node for committing -// so that callers can flush nodes into database with desired scheme. -type NodeWriteFunc = func(owner common.Hash, path []byte, hash common.Hash, blob []byte) +// NewStackTrieOptions initializes an empty options for stackTrie. +func NewStackTrieOptions() *StackTrieOptions { return &StackTrieOptions{} } -func stackTrieFromPool(writenFn NodeWriteFunc, owner common.Hash) *StackTrie { - st := stPool.Get().(*StackTrie) - st.owner = owner - st.writeFn = writenFn - return st +// WithWriter configures trie node writer within the options. +func (o *StackTrieOptions) WithWriter(writer func(path []byte, hash common.Hash, blob []byte)) *StackTrieOptions { + o.Writer = writer + return o +} + +// WithCleaner configures the cleaner in the option for removing dangling nodes. +func (o *StackTrieOptions) WithCleaner(cleaner func(path []byte)) *StackTrieOptions { + o.Cleaner = cleaner + return o } -func returnToPool(st *StackTrie) { - st.Reset() - stPool.Put(st) +// WithSkipBoundary configures whether the left and right boundary nodes are +// filtered for committing, along with a gauge metrics to track how many +// boundary nodes are met. +func (o *StackTrieOptions) WithSkipBoundary(skipLeft, skipRight bool, gauge metrics.Gauge) *StackTrieOptions { + o.SkipLeftBoundary = skipLeft + o.SkipRightBoundary = skipRight + o.boundaryGauge = gauge + return o } // StackTrie is a trie implementation that expects keys to be inserted // in order. Once it determines that a subtree will no longer be inserted // into, it will hash it and free up the memory it uses. type StackTrie struct { - owner common.Hash // the owner of the trie - nodeType uint8 // node type (as in branch, ext, leaf) - val []byte // value contained by this node if it's a leaf - key []byte // key chunk covered by this (full|ext) node - children [16]*StackTrie // list of children (for fullnodes and exts) - writeFn NodeWriteFunc // function for commiting nodes, can be nil + options *StackTrieOptions + root *stNode + h *hasher + + first []byte // The (hex-encoded without terminator) key of first inserted entry, tracked as left boundary. + last []byte // The (hex-encoded without terminator) key of last inserted entry, tracked as right boundary. } // NewStackTrie allocates and initializes an empty trie. -func NewStackTrie(writeFn NodeWriteFunc) *StackTrie { - return &StackTrie{ - nodeType: emptyNode, - writeFn: writeFn, // function for committing nodes, can be nil +func NewStackTrie(options *StackTrieOptions) *StackTrie { + if options == nil { + options = NewStackTrieOptions() } -} - -// NewStackTrieWithOwner allocates and initializes an empty trie, but with -// the additional owner field. -func NewStackTrieWithOwner(writeFn NodeWriteFunc, owner common.Hash) *StackTrie { return &StackTrie{ - owner: owner, - nodeType: emptyNode, - writeFn: writeFn, // function for committing nodes, can be nil + options: options, + root: stPool.Get().(*stNode), + h: newHasher(false), } } -// NewFromBinary initialises a serialized stacktrie with the given db. -func NewFromBinary(data []byte, writeFn NodeWriteFunc) (*StackTrie, error) { - var st StackTrie - if err := st.UnmarshalBinary(data); err != nil { - return nil, err - } - // If a database is used, we need to recursively add it to every child - if writeFn != nil { - st.setWriteFunc(writeFn) +func (t *StackTrie) Update(key, value []byte) { + if err := t.TryUpdate(key, value); err != nil { + log.Error(fmt.Sprintf("Unhandled trie error: %v", err)) } - return &st, nil } -// MarshalBinary implements encoding.BinaryMarshaler -func (st *StackTrie) MarshalBinary() (data []byte, err error) { - var ( - b bytes.Buffer - w = bufio.NewWriter(&b) - ) - if err := gob.NewEncoder(w).Encode(struct { - Owner common.Hash - NodeType uint8 - Val []byte - Key []byte - }{ - st.owner, - st.nodeType, - st.val, - st.key, - }); err != nil { - return nil, err - } - for _, child := range st.children { - if child == nil { - w.WriteByte(0) - continue - } - w.WriteByte(1) - if childData, err := child.MarshalBinary(); err != nil { - return nil, err - } else { - w.Write(childData) - } +// Update inserts a (key, value) pair into the stack trie. +func (t *StackTrie) TryUpdate(key, value []byte) error { + k := keybytesToHex(key) + if len(value) == 0 { + panic("deletion not supported") } - w.Flush() - return b.Bytes(), nil -} - -// UnmarshalBinary implements encoding.BinaryUnmarshaler -func (st *StackTrie) UnmarshalBinary(data []byte) error { - r := bytes.NewReader(data) - return st.unmarshalBinary(r) -} + k = k[:len(k)-1] // chop the termination flag -func (st *StackTrie) unmarshalBinary(r io.Reader) error { - var dec struct { - Owner common.Hash - NodeType uint8 - Val []byte - Key []byte + // track the first and last inserted entries. + if t.first == nil { + t.first = append([]byte{}, k...) } - gob.NewDecoder(r).Decode(&dec) - st.owner = dec.Owner - st.nodeType = dec.NodeType - st.val = dec.Val - st.key = dec.Key - - var hasChild = make([]byte, 1) - for i := range st.children { - if _, err := r.Read(hasChild); err != nil { - return err - } else if hasChild[0] == 0 { - continue - } - var child StackTrie - child.unmarshalBinary(r) - st.children[i] = &child + if t.last == nil { + t.last = append([]byte{}, k...) // allocate key slice + } else { + t.last = append(t.last[:0], k...) // reuse key slice } + t.insert(t.root, k, value, nil) return nil } -func (st *StackTrie) setWriteFunc(writeFn NodeWriteFunc) { - st.writeFn = writeFn - for _, child := range st.children { - if child != nil { - child.setWriteFunc(writeFn) - } - } +// Reset resets the stack trie object to empty state. +func (t *StackTrie) Reset() { + t.options = NewStackTrieOptions() + t.root = stPool.Get().(*stNode) + t.first = nil + t.last = nil } -func newLeaf(owner common.Hash, key, val []byte, writeFn NodeWriteFunc) *StackTrie { - st := stackTrieFromPool(writeFn, owner) - st.nodeType = leafNode +// stNode represents a node within a StackTrie +type stNode struct { + typ uint8 // node type (as in branch, ext, leaf) + key []byte // key chunk covered by this (leaf|ext) node + val []byte // value contained by this node if it's a leaf + children [16]*stNode // list of children (for branch and exts) +} + +// newLeaf constructs a leaf node with provided node key and value. The key +// will be deep-copied in the function and safe to modify afterwards, but +// value is not. +func newLeaf(key, val []byte) *stNode { + st := stPool.Get().(*stNode) + st.typ = leafNode st.key = append(st.key, key...) st.val = val return st } -func newExt(owner common.Hash, key []byte, child *StackTrie, writeFn NodeWriteFunc) *StackTrie { - st := stackTrieFromPool(writeFn, owner) - st.nodeType = extNode +// newExt constructs an extension node with provided node key and child. The +// key will be deep-copied in the function and safe to modify afterwards. +func newExt(key []byte, child *stNode) *stNode { + st := stPool.Get().(*stNode) + st.typ = extNode st.key = append(st.key, key...) st.children[0] = child return st } -// List all values that StackTrie#nodeType can hold +// List all values that stNode#nodeType can hold const ( emptyNode = iota branchNode @@ -198,65 +164,48 @@ const ( hashedNode ) -// TryUpdate inserts a (key, value) pair into the stack trie -func (st *StackTrie) TryUpdate(key, value []byte) error { - k := keybytesToHex(key) - if len(value) == 0 { - panic("deletion not supported") - } - st.insert(k[:len(k)-1], value, nil) - return nil -} - -func (st *StackTrie) Update(key, value []byte) { - if err := st.TryUpdate(key, value); err != nil { - log.Error(fmt.Sprintf("Unhandled trie error: %v", err)) +func (n *stNode) reset() *stNode { + n.key = n.key[:0] + n.val = nil + for i := range n.children { + n.children[i] = nil } -} - -func (st *StackTrie) Reset() { - st.owner = common.Hash{} - st.writeFn = nil - st.key = st.key[:0] - st.val = nil - for i := range st.children { - st.children[i] = nil - } - st.nodeType = emptyNode + n.typ = emptyNode + return n } // Helper function that, given a full key, determines the index // at which the chunk pointed by st.keyOffset is different from // the same chunk in the full key. -func (st *StackTrie) getDiffIndex(key []byte) int { - for idx, nibble := range st.key { +func (n *stNode) getDiffIndex(key []byte) int { + for idx, nibble := range n.key { if nibble != key[idx] { return idx } } - return len(st.key) + return len(n.key) } // Helper function to that inserts a (key, value) pair into -// the trie. Adding the prefix when inserting too. -func (st *StackTrie) insert(key, value, prefix []byte) { - switch st.nodeType { +// the trie. +func (t *StackTrie) insert(st *stNode, key, value []byte, path []byte) { + switch st.typ { case branchNode: /* Branch */ idx := int(key[0]) // Unresolve elder siblings for i := idx - 1; i >= 0; i-- { if st.children[i] != nil { - if st.children[i].nodeType != hashedNode { - st.children[i].hash(append(prefix, byte(i))) + if st.children[i].typ != hashedNode { + t.hash(st.children[i], append(path, byte(i))) } break } } // Add new child if st.children[idx] == nil { - st.children[idx] = newLeaf(st.owner, key[1:], value, st.writeFn) + st.children[idx] = newLeaf(key[1:], value) } else { - st.children[idx].insert(key[1:], value, append(prefix, key[0])) + t.insert(st.children[idx], key[1:], value, append(path, key[0])) } case extNode: /* Ext */ @@ -271,46 +220,45 @@ func (st *StackTrie) insert(key, value, prefix []byte) { if diffidx == len(st.key) { // Ext key and key segment are identical, recurse into // the child node. - st.children[0].insert(key[diffidx:], value, append(prefix, key[:diffidx]...)) + t.insert(st.children[0], key[diffidx:], value, append(path, key[:diffidx]...)) return } // Save the original part. Depending if the break is // at the extension's last byte or not, create an // intermediate extension or use the extension's child // node directly. - var n *StackTrie + var n *stNode if diffidx < len(st.key)-1 { // Break on the non-last byte, insert an intermediate // extension. The path prefix of the newly-inserted // extension should also contain the different byte. - n = newExt(st.owner, st.key[diffidx+1:], st.children[0], st.writeFn) - n.hash(append(prefix, st.key[:diffidx+1]...)) + n = newExt(st.key[diffidx+1:], st.children[0]) + t.hash(n, append(path, st.key[:diffidx+1]...)) } else { // an extension node: reuse the current node. // The path prefix of the original part should // still be same. n = st.children[0] - n.hash(append(prefix, st.key...)) + t.hash(n, append(path, st.key...)) } - - var p *StackTrie + var p *stNode if diffidx == 0 { // the break is on the first byte, so // the current node is converted into // a branch node. st.children[0] = nil p = st - st.nodeType = branchNode + st.typ = branchNode } else { // the common prefix is at least one byte // long, insert a new intermediate branch // node. - st.children[0] = stackTrieFromPool(st.writeFn, st.owner) - st.children[0].nodeType = branchNode + st.children[0] = stPool.Get().(*stNode) + st.children[0].typ = branchNode p = st.children[0] } // Create a leaf for the inserted part - o := newLeaf(st.owner, key[diffidx+1:], value, st.writeFn) + o := newLeaf(key[diffidx+1:], value) // Insert both child leaves where they belong: origIdx := st.key[diffidx] @@ -336,18 +284,18 @@ func (st *StackTrie) insert(key, value, prefix []byte) { // Check if the split occurs at the first nibble of the // chunk. In that case, no prefix extnode is necessary. // Otherwise, create that - var p *StackTrie + var p *stNode if diffidx == 0 { // Convert current leaf into a branch - st.nodeType = branchNode + st.typ = branchNode p = st st.children[0] = nil } else { // Convert current node into an ext, // and insert a child branch node. - st.nodeType = extNode - st.children[0] = NewStackTrieWithOwner(st.writeFn, st.owner) - st.children[0].nodeType = branchNode + st.typ = extNode + st.children[0] = stPool.Get().(*stNode) + st.children[0].typ = branchNode p = st.children[0] } @@ -356,18 +304,18 @@ func (st *StackTrie) insert(key, value, prefix []byte) { // The child leave will be hashed directly in order to // free up some memory. origIdx := st.key[diffidx] - p.children[origIdx] = newLeaf(st.owner, st.key[diffidx+1:], st.val, st.writeFn) - p.children[origIdx].hash(append(prefix, st.key[:diffidx+1]...)) + p.children[origIdx] = newLeaf(st.key[diffidx+1:], st.val) + t.hash(p.children[origIdx], append(path, st.key[:diffidx+1]...)) newIdx := key[diffidx] - p.children[newIdx] = newLeaf(st.owner, key[diffidx+1:], value, st.writeFn) + p.children[newIdx] = newLeaf(key[diffidx+1:], value) // Finally, cut off the key part that has been passed // over to the children. st.key = st.key[:diffidx] st.val = nil case emptyNode: /* Empty */ - st.nodeType = leafNode + st.typ = leafNode st.key = key st.val = value case hashedNode: @@ -390,20 +338,21 @@ func (st *StackTrie) insert(key, value, prefix []byte) { // This method will also: // set 'st.type' to hashedNode // clear 'st.key' -func (st *StackTrie) hash(path []byte) { +func (t *StackTrie) hash(st *stNode, path []byte) { /* Shortcut if node is already hashed */ - if st.nodeType == hashedNode { + if st.typ == hashedNode { return } // The 'hasher' is taken from a pool, but we don't actually // claim an instance until all children are done with their hashing, // and we actually need one + var ( - h *hasher - encodedNode []byte + blob []byte // RLP-encoded node blob + internal [][]byte // List of node paths covered by the extension node ) - switch st.nodeType { + switch st.typ { case branchNode: var node fullNode for i, child := range st.children { @@ -411,22 +360,32 @@ func (st *StackTrie) hash(path []byte) { node.Children[i] = nilValueNode continue } - child.hash(append(path, byte(i))) + t.hash(child, append(path, byte(i))) if len(child.val) < 32 { node.Children[i] = rawNode(child.val) } else { node.Children[i] = hashNode(child.val) } - st.children[i] = nil // Reclaim mem from subtree - returnToPool(child) + st.children[i] = nil // Reclaim mem from subtree + stPool.Put(child.reset()) // Release child back to pool. } - h = newHasher(false) - defer returnHasherToPool(h) - node.encode(h.encbuf) - encodedNode = h.encodedBytes() + node.encode(t.h.encbuf) + blob = t.h.encodedBytes() case extNode: - st.children[0].hash(append(path, st.key...)) + // recursively hash and commit child as the first step + t.hash(st.children[0], append(path, st.key...)) + + // Collect the path of internal nodes between shortNode and its **in disk** + // child. This is essential in the case of path mode scheme to avoid leaving + // danging nodes within the range of this internal path on disk, which would + // break the guarantee for state healing. + if len(st.children[0].val) >= 32 && t.options.Cleaner != nil { + for i := 1; i < len(st.key); i++ { + internal = append(internal, append(path, st.key[:i]...)) + } + } + // encode the extension node sz := hexToCompactInPlace(st.key) n := shortNode{Key: st.key[:sz]} @@ -436,93 +395,83 @@ func (st *StackTrie) hash(path []byte) { n.Val = hashNode(st.children[0].val) } - h = newHasher(false) - defer returnHasherToPool(h) + n.encode(t.h.encbuf) + blob = t.h.encodedBytes() - n.encode(h.encbuf) - encodedNode = h.encodedBytes() - - returnToPool(st.children[0]) - st.children[0] = nil // Reclaim mem from subtree + stPool.Put(st.children[0].reset()) // Release child back to pool. + st.children[0] = nil // Reclaim mem from subtree case leafNode: - h = newHasher(false) - defer returnHasherToPool(h) st.key = append(st.key, byte(16)) sz := hexToCompactInPlace(st.key) n := shortNode{Key: st.key[:sz], Val: valueNode(st.val)} - n.encode(h.encbuf) - encodedNode = h.encodedBytes() + n.encode(t.h.encbuf) + blob = t.h.encodedBytes() case emptyNode: st.val = emptyRoot.Bytes() st.key = st.key[:0] - st.nodeType = hashedNode + st.typ = hashedNode return default: panic("Invalid node type") } st.key = st.key[:0] - st.nodeType = hashedNode - if len(encodedNode) < 32 { + st.typ = hashedNode + // Skip committing the non-root node if the size is smaller than 32 bytes. + if len(blob) < 32 && len(path) > 0 { // If rlp-encoded value was < 32 bytes, then val point directly to the rlp-encoded value - st.val = common.CopyBytes(encodedNode) + st.val = common.CopyBytes(blob) return } - h = newHasher(false) - defer returnHasherToPool(h) - - st.val = h.hashData(encodedNode) + st.val = t.h.hashData(blob) - if st.writeFn != nil { - st.writeFn(st.owner, path, common.BytesToHash(st.val), encodedNode) + // Short circuit if the stack trie is not configured for writing. + if t.options.Writer == nil { + return + } + // Skip committing if the node is on the left boundary and stackTrie is + // configured to filter the boundary. + if t.options.SkipLeftBoundary && bytes.HasPrefix(t.first, path) { + if t.options.boundaryGauge != nil { + t.options.boundaryGauge.Inc(1) + } + return + } + // Skip committing if the node is on the right boundary and stackTrie is + // configured to filter the boundary. + if t.options.SkipRightBoundary && bytes.HasPrefix(t.last, path) { + if t.options.boundaryGauge != nil { + t.options.boundaryGauge.Inc(1) + } + return } + // Clean up the internal dangling nodes covered by the extension node. + // This should be done before writing the node to adhere to the committing + // order from bottom to top. + for _, path := range internal { + t.options.Cleaner(path) + } + t.options.Writer(path, common.BytesToHash(st.val), blob) } -// Hash returns the hash of the current node -func (st *StackTrie) Hash() (h common.Hash) { - st.hash(nil) - if len(st.val) != 32 { - // If the node's RLP isn't 32 bytes long, the node will not - // be hashed, and instead contain the rlp-encoding of the - // node. For the top level node, we need to force the hashing. - ret := make([]byte, 32) - h := newHasher(false) - defer returnHasherToPool(h) - h.sha.Reset() - h.sha.Write(st.val) - h.sha.Read(ret) - return common.BytesToHash(ret) - } - return common.BytesToHash(st.val) +// Hash will firstly hash the entire trie if it's still not hashed and then commit +// all nodes to the associated database. Actually most of the trie nodes have been +// committed already. The main purpose here is to commit the nodes on right boundary. +// +// For stack trie, Hash and Commit are functionally identical +func (t *StackTrie) Hash() (h common.Hash) { + n := t.root + t.hash(n, nil) + return common.BytesToHash(n.val) } -// Commit will firstly hash the entrie trie if it's still not hashed -// and then commit all nodes to the associated database. Actually most -// of the trie nodes MAY have been committed already. The main purpose -// here is to commit the root node. +// Commit will firstly hash the entire trie if it's still not hashed and then commit +// all nodes to the associated database. Actually most of the trie nodes have been +// committed already. The main purpose here is to commit the nodes on right boundary. // -// The associated database is expected, otherwise the whole commit -// functionality should be disabled. -func (st *StackTrie) Commit() (common.Hash, error) { - if st.writeFn == nil { - return common.Hash{}, ErrCommitDisabled - } - st.hash(nil) - if len(st.val) != 32 { - // If the node's RLP isn't 32 bytes long, the node will not - // be hashed (and committed), and instead contain the rlp-encoding of the - // node. For the top level node, we need to force the hashing+commit. - ret := make([]byte, 32) - h := newHasher(false) - defer returnHasherToPool(h) - h.sha.Reset() - // hash st.val -> ret - h.sha.Write(st.val) - h.sha.Read(ret) - st.writeFn(st.owner, nil, common.BytesToHash(ret), st.val) - return common.BytesToHash(ret), nil - } - return common.BytesToHash(st.val), nil +// For stack trie, Hash and Commit are functionally identical. +func (t *StackTrie) Commit() common.Hash { + return t.Hash() } diff --git a/trie/stacktrie_test.go b/trie/stacktrie_test.go index 10baeaf441..6f93c1f53a 100644 --- a/trie/stacktrie_test.go +++ b/trie/stacktrie_test.go @@ -19,11 +19,14 @@ package trie import ( "bytes" "math/big" + "math/rand" "testing" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/trie/testutil" + "golang.org/x/exp/slices" ) func TestStackTrieInsertAndHash(t *testing.T) { @@ -166,12 +169,11 @@ func TestStackTrieInsertAndHash(t *testing.T) { {"13aa", "x___________________________3", "ff0dc70ce2e5db90ee42a4c2ad12139596b890e90eb4e16526ab38fa465b35cf"}, }, } - st := NewStackTrie(nil) for i, test := range tests { // The StackTrie does not allow Insert(), Hash(), Insert(), ... // so we will create new trie for every sequence length of inserts. for l := 1; l <= len(test); l++ { - st.Reset() + st := NewStackTrie(nil) for j := 0; j < l; j++ { kv := &test[j] if err := st.TryUpdate(common.FromHex(kv.K), []byte(kv.V)); err != nil { @@ -350,47 +352,86 @@ func TestStacktrieNotModifyValues(t *testing.T) { } } -// TestStacktrieSerialization tests that the stacktrie works well if we -// serialize/unserialize it a lot -func TestStacktrieSerialization(t *testing.T) { +func buildPartialTree(entries []*kv, t *testing.T) map[string]common.Hash { var ( - st = NewStackTrie(nil) - nt = NewEmpty(NewDatabase(rawdb.NewMemoryDatabase(), nil)) - keyB = big.NewInt(1) - keyDelta = big.NewInt(1) - vals [][]byte - keys [][]byte + options = NewStackTrieOptions() + nodes = make(map[string]common.Hash) ) - getValue := func(i int) []byte { - if i%2 == 0 { // large - return crypto.Keccak256(big.NewInt(int64(i)).Bytes()) - } else { //small - return big.NewInt(int64(i)).Bytes() + var ( + first int + last = len(entries) - 1 + + noLeft bool + noRight bool + ) + // Enter split mode if there are at least two elements + if rand.Intn(5) != 0 { + for { + first = rand.Intn(len(entries)) + last = rand.Intn(len(entries)) + if first <= last { + break + } + } + if first != 0 { + noLeft = true + } + if last != len(entries)-1 { + noRight = true } } - for i := 0; i < 10; i++ { - vals = append(vals, getValue(i)) - keys = append(keys, common.BigToHash(keyB).Bytes()) - keyB = keyB.Add(keyB, keyDelta) - keyDelta.Add(keyDelta, common.Big1) - } - for i, k := range keys { - nt.TryUpdate(k, common.CopyBytes(vals[i])) + options = options.WithSkipBoundary(noLeft, noRight, nil) + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { + nodes[string(path)] = hash + }) + tr := NewStackTrie(options) + + for i := first; i <= last; i++ { + tr.TryUpdate(entries[i].k, entries[i].v) } + tr.Commit() + return nodes +} + +func TestPartialStackTrie(t *testing.T) { + for round := 0; round < 100; round++ { + var ( + n = rand.Intn(100) + 1 + entries []*kv + ) + for i := 0; i < n; i++ { + var val []byte + if rand.Intn(3) == 0 { + val = testutil.RandBytes(3) + } else { + val = testutil.RandBytes(32) + } + entries = append(entries, &kv{ + k: testutil.RandBytes(32), + v: val, + }) + } + slices.SortFunc(entries, (*kv).cmp) + + var ( + nodes = make(map[string]common.Hash) + options = NewStackTrieOptions().WithWriter(func(path []byte, hash common.Hash, blob []byte) { + nodes[string(path)] = hash + }) + ) + tr := NewStackTrie(options) - for i, k := range keys { - blob, err := st.MarshalBinary() - if err != nil { - t.Fatal(err) + for i := 0; i < len(entries); i++ { + tr.TryUpdate(entries[i].k, entries[i].v) } - newSt, err := NewFromBinary(blob, nil) - if err != nil { - t.Fatal(err) + tr.Commit() + + for j := 0; j < 100; j++ { + for path, hash := range buildPartialTree(entries, t) { + if nodes[path] != hash { + t.Errorf("%v, want %x, got %x", []byte(path), nodes[path], hash) + } + } } - st = newSt - st.TryUpdate(k, common.CopyBytes(vals[i])) - } - if have, want := st.Hash(), nt.Hash(); have != want { - t.Fatalf("have %#x want %#x", have, want) } } diff --git a/trie/sync.go b/trie/sync.go index 2b257fe2f7..f7f45d3ae1 100644 --- a/trie/sync.go +++ b/trie/sync.go @@ -19,6 +19,7 @@ package trie import ( "errors" "fmt" + "sync" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/prque" @@ -49,6 +50,18 @@ var ( // lookupGauge is the metric to track how many trie node lookups are // performed to determine if node needs to be deleted. lookupGauge = metrics.NewRegisteredGauge("trie/sync/lookup", nil) + + // accountNodeSyncedGauge is the metric to track how many account trie + // node are written during the sync. + accountNodeSyncedGauge = metrics.NewRegisteredGauge("trie/sync/nodes/account", nil) + + // storageNodeSyncedGauge is the metric to track how many account trie + // node are written during the sync. + storageNodeSyncedGauge = metrics.NewRegisteredGauge("trie/sync/nodes/storage", nil) + + // codeSyncedGauge is the metric to track how many contract codes are + // written during the sync. + codeSyncedGauge = metrics.NewRegisteredGauge("trie/sync/codes", nil) ) // SyncPath is a path tuple identifying a particular trie node either in a single @@ -86,10 +99,9 @@ func NewSyncPath(path []byte) SyncPath { // nodeRequest represents a scheduled or already in-flight trie node retrieval request. type nodeRequest struct { - hash common.Hash // Hash of the trie node to retrieve - path []byte // Merkle path leading to this node for prioritization - data []byte // Data content of the node, cached until all subtrees complete - deletes [][]byte // List of internal path segments for trie nodes to delete + hash common.Hash // Hash of the trie node to retrieve + path []byte // Merkle path leading to this node for prioritization + data []byte // Data content of the node, cached until all subtrees complete parent *nodeRequest // Parent state node referencing this entry deps int // Number of dependencies before allowed to commit this node @@ -116,37 +128,69 @@ type CodeSyncResult struct { Data []byte // Data content of the retrieved bytecode } +// nodeOp represents an operation upon the trie node. It can either represent a +// deletion to the specific node or a node write for persisting retrieved node. +type nodeOp struct { + owner common.Hash // identifier of the trie (empty for account trie) + path []byte // path from the root to the specified node. + blob []byte // the content of the node (nil for deletion) + hash common.Hash // hash of the node content (empty for node deletion) +} + +// isDelete indicates if the operation is a database deletion. +func (op *nodeOp) isDelete() bool { + return len(op.blob) == 0 +} + // syncMemBatch is an in-memory buffer of successfully downloaded but not yet // persisted data items. type syncMemBatch struct { - nodes map[string][]byte // In-memory membatch of recently completed nodes - hashes map[string]common.Hash // Hashes of recently completed nodes - deletes map[string]struct{} // List of paths for trie node to delete - codes map[common.Hash][]byte // In-memory membatch of recently completed codes + scheme string // State scheme identifier + nodes []nodeOp // In-memory batch of recently completed/deleted nodes + codes map[common.Hash][]byte // In-memory membatch of recently completed codes } // newSyncMemBatch allocates a new memory-buffer for not-yet persisted trie nodes. -func newSyncMemBatch() *syncMemBatch { +func newSyncMemBatch(scheme string) *syncMemBatch { return &syncMemBatch{ - nodes: make(map[string][]byte), - hashes: make(map[string]common.Hash), - deletes: make(map[string]struct{}), - codes: make(map[common.Hash][]byte), + scheme: scheme, + codes: make(map[common.Hash][]byte), } } -// hasNode reports the trie node with specific path is already cached. -func (batch *syncMemBatch) hasNode(path []byte) bool { - _, ok := batch.nodes[string(path)] - return ok -} - // hasCode reports the contract code with specific hash is already cached. func (batch *syncMemBatch) hasCode(hash common.Hash) bool { _, ok := batch.codes[hash] return ok } +// addCode caches a contract code database write operation. +func (batch *syncMemBatch) addCode(hash common.Hash, code []byte) { + batch.codes[hash] = code +} + +// addNode caches a node database write operation. +func (batch *syncMemBatch) addNode(owner common.Hash, path []byte, blob []byte, hash common.Hash) { + batch.nodes = append(batch.nodes, nodeOp{ + owner: owner, + path: path, + blob: blob, + hash: hash, + }) +} + +// delNode caches a node database delete operation. +func (batch *syncMemBatch) delNode(owner common.Hash, path []byte) { + if batch.scheme != rawdb.PathScheme { + log.Error("Unexpected node deletion", "owner", owner, "path", path, "scheme", batch.scheme) + return // deletion is not supported in hash mode. + } + batch.nodes = append(batch.nodes, nodeOp{ + owner: owner, + path: path, + }) +} + // Sync is the main state trie synchronisation scheduler, which provides yet // unknown trie hashes to retrieve, accepts node data associated with said hashes // and reconstructs the trie step by step until all is done. @@ -182,7 +226,7 @@ func NewSync(root common.Hash, database ethdb.KeyValueReader, callback LeafCallb ts := &Sync{ scheme: scheme, database: database, - membatch: newSyncMemBatch(), + membatch: newSyncMemBatch(scheme), nodeReqs: make(map[string]*nodeRequest), codeReqs: make(map[common.Hash]*codeRequest), queue: prque.New(nil), @@ -201,16 +245,18 @@ func (s *Sync) AddSubTrie(root common.Hash, path []byte, parent common.Hash, par if root == emptyRoot { return } - if s.membatch.hasNode(path) { - return - } if s.bloom == nil || s.bloom.Contains(root[:]) { // Bloom filter says this might be a duplicate, double check. // If database says yes, then at least the trie node is present // and we hold the assumption that it's NOT legacy contract code. owner, inner := ResolvePath(path) - if rawdb.HasTrieNode(s.database, owner, inner, root, s.scheme) { + exist, inconsistent := s.hasNode(owner, inner, root) + if exist { + // The entire subtrie is already present in the database. return + } else if inconsistent { + // There is a pre-existing node with the wrong hash in DB, remove it. + s.membatch.delNode(owner, inner) } // False positive, bump fault meter bloomFaultMeter.Mark(1) @@ -370,24 +416,39 @@ func (s *Sync) ProcessNode(result NodeSyncResult) error { } // Commit flushes the data stored in the internal membatch out to persistent -// storage, returning any occurred error. +// storage, returning any occurred error. The whole data set will be flushed +// in an atomic database batch. func (s *Sync) Commit(dbw ethdb.Batch) error { + var ( + account int + storage int + ) // Flush the pending node writes into database batch. - for path, value := range s.membatch.nodes { - owner, inner := ResolvePath([]byte(path)) - rawdb.WriteTrieNode(dbw, owner, inner, s.membatch.hashes[path], value, s.scheme) - hash := s.membatch.hashes[path] + for _, op := range s.membatch.nodes { + if op.isDelete() { + // node deletion is only supported in path mode. + if op.owner == (common.Hash{}) { + rawdb.DeleteAccountTrieNode(dbw, op.path) + } else { + rawdb.DeleteStorageTrieNode(dbw, op.owner, op.path) + } + deletionGauge.Inc(1) + } else { + if op.owner == (common.Hash{}) { + account += 1 + } else { + storage += 1 + } + rawdb.WriteTrieNode(dbw, op.owner, op.path, op.hash, op.blob, s.scheme) + } + hash := op.hash if s.bloom != nil { s.bloom.Add(hash[:]) } } - // Flush the pending node deletes into the database batch. - // Please note that each written and deleted node has a - // unique path, ensuring no duplication occurs. - for path := range s.membatch.deletes { - owner, inner := ResolvePath([]byte(path)) - rawdb.DeleteTrieNode(dbw, owner, inner, common.Hash{} /* unused */, s.scheme) - } + accountNodeSyncedGauge.Inc(int64(account)) + storageNodeSyncedGauge.Inc(int64(storage)) + // Flush the pending code writes into database batch. for hash, value := range s.membatch.codes { rawdb.WriteCode(dbw, hash, value) @@ -395,7 +456,9 @@ func (s *Sync) Commit(dbw ethdb.Batch) error { s.bloom.Add(hash[:]) } } - s.membatch = newSyncMemBatch() // reset the batch + codeSyncedGauge.Inc(int64(len(s.membatch.codes))) + + s.membatch = newSyncMemBatch(s.scheme) // reset the batch return nil } @@ -463,12 +526,15 @@ func (s *Sync) children(req *nodeRequest, object node) ([]*nodeRequest, error) { // child as invalid. This is essential in the case of path mode // scheme; otherwise, state healing might overwrite existing child // nodes silently while leaving a dangling parent node within the - // range of this internal path on disk. This would break the - // guarantee for state healing. + // range of this internal path on disk and the persistent state + // ends up with a very weird situation that nodes on the same path + // are not inconsistent while they all present in disk. This property + // would break the guarantee for state healing. // // While it's possible for this shortNode to overwrite a previously // existing full node, the other branches of the fullNode can be - // retained as they remain untouched and complete. + // retained as they are not accessible with the new shortNode, and + // also the whole sub-trie is still untouched and complete. // // This step is only necessary for path mode, as there is no deletion // in hash mode at all. @@ -485,8 +551,7 @@ func (s *Sync) children(req *nodeRequest, object node) ([]*nodeRequest, error) { exists = rawdb.ExistsStorageTrieNode(s.database, owner, append(inner, key[:i]...)) } if exists { - req.deletes = append(req.deletes, key[:i]) - deletionGauge.Inc(1) + s.membatch.delNode(owner, append(inner, key[:i]...)) log.Debug("Detected dangling node", "owner", owner, "path", append(inner, key[:i]...)) } } @@ -506,6 +571,7 @@ func (s *Sync) children(req *nodeRequest, object node) ([]*nodeRequest, error) { } // Iterate over the children, and request all unknown ones requests := make([]*nodeRequest, 0, len(children)) + var batchMu sync.Mutex for _, child := range children { // Notify any external watcher of a new key/value node if req.callback != nil { @@ -522,20 +588,24 @@ func (s *Sync) children(req *nodeRequest, object node) ([]*nodeRequest, error) { } } } - // If the child references another node, resolve or schedule + // If the child references another node, resolve or schedule. + // We check all children concurrently. if node, ok := (child.node).(hashNode); ok { - // Try to resolve the node from the local database - if s.membatch.hasNode(child.path) { - continue - } - chash := common.BytesToHash(node) + path := child.path + hash := common.BytesToHash(node) if s.bloom == nil || s.bloom.Contains(node) { // Bloom filter says this might be a duplicate, double check. // If database says yes, then at least the trie node is present // and we hold the assumption that it's NOT legacy contract code. - owner, inner := ResolvePath(child.path) - if rawdb.HasTrieNode(s.database, owner, inner, chash, s.scheme) { + owner, inner := ResolvePath(path) + exist, inconsistent := s.hasNode(owner, inner, hash) + if exist { continue + } else if inconsistent { + // There is a pre-existing node with the wrong hash in DB, remove it. + batchMu.Lock() + s.membatch.delNode(owner, inner) + batchMu.Unlock() } // False positive, bump fault meter @@ -543,8 +613,8 @@ func (s *Sync) children(req *nodeRequest, object node) ([]*nodeRequest, error) { } // Locally unknown node, schedule for retrieval requests = append(requests, &nodeRequest{ - path: child.path, - hash: chash, + path: path, + hash: hash, parent: req, callback: req.callback, }) @@ -558,14 +628,10 @@ func (s *Sync) children(req *nodeRequest, object node) ([]*nodeRequest, error) { // committed themselves. func (s *Sync) commitNodeRequest(req *nodeRequest) error { // Write the node content to the membatch - s.membatch.nodes[string(req.path)] = req.data - s.membatch.hashes[string(req.path)] = req.hash + owner, path := ResolvePath(req.path) + s.membatch.addNode(owner, path, req.data, req.hash) - // Delete the internal nodes which are marked as invalid - for _, segment := range req.deletes { - path := append(req.path, segment...) - s.membatch.deletes[string(path)] = struct{}{} - } + // Removed the completed node request delete(s.nodeReqs, string(req.path)) s.fetches[len(req.path)]-- @@ -586,7 +652,9 @@ func (s *Sync) commitNodeRequest(req *nodeRequest) error { // committed themselves. func (s *Sync) commitCodeRequest(req *codeRequest) error { // Write the node content to the membatch - s.membatch.codes[req.hash] = req.data + s.membatch.addCode(req.hash, req.data) + + // Removed the completed code request delete(s.codeReqs, req.hash) s.fetches[len(req.path)]-- @@ -602,6 +670,28 @@ func (s *Sync) commitCodeRequest(req *codeRequest) error { return nil } +// hasNode reports whether the specified trie node is present in the database. +// 'exists' is true when the node exists in the database and matches the given root +// hash. The 'inconsistent' return value is true when the node exists but does not +// match the expected hash. +func (s *Sync) hasNode(owner common.Hash, path []byte, hash common.Hash) (exists bool, inconsistent bool) { + // If node is running with hash scheme, check the presence with node hash. + if s.scheme == rawdb.HashScheme { + return rawdb.HasLegacyTrieNode(s.database, hash), false + } + // If node is running with path scheme, check the presence with node path. + var blob []byte + var dbHash common.Hash + if owner == (common.Hash{}) { + blob, dbHash = rawdb.ReadAccountTrieNode(s.database, path) + } else { + blob, dbHash = rawdb.ReadStorageTrieNode(s.database, owner, path) + } + exists = hash == dbHash + inconsistent = !exists && len(blob) != 0 + return exists, inconsistent +} + // ResolvePath resolves the provided composite node path by separating the // path in account trie if it's existent. func ResolvePath(path []byte) (common.Hash, []byte) { diff --git a/trie/sync_test.go b/trie/sync_test.go index a07dbccd87..75eb5809a2 100644 --- a/trie/sync_test.go +++ b/trie/sync_test.go @@ -689,8 +689,11 @@ func testSyncOrdering(t *testing.T, scheme string) { } } } - func syncWith(t *testing.T, root common.Hash, db ethdb.Database, srcDb *Database) { + syncWithHookWriter(t, root, db, srcDb, nil) +} + +func syncWithHookWriter(t *testing.T, root common.Hash, db ethdb.Database, srcDb *Database, hookWriter ethdb.KeyValueWriter) { // Create a destination trie and sync with the scheduler sched := NewSync(root, db, nil, NewSyncBloom(1, db), srcDb.Scheme()) @@ -728,8 +731,11 @@ func syncWith(t *testing.T, root common.Hash, db ethdb.Database, srcDb *Database if err := sched.Commit(batch); err != nil { t.Fatalf("failed to commit data: %v", err) } - batch.Write() - + if hookWriter != nil { + batch.Replay(hookWriter) + } else { + batch.Write() + } paths, nodes, _ = sched.Missing(0) elements = elements[:0] for i := 0; i < len(paths); i++ { @@ -899,3 +905,116 @@ func testPivotMove(t *testing.T, scheme string, tiny bool) { syncWith(t, rootC, destDisk, srcTrieDB) checkTrieContents(t, destDisk, scheme, srcTrie.Hash().Bytes(), stateC, true) } + +func TestSyncAbort(t *testing.T) { + testSyncAbort(t, rawdb.PathScheme) + testSyncAbort(t, rawdb.HashScheme) +} + +type hookWriter struct { + db ethdb.KeyValueStore + filter func(key []byte, value []byte) bool +} + +// Put inserts the given value into the key-value data store. +func (w *hookWriter) Put(key []byte, value []byte) error { + if w.filter != nil && w.filter(key, value) { + return nil + } + return w.db.Put(key, value) +} + +// Delete removes the key from the key-value data store. +func (w *hookWriter) Delete(key []byte) error { + return w.db.Delete(key) +} + +func testSyncAbort(t *testing.T, scheme string) { + var ( + srcDisk = rawdb.NewMemoryDatabase() + srcTrieDB = newTestDatabase(srcDisk, scheme) + srcTrie, _ = New(TrieID(types.EmptyRootHash), srcTrieDB) + + deleteFn = func(key []byte, tr *Trie, states map[string][]byte) { + tr.Delete(key) + delete(states, string(key)) + } + writeFn = func(key []byte, val []byte, tr *Trie, states map[string][]byte) { + if val == nil { + val = randBytes(32) + } + tr.Update(key, val) + states[string(key)] = common.CopyBytes(val) + } + copyStates = func(states map[string][]byte) map[string][]byte { + cpy := make(map[string][]byte) + for k, v := range states { + cpy[k] = v + } + return cpy + } + ) + var ( + stateA = make(map[string][]byte) + key = randBytes(32) + val = randBytes(32) + ) + for i := 0; i < 256; i++ { + writeFn(randBytes(32), nil, srcTrie, stateA) + } + writeFn(key, val, srcTrie, stateA) + + rootA, nodesA, _ := srcTrie.Commit(false) + if err := srcTrieDB.Update(rootA, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodesA), nil); err != nil { + panic(err) + } + if err := srcTrieDB.Commit(rootA, false); err != nil { + panic(err) + } + // Create a destination trie and sync with the scheduler + destDisk := rawdb.NewMemoryDatabase() + syncWith(t, rootA, destDisk, srcTrieDB) + checkTrieContents(t, destDisk, scheme, srcTrie.Hash().Bytes(), stateA, true) + + // Delete the element from the trie + stateB := copyStates(stateA) + srcTrie, _ = New(TrieID(rootA), srcTrieDB) + deleteFn(key, srcTrie, stateB) + + rootB, nodesB, _ := srcTrie.Commit(false) + if err := srcTrieDB.Update(rootB, rootA, 0, trienode.NewWithNodeSet(nodesB), nil); err != nil { + panic(err) + } + if err := srcTrieDB.Commit(rootB, false); err != nil { + panic(err) + } + + // Sync the new state, but never persist the new root node. Before the + // fix #28595, the original old root node will still be left in database + // which breaks the next healing cycle. + syncWithHookWriter(t, rootB, destDisk, srcTrieDB, &hookWriter{db: destDisk, filter: func(key []byte, value []byte) bool { + if scheme == rawdb.HashScheme { + return false + } + if len(value) == 0 { + return false + } + ok, path := rawdb.ResolveAccountTrieNodeKey(key) + return ok && len(path) == 0 + }}) + + // Add elements to expand trie + stateC := copyStates(stateB) + srcTrie, _ = New(TrieID(rootB), srcTrieDB) + + writeFn(key, val, srcTrie, stateC) + rootC, nodesC, _ := srcTrie.Commit(false) + if err := srcTrieDB.Update(rootC, rootB, 0, trienode.NewWithNodeSet(nodesC), nil); err != nil { + panic(err) + } + if err := srcTrieDB.Commit(rootC, false); err != nil { + panic(err) + } + syncWith(t, rootC, destDisk, srcTrieDB) + checkTrieContents(t, destDisk, scheme, srcTrie.Hash().Bytes(), stateC, true) +} diff --git a/trie/trie_test.go b/trie/trie_test.go index b53f6f83eb..ff087b6ef9 100644 --- a/trie/trie_test.go +++ b/trie/trie_test.go @@ -893,10 +893,11 @@ func TestCommitSequenceStackTrie(t *testing.T) { trie := NewEmpty(db) // Another sponge is used for the stacktrie commits stackTrieSponge := &spongeDb{sponge: sha3.NewLegacyKeccak256(), id: "b"} - writeFn := func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { - rawdb.WriteTrieNode(stackTrieSponge, owner, path, hash, blob, db.Scheme()) - } - stTrie := NewStackTrie(writeFn) + options := NewStackTrieOptions() + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(stackTrieSponge, common.Hash{}, path, hash, blob, db.Scheme()) + }) + stTrie := NewStackTrie(options) // Fill the trie with elements, should start 0, otherwise nodes will be nil in the first time. for i := 0; i < count; i++ { // For the stack trie, we need to do inserts in proper order @@ -919,10 +920,7 @@ func TestCommitSequenceStackTrie(t *testing.T) { // Flush memdb -> disk (sponge) db.Commit(root, false) // And flush stacktrie -> disk - stRoot, err := stTrie.Commit() - if err != nil { - t.Fatalf("Failed to commit stack trie %v", err) - } + stRoot := stTrie.Commit() if stRoot != root { t.Fatalf("root wrong, got %x exp %x", stRoot, root) } @@ -953,10 +951,11 @@ func TestCommitSequenceSmallRoot(t *testing.T) { trie := NewEmpty(db) // Another sponge is used for the stacktrie commits stackTrieSponge := &spongeDb{sponge: sha3.NewLegacyKeccak256(), id: "b"} - writeFn := func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { - rawdb.WriteTrieNode(stackTrieSponge, owner, path, hash, blob, db.Scheme()) - } - stTrie := NewStackTrie(writeFn) + options := NewStackTrieOptions() + options = options.WithWriter(func(path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(stackTrieSponge, common.Hash{}, path, hash, blob, db.Scheme()) + }) + stTrie := NewStackTrie(options) // Add a single small-element to the trie(s) key := make([]byte, 5) key[0] = 1 @@ -968,10 +967,7 @@ func TestCommitSequenceSmallRoot(t *testing.T) { // Flush memdb -> disk (sponge) db.Commit(root, false) // And flush stacktrie -> disk - stRoot, err := stTrie.Commit() - if err != nil { - t.Fatalf("Failed to commit stack trie %v", err) - } + stRoot := stTrie.Commit() if stRoot != root { t.Fatalf("root wrong, got %x exp %x", stRoot, root) } diff --git a/trie/triedb/pathdb/database.go b/trie/triedb/pathdb/database.go index 873418c9f0..88b671a8d1 100644 --- a/trie/triedb/pathdb/database.go +++ b/trie/triedb/pathdb/database.go @@ -167,14 +167,31 @@ func New(diskdb ethdb.Database, config *Config) *Database { log.Crit("Failed to open state history freezer", "err", err) } - // Truncate the extra state histories above the current diskLayer - // in freezer in case it's not aligned with the disk layer. - pruned, err := truncateFromHead(db.diskdb, db.freezer, db.tree.bottom().stateID()) - if err != nil { - log.Crit("Failed to truncate state history freezer", "err", err) - } - if pruned > 0 { - log.Warn("Truncated extra state histories from freezer", "count", pruned) + diskLayerID := db.tree.bottom().stateID() + if diskLayerID == 0 { + // Reset the entire state histories in case the trie database is + // not initialized yet, as these state histories are not expected. + frozen, err := db.freezer.Ancients() + if err != nil { + log.Crit("Failed to retrieve head of state history", "err", err) + } + if frozen != 0 { + err := db.freezer.Reset() + if err != nil { + log.Crit("Failed to reset state histories", "err", err) + } + log.Info("Truncated extraneous state history") + } + } else { + // Truncate the extra state histories above in freezer in case + // it's not aligned with the disk layer. + pruned, err := truncateFromHead(db.diskdb, db.freezer, diskLayerID) + if err != nil { + log.Crit("Failed to truncate extra state histories", "err", err) + } + if pruned != 0 { + log.Warn("Truncated extra state histories", "number", pruned) + } } } // Disable database in case node is still in the initial state sync stage. @@ -405,6 +422,9 @@ func (db *Database) Initialized(genesisRoot common.Hash) bool { inited = true } }) + if !inited { + inited = rawdb.ReadSnapSyncStatusFlag(db.diskdb) != rawdb.StateSyncUnknown + } return inited }