core: handle edge case of tx size mismatch

core/txpool/legacypool/legacypool.go

@@ -907,25 +907,14 @@ func (pool *LegacyPool) add(tx *types.Transaction, local bool) (replaced bool, e
 }
 
 func (pool *LegacyPool) addToOverflowPool(drop types.Transactions, isLocal bool) {
-	// calculate total number of slots in drop. Accordingly add them to OverflowPool (if there is space)
-	availableSlotsOverflowPool := pool.availableSlotsOverflowPool()
-	if availableSlotsOverflowPool > 0 {
-		// transfer availableSlotsOverflowPool number of transactions slots from drop to OverflowPool
-		currentSlotsUsed := 0
-		for i, tx := range drop {
-			txSlots := numSlots(tx)
-			if currentSlotsUsed+txSlots <= availableSlotsOverflowPool {
-				from, _ := types.Sender(pool.signer, tx)
-				pool.localBufferPool.Add(tx)
-				log.Debug("adding to OverflowPool", "transaction", tx.Hash().String(), "from", from.String())
-				currentSlotsUsed += txSlots
-			} else {
-				log.Debug("not all got added to OverflowPool", "totalAdded", i+1)
-				return
-			}
+	for _, tx := range drop {
+		added := pool.localBufferPool.Add(tx)
+		if added {
+			from, _ := types.Sender(pool.signer, tx)
+			log.Debug("Added to OverflowPool", "transaction", tx.Hash().String(), "from", from.String())
+		} else {
+			log.Debug("Failed to add transaction to OverflowPool", "transaction", tx.Hash().String())
 		}
-	} else {
-		log.Debug("adding to OverflowPool unsuccessful", "availableSlotsOverflowPool", availableSlotsOverflowPool)
 	}
 }
 

core/txpool/legacypool/tx_overflowpool.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/log"
 )
 
 // txHeapItem implements the Interface interface (https://pkg.go.dev/container/heap#Interface) of heap so that it can be heapified
@@ -65,8 +66,8 @@ type TxOverflowPool struct {
 	txHeap    txHeap
 	index     map[common.Hash]*txHeapItem
 	mu        sync.RWMutex
-	maxSize   uint64
-	totalSize int
+	maxSize   uint64 // Maximum slots
+	totalSize int    // Total number of slots currently
 }
 
 func NewTxOverflowPoolHeap(estimatedMaxSize uint64) *TxOverflowPool {
@@ -77,34 +78,52 @@ func NewTxOverflowPoolHeap(estimatedMaxSize uint64) *TxOverflowPool {
 	}
 }
 
-func (tp *TxOverflowPool) Add(tx *types.Transaction) {
+func (tp *TxOverflowPool) Add(tx *types.Transaction) bool {
 	tp.mu.Lock()
 	defer tp.mu.Unlock()
 
 	if _, exists := tp.index[tx.Hash()]; exists {
 		// Transaction already in pool, ignore
-		return
+		return false
+	}
+
+	txSlots := numSlots(tx)
+
+	// If the transaction is too big to ever fit (and the pool isn't empty right now), reject it
+	if uint64(txSlots) >= tp.maxSize && tp.totalSize != 0 {
+		log.Warn("Transaction too large to fit in OverflowPool", "transaction", tx.Hash().String(), "requiredSlots", txSlots, "maxSlots", tp.maxSize)
+		return false
 	}
 
-	if uint64(len(tp.txHeap)) >= tp.maxSize {
-		// Remove the oldest transaction to make space
+	// Remove transactions until there is room for the new transaction
+	for uint64(tp.totalSize+txSlots) > tp.maxSize {
+		if tp.txHeap.Len() == 0 {
+			// No transactions left to remove, cannot make room
+			log.Warn("Not enough space in OverflowPool even after clearing", "transaction", tx.Hash().String())
+			return false
+		}
+		// Remove the oldest transaction
 		oldestItem, ok := heap.Pop(&tp.txHeap).(*txHeapItem)
 		if !ok || oldestItem == nil {
-			return
+			log.Error("Failed to pop from txHeap during Add")
+			return false
 		}
 		delete(tp.index, oldestItem.tx.Hash())
 		tp.totalSize -= numSlots(oldestItem.tx)
 		OverflowPoolGauge.Dec(1)
 	}
 
+	// Add the new transaction
 	item := &txHeapItem{
 		tx:        tx,
 		timestamp: time.Now().UnixNano(),
 	}
 	heap.Push(&tp.txHeap, item)
 	tp.index[tx.Hash()] = item
-	tp.totalSize += numSlots(tx)
+	tp.totalSize += txSlots
 	OverflowPoolGauge.Inc(1)
+
+	return true
 }
 
 func (tp *TxOverflowPool) Get(hash common.Hash) (*types.Transaction, bool) {

core/txpool/legacypool/tx_overflowpool_test.go

@@ -1,6 +1,7 @@
 package legacypool
 
 import (
+	rand3 "crypto/rand"
 	"math/big"
 	rand2 "math/rand"
 	"testing"
@@ -9,6 +10,7 @@ import (
 	"github.com/cometbft/cometbft/libs/rand"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/stretchr/testify/assert"
 )
 
 // Helper function to create a test transaction
@@ -157,6 +159,64 @@ func TestTxOverflowPoolHeapLen(t *testing.T) {
 	}
 }
 
+func TestTxOverflowPoolSlotCalculation(t *testing.T) {
+	// Initialize the pool with a maximum size of 2
+	pool := NewTxOverflowPoolHeap(2)
+
+	// Create two transactions with different slot requirements
+	tx1 := createTestTx(1, big.NewInt(1000)) // tx1 takes 1 slot
+	tx2 := createTestTx(2, big.NewInt(2000)) // tx2 takes 1 slot
+
+	// Add both transactions to fill the pool
+	pool.Add(tx1)
+	pool.Add(tx2)
+
+	if pool.Len() != 2 {
+		t.Fatalf("Expected pool size 2, but got %d", pool.Len())
+	}
+
+	// Capture the initial total size
+	initialTotalSize := pool.totalSize
+
+	dataSize := 40000
+	tx3 := createLargeTestTx(
+		3,                        // nonce
+		big.NewInt(100000000000), // gasPrice: 100 Gwei
+		dataSize,
+	) // takes 3 slots
+
+	// Create a third transaction with more slots than tx1
+	tx3Added := pool.Add(tx3)
+	assert.Equal(t, false, tx3Added)
+	assert.Equal(t, 2, pool.totalSize)
+
+	// Verify that the pool length remains at 2 after popping the oldest transaction
+	if pool.Len() != 2 {
+		t.Errorf("Expected pool size 2 after overflow, but got %d", pool.Len())
+	}
+
+	// Attempt to add a duplicate transaction (tx3) to see if it increases currentSlotsUsed erroneously
+	initialTotalSize = pool.totalSize
+	pool.Add(tx3)
+
+	// The total size should not change since tx3 is already in the pool
+	if pool.totalSize != initialTotalSize {
+		t.Errorf("Duplicate transaction incorrectly updated totalSize; expected %d but got %d", initialTotalSize, pool.totalSize)
+	}
+}
+
+func TestBiggerTx(t *testing.T) {
+	// Create a transaction with 40KB of data (which should take 2 slots)
+	dataSize := 40000
+	tx := createLargeTestTx(
+		0,                        // nonce
+		big.NewInt(100000000000), // gasPrice: 100 Gwei
+		dataSize,
+	)
+	numberOfSlots := numSlots(tx)
+	assert.Equal(t, 2, numberOfSlots)
+}
+
 // Helper function to create a random test transaction
 func createRandomTestTx() *types.Transaction {
 	nonce := uint64(rand.Intn(1000000))
@@ -176,6 +236,28 @@ func createRandomTestTxs(n int) []*types.Transaction {
 	return txs
 }
 
+// createLargeTestTx creates a transaction with a large data payload
+func createLargeTestTx(nonce uint64, gasPrice *big.Int, dataSize int) *types.Transaction {
+	// Generate random data of specified size
+	data := make([]byte, dataSize)
+	rand3.Read(data)
+
+	to := common.HexToAddress("0x1234567890123456789012345678901234567890")
+
+	// Calculate gas needed for the data
+	// Gas costs: 21000 (base) + 16 (per non-zero byte) or 4 (per zero byte)
+	gasLimit := uint64(21000 + (16 * len(data)))
+
+	return types.NewTransaction(
+		nonce,
+		to,
+		big.NewInt(1000),
+		gasLimit,
+		gasPrice,
+		data,
+	)
+}
+
 // goos: darwin
 // goarch: arm64
 // pkg: github.com/ethereum/go-ethereum/core/txpool/legacypool