Skip to content

Commit

Permalink
core/txpool: move setcode tx validation into legacyPool (#31209)
Browse files Browse the repository at this point in the history 
In this PR, several improvements have been made:

Authorization-related validations have been moved to legacyPool. 
Previously, these checks were part of the standard validation procedure,
which applies common validations across different pools. Since these 
checks are specific to SetCode transactions, relocating them to
legacyPool
is a more reasonable choice.

Additionally, authorization conflict checks are now performed regardless
of whether the transaction is a replacement or not.

---------

Co-authored-by: lightclient <lightclient@protonmail.com>
  • Loading branch information
@rjl493456442 @lightclient
rjl493456442 and lightclient authored Feb 24, 2025
1 parent fbe0005 commit 9211a0e
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 69 deletions.
13 changes: 0 additions & 13 deletions core/txpool/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,22 +51,9 @@ var (
// making the transaction invalid, rather a DOS protection.
ErrOversizedData = errors.New("oversized data")

// ErrFutureReplacePending is returned if a future transaction replaces a pending
// one. Future transactions should only be able to replace other future transactions.
ErrFutureReplacePending = errors.New("future transaction tries to replace pending")

// ErrAlreadyReserved is returned if the sender address has a pending transaction
// in a different subpool. For example, this error is returned in response to any
// input transaction of non-blob type when a blob transaction from this sender
// remains pending (and vice-versa).
ErrAlreadyReserved = errors.New("address already reserved")

// ErrAuthorityReserved is returned if a transaction has an authorization
// 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")
)
84 changes: 55 additions & 29 deletions core/txpool/legacypool/legacypool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,19 @@ var (
// ErrTxPoolOverflow is returned if the transaction pool is full and can't accept
// another remote transaction.
ErrTxPoolOverflow = errors.New("txpool is full")

// 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")

// ErrAuthorityReserved is returned if a transaction has an authorization
// signed by an address which already has in-flight transactions known to the
// pool.
ErrAuthorityReserved = errors.New("authority already reserved")

// ErrFutureReplacePending is returned if a future transaction replaces a pending
// one. Future transactions should only be able to replace other future transactions.
ErrFutureReplacePending = errors.New("future transaction tries to replace pending")
)

var (
Expand Down Expand Up @@ -572,22 +585,8 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction) error {
opts := &txpool.ValidationOptionsWithState{
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()
}
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
},
FirstNonceGap: nil, // Pool allows arbitrary arrival order, don't invalidate nonce gaps
UsedAndLeftSlots: nil, // Pool has own mechanism to limit the number of transactions
ExistingExpenditure: func(addr common.Address) *big.Int {
if list := pool.pending[addr]; list != nil {
return list.totalcost.ToBig()
Expand All @@ -602,22 +601,49 @@ 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
}
return pool.validateAuth(tx)
}

// 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()
exists = exists || queue.Contains(tx.Nonce())
}
// Replace the existing in-flight transaction for delegated accounts
// are still supported
if count >= 1 && !exists {
return 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 ErrAuthorityReserved
}
}
}
return nil
}

Expand Down Expand Up @@ -709,7 +735,7 @@ func (pool *LegacyPool) add(tx *types.Transaction) (replaced bool, err error) {
pool.priced.Put(dropTx)
}
log.Trace("Discarding future transaction replacing pending tx", "hash", hash)
return false, txpool.ErrFutureReplacePending
return false, ErrFutureReplacePending
}
}

Expand Down
24 changes: 12 additions & 12 deletions core/txpool/legacypool/legacypool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1645,8 +1645,8 @@ func TestUnder#(t *testing.T) {
t.Fatalf("failed to add well priced transaction: %v", err)
}
// Ensure that replacing a pending transaction with a future transaction fails
if err := pool.addRemoteSync(pricedTransaction(5, 100000, big.NewInt(6), keys[1])); err != txpool.ErrFutureReplacePending {
t.Fatalf("adding future replace transaction error mismatch: have %v, want %v", err, txpool.ErrFutureReplacePending)
if err := pool.addRemoteSync(pricedTransaction(5, 100000, big.NewInt(6), keys[1])); err != ErrFutureReplacePending {
t.Fatalf("adding future replace transaction error mismatch: have %v, want %v", err, ErrFutureReplacePending)
}
pending, queued = pool.Stats()
if pending != 4 {
Expand Down Expand Up @@ -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, ErrInflightTxLimitReached) {
t.Fatalf("%s: error mismatch: want %v, have %v", name, 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, ErrInflightTxLimitReached) {
t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrInflightTxLimitReached, err)
}
// Replace by fee.
if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(10), keyA)); err != nil {
Expand Down Expand Up @@ -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, ErrInflightTxLimitReached) {
t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrInflightTxLimitReached, err)
}
},
},
Expand Down Expand Up @@ -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)), ErrInflightTxLimitReached; !errors.Is(err, want) {
t.Fatalf("%s: error mismatch: want %v, have %v", name, want, err)
}
},
Expand All @@ -2376,7 +2376,7 @@ func TestSetCodeTransactions(t *testing.T) {
if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keyC)); err != nil {
t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err)
}
if err, want := pool.addRemoteSync(setCodeTx(0, keyA, []unsignedAuth{{1, keyC}})), txpool.ErrAuthorityReserved; !errors.Is(err, want) {
if err, want := pool.addRemoteSync(setCodeTx(0, keyA, []unsignedAuth{{1, keyC}})), ErrAuthorityReserved; !errors.Is(err, want) {
t.Fatalf("%s: error mismatch: want %v, have %v", name, want, err)
}
},
Expand Down Expand Up @@ -2440,8 +2440,8 @@ func TestSetCodeTransactionsReorg(t *testing.T) {
t.Fatalf("failed to add with remote setcode transaction: %v", err)
}
// Try to add a transactions in
if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1000), keyA)); !errors.Is(err, txpool.ErrAccountLimitExceeded) {
t.Fatalf("unexpected error %v, expecting %v", err, txpool.ErrAccountLimitExceeded)
if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1000), keyA)); !errors.Is(err, ErrInflightTxLimitReached) {
t.Fatalf("unexpected error %v, expecting %v", err, ErrInflightTxLimitReached)
}
// Simulate the chain moving
blockchain.statedb.SetNonce(addrA, 2, tracing.NonceChangeAuthorization)
Expand Down
19 changes: 4 additions & 15 deletions core/txpool/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ 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
// UsedAndLeftSlots is an optional 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)
Expand All @@ -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
Expand Down Expand Up @@ -273,15 +268,9 @@ func ValidateTransactionWithState(tx *types.Transaction, signer types.Signer, op
// Transaction takes a new nonce value out of the pool. Ensure it doesn't
// overflow the number of permitted transactions from a single account
// (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)
}

// 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)
if opts.UsedAndLeftSlots != nil {
if used, left := opts.UsedAndLeftSlots(from); left <= 0 {
return fmt.Errorf("%w: pooled %d txs", ErrAccountLimitExceeded, used)
}
}
}
Expand Down

0 comments on commit 9211a0e

Please # to comment.