fix(core, trie): revert error removal in (*state.Trie).Commit #27544 (#1168)

* trie, core/state: revert error removal in (*state.Trie).Commit

* Gary's nitpick :)



---------

Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
This commit is contained in:
Daniel Liu 2026-02-05 14:13:33 +08:00 committed by GitHub
parent d749d54ea5
commit a51eaa7200
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 56 additions and 44 deletions

View file

@ -165,7 +165,10 @@ 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 := t.trie.Commit(false)
root, nodes, err := t.trie.Commit(false)
if err != nil {
return common.Hash{}, err
}
if nodes != nil {
if err := t.trie.UpdateDb(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes)); err != nil {
return common.Hash{}, err

View file

@ -161,7 +161,10 @@ 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 := t.trie.Commit(false)
root, nodes, err := t.trie.Commit(false)
if err != nil {
return common.Hash{}, err
}
if nodes != nil {
if err := t.trie.UpdateDb(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes)); err != nil {
return common.Hash{}, err

View file

@ -114,7 +114,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, *trienode.NodeSet)
Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet, error)
// NodeIterator returns an iterator that returns nodes of the trie. Iteration
// starts at the key after the given start key. An error will be returned

View file

@ -306,7 +306,10 @@ func (s *stateObject) commitTrie() (*trienode.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 := tr.Commit(false)
root, nodes, err := tr.Commit(false)
if err != nil {
return nil, err
}
s.data.Root = root
return nodes, nil
}

View file

@ -954,7 +954,10 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
}
// 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 {

View file

