Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core/state, trie: remove unused error-return from trie Commit operation #26641

Merged
merged 1 commit into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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