core/trie: remove trie tracer (#26665)

This PR contains a small portion of the full pbss PR, namely

    Remove the tracer from trie (and comitter), and instead using an accessList.
    Related changes to the Nodeset.


---------

Co-authored-by: Gary Rong <garyrong0905@gmail.com>

core/state/metrics.go

@@ -19,12 +19,10 @@ package state
 import "github.com/ethereum/go-ethereum/metrics"
 
 var (
-	accountUpdatedMeter      = metrics.NewRegisteredMeter("state/update/account", nil)
-	storageUpdatedMeter      = metrics.NewRegisteredMeter("state/update/storage", nil)
-	accountDeletedMeter      = metrics.NewRegisteredMeter("state/delete/account", nil)
-	storageDeletedMeter      = metrics.NewRegisteredMeter("state/delete/storage", nil)
-	accountTrieUpdatedMeter  = metrics.NewRegisteredMeter("state/update/accountnodes", nil)
-	storageTriesUpdatedMeter = metrics.NewRegisteredMeter("state/update/storagenodes", nil)
-	accountTrieDeletedMeter  = metrics.NewRegisteredMeter("state/delete/accountnodes", nil)
-	storageTriesDeletedMeter = metrics.NewRegisteredMeter("state/delete/storagenodes", nil)
+	accountUpdatedMeter    = metrics.NewRegisteredMeter("state/update/account", nil)
+	storageUpdatedMeter    = metrics.NewRegisteredMeter("state/update/storage", nil)
+	accountDeletedMeter    = metrics.NewRegisteredMeter("state/delete/account", nil)
+	storageDeletedMeter    = metrics.NewRegisteredMeter("state/delete/storage", nil)
+	accountTrieNodesMeter  = metrics.NewRegisteredMeter("state/trie/account", nil)
+	storageTriesNodesMeter = metrics.NewRegisteredMeter("state/trie/storage", nil)
 )

core/state/statedb.go

@@ -970,13 +970,11 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
 
 	// Commit objects to the trie, measuring the elapsed time
 	var (
-		accountTrieNodesUpdated int
-		accountTrieNodesDeleted int
-		storageTrieNodesUpdated int
-		storageTrieNodesDeleted int
-		nodes                   = trie.NewMergedNodeSet()
+		accountTrieNodes int
+		storageTrieNodes int
+		nodes            = trie.NewMergedNodeSet()
+		codeWriter       = s.db.DiskDB().NewBatch()
 	)
-	codeWriter := s.db.DiskDB().NewBatch()
 	for addr := range s.stateObjectsDirty {
 		if obj := s.stateObjects[addr]; !obj.deleted {
 			// Write any contract code associated with the state object
@@ -994,17 +992,9 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
 				if err := nodes.Merge(set); err != nil {
 					return common.Hash{}, err
 				}
-				updates, deleted := set.Size()
-				storageTrieNodesUpdated += updates
-				storageTrieNodesDeleted += deleted
+				storageTrieNodes += set.Size()
 			}
 		}
-		// If the contract is destructed, the storage is still left in the
-		// database as dangling data. Theoretically it's should be wiped from
-		// database as well, but in hash-based-scheme it's extremely hard to
-		// determine that if the trie nodes are also referenced by other storage,
-		// and in path-based-scheme some technical challenges are still unsolved.
-		// Although it won't affect the correctness but please fix it TODO(rjl493456442).
 	}
 	if len(s.stateObjectsDirty) > 0 {
 		s.stateObjectsDirty = make(map[common.Address]struct{})
@@ -1025,7 +1015,7 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
 		if err := nodes.Merge(set); err != nil {
 			return common.Hash{}, err
 		}
-		accountTrieNodesUpdated, accountTrieNodesDeleted = set.Size()
+		accountTrieNodes = set.Size()
 	}
 	if metrics.EnabledExpensive {
 		s.AccountCommits += time.Since(start)
@@ -1034,10 +1024,9 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
 		storageUpdatedMeter.Mark(int64(s.StorageUpdated))
 		accountDeletedMeter.Mark(int64(s.AccountDeleted))
 		storageDeletedMeter.Mark(int64(s.StorageDeleted))
-		accountTrieUpdatedMeter.Mark(int64(accountTrieNodesUpdated))
-		accountTrieDeletedMeter.Mark(int64(accountTrieNodesDeleted))
-		storageTriesUpdatedMeter.Mark(int64(storageTrieNodesUpdated))
-		storageTriesDeletedMeter.Mark(int64(storageTrieNodesDeleted))
+		accountTrieNodesMeter.Mark(int64(accountTrieNodes))
+		storageTriesNodesMeter.Mark(int64(storageTrieNodes))
+
 		s.AccountUpdated, s.AccountDeleted = 0, 0
 		s.StorageUpdated, s.StorageDeleted = 0, 0
 	}

