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

Co-authored-by: Martin Holst Swende <martin@swende.se>

XDCx/tradingstate/XDCx_trie.go

@@ -163,16 +163,13 @@ func (t *XDCXTrie) Commit(onleaf trie.LeafCallback) (common.Hash, error) {
 	// PR #1103 causes TestRevertStates and TestDumpState to fail,
 	// but we will not fix them since XDCx has been abandoned.
 	// TODO(daniel): The following code may be incorrect, ref PR #25320:
-	root, nodes, err := t.trie.Commit(false)
+	root, nodes := t.trie.Commit(false)
-	if err != nil {
-		return common.Hash{}, err
-	}
 	if nodes != nil {
 		if err := t.trie.UpdateDb(trie.NewWithNodeSet(nodes)); err != nil {
 			return common.Hash{}, err
 		}
 	}
-	return root, err
+	return root, nil
 }
 
 func (t *XDCXTrie) Hash() common.Hash {

XDCxlending/lendingstate/XDCx_trie.go

@@ -159,16 +159,13 @@ func (t *XDCXTrie) Commit(onleaf trie.LeafCallback) (common.Hash, error) {
 	// PR #1103 causes TestRevertStates and TestDumpState to fail,
 	// but we will not fix them since XDCx has been abandoned.
 	// TODO(daniel): The following code may be incorrect, ref PR #25320:
-	root, nodes, err := t.trie.Commit(false)
+	root, nodes := t.trie.Commit(false)
-	if err != nil {
-		return common.Hash{}, err
-	}
 	if nodes != nil {
 		if err := t.trie.UpdateDb(trie.NewWithNodeSet(nodes)); err != nil {
 			return common.Hash{}, err
 		}
 	}
-	return root, err
+	return root, nil
 }
 
 func (t *XDCXTrie) Hash() common.Hash {

core/state/database.go

@@ -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.

core/state/state_object.go

@@ -331,11 +331,9 @@ func (s *stateObject) commitTrie(db Database) (*trie.NodeSet, error) {
 	}
 	// Track the amount of time wasted on committing the storage trie
 	defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now())
-	root, nodes, err := tr.Commit(false)
+	root, nodes := tr.Commit(false)
-	if err == nil {
+	s.data.Root = root
-		s.data.Root = root
+	return nodes, nil
-	}
-	return nodes, err
 }
 
 // AddBalance adds amount to s's balance.

core/state/statedb.go

@@ -923,11 +923,7 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
 	}
 	// Write the account trie changes, measuring the amount of wasted time
 	start := time.Now()
-
+	root, set := s.trie.Commit(true)
-	root, set, err := s.trie.Commit(true)
-	if err != nil {
-		return common.Hash{}, err
-	}
 	// Merge the dirty nodes of account trie into global set
 	if set != nil {
 		if err := nodes.Merge(set); err != nil {

trie/committer.go

@@ -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) {
+func (c *committer) Commit(n node) (hashNode, *NodeSet) {
-	h, err := c.commit(nil, n)
+	h := c.commit(nil, n)
-	if err != nil {
-		return nil, nil, err
-	}
 	// 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) {
@@ -77,10 +74,8 @@ 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)
+			childV := c.commit(append(path, cn.Key...), cn.Val)
-			if err != nil {
+
-				return nil, err
-			}
 			collapsed.Val = childV
 		}
 		// The key needs to be copied, since we're adding it to the
@@ -88,7 +83,7 @@ func (c *committer) commit(path []byte, n node) (node, error) {
 		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
@@ -96,18 +91,15 @@ func (c *committer) commit(path []byte, n node) (node, error) {
 		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)
+		hashedKids := c.commitChildren(path, cn)
-		if err != nil {
-			return nil, err
-		}
 		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
@@ -115,9 +107,9 @@ func (c *committer) commit(path []byte, n node) (node, error) {
 		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))
@@ -125,7 +117,7 @@ func (c *committer) commit(path []byte, n node) (node, error) {
 }
 
 // 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]
@@ -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)
+		hashed := c.commit(append(path, byte(i)), child)
-		if err != nil {
-			return children, err
-		}
 		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

trie/iterator_test.go

@@ -60,10 +60,7 @@ func TestIterator(t *testing.T) {
 		all[val.k] = val.v
 		trie.Update([]byte(val.k), []byte(val.v))
 	}
-	root, nodes, err := trie.Commit(false)
+	root, nodes := trie.Commit(false)
-	if err != nil {
-		t.Fatalf("Failed to commit trie %v", err)
-	}
 	db.Update(NewWithNodeSet(nodes))
 
 	trie, _ = New(TrieID(root), db)
