Skip to content

Commit

Permalink
core/state, trie: remove unused error-return from trie Commit operati…
Browse files Browse the repository at this point in the history
  • Loading branch information
holiman authored and shekhirin committed Jun 6, 2023
1 parent 7ea3760 commit 64872d7
Show file tree
Hide file tree
Showing 18 changed files with 72 additions and 131 deletions.
2 changes: 1 addition & 1 deletion core/state/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ type Trie interface {
// The returned nodeset can be nil if the trie is clean(nothing to commit).
// Once the trie is committed, it's not usable anymore. A new trie must
// be created with new root and updated trie database for following usage
Commit(collectLeaf bool) (common.Hash, *trie.NodeSet, error)
Commit(collectLeaf bool) (common.Hash, *trie.NodeSet)

// NodeIterator returns an iterator that returns nodes of the trie. Iteration
// starts at the key after the given start key.
Expand Down
4 changes: 2 additions & 2 deletions core/state/snapshot/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,8 @@ func (dl *diskLayer) generateRange(ctx *generatorContext, trieId *trie.ID, prefi
for i, key := range result.keys {
snapTrie.Update(key, result.vals[i])
}
root, nodes, err := snapTrie.Commit(false)
if err == nil && nodes != nil {
root, nodes := snapTrie.Commit(false)
if nodes != nil {
tdb.Update(trie.NewWithNodeSet(nodes))
tdb.Commit(root, false)
}
Expand Down
4 changes: 2 additions & 2 deletions core/state/snapshot/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,15 @@ func (t *testHelper) makeStorageTrie(stateRoot, owner common.Hash, keys []string
if !commit {
return stTrie.Hash().Bytes()
}
root, nodes, _ := stTrie.Commit(false)
root, nodes := stTrie.Commit(false)
if nodes != nil {
t.nodes.Merge(nodes)
}
return root.Bytes()
}

func (t *testHelper) Commit() common.Hash {
root, nodes, _ := t.accTrie.Commit(true)
root, nodes := t.accTrie.Commit(true)
if nodes != nil {
t.nodes.Merge(nodes)
}
Expand Down
6 changes: 2 additions & 4 deletions core/state/state_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,10 +385,8 @@ func (s *stateObject) commitTrie(db Database) (*trie.NodeSet, error) {
if metrics.EnabledExpensive {
defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now())
}
root, nodes, err := tr.Commit(false)
if err == nil {
s.data.Root = root
}
root, nodes := tr.Commit(false)
s.data.Root = root
return nodes, err
}

Expand Down
5 changes: 1 addition & 4 deletions core/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -1019,10 +1019,7 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
if metrics.EnabledExpensive {
start = time.Now()
}
root, set, err := s.trie.Commit(true)
if err != nil {
return common.Hash{}, err
}
root, set := s.trie.Commit(true)
// Merge the dirty nodes of account trie into global set
if set != nil {
if err := nodes.Merge(set); err != nil {
Expand Down
12 changes: 6 additions & 6 deletions eth/protocols/snap/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1388,7 +1388,7 @@ func makeAccountTrieNoStorage(n int) (string, *trie.Trie, entrySlice) {

// Commit the state changes into db and re-create the trie
// for accessing later.
root, nodes, _ := accTrie.Commit(false)
root, nodes := accTrie.Commit(false)
db.Update(trie.NewWithNodeSet(nodes))

accTrie, _ = trie.New(trie.StateTrieID(root), db)
Expand Down Expand Up @@ -1450,7 +1450,7 @@ func makeBoundaryAccountTrie(n int) (string, *trie.Trie, entrySlice) {

// Commit the state changes into db and re-create the trie
// for accessing later.
root, nodes, _ := accTrie.Commit(false)
root, nodes := accTrie.Commit(false)
db.Update(trie.NewWithNodeSet(nodes))

accTrie, _ = trie.New(trie.StateTrieID(root), db)
Expand Down Expand Up @@ -1496,7 +1496,7 @@ func makeAccountTrieWithStorageWithUniqueStorage(accounts, slots int, code bool)
sort.Sort(entries)

// Commit account trie
root, set, _ := accTrie.Commit(true)
root, set := accTrie.Commit(true)
nodes.Merge(set)

// Commit gathered dirty nodes into database
Expand Down Expand Up @@ -1561,7 +1561,7 @@ func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool) (strin
sort.Sort(entries)

// Commit account trie
root, set, _ := accTrie.Commit(true)
root, set := accTrie.Commit(true)
nodes.Merge(set)

// Commit gathered dirty nodes into database
Expand Down Expand Up @@ -1603,7 +1603,7 @@ func makeStorageTrieWithSeed(owner common.Hash, n, seed uint64, db *trie.Databas
entries = append(entries, elem)
}
sort.Sort(entries)
root, nodes, _ := trie.Commit(false)
root, nodes := trie.Commit(false)
return root, nodes, entries
}

Expand Down Expand Up @@ -1654,7 +1654,7 @@ func makeBoundaryStorageTrie(owner common.Hash, n int, db *trie.Database) (commo
entries = append(entries, elem)
}
sort.Sort(entries)
root, nodes, _ := trie.Commit(false)
root, nodes := trie.Commit(false)
return root, nodes, entries
}

Expand Down
12 changes: 4 additions & 8 deletions light/postprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,7 @@ func (c *ChtIndexerBackend) Process(ctx context.Context, header *types.Header) e

// Commit implements core.ChainIndexerBackend
func (c *ChtIndexerBackend) Commit() error {
root, nodes, err := c.trie.Commit(false)
if err != nil {
return err
}
root, nodes := c.trie.Commit(false)
// Commit trie changes into trie database in case it's not nil.
if nodes != nil {
if err := c.triedb.Update(trie.NewWithNodeSet(nodes)); err != nil {
Expand All @@ -226,6 +223,7 @@ func (c *ChtIndexerBackend) Commit() error {
}
}
// Re-create trie with newly generated root and updated database.
var err error
c.trie, err = trie.New(trie.TrieID(root), c.triedb)
if err != nil {
return err
Expand Down Expand Up @@ -458,10 +456,7 @@ func (b *BloomTrieIndexerBackend) Commit() error {
b.trie.Delete(encKey[:])
}
}
root, nodes, err := b.trie.Commit(false)
if err != nil {
return err
}
root, nodes := b.trie.Commit(false)
// Commit trie changes into trie database in case it's not nil.
if nodes != nil {
if err := b.triedb.Update(trie.NewWithNodeSet(nodes)); err != nil {
Expand All @@ -472,6 +467,7 @@ func (b *BloomTrieIndexerBackend) Commit() error {
}
}
// Re-create trie with newly generated root and updated database.
var err error
b.trie, err = trie.New(trie.TrieID(root), b.triedb)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions light/trie.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ func (t *odrTrie) TryDeleteAccount(address common.Address) error {
})
}

func (t *odrTrie) Commit(collectLeaf bool) (common.Hash, *trie.NodeSet, error) {
func (t *odrTrie) Commit(collectLeaf bool) (common.Hash, *trie.NodeSet) {
if t.trie == nil {
return t.id.Root, nil, nil
return t.id.Root, nil
}
return t.trie.Commit(collectLeaf)
}
Expand Down
9 changes: 2 additions & 7 deletions tests/fuzzers/stacktrie/trie_fuzzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,7 @@ func (f *fuzzer) fuzz() int {
return 0
}
// Flush trie -> database
rootA, nodes, err := trieA.Commit(false)
if err != nil {
panic(err)
}
rootA, nodes := trieA.Commit(false)
if nodes != nil {
dbA.Update(trie.NewWithNodeSet(nodes))
}
Expand All @@ -201,9 +198,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))
}
Expand Down
5 changes: 1 addition & 4 deletions tests/fuzzers/trie/trie-fuzzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,7 @@ func runRandTest(rt randTest) error {
case opHash:
tr.Hash()
case opCommit:
hash, nodes, err := tr.Commit(false)
if err != nil {
return err
}
hash, nodes := tr.Commit(false)
if nodes != nil {
if err := triedb.Update(trie.NewWithNodeSet(nodes)); err != nil {
return err
Expand Down
43 changes: 16 additions & 27 deletions trie/committer.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,25 +48,22 @@ func newCommitter(owner common.Hash, tracer *tracer, collectLeaf bool) *committe

// 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, error) {
h, err := c.commit(nil, n)
if err != nil {
return nil, nil, err
}
func (c *committer) Commit(n node) (hashNode, *NodeSet) {
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, nil
return h.(hashNode), c.nodes
}

// commit collapses a node down into a hash node and returns it.
func (c *committer) commit(path []byte, n node) (node, error) {
func (c *committer) commit(path []byte, n node) node {
// if this path is clean, use available cached data
hash, dirty := n.cache()
if hash != nil && !dirty {
return hash, nil
return hash
}
// Commit children, then parent, and remove the dirty flag.
switch cn := n.(type) {
Expand All @@ -77,55 +74,50 @@ func (c *committer) commit(path []byte, n node) (node, error) {
// If the child is fullNode, recursively commit,
// otherwise it can only be hashNode or valueNode.
if _, ok := cn.Val.(*fullNode); ok {
childV, err := c.commit(append(path, cn.Key...), cn.Val)
if err != nil {
return nil, err
}
childV := c.commit(append(path, cn.Key...), cn.Val)

collapsed.Val = childV
}
// The key needs to be copied, since we're adding it to the
// modified nodeset.
collapsed.Key = hexToCompact(cn.Key)
hashedNode := c.store(path, collapsed)
if hn, ok := hashedNode.(hashNode); ok {
return hn, nil
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, nil
return collapsed
case *fullNode:
hashedKids, err := c.commitChildren(path, cn)
if err != nil {
return nil, err
}
hashedKids := c.commitChildren(path, cn)
collapsed := cn.copy()
collapsed.Children = hashedKids

hashedNode := c.store(path, collapsed)
if hn, ok := hashedNode.(hashNode); ok {
return hn, nil
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, nil
return collapsed
case hashNode:
return cn, nil
return cn
default:
// nil, valuenode shouldn't be committed
panic(fmt.Sprintf("%T: invalid node: %v", n, n))
}
}

// commitChildren commits the children of the given fullnode
func (c *committer) commitChildren(path []byte, n *fullNode) ([17]node, error) {
func (c *committer) commitChildren(path []byte, n *fullNode) [17]node {
var children [17]node
for i := 0; i < 16; i++ {
child := n.Children[i]
Expand All @@ -142,17 +134,14 @@ func (c *committer) commitChildren(path []byte, n *fullNode) ([17]node, error) {
// Commit the child recursively and store the "hashed" value.
// Note the returned node can be some embedded nodes, so it's
// possible the type is not hashNode.
hashed, err := c.commit(append(path, byte(i)), child)
if err != nil {
return children, err
}
hashed := c.commit(append(path, byte(i)), child)
children[i] = hashed
}
// For the 17th child, it's possible the type is valuenode.
if n.Children[16] != nil {
children[16] = n.Children[16]
}
return children, nil
return children
}

// store hashes the node n and adds it to the modified nodeset. If leaf collection
Expand Down
Loading

0 comments on commit 64872d7

Please sign in to comment.