diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 0d5a1fb183..a1e75bc376 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/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) } } diff --git a/core/txpool/legacypool/tx_overflowpool.go b/core/txpool/legacypool/tx_overflowpool.go index 4bfd4b6f5a..290cb2220c 100644 --- a/core/txpool/legacypool/tx_overflowpool.go +++ b/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) { diff --git a/core/txpool/legacypool/tx_overflowpool_test.go b/core/txpool/legacypool/tx_overflowpool_test.go index 9a4aee5008..e0976eca2f 100644 --- a/core/txpool/legacypool/tx_overflowpool_test.go +++ b/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