From ed64216928c2d793fff6b7bfde7985eff2fa5d0f Mon Sep 17 00:00:00 2001
From: Goran Rojovic <100121253+goran-ethernal@users.noreply.github.com>
Date: Mon, 12 Jun 2023 15:07:48 +0200
Subject: [PATCH] [EVM-528]: Resolve TODOs in state code (#1597)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Resolve TODOs

* Try to reuse single arena pool instance

* Get rid of some panics in state/txn.go

* Fix block builder UT

---------

Co-authored-by: Stefan Negovanović <stefan@ethernal.tech>
---
 blockchain/blockchain.go                 |   5 +-
 consensus/dev/dev.go                     |   5 +-
 consensus/ibft/consensus_backend.go      |   6 +-
 consensus/polybft/block_builder.go       |   8 +-
 consensus/polybft/blockchain_wrapper.go  |   5 +-
 helper/predeployment/predeployment.go    |   5 +-
 state/executor.go                        |  49 ++++++++--
 state/immutable-trie/snapshot.go         |   9 +-
 state/immutable-trie/trie.go             |   9 +-
 state/runtime/precompiled/precompiled.go |   2 -
 state/testing.go                         | 119 ++++++++++++++++++-----
 state/txn.go                             |  30 +++---
 state/txn_test.go                        |   2 +-
 tests/state_test.go                      |   9 +-
 tests/testing.go                         |  10 +-
 15 files changed, 201 insertions(+), 72 deletions(-)

diff --git a/blockchain/blockchain.go b/blockchain/blockchain.go
index d720b0ce3a..c33c1d0d36 100644
--- a/blockchain/blockchain.go
+++ b/blockchain/blockchain.go
@@ -840,7 +840,10 @@ func (b *Blockchain) executeBlockTransactions(block *types.Block) (*BlockResult,
 		return nil, err
 	}
 
-	_, root := txn.Commit()
+	_, root, err := txn.Commit()
+	if err != nil {
+		return nil, fmt.Errorf("failed to commit the state changes: %w", err)
+	}
 
 	// Append the receipts to the receipts cache
 	b.receiptsCache.Add(header.Hash, txn.Receipts())
diff --git a/consensus/dev/dev.go b/consensus/dev/dev.go
index ca10891878..00ba97a617 100644
--- a/consensus/dev/dev.go
+++ b/consensus/dev/dev.go
@@ -186,7 +186,10 @@ func (d *Dev) writeNewBlock(parent *types.Header) error {
 	txns := d.writeTransactions(baseFee, gasLimit, transition)
 
 	// Commit the changes
-	_, root := transition.Commit()
+	_, root, err := transition.Commit()
+	if err != nil {
+		return fmt.Errorf("failed to commit the state changes: %w", err)
+	}
 
 	// Update the header
 	header.StateRoot = root
diff --git a/consensus/ibft/consensus_backend.go b/consensus/ibft/consensus_backend.go
index 149e2cdb70..19094e41ef 100644
--- a/consensus/ibft/consensus_backend.go
+++ b/consensus/ibft/consensus_backend.go
@@ -219,7 +219,11 @@ func (i *backendIBFT) buildBlock(parent *types.Header) (*types.Block, error) {
 		return nil, err
 	}
 
-	_, root := transition.Commit()
+	_, root, err := transition.Commit()
+	if err != nil {
+		return nil, fmt.Errorf("failed to commit the state changes: %w", err)
+	}
+
 	header.StateRoot = root
 	header.GasUsed = transition.TotalGas()
 
diff --git a/consensus/polybft/block_builder.go b/consensus/polybft/block_builder.go
index 18ef8ec0e5..f30d375c63 100644
--- a/consensus/polybft/block_builder.go
+++ b/consensus/polybft/block_builder.go
@@ -1,6 +1,7 @@
 package polybft
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/0xPolygon/polygon-edge/consensus"
@@ -112,7 +113,12 @@ func (b *BlockBuilder) Build(handler func(h *types.Header)) (*types.FullBlock, e
 		handler(b.header)
 	}
 
-	_, b.header.StateRoot = b.state.Commit()
+	_, stateRoot, err := b.state.Commit()
+	if err != nil {
+		return nil, fmt.Errorf("failed to commit the state changes: %w", err)
+	}
+
+	b.header.StateRoot = stateRoot
 	b.header.GasUsed = b.state.TotalGas()
 	b.header.LogsBloom = types.CreateBloom(b.Receipts())
 
diff --git a/consensus/polybft/blockchain_wrapper.go b/consensus/polybft/blockchain_wrapper.go
index 1ae7b1127c..dc350f73d7 100644
--- a/consensus/polybft/blockchain_wrapper.go
+++ b/consensus/polybft/blockchain_wrapper.go
@@ -107,7 +107,10 @@ func (p *blockchainWrapper) ProcessBlock(parent *types.Header, block *types.Bloc
 		}
 	}
 
-	_, root := transition.Commit()
+	_, root, err := transition.Commit()
+	if err != nil {
+		return nil, fmt.Errorf("failed to commit the state changes: %w", err)
+	}
 
 	updateBlockExecutionMetric(start)
 
diff --git a/helper/predeployment/predeployment.go b/helper/predeployment/predeployment.go
index ce6d0f4ce6..e050b064b9 100644
--- a/helper/predeployment/predeployment.go
+++ b/helper/predeployment/predeployment.go
@@ -153,7 +153,10 @@ func getPredeployAccount(address types.Address, input, deployedBytecode []byte)
 	// the state needs to be walked to collect all touched all storage slots
 	storageMap := getModifiedStorageMap(radix, address)
 
-	transition.Commit()
+	_, _, err := transition.Commit()
+	if err != nil {
+		return nil, fmt.Errorf("failed to commit the state changes: %w", err)
+	}
 
 	return &chain.GenesisAccount{
 		Balance: transition.GetBalance(address),
diff --git a/state/executor.go b/state/executor.go
index 8aad64d5d7..f204b74c10 100644
--- a/state/executor.go
+++ b/state/executor.go
@@ -112,7 +112,11 @@ func (e *Executor) WriteGenesis(
 		}
 	}
 
-	objs := txn.Commit(false)
+	objs, err := txn.Commit(false)
+	if err != nil {
+		return types.Hash{}, err
+	}
+
 	_, root := snap.Commit(objs)
 
 	return types.BytesToHash(root), nil
@@ -363,7 +367,9 @@ func (t *Transition) Write(txn *types.Transaction) error {
 	}
 
 	// The suicided accounts are set as deleted for the next iteration
-	t.state.CleanDeleteObjects(true)
+	if err := t.state.CleanDeleteObjects(true); err != nil {
+		return fmt.Errorf("failed to clean deleted objects: %w", err)
+	}
 
 	if result.Failed() {
 		receipt.SetStatus(types.ReceiptFailed)
@@ -385,11 +391,15 @@ func (t *Transition) Write(txn *types.Transaction) error {
 }
 
 // Commit commits the final result
-func (t *Transition) Commit() (Snapshot, types.Hash) {
-	objs := t.state.Commit(t.config.EIP155)
+func (t *Transition) Commit() (Snapshot, types.Hash, error) {
+	objs, err := t.state.Commit(t.config.EIP155)
+	if err != nil {
+		return nil, types.ZeroHash, err
+	}
+
 	s2, root := t.snap.Commit(objs)
 
-	return s2, types.BytesToHash(root)
+	return s2, types.BytesToHash(root), nil
 }
 
 func (t *Transition) subGasPool(amount uint64) error {
@@ -416,7 +426,9 @@ func (t *Transition) Apply(msg *types.Transaction) (*runtime.ExecutionResult, er
 
 	result, err := t.apply(msg)
 	if err != nil {
-		t.state.RevertToSnapshot(s)
+		if revertErr := t.state.RevertToSnapshot(s); revertErr != nil {
+			return nil, revertErr
+		}
 	}
 
 	if t.PostHook != nil {
@@ -785,7 +797,12 @@ func (t *Transition) applyCall(
 
 	result = t.run(c, host)
 	if result.Failed() {
-		t.state.RevertToSnapshot(snapshot)
+		if err := t.state.RevertToSnapshot(snapshot); err != nil {
+			return &runtime.ExecutionResult{
+				GasLeft: c.Gas,
+				Err:     err,
+			}
+		}
 	}
 
 	t.captureCallEnd(c, result)
@@ -873,14 +890,22 @@ func (t *Transition) applyCreate(c *runtime.Contract, host runtime.Host) *runtim
 
 	result = t.run(c, host)
 	if result.Failed() {
-		t.state.RevertToSnapshot(snapshot)
+		if err := t.state.RevertToSnapshot(snapshot); err != nil {
+			return &runtime.ExecutionResult{
+				Err: err,
+			}
+		}
 
 		return result
 	}
 
 	if t.config.EIP158 && len(result.ReturnValue) > SpuriousDragonMaxCodeSize {
 		// Contract size exceeds 'SpuriousDragon' size limit
-		t.state.RevertToSnapshot(snapshot)
+		if err := t.state.RevertToSnapshot(snapshot); err != nil {
+			return &runtime.ExecutionResult{
+				Err: err,
+			}
+		}
 
 		return &runtime.ExecutionResult{
 			GasLeft: 0,
@@ -896,7 +921,11 @@ func (t *Transition) applyCreate(c *runtime.Contract, host runtime.Host) *runtim
 
 		// Out of gas creating the contract
 		if t.config.Homestead {
-			t.state.RevertToSnapshot(snapshot)
+			if err := t.state.RevertToSnapshot(snapshot); err != nil {
+				return &runtime.ExecutionResult{
+					Err: err,
+				}
+			}
 
 			result.GasLeft = 0
 		}
diff --git a/state/immutable-trie/snapshot.go b/state/immutable-trie/snapshot.go
index 935fc52da4..84a06fe1fa 100644
--- a/state/immutable-trie/snapshot.go
+++ b/state/immutable-trie/snapshot.go
@@ -79,11 +79,8 @@ func (s *Snapshot) Commit(objs []*state.Object) (state.Snapshot, []byte) {
 	tt := s.trie.Txn(s.state.storage)
 	tt.batch = batch
 
-	arena := accountArenaPool.Get()
-	defer accountArenaPool.Put(arena)
-
-	ar1 := stateArenaPool.Get()
-	defer stateArenaPool.Put(ar1)
+	arena := stateArenaPool.Get()
+	defer stateArenaPool.Put(arena)
 
 	for _, obj := range objs {
 		if obj.Deleted {
@@ -110,7 +107,7 @@ func (s *Snapshot) Commit(objs []*state.Object) (state.Snapshot, []byte) {
 					if entry.Deleted {
 						localTxn.Delete(k)
 					} else {
-						vv := ar1.NewBytes(bytes.TrimLeft(entry.Val, "\x00"))
+						vv := arena.NewBytes(bytes.TrimLeft(entry.Val, "\x00"))
 						localTxn.Insert(k, vv.MarshalTo(nil))
 					}
 				}
diff --git a/state/immutable-trie/trie.go b/state/immutable-trie/trie.go
index 8143893306..154cd811ff 100644
--- a/state/immutable-trie/trie.go
+++ b/state/immutable-trie/trie.go
@@ -113,12 +113,9 @@ func hashit(k []byte) []byte {
 	return h.Sum(nil)
 }
 
-var accountArenaPool fastrlp.ArenaPool
-
-// TODO, Remove once we do update in fastrlp (to be fixed in EVM-528)
-//
-//nolint:godox
-var stateArenaPool fastrlp.ArenaPool
+var (
+	stateArenaPool fastrlp.ArenaPool
+)
 
 // Hash returns the root hash of the trie. It does not write to the
 // database and can be used even if the trie doesn't have one.
diff --git a/state/runtime/precompiled/precompiled.go b/state/runtime/precompiled/precompiled.go
index 2a661bba13..8b33d5eb80 100644
--- a/state/runtime/precompiled/precompiled.go
+++ b/state/runtime/precompiled/precompiled.go
@@ -161,8 +161,6 @@ func (p *Precompiled) Run(c *runtime.Contract, host runtime.Host, config *chain.
 var zeroPadding = make([]byte, 64)
 
 func (p *Precompiled) leftPad(buf []byte, n int) []byte {
-	//nolint:godox
-	// TODO, avoid buffer allocation (to be fixed in EVM-528)
 	l := len(buf)
 	if l > n {
 		return buf
diff --git a/state/testing.go b/state/testing.go
index 9ff6273821..44d7af1e7d 100644
--- a/state/testing.go
+++ b/state/testing.go
@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/0xPolygon/polygon-edge/types"
 )
@@ -53,10 +54,15 @@ func TestState(t *testing.T, buildPreState buildPreState) {
 
 		testWriteEmptyState(t, buildPreState)
 	})
-	t.Run("update state with empty", func(t *testing.T) {
+	t.Run("update state with empty - delete empty objects", func(t *testing.T) {
 		t.Parallel()
 
-		testUpdateStateWithEmpty(t, buildPreState)
+		testUpdateStateWithEmpty(t, buildPreState, true)
+	})
+	t.Run("update state with empty - do not delete empty objects", func(t *testing.T) {
+		t.Parallel()
+
+		testUpdateStateWithEmpty(t, buildPreState, false)
 	})
 	t.Run("suicide account in pre-state", func(t *testing.T) {
 		t.Parallel()
@@ -131,13 +137,19 @@ func testDeleteCommonStateRoot(t *testing.T, buildPreState buildPreState) {
 	txn.SetState(addr2, hash1, hash1)
 	txn.SetState(addr2, hash2, hash1)
 
-	snap2, _ := snap.Commit(txn.Commit(false))
+	objs, err := txn.Commit(false)
+	require.NoError(t, err)
+
+	snap2, _ := snap.Commit(objs)
 	txn2 := newTxn(snap2)
 
 	txn2.SetState(addr1, hash0, hash0)
 	txn2.SetState(addr1, hash1, hash0)
 
-	snap3, _ := snap2.Commit(txn2.Commit(false))
+	objs, err = txn2.Commit(false)
+	require.NoError(t, err)
+
+	snap3, _ := snap2.Commit(objs)
 
 	txn3 := newTxn(snap3)
 	assert.Equal(t, hash1, txn3.GetState(addr1, hash2))
@@ -158,7 +170,10 @@ func testWriteState(t *testing.T, buildPreState buildPreState) {
 	assert.Equal(t, hash1, txn.GetState(addr1, hash1))
 	assert.Equal(t, hash2, txn.GetState(addr1, hash2))
 
-	snap, _ = snap.Commit(txn.Commit(false))
+	objs, err := txn.Commit(false)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
 	assert.Equal(t, hash1, txn.GetState(addr1, hash1))
@@ -173,7 +188,11 @@ func testWriteEmptyState(t *testing.T, buildPreState buildPreState) {
 
 	// Without EIP150 the data is added
 	txn.SetState(addr1, hash1, hash0)
-	snap, _ = snap.Commit(txn.Commit(false))
+
+	objs, err := txn.Commit(false)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
 	assert.True(t, txn.Exist(addr1))
@@ -183,13 +202,17 @@ func testWriteEmptyState(t *testing.T, buildPreState buildPreState) {
 
 	// With EIP150 the empty data is removed
 	txn.SetState(addr1, hash1, hash0)
-	snap, _ = snap.Commit(txn.Commit(true))
+
+	objs, err = txn.Commit(true)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
 	assert.False(t, txn.Exist(addr1))
 }
 
-func testUpdateStateWithEmpty(t *testing.T, buildPreState buildPreState) {
+func testUpdateStateWithEmpty(t *testing.T, buildPreState buildPreState, deleteEmptyObjects bool) {
 	t.Helper()
 
 	// If the state (in prestate) is updated to empty it should be removed
@@ -198,13 +221,38 @@ func testUpdateStateWithEmpty(t *testing.T, buildPreState buildPreState) {
 	txn := newTxn(snap)
 	txn.SetState(addr1, hash1, hash0)
 
-	//nolint:godox
-	// TODO, test with false (should not be deleted) (to be fixed in EVM-528)
-	// TODO, test with balance on the account and nonce (to be fixed in EVM-528)
-	snap, _ = snap.Commit(txn.Commit(true))
+	objs, err := txn.Commit(deleteEmptyObjects)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
-	assert.False(t, txn.Exist(addr1))
+	assert.Equal(t, !deleteEmptyObjects, txn.Exist(addr1))
+
+	// if balance is set, no matter what is passed to deleteEmptyObjects,
+	// addr1 should exist in state objects of txn
+	txn.SetBalance(addr1, big.NewInt(100))
+
+	objs, err = txn.Commit(deleteEmptyObjects)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
+
+	txn = newTxn(snap)
+	assert.Equal(t, true, txn.Exist(addr1))
+
+	// if nonce is set, no matter what is passed to deleteEmptyObjects,
+	// addr1 should exist in state objects of txn
+	txn.SetBalance(addr1, big.NewInt(0))
+	txn.SetNonce(addr1, 1)
+
+	objs, err = txn.Commit(deleteEmptyObjects)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
+
+	txn = newTxn(snap)
+	assert.Equal(t, true, txn.Exist(addr1))
 }
 
 func testSuicideAccountInPreState(t *testing.T, buildPreState buildPreState) {
@@ -215,7 +263,11 @@ func testSuicideAccountInPreState(t *testing.T, buildPreState buildPreState) {
 
 	txn := newTxn(snap)
 	txn.Suicide(addr1)
-	snap, _ = snap.Commit(txn.Commit(true))
+
+	objs, err := txn.Commit(true)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
 	assert.False(t, txn.Exist(addr1))
@@ -233,7 +285,10 @@ func testSuicideAccount(t *testing.T, buildPreState buildPreState) {
 	// Note, even if has commit suicide it still exists in the current txn
 	assert.True(t, txn.Exist(addr1))
 
-	snap, _ = snap.Commit(txn.Commit(true))
+	objs, err := txn.Commit(true)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
 	assert.False(t, txn.Exist(addr1))
@@ -254,7 +309,11 @@ func testSuicideAccountWithData(t *testing.T, buildPreState buildPreState) {
 	txn.SetState(addr1, hash1, hash1)
 
 	txn.Suicide(addr1)
-	snap, _ = snap.Commit(txn.Commit(true))
+
+	objs, err := txn.Commit(true)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
 
@@ -277,7 +336,10 @@ func testSuicideCoinbase(t *testing.T, buildPreState buildPreState) {
 	txn := newTxn(snap)
 	txn.Suicide(addr1)
 	txn.AddSealingReward(addr1, big.NewInt(10))
-	snap, _ = snap.Commit(txn.Commit(true))
+	objs, err := txn.Commit(true)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
 	assert.Equal(t, big.NewInt(10), txn.GetBalance(addr1))
@@ -294,10 +356,11 @@ func testSuicideWithIntermediateCommit(t *testing.T, buildPreState buildPreState
 
 	assert.Equal(t, uint64(10), txn.GetNonce(addr1))
 
-	txn.CleanDeleteObjects(true)
+	assert.NoError(t, txn.CleanDeleteObjects(true))
 	assert.Equal(t, uint64(0), txn.GetNonce(addr1))
 
-	txn.Commit(true)
+	_, err := txn.Commit(true)
+	assert.NoError(t, err)
 	assert.Equal(t, uint64(0), txn.GetNonce(addr1))
 }
 
@@ -312,7 +375,8 @@ func testRestartRefunds(t *testing.T, buildPreState buildPreState) {
 	txn.AddRefund(1000)
 	assert.Equal(t, uint64(1000), txn.GetRefund())
 
-	txn.Commit(false)
+	_, err := txn.Commit(false)
+	assert.NoError(t, err)
 
 	// refund should be empty after the commit
 	assert.Equal(t, uint64(0), txn.GetRefund())
@@ -331,7 +395,11 @@ func testChangePrestateAccountBalanceToZero(t *testing.T, buildPreState buildPre
 
 	txn := newTxn(snap)
 	txn.SetBalance(addr1, big.NewInt(0))
-	snap, _ = snap.Commit(txn.Commit(true))
+
+	objs, err := txn.Commit(true)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
 	assert.False(t, txn.Exist(addr1))
@@ -346,7 +414,10 @@ func testChangeAccountBalanceToZero(t *testing.T, buildPreState buildPreState) {
 	txn.SetBalance(addr1, big.NewInt(10))
 	txn.SetBalance(addr1, big.NewInt(0))
 
-	snap, _ = snap.Commit(txn.Commit(true))
+	objs, err := txn.Commit(true)
+	require.NoError(t, err)
+
+	snap, _ = snap.Commit(objs)
 
 	txn = newTxn(snap)
 	assert.False(t, txn.Exist(addr1))
@@ -373,7 +444,9 @@ func testSetAndGetCode(t *testing.T, buildPreState buildPreState) {
 	txn := newTxn(snap)
 	txn.SetCode(addr1, testCode)
 
-	affectedObjs := txn.Commit(true)
+	affectedObjs, err := txn.Commit(true)
+	require.NoError(t, err)
+
 	snap, _ = snap.Commit(affectedObjs)
 	assert.Len(t, affectedObjs, 1)
 
diff --git a/state/txn.go b/state/txn.go
index 95b432388e..2bf4a12dd3 100644
--- a/state/txn.go
+++ b/state/txn.go
@@ -1,6 +1,8 @@
 package state
 
 import (
+	"errors"
+	"fmt"
 	"math/big"
 
 	iradix "github.com/hashicorp/go-immutable-radix"
@@ -68,13 +70,15 @@ func (txn *Txn) Snapshot() int {
 }
 
 // RevertToSnapshot reverts to a given snapshot
-func (txn *Txn) RevertToSnapshot(id int) {
-	if id > len(txn.snapshots) {
-		panic("") //nolint:gocritic
+func (txn *Txn) RevertToSnapshot(id int) error {
+	if id > len(txn.snapshots)-1 {
+		return fmt.Errorf("snapshot id %d out of the range", id)
 	}
 
 	tree := txn.snapshots[id]
 	txn.txn = tree.Txn()
+
+	return nil
 }
 
 // GetAccount returns an account
@@ -492,9 +496,6 @@ func (txn *Txn) TouchAccount(addr types.Address) {
 	})
 }
 
-//nolint:godox
-// TODO, check panics with this ones (to be fixed in EVM-528)
-
 func (txn *Txn) Exist(addr types.Address) bool {
 	_, exists := txn.getStateObject(addr)
 
@@ -537,7 +538,7 @@ func (txn *Txn) CreateAccount(addr types.Address) {
 	txn.txn.Insert(addr.Bytes(), obj)
 }
 
-func (txn *Txn) CleanDeleteObjects(deleteEmptyObjects bool) {
+func (txn *Txn) CleanDeleteObjects(deleteEmptyObjects bool) error {
 	remove := [][]byte{}
 
 	txn.txn.Root().Walk(func(k []byte, v interface{}) bool {
@@ -555,13 +556,12 @@ func (txn *Txn) CleanDeleteObjects(deleteEmptyObjects bool) {
 	for _, k := range remove {
 		v, ok := txn.txn.Get(k)
 		if !ok {
-			panic("it should not happen") //nolint:gocritic
+			return fmt.Errorf("failed to retrieve value for %s key", string(k))
 		}
 
 		obj, ok := v.(*StateObject)
-
 		if !ok {
-			panic("it should not happen") //nolint:gocritic
+			return errors.New("found object is not of StateObject type")
 		}
 
 		obj2 := obj.Copy()
@@ -571,10 +571,14 @@ func (txn *Txn) CleanDeleteObjects(deleteEmptyObjects bool) {
 
 	// delete refunds
 	txn.txn.Delete(refundIndex)
+
+	return nil
 }
 
-func (txn *Txn) Commit(deleteEmptyObjects bool) []*Object {
-	txn.CleanDeleteObjects(deleteEmptyObjects)
+func (txn *Txn) Commit(deleteEmptyObjects bool) ([]*Object, error) {
+	if err := txn.CleanDeleteObjects(deleteEmptyObjects); err != nil {
+		return nil, err
+	}
 
 	x := txn.txn.Commit()
 
@@ -620,5 +624,5 @@ func (txn *Txn) Commit(deleteEmptyObjects bool) []*Object {
 		return false
 	})
 
-	return objs
+	return objs, nil
 }
diff --git a/state/txn_test.go b/state/txn_test.go
index 0f1dde2acd..2a00ddef0b 100644
--- a/state/txn_test.go
+++ b/state/txn_test.go
@@ -67,6 +67,6 @@ func TestSnapshotUpdateData(t *testing.T) {
 	txn.SetState(addr1, hash1, hash2)
 	assert.Equal(t, hash2, txn.GetState(addr1, hash1))
 
-	txn.RevertToSnapshot(ss)
+	assert.NoError(t, txn.RevertToSnapshot(ss))
 	assert.Equal(t, hash1, txn.GetState(addr1, hash1))
 }
diff --git a/tests/state_test.go b/tests/state_test.go
index 198f39b5c1..540e63c672 100644
--- a/tests/state_test.go
+++ b/tests/state_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/0xPolygon/polygon-edge/state"
 	"github.com/0xPolygon/polygon-edge/types"
 	"github.com/hashicorp/go-hclog"
+	"github.com/stretchr/testify/require"
 )
 
 // Currently used test cases suite version is v10.4.
@@ -57,7 +58,9 @@ func RunSpecificTest(t *testing.T, file string, c testCase, name, fork string, i
 		t.Fatalf("failed to create transaction: %v", err)
 	}
 
-	s, snapshot, pastRoot := buildState(c.Pre)
+	s, snapshot, pastRoot, err := buildState(c.Pre)
+	require.NoError(t, err)
+
 	forks := config.At(uint64(env.Number))
 
 	xxx := state.NewExecutor(&chain.Params{
@@ -88,7 +91,9 @@ func RunSpecificTest(t *testing.T, file string, c testCase, name, fork string, i
 	// mining rewards
 	txn.AddSealingReward(env.Coinbase, big.NewInt(0))
 
-	objs := txn.Commit(forks.EIP155)
+	objs, err := txn.Commit(forks.EIP155)
+	require.NoError(t, err)
+
 	_, root := snapshot.Commit(objs)
 
 	// Check block root
diff --git a/tests/testing.go b/tests/testing.go
index fca394139e..3d2e897379 100644
--- a/tests/testing.go
+++ b/tests/testing.go
@@ -186,7 +186,7 @@ func (e *env) ToEnv(t *testing.T) runtime.TxContext {
 
 func buildState(
 	allocs map[types.Address]*chain.GenesisAccount,
-) (state.State, state.Snapshot, types.Hash) {
+) (state.State, state.Snapshot, types.Hash, error) {
 	s := itrie.NewState(itrie.NewMemoryStorage())
 	snap := s.NewSnapshot()
 
@@ -206,10 +206,14 @@ func buildState(
 		}
 	}
 
-	objs := txn.Commit(false)
+	objs, err := txn.Commit(false)
+	if err != nil {
+		return nil, nil, types.ZeroHash, err
+	}
+
 	snap, root := snap.Commit(objs)
 
-	return s, snap, types.BytesToHash(root)
+	return s, snap, types.BytesToHash(root), nil
 }
 
 type indexes struct {