@@ -229,7 +226,7 @@ func TestDifferenceIterator(t *testing.T) {
 	for _, val := range testdata1 {
 		triea.Update([]byte(val.k), []byte(val.v))
 	}
-	rootA, nodesA, _ := triea.Commit(false)
+	rootA, nodesA := triea.Commit(false)
 	dba.Update(NewWithNodeSet(nodesA))
 	triea, _ = New(TrieID(rootA), dba)
 
@@ -238,7 +235,7 @@ func TestDifferenceIterator(t *testing.T) {
 	for _, val := range testdata2 {
 		trieb.Update([]byte(val.k), []byte(val.v))
 	}
-	rootB, nodesB, _ := trieb.Commit(false)
+	rootB, nodesB := trieb.Commit(false)
 	dbb.Update(NewWithNodeSet(nodesB))
 	trieb, _ = New(TrieID(rootB), dbb)
 
@@ -271,7 +268,7 @@ func TestUnionIterator(t *testing.T) {
 	for _, val := range testdata1 {
 		triea.Update([]byte(val.k), []byte(val.v))
 	}
-	rootA, nodesA, _ := triea.Commit(false)
+	rootA, nodesA := triea.Commit(false)
 	dba.Update(NewWithNodeSet(nodesA))
 	triea, _ = New(TrieID(rootA), dba)
 
@@ -280,7 +277,7 @@ func TestUnionIterator(t *testing.T) {
 	for _, val := range testdata2 {
 		trieb.Update([]byte(val.k), []byte(val.v))
 	}
-	rootB, nodesB, _ := trieb.Commit(false)
+	rootB, nodesB := trieb.Commit(false)
 	dbb.Update(NewWithNodeSet(nodesB))
 	trieb, _ = New(TrieID(rootB), dbb)
 
@@ -338,7 +335,7 @@ func testIteratorContinueAfterError(t *testing.T, memonly bool) {
 	for _, val := range testdata1 {
 		tr.Update([]byte(val.k), []byte(val.v))
 	}
-	_, nodes, _ := tr.Commit(false)
+	_, nodes := tr.Commit(false)
 	triedb.Update(NewWithNodeSet(nodes))
 	if !memonly {
 		triedb.Commit(tr.Hash(), false)
@@ -430,7 +427,7 @@ func testIteratorContinueAfterSeekError(t *testing.T, memonly bool) {
 	for _, val := range testdata1 {
 		ctr.Update([]byte(val.k), []byte(val.v))
 	}
-	root, nodes, _ := ctr.Commit(false)
+	root, nodes := ctr.Commit(false)
 	triedb.Update(NewWithNodeSet(nodes))
 	if !memonly {
 		triedb.Commit(root, false)
@@ -545,7 +542,7 @@ func makeLargeTestTrie() (*Database, *StateTrie, *loggingDb) {
 		val = crypto.Keccak256(val)
 		trie.Update(key, val)
 	}
-	_, nodes, _ := trie.Commit(false)
+	_, nodes := trie.Commit(false)
 	triedb.Update(NewWithNodeSet(nodes))
 	// Return the generated trie
 	return triedb, trie, logDb
@@ -585,7 +582,7 @@ func TestIteratorNodeBlob(t *testing.T) {
 		all[val.k] = val.v
 		trie.Update([]byte(val.k), []byte(val.v))
 	}
-	_, nodes, _ := trie.Commit(false)
+	_, nodes := trie.Commit(false)
 	triedb.Update(NewWithNodeSet(nodes))
 	triedb.Cap(0)
 

trie/secure_trie.go

@@ -211,7 +211,7 @@ func (t *StateTrie) GetKey(shaKey []byte) []byte {
 // All cached preimages will be also flushed if preimages recording is enabled.
 // 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
-func (t *StateTrie) Commit(collectLeaf bool) (common.Hash, *NodeSet, error) {
+func (t *StateTrie) Commit(collectLeaf bool) (common.Hash, *NodeSet) {
 	// Write all the pre-images to the actual disk database
 	if len(t.getSecKeyCache()) > 0 {
 		if t.preimages != nil {

trie/secure_trie_test.go

@@ -58,10 +58,7 @@ func makeTestStateTrie() (*Database, *StateTrie, map[string][]byte) {
 			trie.Update(key, val)
 		}
 	}
-	root, nodes, err := trie.Commit(false)
+	root, nodes := trie.Commit(false)
-	if err != nil {
-		panic(fmt.Errorf("failed to commit trie %v", err))
-	}
 	if err := triedb.Update(NewWithNodeSet(nodes)); err != nil {
 		panic(fmt.Errorf("failed to commit db %v", err))
 	}

trie/sync_test.go

@@ -53,10 +53,7 @@ func makeTestTrie() (*Database, *StateTrie, map[string][]byte) {
 			trie.Update(key, val)
 		}
 	}
-	root, nodes, err := trie.Commit(false)
+	root, nodes := trie.Commit(false)
-	if err != nil {
-		panic(fmt.Errorf("failed to commit trie %v", err))
-	}
 	if err := triedb.Update(NewWithNodeSet(nodes)); err != nil {
 		panic(fmt.Errorf("failed to commit db %v", err))
 	}

trie/trie.go

@@ -753,7 +753,7 @@ func (t *Trie) Hash() common.Hash {
 // 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
-func (t *Trie) Commit(collectLeaf bool) (common.Hash, *NodeSet, error) {
+func (t *Trie) Commit(collectLeaf bool) (common.Hash, *NodeSet) {
 	defer t.tracer.reset()
 
 	// Trie is empty and can be classified into two types of situations:
@@ -763,7 +763,7 @@ func (t *Trie) Commit(collectLeaf bool) (common.Hash, *NodeSet, error) {
 		// Wrap tracked deletions as the return
 		set := NewNodeSet(t.owner)
 		t.tracer.markDeletions(set)
-		return types.EmptyRootHash, set, nil
+		return types.EmptyRootHash, set
 	}
 	// Derive the hash for all dirty nodes first. We hold the assumption
 	// in the following procedure that all nodes are hashed.
@@ -775,15 +775,12 @@ func (t *Trie) Commit(collectLeaf bool) (common.Hash, *NodeSet, error) {
 		// Replace the root node with the origin hash in order to
 		// ensure all resolved nodes are dropped after the commit.
 		t.root = hashedNode
-		return rootHash, nil, nil
+		return rootHash, nil
 	}
 	h := newCommitter(t.owner, t.tracer, collectLeaf)
-	newRoot, nodes, err := h.Commit(t.root)
+	newRoot, nodes := h.Commit(t.root)
-	if err != nil {
-		return common.Hash{}, nil, err
-	}
 	t.root = newRoot
-	return rootHash, nodes, nil
+	return rootHash, nodes
 }
 
 // hashRoot calculates the root hash of the given trie

trie/trie_test.go

@@ -85,7 +85,7 @@ func testMissingNode(t *testing.T, memonly bool) {
 	trie := NewEmpty(triedb)
 	updateString(trie, "120000", "qwerqwerqwerqwerqwerqwerqwerqwer")
 	updateString(trie, "123456", "asdfasdfasdfasdfasdfasdfasdfasdf")
-	root, nodes, _ := trie.Commit(false)
+	root, nodes := trie.Commit(false)
 	triedb.Update(NewWithNodeSet(nodes))
 	if !memonly {
 		triedb.Commit(root, false)
@@ -168,10 +168,7 @@ func TestInsert(t *testing.T) {
 	updateString(trie, "A", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
 
 	exp = common.HexToHash("d23786fb4a010da3ce639d66d5e904a11dbc02746d1ce25029e53290cabf28ab")
-	root, _, err := trie.Commit(false)
+	root, _ = trie.Commit(false)
-	if err != nil {
-		t.Fatalf("commit error: %v", err)
-	}
 	if root != exp {
 		t.Errorf("case 2: exp %x got %x", exp, root)
 	}
@@ -196,7 +193,7 @@ func TestGet(t *testing.T) {
 		if i == 1 {
 			return
 		}
-		root, nodes, _ := trie.Commit(false)
+		root, nodes := trie.Commit(false)
 		db.Update(NewWithNodeSet(nodes))
 		trie, _ = New(TrieID(root), db)
 	}
@@ -268,10 +265,7 @@ func TestReplication(t *testing.T) {
 	for _, val := range vals {
 		updateString(trie, val.k, val.v)
 	}
-	exp, nodes, err := trie.Commit(false)
+	exp, nodes := trie.Commit(false)
-	if err != nil {
-		t.Fatalf("commit error: %v", err)
-	}
 	triedb.Update(NewWithNodeSet(nodes))
 
 	// create a new trie on top of the database and check that lookups work.
@@ -284,10 +278,7 @@ func TestReplication(t *testing.T) {
 			t.Errorf("trie2 doesn't have %q => %q", kv.k, kv.v)
 		}
 	}
-	hash, nodes, err := trie2.Commit(false)
+	hash, nodes := trie2.Commit(false)
-	if err != nil {
-		t.Fatalf("commit error: %v", err)
-	}
 	if hash != exp {
 		t.Errorf("root failure. expected %x got %x", exp, hash)
 	}
@@ -474,11 +465,7 @@ func runRandTest(rt randTest) error {
 		case opHash:
 			tr.Hash()
 		case opCommit:
-			root, nodes, err := tr.Commit(true)
+			root, nodes := tr.Commit(true)
-			if err != nil {
-				rt[i].err = err
-				return err
-			}
 			// Validity the returned nodeset
 			if nodes != nil {
 				for path, node := range nodes.updates.nodes {
@@ -730,7 +717,7 @@ func TestCommitAfterHash(t *testing.T) {
 	if exp != root {
 		t.Errorf("got %x, exp %x", root, exp)
 	}
-	root, _, _ = trie.Commit(false)
+	root, _ = trie.Commit(false)
 	if exp != root {
 		t.Errorf("got %x, exp %x", root, exp)
 	}
@@ -832,7 +819,7 @@ func TestCommitSequence(t *testing.T) {
 			trie.Update(crypto.Keccak256(addresses[i][:]), accounts[i])
 		}
 		// Flush trie -> database
-		root, nodes, _ := trie.Commit(false)
+		root, nodes := trie.Commit(false)
 		db.Update(NewWithNodeSet(nodes))
 		// Flush memdb -> disk (sponge)
 		db.Commit(root, false)
@@ -873,7 +860,7 @@ func TestCommitSequenceRandomBlobs(t *testing.T) {
 			trie.Update(key, val)
 		}
 		// Flush trie -> database
-		root, nodes, _ := trie.Commit(false)
+		root, nodes := trie.Commit(false)
 		db.Update(NewWithNodeSet(nodes))
 		// Flush memdb -> disk (sponge)
 		db.Commit(root, false)
@@ -912,7 +899,7 @@ func TestCommitSequenceStackTrie(t *testing.T) {
 			stTrie.TryUpdate(key, val)
 		}
 		// Flush trie -> database
-		root, nodes, _ := trie.Commit(false)
+		root, nodes := trie.Commit(false)
 		// Flush memdb -> disk (sponge)
 		db.Update(NewWithNodeSet(nodes))
 		db.Commit(root, false)
@@ -960,7 +947,7 @@ func TestCommitSequenceSmallRoot(t *testing.T) {
 	trie.TryUpdate(key, []byte{0x1})
 	stTrie.TryUpdate(key, []byte{0x1})
 	// Flush trie -> database
-	root, nodes, _ := trie.Commit(false)
+	root, nodes := trie.Commit(false)
 	// Flush memdb -> disk (sponge)
 	db.Update(NewWithNodeSet(nodes))
 	db.Commit(root, false)
@@ -1132,7 +1119,7 @@ func benchmarkDerefRootFixedSize(b *testing.B, addresses [][20]byte, accounts []
 		trie.Update(crypto.Keccak256(addresses[i][:]), accounts[i])
 	}
 	h := trie.Hash()
-	_, nodes, _ := trie.Commit(false)
+	_, nodes := trie.Commit(false)
 	triedb.Update(NewWithNodeSet(nodes))
 	b.StartTimer()
 	triedb.Dereference(h)

trie/util_test.go

@@ -70,7 +70,7 @@ func TestTrieTracer(t *testing.T) {
 	}
 
 	// Commit the changes and re-create with new root
-	root, nodes, _ := trie.Commit(false)
+	root, nodes := trie.Commit(false)
 	if err := db.Update(NewWithNodeSet(nodes)); err != nil {
 		t.Fatal(err)
 	}
@@ -155,7 +155,7 @@ func TestTrieTracePrevValue(t *testing.T) {
 	}
 
 	// Commit the changes and re-create with new root
-	root, nodes, _ := trie.Commit(false)
+	root, nodes := trie.Commit(false)
 	if err := db.Update(NewWithNodeSet(nodes)); err != nil {
 		t.Fatal(err)
 	}
@@ -262,10 +262,7 @@ func TestDeleteAll(t *testing.T) {
 	for _, val := range vals {
 		trie.Update([]byte(val.k), []byte(val.v))
 	}
-	root, set, err := trie.Commit(false)
+	root, set := trie.Commit(false)
-	if err != nil {
-		t.Fatal(err)
-	}
 	if err := db.Update(NewWithNodeSet(set)); err != nil {
 		t.Fatal(err)
 	}
@@ -289,10 +286,7 @@ func TestDeleteAll(t *testing.T) {
 	for _, val := range vals {
 		trie.Delete([]byte(val.k))
 	}
-	root, set, err = trie.Commit(false)
+	root, set = trie.Commit(false)
-	if err != nil {
-		t.Fatalf("Failed to delete trie %v", err)
-	}
 	if root != types.EmptyRootHash {
 		t.Fatalf("Invalid trie root %v", root)
 	}