trie: wrap deletion in case trie.root is nil (ethereum#26365)

This PR fixes an error in trie commit. If the trie.root is nil, it can be two possible scenarios:

-  The trie was empty, and no change happens
-  The trie was non-empty and all nodes are dropped

For the latter one, we should collect the deletions and apply them into database(e.g. in PBSS).

Author: rjl493456442

trie/committer.go

@@ -53,20 +53,11 @@ func (c *committer) Commit(n node) (hashNode, *NodeSet, error) {
 	if err != nil {
 		return nil, nil, err
 	}
-	// Some nodes can be deleted from trie which can't be captured by committer
-	// itself. Iterate all deleted nodes tracked by tracer and marked them as
-	// deleted only if they are present in database previously.
-	for _, path := range c.tracer.deleteList() {
-		// There are a few possibilities for this scenario(the node is deleted
-		// but not present in database previously), for example the node was
-		// embedded in the parent and now deleted from the trie. In this case
-		// it's noop from database's perspective.
-		val := c.tracer.getPrev(path)
-		if len(val) == 0 {
-			continue
-		}
-		c.nodes.markDeleted(path, val)
-	}
+	// Some nodes can be deleted from trie which can't be captured
+	// by committer itself. Iterate all deleted nodes tracked by
+	// tracer and marked them as deleted only if they are present
+	// in database previously.
+	c.tracer.markDeletions(c.nodes)
 	return h.(hashNode), c.nodes, nil
 }
 

trie/trie.go

@@ -569,8 +569,14 @@ func (t *Trie) Hash() common.Hash {
 func (t *Trie) Commit(collectLeaf bool) (common.Hash, *NodeSet, error) {
 	defer t.tracer.reset()
 
+	// Trie is empty and can be classified into two types of situations:
+	// - The trie was empty and no update happens
+	// - The trie was non-empty and all nodes are dropped
 	if t.root == nil {
-		return emptyRoot, nil, nil
+		// Wrap tracked deletions as the return
+		set := NewNodeSet(t.owner)
+		t.tracer.markDeletions(set)
+		return emptyRoot, set, nil
 	}
 	// Derive the hash for all dirty nodes first. We hold the assumption
 	// in the following procedure that all nodes are hashed.

trie/util_test.go

@@ -242,3 +242,69 @@ func TestTrieTracePrevValue(t *testing.T) {
 		}
 	}
 }
+
+func TestDeleteAll(t *testing.T) {
+	db := NewDatabase(rawdb.NewMemoryDatabase())
+	trie := NewEmpty(db)
+	trie.tracer = newTracer()
+
+	// Insert a batch of entries, all the nodes should be marked as inserted
+	vals := []struct{ k, v string }{
+		{"do", "verb"},
+		{"ether", "wookiedoo"},
+		{"horse", "stallion"},
+		{"shaman", "horse"},
+		{"doge", "coin"},
+		{"dog", "puppy"},
+		{"somethingveryoddindeedthis is", "myothernodedata"},
+	}
+	for _, val := range vals {
+		trie.Update([]byte(val.k), []byte(val.v))
+	}
+	root, set, err := trie.Commit(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := db.Update(NewWithNodeSet(set)); err != nil {
+		t.Fatal(err)
+	}
+	// Delete entries from trie, ensure all values are detected
+	trie, _ = New(TrieID(root), db)
+	trie.tracer = newTracer()
+	trie.resolveAndTrack(root.Bytes(), nil)
+
+	// Iterate all existent nodes
+	var (
+		it    = trie.NodeIterator(nil)
+		nodes = make(map[string][]byte)
+	)
+	for it.Next(true) {
+		if it.Hash() != (common.Hash{}) {
+			nodes[string(it.Path())] = common.CopyBytes(it.NodeBlob())
+		}
+	}
+
+	// Perform deletion to purge the entire trie
+	for _, val := range vals {
+		trie.Delete([]byte(val.k))
+	}
+	root, set, err = trie.Commit(false)
+	if err != nil {
+		t.Fatalf("Failed to delete trie %v", err)
+	}
+	if root != emptyRoot {
+		t.Fatalf("Invalid trie root %v", root)
+	}
+	for path, blob := range set.deletes {
+		prev, ok := nodes[path]
+		if !ok {
+			t.Fatalf("Extra node deleted %v", []byte(path))
+		}
+		if !bytes.Equal(prev, blob) {
+			t.Fatalf("Unexpected previous value %v", []byte(path))
+		}
+	}
+	if len(set.deletes) != len(nodes) {
+		t.Fatalf("Unexpected deletion set")
+	}
+}

trie/utils.go

@@ -178,3 +178,22 @@ func (t *tracer) copy() *tracer {
 		origin: origin,
 	}
 }
+
+// markDeletions puts all tracked deletions into the provided nodeset.
+func (t *tracer) markDeletions(set *NodeSet) {
+	// Tracer isn't used right now, remove this check later.
+	if t == nil {
+		return
+	}
+	for _, path := range t.deleteList() {
+		// There are a few possibilities for this scenario(the node is deleted
+		// but not present in database previously), for example the node was
+		// embedded in the parent and now deleted from the trie. In this case
+		// it's noop from database's perspective.
+		val := t.getPrev(path)
+		if len(val) == 0 {
+			continue
+		}
+		set.markDeleted(path, val)
+	}
+}