From 8d49623f819ee5b3b72f8583726f9c429362f4fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 26 Jul 2023 23:39:08 +0300 Subject: [PATCH] core/txpool: limit blob transactions to 16 per account --- core/txpool/blobpool/blobpool.go | 52 ++++++++++++ core/txpool/blobpool/blobpool_test.go | 118 ++++++++++++++++++++++++++ core/txpool/errors.go | 4 + core/txpool/legacypool/legacypool.go | 10 +++ core/txpool/validation.go | 11 +++ 5 files changed, 195 insertions(+) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index f2a77204e4b46..feceed8dd236e 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -65,6 +65,14 @@ const ( // limit can never hurt. txMaxSize = 1024 * 1024 + // maxTxsPerAccount is the maximum number of blob transactions admitted from + // a single account. The limit is enforced to minimize the DoS potential of + // a private tx cancelling publicly propagated blobs. + // + // Note, transactions resurrected by a reorg are also subject to this limit, + // so pushing it down too agressively might make resurrections non-functional. + maxTxsPerAccount = 16 + // pendingTransactionStore is the subfolder containing the currently queued // blob transactions. pendingTransactionStore = "queue" @@ -165,6 +173,13 @@ func newBlobTxMeta(id uint64, size uint32, tx *types.Transaction) *blobTxMeta { // txs are disallowed; c) the presence of blob transactions exclude non-blob // transactions. // +// - Malicious cancellations are possible. Although the pool might prevent txs +// that cancel blobs, blocks might contain such transaction (malicious miner +// or flashbotter). The pool should cap the total number of blob transactions +// per account as to prevent propagating too much data before cancelling it +// via a normal transaction. It should nonetheless be high enough to support +// resurrecting reorged transactions. Perhaps 4-16. +// // - Local txs are meaningless. Mining pools historically used local transactions // for payouts or for backdoor deals. With 1559 in place, the basefee usually // dominates the final price, so 0 or non-0 tip doesn't change much. Blob txs @@ -652,6 +667,36 @@ func (p *BlobPool) recheck(addr common.Address, inclusions map[common.Hash]uint6 } } } + // Sanity check that no account can have more queued transactions than the + // DoS protection threshold. + if len(txs) > maxTxsPerAccount { + // Evict the highest nonce transactions until the pending set falls under + // the account's transaction cap + var ( + ids []uint64 + nonces []uint64 + ) + for len(txs) > maxTxsPerAccount { + last := txs[len(txs)-1] + txs[len(txs)-1] = nil + txs = txs[:len(txs)-1] + + ids = append(ids, last.id) + nonces = append(nonces, last.nonce) + + p.spent[addr] = new(uint256.Int).Sub(p.spent[addr], last.costCap) + p.stored -= uint64(last.size) + delete(p.lookup, last.hash) + } + p.index[addr] = txs + + log.Warn("Dropping overcapped blob transactions", "from", addr, "kept", len(txs), "drop", nonces, "ids", ids) + for _, id := range ids { + if err := p.store.Delete(id); err != nil { + log.Error("Failed to delete blob transaction", "from", addr, "id", id, "err", err) + } + } + } // Included cheap transactions might have left the remaining ones better from // an eviction point, fix any potential issues in the heap. if _, ok := p.index[addr]; ok && inclusions != nil { @@ -998,6 +1043,13 @@ func (p *BlobPool) validateTx(tx *types.Transaction, blobs []kzg4844.Blob, commi // have pooled. return p.state.GetNonce(addr) + uint64(len(p.index[addr])) }, + UsedAndLeftSlots: func(addr common.Address) (int, int) { + have := len(p.index[addr]) + if have >= maxTxsPerAccount { + return have, 0 + } + return have, maxTxsPerAccount - have + }, ExistingExpenditure: func(addr common.Address) *big.Int { if spent := p.spent[addr]; spent != nil { return spent.ToBig() diff --git a/core/txpool/blobpool/blobpool_test.go b/core/txpool/blobpool/blobpool_test.go index 0403e2e1100f3..6e492e9031cbe 100644 --- a/core/txpool/blobpool/blobpool_test.go +++ b/core/txpool/blobpool/blobpool_test.go @@ -486,6 +486,22 @@ func TestOpenDrops(t *testing.T) { overdrafted[id] = struct{}{} } } + // Insert a sequence of transactions overflowing the account cap to verify + // that part of the set will get invalidated. + var ( + overcapper, _ = crypto.GenerateKey() + overcapped = make(map[uint64]struct{}) + ) + for nonce := uint64(0); nonce < maxTxsPerAccount+3; nonce++ { + blob, _ := rlp.EncodeToBytes(&blobTx{Tx: makeTx(nonce, 1, 1, 1, overcapper)}) + + id, _ := store.Put(blob) + if nonce < maxTxsPerAccount { + valids[id] = struct{}{} + } else { + overcapped[id] = struct{}{} + } + } store.Close() // Create a blob pool out of the pre-seeded data @@ -500,6 +516,7 @@ func TestOpenDrops(t *testing.T) { statedb.AddBalance(crypto.PubkeyToAddress(outpricer.PublicKey), big.NewInt(1000000)) statedb.AddBalance(crypto.PubkeyToAddress(exceeder.PublicKey), big.NewInt(1000000)) statedb.AddBalance(crypto.PubkeyToAddress(overdrafter.PublicKey), big.NewInt(1000000)) + statedb.AddBalance(crypto.PubkeyToAddress(overcapper.PublicKey), big.NewInt(10000000)) statedb.Commit(true) chain := &testBlockChain{ @@ -541,6 +558,8 @@ func TestOpenDrops(t *testing.T) { t.Errorf("fully overdrafted transaction remained in storage: %d", tx.id) } else if _, ok := overdrafted[tx.id]; ok { t.Errorf("partially overdrafted transaction remained in storage: %d", tx.id) + } else if _, ok := overcapped[tx.id]; ok { + t.Errorf("overcapped transaction remained in storage: %d", tx.id) } else { alive[tx.id] = struct{}{} } @@ -981,6 +1000,105 @@ func TestAdd(t *testing.T) { }, }, }, + // Transactions should only be accepted into the pool if the total count + // from the same account doesn't overflow the pool limits + { + seeds: map[string]seed{ + "alice": {balance: 10000000}, + }, + adds: []addtx{ + { // New account, no previous txs, 16 slots left: accept nonce 0 + from: "alice", + tx: makeUnsignedTx(0, 1, 1, 1), + err: nil, + }, + { // New account, 1 pooled tx, 15 slots left: accept nonce 1 + from: "alice", + tx: makeUnsignedTx(1, 1, 1, 1), + err: nil, + }, + { // New account, 2 pooled tx, 14 slots left: accept nonce 2 + from: "alice", + tx: makeUnsignedTx(2, 1, 1, 1), + err: nil, + }, + { // New account, 3 pooled tx, 13 slots left: accept nonce 3 + from: "alice", + tx: makeUnsignedTx(3, 1, 1, 1), + err: nil, + }, + { // New account, 4 pooled tx, 12 slots left: accept nonce 4 + from: "alice", + tx: makeUnsignedTx(4, 1, 1, 1), + err: nil, + }, + { // New account, 5 pooled tx, 11 slots left: accept nonce 5 + from: "alice", + tx: makeUnsignedTx(5, 1, 1, 1), + err: nil, + }, + { // New account, 6 pooled tx, 10 slots left: accept nonce 6 + from: "alice", + tx: makeUnsignedTx(6, 1, 1, 1), + err: nil, + }, + { // New account, 7 pooled tx, 9 slots left: accept nonce 7 + from: "alice", + tx: makeUnsignedTx(7, 1, 1, 1), + err: nil, + }, + { // New account, 8 pooled tx, 8 slots left: accept nonce 8 + from: "alice", + tx: makeUnsignedTx(8, 1, 1, 1), + err: nil, + }, + { // New account, 9 pooled tx, 7 slots left: accept nonce 9 + from: "alice", + tx: makeUnsignedTx(9, 1, 1, 1), + err: nil, + }, + { // New account, 10 pooled tx, 6 slots left: accept nonce 10 + from: "alice", + tx: makeUnsignedTx(10, 1, 1, 1), + err: nil, + }, + { // New account, 11 pooled tx, 5 slots left: accept nonce 11 + from: "alice", + tx: makeUnsignedTx(11, 1, 1, 1), + err: nil, + }, + { // New account, 12 pooled tx, 4 slots left: accept nonce 12 + from: "alice", + tx: makeUnsignedTx(12, 1, 1, 1), + err: nil, + }, + { // New account, 13 pooled tx, 3 slots left: accept nonce 13 + from: "alice", + tx: makeUnsignedTx(13, 1, 1, 1), + err: nil, + }, + { // New account, 14 pooled tx, 2 slots left: accept nonce 14 + from: "alice", + tx: makeUnsignedTx(14, 1, 1, 1), + err: nil, + }, + { // New account, 15 pooled tx, 1 slots left: accept nonce 15 + from: "alice", + tx: makeUnsignedTx(15, 1, 1, 1), + err: nil, + }, + { // New account, 16 pooled tx, 0 slots left: accept nonce 15 replacement + from: "alice", + tx: makeUnsignedTx(15, 10, 10, 10), + err: nil, + }, + { // New account, 16 pooled tx, 0 slots left: reject nonce 16 with overcap + from: "alice", + tx: makeUnsignedTx(16, 1, 1, 1), + err: txpool.ErrAccountLimitExceeded, + }, + }, + }, // Previously existing transactions should be allowed to be replaced iff // the new cumulative expenditure can be covered by the account and the // prices are bumped all around (no percentage check here). diff --git a/core/txpool/errors.go b/core/txpool/errors.go index b8c1c914a347d..bc26550f78ca2 100644 --- a/core/txpool/errors.go +++ b/core/txpool/errors.go @@ -34,6 +34,10 @@ var ( // with a different one without the required price bump. ErrReplaceUnderpriced = errors.New("replacement transaction underpriced") + // ErrAccountLimitExceeded is returned if a transaction would exceed the number + // allowed by a pool for a single account. + ErrAccountLimitExceeded = errors.New("account limit exceeded") + // ErrGasLimit is returned if a transaction's requested gas limit exceeds the // maximum allowance of the current block. ErrGasLimit = errors.New("exceeds block gas limit") diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 184856df59b24..782f0facb5195 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -601,6 +601,16 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction, local bool) error { State: pool.currentState, FirstNonceGap: nil, // Pool allows arbitrary arrival order, don't invalidate nonce gaps + UsedAndLeftSlots: func(addr common.Address) (int, int) { + var have int + if list := pool.pending[addr]; list != nil { + have += list.Len() + } + if list := pool.queue[addr]; list != nil { + have += list.Len() + } + return have, math.MaxInt + }, ExistingExpenditure: func(addr common.Address) *big.Int { if list := pool.pending[addr]; list != nil { return list.totalcost diff --git a/core/txpool/validation.go b/core/txpool/validation.go index af678277f868a..67c39a99396f9 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -166,6 +166,11 @@ type ValidationOptionsWithState struct { // nonce gaps will be ignored and permitted. FirstNonceGap func(addr common.Address) uint64 + // UsedAndLeftSlots is a mandatory callback to retrieve the number of tx slots + // used and the number still permitted for an account. New transactions will + // be rejected once the number of remaining slots reaches zero. + UsedAndLeftSlots func(addr common.Address) (int, int) + // ExistingExpenditure is a mandatory callback to retrieve the cummulative // cost of the already pooled transactions to check for overdrafts. ExistingExpenditure func(addr common.Address) *big.Int @@ -220,6 +225,12 @@ func ValidateTransactionWithState(tx *types.Transaction, signer types.Signer, op if balance.Cmp(need) < 0 { return fmt.Errorf("%w: balance %v, queued cost %v, tx cost %v, overshot %v", core.ErrInsufficientFunds, balance, spent, cost, new(big.Int).Sub(need, balance)) } + // Transaction takes a new nonce value out of the pool. Ensure it doesn't + // overflow the number of permitted transactions from a single accoun + // (i.e. max cancellable via out-of-bound transaction). + if used, left := opts.UsedAndLeftSlots(from); left <= 0 { + return fmt.Errorf("%w: pooled %d txs", ErrAccountLimitExceeded, used) + } } return nil }