trie: separate opTracer and prevalueTracer

trie/committer.go

@@ -29,12 +29,12 @@ import (
 // insertion order.
 type committer struct {
 	nodes       *trienode.NodeSet
-	tracer      *tracer
+	tracer      *prevalueTracer
 	collectLeaf bool
 }
 
 // newCommitter creates a new committer or picks one from the pool.
-func newCommitter(nodeset *trienode.NodeSet, tracer *tracer, collectLeaf bool) *committer {
+func newCommitter(nodeset *trienode.NodeSet, tracer *prevalueTracer, collectLeaf bool) *committer {
 	return &committer{
 		nodes:       nodeset,
 		tracer:      tracer,
@@ -140,8 +140,7 @@ func (c *committer) store(path []byte, n node) node {
 		// The node is embedded in its parent, in other words, this node
 		// will not be stored in the database independently, mark it as
 		// deleted only if the node was existent in database before.
-		_, ok := c.tracer.accessList[string(path)]
-		if ok {
+		if c.tracer.has(path) {
 			c.nodes.AddNode(path, trienode.NewDeleted())
 		}
 		return n

trie/proof.go

@@ -573,7 +573,12 @@ func VerifyRangeProof(rootHash common.Hash, firstKey []byte, keys [][]byte, valu
 	}
 	// Rebuild the trie with the leaf stream, the shape of trie
 	// should be same with the original one.
-	tr := &Trie{root: root, reader: newEmptyReader(), tracer: newTracer()}
+	tr := &Trie{
+		root:           root,
+		reader:         newEmptyReader(),
+		opTracer:       newOpTracer(),
+		prevalueTracer: newPrevalueTracer(),
+	}
 	if empty {
 		tr.root = nil
 	}

trie/tracer.go

@@ -18,14 +18,14 @@ package trie
 
 import (
 	"maps"
-
-	"github.com/ethereum/go-ethereum/common"
+	"slices"
+	"sync"
 )
 
-// tracer tracks the changes of trie nodes. During the trie operations,
+// opTracer tracks the changes of trie nodes. During the trie operations,
 // some nodes can be deleted from the trie, while these deleted nodes
 // won't be captured by trie.Hasher or trie.Committer. Thus, these deleted
-// nodes won't be removed from the disk at all. Tracer is an auxiliary tool
+// nodes won't be removed from the disk at all. opTracer is an auxiliary tool
 // used to track all insert and delete operations of trie and capture all
 // deleted nodes eventually.
 //
@@ -35,38 +35,25 @@ import (
 // This tool can track all of them no matter the node is embedded in its
 // parent or not, but valueNode is never tracked.
 //
-// Besides, it's also used for recording the original value of the nodes
-// when they are resolved from the disk. The pre-value of the nodes will
-// be used to construct trie history in the future.
-//
-// Note tracer is not thread-safe, callers should be responsible for handling
+// Note opTracer is not thread-safe, callers should be responsible for handling
 // the concurrency issues by themselves.
-type tracer struct {
-	inserts    map[string]struct{}
-	deletes    map[string]struct{}
-	accessList map[string][]byte
+type opTracer struct {
+	inserts map[string]struct{}
+	deletes map[string]struct{}
 }
 
-// newTracer initializes the tracer for capturing trie changes.
-func newTracer() *tracer {
-	return &tracer{
-		inserts:    make(map[string]struct{}),
-		deletes:    make(map[string]struct{}),
-		accessList: make(map[string][]byte),
+// newOpTracer initializes the tracer for capturing trie changes.
+func newOpTracer() *opTracer {
+	return &opTracer{
+		inserts: make(map[string]struct{}),
+		deletes: make(map[string]struct{}),
 	}
 }
 
-// onRead tracks the newly loaded trie node and caches the rlp-encoded
-// blob internally. Don't change the value outside of function since
-// it's not deep-copied.
-func (t *tracer) onRead(path []byte, val []byte) {
-	t.accessList[string(path)] = val
-}
-
 // onInsert tracks the newly inserted trie node. If it's already
 // in the deletion set (resurrected node), then just wipe it from
 // the deletion set as it's "untouched".
-func (t *tracer) onInsert(path []byte) {
+func (t *opTracer) onInsert(path []byte) {
 	if _, present := t.deletes[string(path)]; present {
 		delete(t.deletes, string(path))
 		return
@@ -77,7 +64,7 @@ func (t *tracer) onInsert(path []byte) {
 // onDelete tracks the newly deleted trie node. If it's already
 // in the addition set, then just wipe it from the addition set
 // as it's untouched.
-func (t *tracer) onDelete(path []byte) {
+func (t *opTracer) onDelete(path []byte) {
 	if _, present := t.inserts[string(path)]; present {
 		delete(t.inserts, string(path))
 		return
@@ -86,37 +73,102 @@ func (t *tracer) onDelete(path []byte) {
 }
 
 // reset clears the content tracked by tracer.
-func (t *tracer) reset() {
-	t.inserts = make(map[string]struct{})
-	t.deletes = make(map[string]struct{})
-	t.accessList = make(map[string][]byte)
+func (t *opTracer) reset() {
+	clear(t.inserts)
+	clear(t.deletes)
 }
 
 // copy returns a deep copied tracer instance.
-func (t *tracer) copy() *tracer {
-	accessList := make(map[string][]byte, len(t.accessList))
-	for path, blob := range t.accessList {
-		accessList[path] = common.CopyBytes(blob)
-	}
-	return &tracer{
-		inserts:    maps.Clone(t.inserts),
-		deletes:    maps.Clone(t.deletes),
-		accessList: accessList,
+func (t *opTracer) copy() *opTracer {
+	return &opTracer{
+		inserts: maps.Clone(t.inserts),
+		deletes: maps.Clone(t.deletes),
 	}
 }
 
-// deletedNodes returns a list of node paths which are deleted from the trie.
-func (t *tracer) deletedNodes() []string {
-	var paths []string
+// deletedList returns a list of node paths which are deleted from the trie.
+func (t *opTracer) deletedList() [][]byte {
+	paths := make([][]byte, 0, len(t.deletes))
 	for path := range t.deletes {
-		// It's possible a few deleted nodes were embedded
-		// in their parent before, the deletions can be no
-		// effect by deleting nothing, filter them out.
-		_, ok := t.accessList[path]
-		if !ok {
-			continue
-		}
-		paths = append(paths, path)
+		paths = append(paths, []byte(path))
 	}
 	return paths
 }
+
+// prevalueTracer tracks the original values of resolved trie nodes. Cached trie
+// node values are expected to be immutable.
+//
+// prevalueTracer is protected by a lock to ensure concurrency safety, as
+// concurrent map writes may occur during concurrent trie read operations.
+type prevalueTracer struct {
+	data map[string][]byte
+	lock sync.RWMutex
+}
+
+// newPrevalueTracer initializes the tracer for capturing resolved trie nodes.
+func newPrevalueTracer() *prevalueTracer {
+	return &prevalueTracer{
+		data: make(map[string][]byte),
+	}
+}
+
+// put tracks the newly loaded trie node and caches its RLP-encoded
+// blob internally. Do not modify the value outside this function,
+// as it is not deep-copied.
+func (t *prevalueTracer) put(path []byte, val []byte) {
+	t.lock.Lock()
+	defer t.lock.Unlock()
+
+	t.data[string(path)] = val
+}
+
+// has returns a flag indicating whether the corresponding trie node specified
+// by the path exists in the trie.
+func (t *prevalueTracer) has(path []byte) bool {
+	t.lock.RLock()
+	defer t.lock.RUnlock()
+
+	_, ok := t.data[string(path)]
+	return ok
+}
+
+// hasList returns a list of flag indicating whether the corresponding trie nodes
+// specified by the path exists in the trie.
+func (t *prevalueTracer) hasList(list [][]byte) []bool {
+	t.lock.RLock()
+	defer t.lock.RUnlock()
+
+	exists := make([]bool, 0, len(list))
+	for _, path := range list {
+		_, ok := t.data[string(path)]
+		exists = append(exists, ok)
+	}
+	return exists
+}
+
+// values returns a list of values of the cached trie nodes.
+func (t *prevalueTracer) values() [][]byte {
+	t.lock.RLock()
+	defer t.lock.RUnlock()
+
+	return slices.Collect(maps.Values(t.data))
+}
+
+// reset resets the cached content in the prevalueTracer.
+func (t *prevalueTracer) reset() {
+	t.lock.Lock()
+	defer t.lock.Unlock()
+
+	clear(t.data)
+}
+
+// copy returns a copied prevalueTracer instance.
+func (t *prevalueTracer) copy() *prevalueTracer {
+	t.lock.RLock()
+	defer t.lock.RUnlock()
+
+	// Shadow clone is used, as the cached trie node values are immutable
+	return &prevalueTracer{
+		data: maps.Clone(t.data),
+	}
+}

trie/tracer_test.go

@@ -68,8 +68,8 @@ func testTrieTracer(t *testing.T, vals []struct{ k, v string }) {
 	for _, val := range vals {
 		trie.MustUpdate([]byte(val.k), []byte(val.v))
 	}
-	insertSet := copySet(trie.tracer.inserts) // copy before commit
-	deleteSet := copySet(trie.tracer.deletes) // copy before commit
+	insertSet := copySet(trie.opTracer.inserts) // copy before commit
+	deleteSet := copySet(trie.opTracer.deletes) // copy before commit
 	root, nodes := trie.Commit(false)
 	db.Update(root, types.EmptyRootHash, trienode.NewWithNodeSet(nodes))
 
@@ -86,7 +86,7 @@ func testTrieTracer(t *testing.T, vals []struct{ k, v string }) {
 	for _, val := range vals {
 		trie.MustDelete([]byte(val.k))
 	}
-	insertSet, deleteSet = copySet(trie.tracer.inserts), copySet(trie.tracer.deletes)
+	insertSet, deleteSet = copySet(trie.opTracer.inserts), copySet(trie.opTracer.deletes)
 	if !compareSet(insertSet, nil) {
 		t.Fatal("Unexpected insertion set")
 	}
@@ -112,10 +112,10 @@ func testTrieTracerNoop(t *testing.T, vals []struct{ k, v string }) {
 	for _, val := range vals {
 		trie.MustDelete([]byte(val.k))
 	}
-	if len(trie.tracer.inserts) != 0 {
+	if len(trie.opTracer.inserts) != 0 {
 		t.Fatal("Unexpected insertion set")
 	}
-	if len(trie.tracer.deletes) != 0 {
+	if len(trie.opTracer.deletes) != 0 {
 		t.Fatal("Unexpected deletion set")
 	}
 }
@@ -249,9 +249,9 @@ func TestAccessListLeak(t *testing.T) {
 	}
 	for _, c := range cases {
 		trie, _ = New(TrieID(root), db)
-		n1 := len(trie.tracer.accessList)
+		n1 := len(trie.prevalueTracer.data)
 		c.op(trie)
-		n2 := len(trie.tracer.accessList)
+		n2 := len(trie.prevalueTracer.data)
 
 		if n1 != n2 {
 			t.Fatalf("AccessList is leaked, prev %d after %d", n1, n2)

trie/trie.go

@@ -55,8 +55,9 @@ type Trie struct {
 	// reader is the handler trie can retrieve nodes from.
 	reader *trieReader
 
-	// tracer is the tool to track the trie changes.
-	tracer *tracer
+	// Various tracers for capturing the modification to trie
+	opTracer       *opTracer
+	prevalueTracer *prevalueTracer
 }
 
 // newFlag returns the cache flag value for a newly created node.
@@ -67,13 +68,14 @@ func (t *Trie) newFlag() nodeFlag {
 // Copy returns a copy of Trie.
 func (t *Trie) Copy() *Trie {
 	return &Trie{
-		root:        copyNode(t.root),
-		owner:       t.owner,
-		committed:   t.committed,
-		unhashed:    t.unhashed,
-		uncommitted: t.uncommitted,
-		reader:      t.reader,
-		tracer:      t.tracer.copy(),
+		root:           copyNode(t.root),
+		owner:          t.owner,
+		committed:      t.committed,
+		unhashed:       t.unhashed,
+		uncommitted:    t.uncommitted,
+		reader:         t.reader,
+		opTracer:       t.opTracer.copy(),
+		prevalueTracer: t.prevalueTracer.copy(),
 	}
 }
 
@@ -89,9 +91,10 @@ func New(id *ID, db database.NodeDatabase) (*Trie, error) {
 		return nil, err
 	}
 	trie := &Trie{
-		owner:  id.Owner,
-		reader: reader,
-		tracer: newTracer(),
+		owner:          id.Owner,
+		reader:         reader,
+		opTracer:       newOpTracer(),
+		prevalueTracer: newPrevalueTracer(),
 	}
 	if id.Root != (common.Hash{}) && id.Root != types.EmptyRootHash {
 		rootnode, err := trie.resolveAndTrack(id.Root[:], nil)
@@ -361,7 +364,7 @@ func (t *Trie) insert(n node, prefix, key []byte, value node) (bool, node, error
 		// New branch node is created as a child of the original short node.
 		// Track the newly inserted node in the tracer. The node identifier
 		// passed is the path from the root node.
-		t.tracer.onInsert(append(prefix, key[:matchlen]...))
+		t.opTracer.onInsert(append(prefix, key[:matchlen]...))
 
 		// Replace it with a short node leading up to the branch.
 		return true, &shortNode{key[:matchlen], branch, t.newFlag()}, nil
@@ -379,7 +382,7 @@ func (t *Trie) insert(n node, prefix, key []byte, value node) (bool, node, error
 		// New short node is created and track it in the tracer. The node identifier
 		// passed is the path from the root node. Note the valueNode won't be tracked
 		// since it's always embedded in its parent.
-		t.tracer.onInsert(prefix)
+		t.opTracer.onInsert(prefix)
 
 		return true, &shortNode{key, value, t.newFlag()}, nil
 
@@ -444,7 +447,7 @@ func (t *Trie) delete(n node, prefix, key []byte) (bool, node, error) {
 			// The matched short node is deleted entirely and track
 			// it in the deletion set. The same the valueNode doesn't
 			// need to be tracked at all since it's always embedded.
-			t.tracer.onDelete(prefix)
+			t.opTracer.onDelete(prefix)
 
 			return true, nil, nil // remove n entirely for whole matches
 		}
@@ -460,7 +463,7 @@ func (t *Trie) delete(n node, prefix, key []byte) (bool, node, error) {
 		case *shortNode:
 			// The child shortNode is merged into its parent, track
 			// is deleted as well.
-			t.tracer.onDelete(append(prefix, n.Key...))
+			t.opTracer.onDelete(append(prefix, n.Key...))
 
 			// Deleting from the subtrie reduced it to another
 			// short node. Merge the nodes to avoid creating a
@@ -525,7 +528,7 @@ func (t *Trie) delete(n node, prefix, key []byte) (bool, node, error) {
 					// Replace the entire full node with the short node.
 					// Mark the original short node as deleted since the
 					// value is embedded into the parent now.
-					t.tracer.onDelete(append(prefix, byte(pos)))
+					t.opTracer.onDelete(append(prefix, byte(pos)))
 
 					k := append([]byte{byte(pos)}, cnode.Key...)
 					return true, &shortNode{k, cnode.Val, t.newFlag()}, nil
@@ -616,13 +619,33 @@ func (t *Trie) resolveAndTrack(n hashNode, prefix []byte) (node, error) {
 	if err != nil {
 		return nil, err
 	}
-	t.tracer.onRead(prefix, blob)
+	t.prevalueTracer.put(prefix, blob)
 
 	// The returned node blob won't be changed afterward. No need to
 	// deep-copy the slice.
 	return decodeNodeUnsafe(n, blob)
 }
 
+// deletedNodes returns a list of node paths, referring the nodes being deleted
+// from the trie.
+func (t *Trie) deletedNodes() [][]byte {
+	list := t.opTracer.deletedList()
+
+	// It's possible a few deleted nodes were embedded in their parent before,
+	// the deletions can be no effect by deleting nothing, filter them out.
+	var (
+		pos   int
+		flags = t.prevalueTracer.hasList(list)
+	)
+	for i := 0; i < len(list); i++ {
+		if flags[i] { // exist, keep it
+			list[pos] = list[i]
+			pos++
+		}
+	}
+	return list[:pos] // trim to the new length
+}
+
 // Hash returns the root hash of the trie. It does not write to the
 // database and can be used even if the trie doesn't have one.
 func (t *Trie) Hash() common.Hash {
@@ -644,13 +667,13 @@ func (t *Trie) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet) {
 	// (b) The trie was non-empty and all nodes are dropped => return
 	//     the node set includes all deleted nodes
 	if t.root == nil {
-		paths := t.tracer.deletedNodes()
+		paths := t.deletedNodes()
 		if len(paths) == 0 {
 			return types.EmptyRootHash, nil // case (a)
 		}
 		nodes := trienode.NewNodeSet(t.owner)
 		for _, path := range paths {
-			nodes.AddNode([]byte(path), trienode.NewDeleted())
+			nodes.AddNode(path, trienode.NewDeleted())
 		}
 		return types.EmptyRootHash, nodes // case (b)
 	}
@@ -667,11 +690,11 @@ func (t *Trie) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet) {
 		return rootHash, nil
 	}
 	nodes := trienode.NewNodeSet(t.owner)
-	for _, path := range t.tracer.deletedNodes() {
-		nodes.AddNode([]byte(path), trienode.NewDeleted())
+	for _, path := range t.deletedNodes() {
+		nodes.AddNode(path, trienode.NewDeleted())
 	}
 	// If the number of changes is below 100, we let one thread handle it
-	t.root = newCommitter(nodes, t.tracer, collectLeaf).Commit(t.root, t.uncommitted > 100)
+	t.root = newCommitter(nodes, t.prevalueTracer, collectLeaf).Commit(t.root, t.uncommitted > 100)
 	t.uncommitted = 0
 	return rootHash, nodes
 }
@@ -692,12 +715,13 @@ func (t *Trie) hashRoot() node {
 
 // Witness returns a set containing all trie nodes that have been accessed.
 func (t *Trie) Witness() map[string]struct{} {
-	if len(t.tracer.accessList) == 0 {
+	values := t.prevalueTracer.values()
+	if len(values) == 0 {
 		return nil
 	}
-	witness := make(map[string]struct{}, len(t.tracer.accessList))
-	for _, node := range t.tracer.accessList {
-		witness[string(node)] = struct{}{}
+	witness := make(map[string]struct{}, len(values))
+	for _, val := range values {
+		witness[string(val)] = struct{}{}
 	}
 	return witness
 }
@@ -708,6 +732,7 @@ func (t *Trie) Reset() {
 	t.owner = common.Hash{}
 	t.unhashed = 0
 	t.uncommitted = 0
-	t.tracer.reset()
+	t.opTracer.reset()
+	t.prevalueTracer.reset()
 	t.committed = false
 }

trie/trie_test.go

@@ -595,18 +595,18 @@ func runRandTest(rt randTest) error {
 					deleteExp[path] = struct{}{}
 				}
 			}
-			if len(insertExp) != len(tr.tracer.inserts) {
+			if len(insertExp) != len(tr.opTracer.inserts) {
 				rt[i].err = errors.New("insert set mismatch")
 			}
-			if len(deleteExp) != len(tr.tracer.deletes) {
+			if len(deleteExp) != len(tr.opTracer.deletes) {
 				rt[i].err = errors.New("delete set mismatch")
 			}
-			for insert := range tr.tracer.inserts {
+			for insert := range tr.opTracer.inserts {
 				if _, present := insertExp[insert]; !present {
 					rt[i].err = errors.New("missing inserted node")
 				}
 			}
-			for del := range tr.tracer.deletes {
+			for del := range tr.opTracer.deletes {
 				if _, present := deleteExp[del]; !present {
 					rt[i].err = errors.New("missing deleted node")
 				}