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

Prior to this change, it was possible that transactions are erroneously deemed as 'future' although they are in fact 'pending', causing them to be dropped due to 'future' not being allowed to replace 'pending'. 

This change fixes that, by doing a more in-depth inspection of the queue.
This commit is contained in:
Marius van der Wijden 2023-04-05 10:59:32 +02:00 committed by GitHub
parent 2adce0b066
commit 230df98e4d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 51 additions and 19 deletions

View File

@ -270,10 +270,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 // Add tries to insert a new transaction into the list, returning whether the

View File

@ -745,11 +745,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 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 var replacesPending bool
for _, dropTx := range drop { for _, dropTx := range drop {
dropSender, _ := types.Sender(pool.signer, dropTx) 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 replacesPending = true
break break
} }
@ -774,7 +774,7 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e
} }
// Try to replace an existing transaction in the pending pool // 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 // Nonce already pending, check if required price bump is met
inserted, old := list.Add(tx, pool.config.PriceBump) inserted, old := list.Add(tx, pool.config.PriceBump)
if !inserted { if !inserted {
@ -817,18 +817,26 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e
return replaced, nil 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 {
return false
} }
// Sender has pending transactions. // The transaction has a nonce gap with pending list, it's only considered
if old := list.txs.Get(tx.Nonce()); old != nil { // as executable if transactions in queue can fill up the nonce gap.
return false // It replaces a pending transaction. queue, ok := pool.queue[from]
if !ok {
return true
} }
// Not replacing, check if parent nonce exists in pending. for nonce := next; nonce < tx.Nonce(); nonce++ {
return list.txs.Get(tx.Nonce()-1) == nil 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. // enqueueTx inserts a new transaction into the non-executable transaction queue.

View File

@ -42,7 +42,7 @@ func count(t *testing.T, pool *TxPool) (pending int, queued int) {
return pending, queued return pending, queued
} }
func fillPool(t *testing.T, pool *TxPool) { func fillPool(t testing.TB, pool *TxPool) {
t.Helper() t.Helper()
// Create a number of test accounts, fund them and make transactions // Create a number of test accounts, fund them and make transactions
executableTxs := types.Transactions{} executableTxs := types.Transactions{}
@ -189,7 +189,7 @@ func TestTransactionZAttack(t *testing.T) {
key, _ := crypto.GenerateKey() key, _ := crypto.GenerateKey()
pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(100000000000)) pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(100000000000))
for j := 0; j < int(pool.config.GlobalSlots); j++ { 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) pool.AddRemotesSync(overDraftTxs)
@ -210,3 +210,27 @@ func TestTransactionZAttack(t *testing.T) {
newIvPending, ivPending, pool.config.GlobalSlots, newQueued) newIvPending, ivPending, 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()), nil)
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))
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)
}
}