core/txpool: disallow future churn by remote txs #26907 (#1860)

core/txpool/list.go

@@ -289,10 +289,10 @@ func newList(strict bool) *list {
 	}
 }
 
-// Overlaps returns whether the transaction specified has the same nonce as one
+// Contains returns whether the list contains a transaction
-// already contained within the list.
+// with the provided nonce.
-func (l *list) Overlaps(tx *types.Transaction) bool {
+func (l *list) Contains(nonce uint64) bool {
-	return l.txs.Get(tx.Nonce()) != nil
+	return l.txs.Get(nonce) != nil
 }
 
 // Add tries to insert a new transaction into the list, returning whether the

core/txpool/txpool.go

@@ -845,11 +845,11 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e
 		}
 
 		// If the new transaction is a future transaction it should never churn pending transactions
-		if !isLocal && pool.isFuture(from, tx) {
+		if !isLocal && pool.isGapped(from, tx) {
 			var replacesPending bool
 			for _, dropTx := range drop {
 				dropSender, _ := types.Sender(pool.signer, dropTx)
-				if list := pool.pending[dropSender]; list != nil && list.Overlaps(dropTx) {
+				if list := pool.pending[dropSender]; list != nil && list.Contains(dropTx.Nonce()) {
 					replacesPending = true
 					break
 				}
@@ -874,7 +874,7 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e
 	}
 
 	// Try to replace an existing transaction in the pending pool
-	if list := pool.pending[from]; list != nil && list.Overlaps(tx) {
+	if list := pool.pending[from]; list != nil && list.Contains(tx.Nonce()) {
 		// Nonce already pending, check if required price bump is met
 		inserted, old := list.Add(tx, pool.config.PriceBump)
 		if !inserted {
@@ -917,23 +917,26 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e
 	return replaced, nil
 }
 
-// isFuture reports whether the given transaction is immediately executable.
+// isGapped reports whether the given transaction is immediately executable.
-func (pool *TxPool) isFuture(from common.Address, tx *types.Transaction) bool {
+func (pool *TxPool) isGapped(from common.Address, tx *types.Transaction) bool {
-	list := pool.pending[from]
+	// Short circuit if transaction matches pending nonce and can be promoted
-	if list == nil {
+	// to pending list as an executable transaction.
-		return pool.pendingNonces.get(from) != tx.Nonce()
+	next := pool.pendingNonces.get(from)
-	}
+	if tx.Nonce() == next {
-	// Sender has pending transactions.
-	if old := list.txs.Get(tx.Nonce()); old != nil {
-		return false // It replaces a pending transaction.
-	}
-	// Explicitly handle nonce 0 to avoid unsigned underflow.
-	nonce := tx.Nonce()
-	if nonce == 0 {
 		return false
 	}
-	// Not replacing, check if parent nonce exists in pending.
+	// The transaction has a nonce gap with pending list, it's only considered
-	return list.txs.Get(nonce-1) == nil
+	// as executable if transactions in queue can fill up the nonce gap.
+	queue, ok := pool.queue[from]
+	if !ok {
+		return true
+	}
+	for nonce := next; nonce < tx.Nonce(); nonce++ {
+		if !queue.Contains(nonce) {
+			return true // txs in queue can't fill up the nonce gap
+		}
+	}
+	return false
 }
 
 // enqueueTx inserts a new transaction into the non-executable transaction queue.

core/txpool/txpool2_test.go

@@ -43,7 +43,7 @@ func count(t *testing.T, pool *TxPool) (pending int, queued int) {
 	return pending, queued
 }
 
-func fillPool(t *testing.T, pool *TxPool) {
+func fillPool(t testing.TB, pool *TxPool) {
 	t.Helper()
 	// Create a number of test accounts, fund them and make transactions
 	executableTxs := types.Transactions{}
@@ -190,7 +190,7 @@ func TestTransactionZAttack(t *testing.T) {
 		key, _ := crypto.GenerateKey()
 		pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(100000000000), tracing.BalanceChangeUnspecified)
 		for j := 0; j < int(pool.config.GlobalSlots); j++ {
-			overDraftTxs = append(overDraftTxs, pricedValuedTransaction(uint64(j), 60000000000, 21000, big.NewInt(500), key))
+			overDraftTxs = append(overDraftTxs, pricedValuedTransaction(uint64(j), 600000000000, 21000, big.NewInt(500), key))
 		}
 	}
 	pool.AddRemotesSync(overDraftTxs)
@@ -214,3 +214,27 @@ func TestTransactionZAttack(t *testing.T) {
 			newIvPending, ivPending, delta, common.LimitThresholdNonceInQueue, pool.config.GlobalSlots, newQueued)
 	}
 }
+
+func BenchmarkFutureAttack(b *testing.B) {
+	// Create the pool to test the limit enforcement with
+	statedb, _ := state.New(common.Hash{}, state.NewDatabase(rawdb.NewMemoryDatabase()))
+	blockchain := newTestBlockChain(1000000, statedb, new(event.Feed))
+	config := testTxPoolConfig
+	config.GlobalQueue = 100
+	config.GlobalSlots = 100
+	pool := NewTxPool(config, eip1559Config, blockchain)
+	defer pool.Stop()
+	fillPool(b, pool)
+
+	key, _ := crypto.GenerateKey()
+	pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(100000000000), tracing.BalanceChangeUnspecified)
+	futureTxs := types.Transactions{}
+
+	for n := 0; n < b.N; n++ {
+		futureTxs = append(futureTxs, pricedTransaction(1000+uint64(n), 100000, big.NewInt(500), key))
+	}
+	b.ResetTimer()
+	for i := 0; i < 5; i++ {
+		pool.AddRemotesSync(futureTxs)
+	}
+}