From e879377319048294c1ac0839acf89547c416bcc7 Mon Sep 17 00:00:00 2001
From: Andrew Ashikhmin <34320705+yperbasis@users.noreply.github.com>
Date: Wed, 1 Feb 2023 16:27:43 +0100
Subject: [PATCH] Fix index out of range in TxPool.best (#876)

This should fix
```
CLMocker: Could not getPayload (a55589c9, 0x0000000000000003): runtime error: index out of range [15] with length 15, trace: [stageloop.go:262 panic.go:884 panic.go:113 pool.go:670 pool.go:677 pool.go:699 stage_mining_exec.go:204 kv_mdbx.go:622 stage_mining_exec.go:199 stage_mining_exec.go:135 stagebuilder.go:41 sync.go:353 sync.go:255 stageloop.go:275 backend.go:518 block_builder.go:30 asm_amd64.s:1594]
```
which I occasionally observed in my Hive test runs.
---
 txpool/pool.go | 104 ++++++++++++++++++++++++-------------------------
 1 file changed, 50 insertions(+), 54 deletions(-)

diff --git a/txpool/pool.go b/txpool/pool.go
index 791298679..7cb3e78b0 100644
--- a/txpool/pool.go
+++ b/txpool/pool.go
@@ -613,77 +613,73 @@ func (p *TxPool) AddNewGoodPeer(peerID types.PeerID) { p.recentlyConnectedPeers.
 func (p *TxPool) Started() bool                      { return p.started.Load() }
 
 func (p *TxPool) best(n uint16, txs *types.TxsRlp, tx kv.Tx, onTopOf, availableGas uint64, toSkip mapset.Set[[32]byte]) (bool, int, error) {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+
 	// First wait for the corresponding block to arrive
 	if p.lastSeenBlock.Load() < onTopOf {
 		return false, 0, nil // Too early
 	}
 
 	isShanghai := p.isShanghai()
+	best := p.pending.best
 
-	txs.Resize(uint(cmp.Min(int(n), len(p.pending.best.ms))))
+	txs.Resize(uint(cmp.Min(int(n), len(best.ms))))
 	var toRemove []*metaTx
 	count := 0
 
-	success, err := func() (bool, error) {
-		p.lock.Lock()
-		defer p.lock.Unlock()
-
-		best := p.pending.best
-		for i := 0; count < int(n) && i < len(best.ms); i++ {
-			// if we wouldn't have enough gas for a standard transaction then quit out early
-			if availableGas < fixedgas.TxGas {
-				break
-			}
-
-			mt := best.ms[i]
-
-			if toSkip.Contains(mt.Tx.IDHash) {
-				continue
-			}
-
-			if mt.Tx.Gas >= p.blockGasLimit.Load() {
-				// Skip transactions with very large gas limit
-				continue
-			}
-			rlpTx, sender, isLocal, err := p.getRlpLocked(tx, mt.Tx.IDHash[:])
-			if err != nil {
-				return false, err
-			}
-			if len(rlpTx) == 0 {
-				toRemove = append(toRemove, mt)
-				continue
-			}
-
-			// make sure we have enough gas in the caller to add this transaction.
-			// not an exact science using intrinsic gas but as close as we could hope for at
-			// this stage
-			intrinsicGas, _ := CalcIntrinsicGas(uint64(mt.Tx.DataLen), uint64(mt.Tx.DataNonZeroLen), nil, mt.Tx.Creation, true, true, isShanghai)
-			if intrinsicGas > availableGas {
-				// we might find another TX with a low enough intrinsic gas to include so carry on
-				continue
-			}
-
-			if intrinsicGas <= availableGas { // check for potential underflow
-				availableGas -= intrinsicGas
-			}
-
-			txs.Txs[count] = rlpTx
-			copy(txs.Senders.At(count), sender)
-			txs.IsLocal[count] = isLocal
-			toSkip.Add(mt.Tx.IDHash)
-			count++
+	for i := 0; count < int(n) && i < len(best.ms); i++ {
+		// if we wouldn't have enough gas for a standard transaction then quit out early
+		if availableGas < fixedgas.TxGas {
+			break
 		}
-		return true, nil
-	}()
+
+		mt := best.ms[i]
+
+		if toSkip.Contains(mt.Tx.IDHash) {
+			continue
+		}
+
+		if mt.Tx.Gas >= p.blockGasLimit.Load() {
+			// Skip transactions with very large gas limit
+			continue
+		}
+		rlpTx, sender, isLocal, err := p.getRlpLocked(tx, mt.Tx.IDHash[:])
+		if err != nil {
+			return false, count, err
+		}
+		if len(rlpTx) == 0 {
+			toRemove = append(toRemove, mt)
+			continue
+		}
+
+		// make sure we have enough gas in the caller to add this transaction.
+		// not an exact science using intrinsic gas but as close as we could hope for at
+		// this stage
+		intrinsicGas, _ := CalcIntrinsicGas(uint64(mt.Tx.DataLen), uint64(mt.Tx.DataNonZeroLen), nil, mt.Tx.Creation, true, true, isShanghai)
+		if intrinsicGas > availableGas {
+			// we might find another TX with a low enough intrinsic gas to include so carry on
+			continue
+		}
+
+		if intrinsicGas <= availableGas { // check for potential underflow
+			availableGas -= intrinsicGas
+		}
+
+		txs.Txs[count] = rlpTx
+		copy(txs.Senders.At(count), sender)
+		txs.IsLocal[count] = isLocal
+		toSkip.Add(mt.Tx.IDHash)
+		count++
+	}
+
 	txs.Resize(uint(count))
 	if len(toRemove) > 0 {
-		p.lock.Lock()
-		defer p.lock.Unlock()
 		for _, mt := range toRemove {
 			p.pending.Remove(mt)
 		}
 	}
-	return success, count, err
+	return true, count, nil
 }
 
 func (p *TxPool) ResetYieldedStatus() {