Skip to content

Commit

Permalink
Revert "core/trie: remove trie tracer (ethereum#26665)" (ethereum#26732)
Browse files Browse the repository at this point in the history
This reverts commit 7c749c9.
  • Loading branch information
rjl493456442 authored and shekhirin committed Jun 6, 2023
1 parent a4fd50c commit 67a4a11
Show file tree
Hide file tree
Showing 9 changed files with 725 additions and 116 deletions.
14 changes: 8 additions & 6 deletions core/state/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ 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)
accountTrieNodesMeter = metrics.NewRegisteredMeter("state/trie/account", nil)
storageTriesNodesMeter = metrics.NewRegisteredMeter("state/trie/storage", 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)
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)
)
29 changes: 20 additions & 9 deletions core/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -970,11 +970,13 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {

// Commit objects to the trie, measuring the elapsed time
var (
accountTrieNodes int
storageTrieNodes int
nodes = trie.NewMergedNodeSet()
codeWriter = s.db.DiskDB().NewBatch()
accountTrieNodesUpdated int
accountTrieNodesDeleted int
storageTrieNodesUpdated int
storageTrieNodesDeleted int
nodes = trie.NewMergedNodeSet()
)
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
Expand All @@ -992,9 +994,17 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
if err := nodes.Merge(set); err != nil {
return common.Hash{}, err
}
storageTrieNodes += set.Size()
updates, deleted := set.Size()
storageTrieNodesUpdated += updates
storageTrieNodesDeleted += deleted
}
}
// 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{})
Expand All @@ -1015,7 +1025,7 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
if err := nodes.Merge(set); err != nil {
return common.Hash{}, err
}
accountTrieNodes = set.Size()
accountTrieNodesUpdated, accountTrieNodesDeleted = set.Size()
}
if metrics.EnabledExpensive {
s.AccountCommits += time.Since(start)
Expand All @@ -1024,9 +1034,10 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
storageUpdatedMeter.Mark(int64(s.StorageUpdated))
accountDeletedMeter.Mark(int64(s.AccountDeleted))
storageDeletedMeter.Mark(int64(s.StorageDeleted))
accountTrieNodesMeter.Mark(int64(accountTrieNodes))
storageTriesNodesMeter.Mark(int64(storageTrieNodes))

accountTrieUpdatedMeter.Mark(int64(accountTrieNodesUpdated))
accountTrieDeletedMeter.Mark(int64(accountTrieNodesDeleted))
storageTriesUpdatedMeter.Mark(int64(storageTrieNodesUpdated))
storageTriesDeletedMeter.Mark(int64(storageTrieNodesDeleted))
s.AccountUpdated, s.AccountDeleted = 0, 0
s.StorageUpdated, s.StorageDeleted = 0, 0
}
Expand Down
29 changes: 24 additions & 5 deletions trie/committer.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,29 @@ 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(nodes *NodeSet, collectLeaf bool) *committer {
func newCommitter(owner common.Hash, tracer *tracer, collectLeaf bool) *committer {
return &committer{
nodes: nodes,
nodes: NewNodeSet(owner),
tracer: tracer,
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 {
func (c *committer) Commit(n node) (hashNode, *NodeSet) {
h := c.commit(nil, n)
return h.(hashNode)
// 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
}

// commit collapses a node down into a hash node and returns it.
Expand Down Expand Up @@ -78,6 +85,12 @@ 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)
Expand All @@ -88,6 +101,12 @@ 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
Expand Down Expand Up @@ -150,7 +169,7 @@ func (c *committer) store(path []byte, n node) node {
}
)
// Collect the dirty node to nodeset for return.
c.nodes.markUpdated(path, mnode)
c.nodes.markUpdated(path, mnode, c.tracer.getPrev(path))

// 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
Expand Down
9 changes: 5 additions & 4 deletions trie/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -792,12 +792,13 @@ func (db *Database) Update(nodes *MergedNodeSet) error {
}
for _, owner := range order {
subset := nodes.sets[owner]
subset.forEachWithOrder(false, func(path string, n *memoryNode) {
if n.isDeleted() {
return // ignore deletion
for _, path := range subset.updates.order {
n, ok := subset.updates.nodes[path]
if !ok {
return fmt.Errorf("missing node %x %v", owner, path)
}
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.
Expand Down
120 changes: 55 additions & 65 deletions trie/nodeset.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package trie
import (
"fmt"
"reflect"
"sort"
"strings"

"github.com/ethereum/go-ethereum/common"
Expand All @@ -41,8 +40,8 @@ var memoryNodeSize = int(reflect.TypeOf(memoryNode{}).Size())

// memorySize returns the total memory size used by this node.
// nolint:unused
func (n *memoryNode) memorySize(pathlen int) int {
return int(n.size) + memoryNodeSize + pathlen
func (n *memoryNode) memorySize(key int) int {
return int(n.size) + memoryNodeSize + key
}

// rlp returns the raw rlp encoded blob of the cached trie node, either directly
Expand All @@ -65,107 +64,96 @@ 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(pathlen int) int {
return n.memoryNode.memorySize(pathlen) + len(n.prev)
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
}

// 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
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
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
}

// NewNodeSet initializes an empty node set to be used for tracking dirty nodes
// 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)
}
// 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 {
return &NodeSet{
owner: owner,
nodes: make(map[string]*memoryNode),
accessList: accessList,
owner: owner,
updates: &nodesWithOrder{
nodes: make(map[string]*nodeWithPrev),
},
deletes: make(map[string][]byte),
}
}

// 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])
/*
// 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])
}
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) {
set.nodes[string(path)] = node
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,
}
}

// markDeleted marks the node as deleted with provided path and previous value.
// nolint: unused
func (set *NodeSet) markDeleted(path []byte) {
set.nodes[string(path)] = &memoryNode{}
func (set *NodeSet) markDeleted(path []byte, prev []byte) {
set.deletes[string(path)] = prev
}

// 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 dirty nodes contained in the set.
func (set *NodeSet) Size() int {
return len(set.nodes)
// 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)
}

// 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.nodes {
for _, node := range set.updates.nodes {
ret = append(ret, node.hash)
}
return ret
Expand All @@ -175,17 +163,19 @@ 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.nodes != nil {
for path, n := range set.nodes {
// Deletion
if n.isDeleted() {
fmt.Fprintf(out, " [-]: %x\n", path)
continue
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)
}
// 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)
}
Expand Down
Loading

0 comments on commit 67a4a11

Please sign in to comment.