@ -62,7 +62,7 @@ func TestIterator(t *testing.T) {
all[val.k] = val.v
trie.MustUpdate([]byte(val.k), []byte(val.v))
}
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
trie, _ = New(TrieID(root), db)
@ -254,7 +254,7 @@ func TestDifferenceIterator(t *testing.T) {
for _, val := range testdata1 {
triea.MustUpdate([]byte(val.k), []byte(val.v))
}
rootA, nodesA := triea.Commit(false)
rootA, nodesA, _ := triea.Commit(false)
dba.Update(rootA, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodesA))
triea, _ = New(TrieID(rootA), dba)
@ -263,7 +263,7 @@ func TestDifferenceIterator(t *testing.T) {
for _, val := range testdata2 {
trieb.MustUpdate([]byte(val.k), []byte(val.v))
}
rootB, nodesB := trieb.Commit(false)
rootB, nodesB, _ := trieb.Commit(false)
dbb.Update(rootB, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodesB))
trieb, _ = New(TrieID(rootB), dbb)
@ -296,7 +296,7 @@ func TestUnionIterator(t *testing.T) {
for _, val := range testdata1 {
triea.MustUpdate([]byte(val.k), []byte(val.v))
}
rootA, nodesA := triea.Commit(false)
rootA, nodesA, _ := triea.Commit(false)
dba.Update(rootA, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodesA))
triea, _ = New(TrieID(rootA), dba)
@ -305,7 +305,7 @@ func TestUnionIterator(t *testing.T) {
for _, val := range testdata2 {
trieb.MustUpdate([]byte(val.k), []byte(val.v))
}
rootB, nodesB := trieb.Commit(false)
rootB, nodesB, _ := trieb.Commit(false)
dbb.Update(rootB, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodesB))
trieb, _ = New(TrieID(rootB), dbb)
@ -367,7 +367,7 @@ func testIteratorContinueAfterError(t *testing.T, memonly bool, scheme string) {
for _, val := range testdata1 {
tr.MustUpdate([]byte(val.k), []byte(val.v))
}
root, nodes := tr.Commit(false)
root, nodes, _ := tr.Commit(false)
tdb.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
if !memonly {
tdb.Commit(root, false)
@ -477,7 +477,7 @@ func testIteratorContinueAfterSeekError(t *testing.T, memonly bool, scheme strin
for _, val := range testdata1 {
ctr.MustUpdate([]byte(val.k), []byte(val.v))
}
root, nodes := ctr.Commit(false)
root, nodes, _ := ctr.Commit(false)
for path, n := range nodes.Nodes {
if n.Hash == barNodeHash {
barNodePath = []byte(path)
@ -600,7 +600,7 @@ func makeLargeTestTrie() (*Database, *StateTrie, *loggingDb) {
val = crypto.Keccak256(val)
trie.MustUpdate(key, val)
}
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
triedb.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
triedb.Commit(root, false)
@ -643,7 +643,7 @@ func testIteratorNodeBlob(t *testing.T, scheme string) {
all[val.k] = val.v
trie.MustUpdate([]byte(val.k), []byte(val.v))
}
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
triedb.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
triedb.Commit(root, false)

View file

@ -223,7 +223,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, *trienode.NodeSet) {
func (t *StateTrie) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet, error) {
// Write all the pre-images to the actual disk database
if len(t.getSecKeyCache()) > 0 {
if t.preimages != nil {

View file

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

View file

@ -56,7 +56,7 @@ func makeTestTrie(scheme string) (ethdb.Database, *Database, *StateTrie, map[str
trie.MustUpdate(key, val)
}
}
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
if err := triedb.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes)); err != nil {
panic(fmt.Errorf("failed to commit db %v", err))
}
@ -739,7 +739,7 @@ func testSyncMovingTarget(t *testing.T, scheme string) {
srcTrie.MustUpdate(key, val)
diff[string(key)] = val
}
root, nodes := srcTrie.Commit(false)
root, nodes, _ := srcTrie.Commit(false)
if err := srcDb.Update(root, preRoot, 0, trienode.NewWithNodeSet(nodes)); err != nil {
panic(err)
}
@ -764,7 +764,7 @@ func testSyncMovingTarget(t *testing.T, scheme string) {
srcTrie.MustUpdate([]byte(k), val)
reverted[k] = val
}
root, nodes = srcTrie.Commit(false)
root, nodes, _ = srcTrie.Commit(false)
if err := srcDb.Update(root, preRoot, 0, trienode.NewWithNodeSet(nodes)); err != nil {
panic(err)
}

View file

@ -70,7 +70,7 @@ func testTrieTracer(t *testing.T, vals []struct{ k, v string }) {
}
insertSet := copySet(trie.tracer.inserts) // copy before commit
deleteSet := copySet(trie.tracer.deletes) // copy before commit
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
seen := setKeys(iterNodes(db, root))
@ -136,7 +136,7 @@ func testAccessList(t *testing.T, vals []struct{ k, v string }) {
for _, val := range vals {
trie.MustUpdate([]byte(val.k), []byte(val.v))
}
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
trie, _ = New(TrieID(root), db)
@ -151,7 +151,7 @@ func testAccessList(t *testing.T, vals []struct{ k, v string }) {
for _, val := range vals {
trie.MustUpdate([]byte(val.k), randBytes(32))
}
root, nodes = trie.Commit(false)
root, nodes, _ = trie.Commit(false)
db.Update(root, parent, 0, trienode.NewWithNodeSet(nodes))
trie, _ = New(TrieID(root), db)
@ -169,7 +169,7 @@ func testAccessList(t *testing.T, vals []struct{ k, v string }) {
keys = append(keys, string(key))
trie.MustUpdate(key, randBytes(32))
}
root, nodes = trie.Commit(false)
root, nodes, _ = trie.Commit(false)
db.Update(root, parent, 0, trienode.NewWithNodeSet(nodes))
trie, _ = New(TrieID(root), db)
@ -184,7 +184,7 @@ func testAccessList(t *testing.T, vals []struct{ k, v string }) {
for _, key := range keys {
trie.MustUpdate([]byte(key), nil)
}
root, nodes = trie.Commit(false)
root, nodes, _ = trie.Commit(false)
db.Update(root, parent, 0, trienode.NewWithNodeSet(nodes))
trie, _ = New(TrieID(root), db)
@ -199,7 +199,7 @@ func testAccessList(t *testing.T, vals []struct{ k, v string }) {
for _, val := range vals {
trie.MustUpdate([]byte(val.k), nil)
}
root, nodes = trie.Commit(false)
root, nodes, _ = trie.Commit(false)
db.Update(root, parent, 0, trienode.NewWithNodeSet(nodes))
trie, _ = New(TrieID(root), db)
@ -218,7 +218,7 @@ func TestAccessListLeak(t *testing.T) {
for _, val := range standard {
trie.MustUpdate([]byte(val.k), []byte(val.v))
}
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
var cases = []struct {
@ -268,7 +268,7 @@ func TestTinyTree(t *testing.T) {
for _, val := range tiny {
trie.MustUpdate([]byte(val.k), randBytes(32))
}
root, set := trie.Commit(false)
root, set, _ := trie.Commit(false)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(set))
parent := root
@ -277,7 +277,7 @@ func TestTinyTree(t *testing.T) {
for _, val := range tiny {
trie.MustUpdate([]byte(val.k), []byte(val.v))
}
root, set = trie.Commit(false)
root, set, _ = trie.Commit(false)
db.Update(root, parent, 0, trienode.NewWithNodeSet(set))
trie, _ = New(TrieID(root), db)

View file

@ -802,7 +802,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, *trienode.NodeSet) {
func (t *Trie) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet, error) {
defer t.tracer.reset()
defer func() {
t.committed = true
@ -814,7 +814,7 @@ func (t *Trie) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet) {
// - The trie was empty and no update happens
// - The trie was non-empty and all nodes are dropped
if t.root == nil {
return types.EmptyRootHash, nodes
return types.EmptyRootHash, nodes, nil
}
// Derive the hash for all dirty nodes first. We hold the assumption
// in the following procedure that all nodes are hashed.
@ -826,10 +826,10 @@ func (t *Trie) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet) {
// 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
return rootHash, nil, nil
}
t.root = newCommitter(nodes, t.tracer, collectLeaf).Commit(t.root)
return rootHash, nodes
return rootHash, nodes, nil
}
// hashRoot calculates the root hash of the given trie

View file

@ -89,7 +89,7 @@ func testMissingNode(t *testing.T, memonly bool, scheme string) {
trie := NewEmpty(triedb)
updateString(trie, "120000", "qwerqwerqwerqwerqwerqwerqwerqwer")
updateString(trie, "123456", "asdfasdfasdfasdfasdfasdfasdfasdf")
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
triedb.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
if !memonly {
@ -178,7 +178,7 @@ func TestInsert(t *testing.T) {
updateString(trie, "A", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
exp = common.HexToHash("d23786fb4a010da3ce639d66d5e904a11dbc02746d1ce25029e53290cabf28ab")
root, _ = trie.Commit(false)
root, _, _ = trie.Commit(false)
if root != exp {
t.Errorf("case 2: exp %x got %x", exp, root)
}
@ -203,7 +203,7 @@ func TestGet(t *testing.T) {
if i == 1 {
return
}
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
trie, _ = New(TrieID(root), db)
}
@ -275,7 +275,7 @@ func TestReplication(t *testing.T) {
for _, val := range vals {
updateString(trie, val.k, val.v)
}
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
// create a new trie on top of the database and check that lookups work.
@ -288,7 +288,7 @@ func TestReplication(t *testing.T) {
t.Errorf("trie2 doesn't have %q => %q", kv.k, kv.v)
}
}
hash, nodes := trie2.Commit(false)
hash, nodes, _ := trie2.Commit(false)
if hash != root {
t.Errorf("root failure. expected %x got %x", root, hash)
}
@ -520,7 +520,7 @@ func runRandTest(rt randTest) error {
case opHash:
tr.Hash()
case opCommit:
root, nodes := tr.Commit(true)
root, nodes, _ := tr.Commit(true)
if nodes != nil {
triedb.Update(root, origin, 0, trienode.NewWithNodeSet(nodes))
}
@ -753,7 +753,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)
}
@ -855,7 +855,7 @@ func TestCommitSequence(t *testing.T) {
trie.MustUpdate(crypto.Keccak256(addresses[i][:]), accounts[i])
}
// Flush trie -> database
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
// Flush memdb -> disk (sponge)
db.Commit(root, false)
@ -896,7 +896,7 @@ func TestCommitSequenceRandomBlobs(t *testing.T) {
trie.MustUpdate(key, val)
}
// Flush trie -> database
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
// Flush memdb -> disk (sponge)
db.Commit(root, false)
@ -935,7 +935,7 @@ func TestCommitSequenceStackTrie(t *testing.T) {
stTrie.Update(key, val)
}
// Flush trie -> database
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
// Flush memdb -> disk (sponge)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
db.Commit(root, false)
@ -983,7 +983,7 @@ func TestCommitSequenceSmallRoot(t *testing.T) {
trie.Update(key, []byte{0x1})
stTrie.Update(key, []byte{0x1})
// Flush trie -> database
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
// Flush memdb -> disk (sponge)
db.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
db.Commit(root, false)
@ -1155,7 +1155,7 @@ func benchmarkDerefRootFixedSize(b *testing.B, addresses [][20]byte, accounts []
trie.MustUpdate(crypto.Keccak256(addresses[i][:]), accounts[i])
}
h := trie.Hash()
root, nodes := trie.Commit(false)
root, nodes, _ := trie.Commit(false)
triedb.Update(root, types.EmptyRootHash, 0, trienode.NewWithNodeSet(nodes))
b.StartTimer()
triedb.Dereference(h)