[EVM-528]: Resolve TODOs in state code (#1597)

* 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

@@ -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())

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

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()
 

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())
 

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)
 

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),

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
 		}

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))
 					}
 				}

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.

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