trie/committer.go

@@ -33,29 +33,22 @@ type leaf struct {
 // insertion order.
 type committer struct {
 	nodes       *NodeSet
-	tracer      *tracer
 	collectLeaf bool
 }
 
 // newCommitter creates a new committer or picks one from the pool.
-func newCommitter(owner common.Hash, tracer *tracer, collectLeaf bool) *committer {
+func newCommitter(nodes *NodeSet, collectLeaf bool) *committer {
 	return &committer{
-		nodes:       NewNodeSet(owner),
-		tracer:      tracer,
+		nodes:       nodes,
 		collectLeaf: collectLeaf,
 	}
 }
 
 // Commit collapses a node down into a hash node and returns it along with
 // the modified nodeset.
-func (c *committer) Commit(n node) (hashNode, *NodeSet) {
+func (c *committer) Commit(n node) hashNode {
 	h := c.commit(nil, n)
-	// 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
+	return h.(hashNode)
 }
 
 // commit collapses a node down into a hash node and returns it.
@@ -85,12 +78,6 @@ func (c *committer) commit(path []byte, n node) node {
 		if hn, ok := hashedNode.(hashNode); ok {
 			return hn
 		}
-		// The short node now is embedded in its parent. Mark the node as
-		// deleted if it's present in database previously. It's equivalent
-		// as deletion from database's perspective.
-		if prev := c.tracer.getPrev(path); len(prev) != 0 {
-			c.nodes.markDeleted(path, prev)
-		}
 		return collapsed
 	case *fullNode:
 		hashedKids := c.commitChildren(path, cn)
@@ -101,12 +88,6 @@ func (c *committer) commit(path []byte, n node) node {
 		if hn, ok := hashedNode.(hashNode); ok {
 			return hn
 		}
-		// The full node now is embedded in its parent. Mark the node as
-		// deleted if it's present in database previously. It's equivalent
-		// as deletion from database's perspective.
-		if prev := c.tracer.getPrev(path); len(prev) != 0 {
-			c.nodes.markDeleted(path, prev)
-		}
 		return collapsed
 	case hashNode:
 		return cn
@@ -169,7 +150,7 @@ func (c *committer) store(path []byte, n node) node {
 		}
 	)
 	// Collect the dirty node to nodeset for return.
-	c.nodes.markUpdated(path, mnode, c.tracer.getPrev(path))
+	c.nodes.markUpdated(path, mnode)
 
 	// Collect the corresponding leaf node if it's required. We don't check
 	// full node since it's impossible to store value in fullNode. The key

trie/database.go

@@ -792,13 +792,12 @@ func (db *Database) Update(nodes *MergedNodeSet) error {
 	}
 	for _, owner := range order {
 		subset := nodes.sets[owner]
-		for _, path := range subset.updates.order {
-			n, ok := subset.updates.nodes[path]
-			if !ok {
-				return fmt.Errorf("missing node %x %v", owner, path)
+		subset.forEachWithOrder(false, func(path string, n *memoryNode) {
+			if n.isDeleted() {
+				return // ignore deletion
 			}
 			db.insert(n.hash, int(n.size), n.node)
-		}
+		})
 	}
 	// Link up the account trie and storage trie if the node points
 	// to an account trie leaf.

trie/nodeset.go

@@ -19,6 +19,7 @@ package trie
 import (
 	"fmt"
 	"reflect"
+	"sort"
 	"strings"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -40,8 +41,8 @@ var memoryNodeSize = int(reflect.TypeOf(memoryNode{}).Size())
 
 // memorySize returns the total memory size used by this node.
 // nolint:unused
-func (n *memoryNode) memorySize(key int) int {
-	return int(n.size) + memoryNodeSize + key
+func (n *memoryNode) memorySize(pathlen int) int {
+	return int(n.size) + memoryNodeSize + pathlen
 }
 
 // rlp returns the raw rlp encoded blob of the cached trie node, either directly
@@ -64,96 +65,107 @@ func (n *memoryNode) obj() node {
 	return expandNode(n.hash[:], n.node)
 }
 
+// isDeleted returns the indicator if the node is marked as deleted.
+func (n *memoryNode) isDeleted() bool {
+	return n.hash == (common.Hash{})
+}
+
 // nodeWithPrev wraps the memoryNode with the previous node value.
+// nolint: unused
 type nodeWithPrev struct {
 	*memoryNode
 	prev []byte // RLP-encoded previous value, nil means it's non-existent
 }
 
 // unwrap returns the internal memoryNode object.
-// nolint:unused
+// nolint: unused
 func (n *nodeWithPrev) unwrap() *memoryNode {
 	return n.memoryNode
 }
 
 // memorySize returns the total memory size used by this node. It overloads
 // the function in memoryNode by counting the size of previous value as well.
 // nolint: unused
-func (n *nodeWithPrev) memorySize(key int) int {
-	return n.memoryNode.memorySize(key) + len(n.prev)
-}
-
-// nodesWithOrder represents a collection of dirty nodes which includes
-// newly-inserted and updated nodes. The modification order of all nodes
-// is represented by order list.
-type nodesWithOrder struct {
-	order []string                 // the path list of dirty nodes, sort by insertion order
-	nodes map[string]*nodeWithPrev // the map of dirty nodes, keyed by node path
+func (n *nodeWithPrev) memorySize(pathlen int) int {
+	return n.memoryNode.memorySize(pathlen) + len(n.prev)
 }
 
 // NodeSet contains all dirty nodes collected during the commit operation.
 // Each node is keyed by path. It's not thread-safe to use.
 type NodeSet struct {
-	owner   common.Hash       // the identifier of the trie
-	updates *nodesWithOrder   // the set of updated nodes(newly inserted, updated)
-	deletes map[string][]byte // the map of deleted nodes, keyed by node
-	leaves  []*leaf           // the list of dirty leaves
+	owner      common.Hash            // the identifier of the trie
+	nodes      map[string]*memoryNode // the set of dirty nodes(inserted, updated, deleted)
+	leaves     []*leaf                // the list of dirty leaves
+	accessList map[string][]byte      // The list of accessed nodes, which records the original node value
 }
 
 // NewNodeSet initializes an empty node set to be used for tracking dirty nodes
-// from a specific account or storage trie. The owner is zero for the account
-// trie and the owning account address hash for storage tries.
-func NewNodeSet(owner common.Hash) *NodeSet {
+// for a specific account or storage trie. The owner is zero for the account trie
+// and the owning account address hash for storage tries. The provided accessList
+// represents the original value of accessed nodes, it can be optional but would
+// be beneficial for speeding up the construction of trie history.
+func NewNodeSet(owner common.Hash, accessList map[string][]byte) *NodeSet {
+	// Don't panic for lazy users
+	if accessList == nil {
+		accessList = make(map[string][]byte)
+	}
 	return &NodeSet{
-		owner: owner,
-		updates: &nodesWithOrder{
-			nodes: make(map[string]*nodeWithPrev),
-		},
-		deletes: make(map[string][]byte),
+		owner:      owner,
+		nodes:      make(map[string]*memoryNode),
+		accessList: accessList,
 	}
 }
 
-/*
-// NewNodeSetWithDeletion initializes the nodeset with provided deletion set.
-func NewNodeSetWithDeletion(owner common.Hash, paths [][]byte, prev [][]byte) *NodeSet {
-	set := NewNodeSet(owner)
-	for i, path := range paths {
-		set.markDeleted(path, prev[i])
+// forEachWithOrder iterates the dirty nodes with the specified order.
+// If topToBottom is true:
+//
+//	then the order of iteration is top to bottom, left to right.
+//
+// If topToBottom is false:
+//
+//	then the order of iteration is bottom to top, right to left.
+func (set *NodeSet) forEachWithOrder(topToBottom bool, callback func(path string, n *memoryNode)) {
+	var paths sort.StringSlice
+	for path := range set.nodes {
+		paths = append(paths, path)
+	}
+	if topToBottom {
+		paths.Sort()
+	} else {
+		sort.Sort(sort.Reverse(paths))
+	}
+	for _, path := range paths {
+		callback(path, set.nodes[path])
 	}
-	return set
 }
-*/
 
 // markUpdated marks the node as dirty(newly-inserted or updated) with provided
 // node path, node object along with its previous value.
-func (set *NodeSet) markUpdated(path []byte, node *memoryNode, prev []byte) {
-	set.updates.order = append(set.updates.order, string(path))
-	set.updates.nodes[string(path)] = &nodeWithPrev{
-		memoryNode: node,
-		prev:       prev,
-	}
+func (set *NodeSet) markUpdated(path []byte, node *memoryNode) {
+	set.nodes[string(path)] = node
 }
 
 // markDeleted marks the node as deleted with provided path and previous value.
-func (set *NodeSet) markDeleted(path []byte, prev []byte) {
-	set.deletes[string(path)] = prev
+// nolint: unused
+func (set *NodeSet) markDeleted(path []byte) {
+	set.nodes[string(path)] = &memoryNode{}
 }
 
 // addLeaf collects the provided leaf node into set.
 func (set *NodeSet) addLeaf(node *leaf) {
 	set.leaves = append(set.leaves, node)
 }
 
-// Size returns the number of updated and deleted nodes contained in the set.
-func (set *NodeSet) Size() (int, int) {
-	return len(set.updates.order), len(set.deletes)
+// Size returns the number of dirty nodes contained in the set.
+func (set *NodeSet) Size() int {
+	return len(set.nodes)
 }
 
 // Hashes returns the hashes of all updated nodes. TODO(rjl493456442) how can
 // we get rid of it?
 func (set *NodeSet) Hashes() []common.Hash {
 	var ret []common.Hash
-	for _, node := range set.updates.nodes {
+	for _, node := range set.nodes {
 		ret = append(ret, node.hash)
 	}
 	return ret
@@ -163,19 +175,17 @@ func (set *NodeSet) Hashes() []common.Hash {
 func (set *NodeSet) Summary() string {
 	var out = new(strings.Builder)
 	fmt.Fprintf(out, "nodeset owner: %v\n", set.owner)
-	if set.updates != nil {
-		for _, key := range set.updates.order {
-			updated := set.updates.nodes[key]
-			if updated.prev != nil {
-				fmt.Fprintf(out, "  [*]: %x -> %v prev: %x\n", key, updated.hash, updated.prev)
-			} else {
-				fmt.Fprintf(out, "  [+]: %x -> %v\n", key, updated.hash)
+	if set.nodes != nil {
+		for path, n := range set.nodes {
+			// Deletion
+			if n.isDeleted() {
+				fmt.Fprintf(out, "  [-]: %x\n", path)
+				continue
 			}
+			// Update
+			fmt.Fprintf(out, "  [+]: %x -> %v\n", path, n.hash)
 		}
 	}
-	for k, n := range set.deletes {
-		fmt.Fprintf(out, "  [-]: %x -> %x\n", k, n)
-	}
 	for _, n := range set.leaves {
 		fmt.Fprintf(out, "[leaf]: %v\n", n)
 	}