Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/txpool/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,10 @@ func newList(strict bool) *list {
}
}

// Overlaps returns whether the transaction specified has the same nonce as one
// already contained within the list.
func (l *list) Overlaps(tx *types.Transaction) bool {
return l.txs.Get(tx.Nonce()) != nil
// Contains returns whether the list contains a transaction
// with the provided nonce.
func (l *list) Contains(nonce uint64) bool {
return l.txs.Get(nonce) != nil
}

// Add tries to insert a new transaction into the list, returning whether the
Expand Down
34 changes: 21 additions & 13 deletions core/txpool/txpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,11 +736,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
}
Expand All @@ -765,7 +765,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 {
Expand Down Expand Up @@ -808,18 +808,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.
func (pool *TxPool) isFuture(from common.Address, tx *types.Transaction) bool {
list := pool.pending[from]
if list == nil {
return pool.pendingNonces.get(from) != tx.Nonce()
// isGapped reports whether the given transaction is immediately executable.
func (pool *TxPool) isGapped(from common.Address, tx *types.Transaction) bool {
// Short circuit if transaction matches pending nonce and can be promoted
// to pending list as an executable transaction.
next := pool.pendingNonces.get(from)
if tx.Nonce() == next {
return false
}
// Sender has pending transactions.
if old := list.txs.Get(tx.Nonce()); old != nil {
return false // It replaces a pending transaction.
// The transaction has a nonce gap with pending list, it's only considered
// as executable if transactions in queue can fill up the nonce gap.
queue, ok := pool.queue[from]
holiman marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
return true
}
// Not replacing, check if parent nonce exists in pending.
return list.txs.Get(tx.Nonce()-1) == nil
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.
Expand Down
28 changes: 26 additions & 2 deletions core/txpool/txpool2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wow today I learned! cool!

t.Helper()
// Create a number of test accounts, fund them and make transactions
executableTxs := types.Transactions{}
Expand Down Expand Up @@ -189,7 +189,7 @@ func TestTransactionZAttack(t *testing.T) {
key, _ := crypto.GenerateKey()
pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(100000000000))
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)
Expand All @@ -210,3 +210,27 @@ func TestTransactionZAttack(t *testing.T) {
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{}

b.ResetTimer()
for n := 0; n < b.N; n++ {
futureTxs = append(futureTxs, pricedTransaction(1000+uint64(n), 100000, big.NewInt(500), key))
}
holiman marked this conversation as resolved.
Show resolved Hide resolved
for i := 0; i < 5; i++ {
pool.AddRemotesSync(futureTxs)
}
}