From 41e765004cfb49a87f74727e6a55ac2e994c6b12 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Wed, 19 Feb 2025 12:02:44 +0800 Subject: [PATCH] core/txpool: move setcode tx validation into legacyPool --- core/txpool/errors.go | 8 +-- core/txpool/legacypool/legacypool.go | 63 +++++++++++++++++------ core/txpool/legacypool/legacypool_test.go | 14 ++--- core/txpool/validation.go | 13 ----- 4 files changed, 57 insertions(+), 41 deletions(-) diff --git a/core/txpool/errors.go b/core/txpool/errors.go index 32e49db87c..2607709d30 100644 --- a/core/txpool/errors.go +++ b/core/txpool/errors.go @@ -38,6 +38,10 @@ var ( // allowed by a pool for a single account. ErrAccountLimitExceeded = errors.New("account limit exceeded") + // ErrInflightTxLimitReached is returned when the maximum number of in-flight + // transactions is reached for specific accounts. + ErrInflightTxLimitReached = errors.New("in-flight transaction limit reached for delegated accounts") + // 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") @@ -65,8 +69,4 @@ var ( // signed by an address which already has in-flight transactions known to the // pool. ErrAuthorityReserved = errors.New("authority already reserved") - - // ErrAuthorityNonce is returned if a transaction has an authorization with - // a nonce that is not currently valid for the authority. - ErrAuthorityNonceTooLow = errors.New("authority nonce too low") ) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index c7ff954a49..ada3f1380c 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -581,11 +581,6 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction) error { if list := pool.queue[addr]; list != nil { have += list.Len() } - if pool.currentState.GetCodeHash(addr) != types.EmptyCodeHash || len(pool.all.auths[addr]) != 0 { - // Allow at most one in-flight tx for delegated accounts or those with - // a pending authorization. - return have, max(0, 1-have) - } return have, math.MaxInt }, ExistingExpenditure: func(addr common.Address) *big.Int { @@ -602,18 +597,6 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction) error { } return nil }, - KnownConflicts: func(from common.Address, auths []common.Address) []common.Address { - var conflicts []common.Address - // Authorities cannot conflict with any pending or queued transactions. - for _, addr := range auths { - if list := pool.pending[addr]; list != nil { - conflicts = append(conflicts, addr) - } else if list := pool.queue[addr]; list != nil { - conflicts = append(conflicts, addr) - } - } - return conflicts - }, } if err := txpool.ValidateTransactionWithState(tx, pool.signer, opts); err != nil { return err @@ -621,6 +604,47 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction) error { return nil } +// validateAuth verifies that the transaction complies with code authorization +// restrictions brought by SetCode transaction type. +func (pool *LegacyPool) validateAuth(tx *types.Transaction) error { + from, _ := types.Sender(pool.signer, tx) // validated + + // Allow at most one in-flight tx for delegated accounts or those with a + // pending authorization. + if pool.currentState.GetCodeHash(from) != types.EmptyCodeHash || len(pool.all.auths[from]) != 0 { + var ( + count int + exists bool + ) + pending := pool.pending[from] + if pending != nil { + count += pending.Len() + exists = pending.Contains(tx.Nonce()) + } + queue := pool.queue[from] + if queue != nil { + count += queue.Len() + if !exists { + exists = queue.Contains(tx.Nonce()) + } + } + // Replace the existing inflight transaction for delegated accounts + // are still supported + if count >= 1 && !exists { + return txpool.ErrInflightTxLimitReached + } + } + // Authorities cannot conflict with any pending or queued transactions. + if auths := tx.SetCodeAuthorities(); len(auths) > 0 { + for _, auth := range auths { + if pool.pending[auth] != nil || pool.queue[auth] != nil { + return txpool.ErrAuthorityReserved + } + } + } + return nil +} + // add validates a transaction and inserts it into the non-executable queue for later // pending promotion and execution. If the transaction is a replacement for an already // pending or queued one, it overwrites the previous transaction if its price is higher. @@ -639,6 +663,11 @@ func (pool *LegacyPool) add(tx *types.Transaction) (replaced bool, err error) { invalidTxMeter.Mark(1) return false, err } + if err := pool.validateAuth(tx); err != nil { + log.Trace("Discarding invalid transaction", "hash", hash, "err", err) + invalidTxMeter.Mark(1) + return false, err + } // already validated by this point from, _ := types.Sender(pool.signer, tx) diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index 7e35e0cc60..81e45055f7 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -2248,12 +2248,12 @@ func TestSetCodeTransactions(t *testing.T) { if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1), keyA)); err != nil { t.Fatalf("%s: failed to add remote transaction: %v", name, err) } - if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keyA)); !errors.Is(err, txpool.ErrAccountLimitExceeded) { - t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrAccountLimitExceeded, err) + if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keyA)); !errors.Is(err, txpool.ErrInflightTxLimitReached) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrInflightTxLimitReached, err) } // Also check gapped transaction. - if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1), keyA)); !errors.Is(err, txpool.ErrAccountLimitExceeded) { - t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrAccountLimitExceeded, err) + if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1), keyA)); !errors.Is(err, txpool.ErrInflightTxLimitReached) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrInflightTxLimitReached, err) } // Replace by fee. if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(10), keyA)); err != nil { @@ -2287,8 +2287,8 @@ func TestSetCodeTransactions(t *testing.T) { t.Fatalf("%s: failed to add with pending delegatio: %v", name, err) } // Also check gapped transaction is rejected. - if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keyC)); !errors.Is(err, txpool.ErrAccountLimitExceeded) { - t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrAccountLimitExceeded, err) + if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keyC)); !errors.Is(err, txpool.ErrInflightTxLimitReached) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrInflightTxLimitReached, err) } }, }, @@ -2363,7 +2363,7 @@ func TestSetCodeTransactions(t *testing.T) { if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keyC)); err != nil { t.Fatalf("%s: failed to added single pooled for account with pending delegation: %v", name, err) } - if err, want := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1000), keyC)), txpool.ErrAccountLimitExceeded; !errors.Is(err, want) { + if err, want := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1000), keyC)), txpool.ErrInflightTxLimitReached; !errors.Is(err, want) { t.Fatalf("%s: error mismatch: want %v, have %v", name, want, err) } }, diff --git a/core/txpool/validation.go b/core/txpool/validation.go index 4d53c386b6..e464839546 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -218,11 +218,6 @@ type ValidationOptionsWithState struct { // ExistingCost is a mandatory callback to retrieve an already pooled // transaction's cost with the given nonce to check for overdrafts. ExistingCost func(addr common.Address, nonce uint64) *big.Int - - // KnownConflicts is an optional callback which iterates over the list of - // addresses and returns all addresses known to the pool with in-flight - // transactions. - KnownConflicts func(sender common.Address, authorizers []common.Address) []common.Address } // ValidateTransactionWithState is a helper method to check whether a transaction @@ -276,14 +271,6 @@ func ValidateTransactionWithState(tx *types.Transaction, signer types.Signer, op if used, left := opts.UsedAndLeftSlots(from); left <= 0 { return fmt.Errorf("%w: pooled %d txs", ErrAccountLimitExceeded, used) } - - // Verify no authorizations will invalidate existing transactions known to - // the pool. - if opts.KnownConflicts != nil { - if conflicts := opts.KnownConflicts(from, tx.SetCodeAuthorities()); len(conflicts) > 0 { - return fmt.Errorf("%w: authorization conflicts with other known tx", ErrAuthorityReserved) - } - } } return nil }