core/txpool: allow tx and authority regardless of admission order (#31373)

This PR proposes a change to the authorizations' validation introduced
in commit cdb66c8. These changes make the expected behavior independent
of the order of admission of authorizations, improving the
predictability of the resulting state and the usability of the system
with it.

The current implementation behavior is dependent on the transaction
submission order: This issue is related to authorities and the sender of
a transaction, and can be reproduced respecting the normal nonce rules.

The issue can be reproduced by the two following cases:
**First case**
- Given an empty pool.
- Submit transaction `{ from: B, auths [ A ] }`: is accepted.
- Submit transaction `{ from: A }`: Is accepted: it becomes the one
in-flight transaction allowed.

**Second case**
- Given an empty pool.
- Submit transaction `{ from: A }`:  is accepted
- Submit transaction `{ from: B, auths [ A ] }`: is rejected since there
is already a queued/pending transaction from A.

The expected behavior is that both sequences of events would lead to the
same sets of accepted and rejected transactions.

**Proposed changes** 
The queued/pending transactions issued from any authority of the
transaction being validated have to be counted, allowing one transaction
from accounts submitting an authorization.

- Notice that the expected behavior was explicitly forbidden in the case
"reject-delegation-from-pending-account", I believe that this behavior
conflicts to the definition of the limitation, and it is removed in this
PR. The expected behavior is tested in
"accept-authorization-from-sender-of-one-inflight-tx".
- Replacement tests have been separated to improve readability of the
acceptance test.
- The test "allow-more-than-one-tx-from-replaced-authority" has been
extended with one extra transaction, since the system would always have
accepted one transaction (but not two).
- The test "accept-one-inflight-tx-of-delegated-account" is extended to
clean-up state, avoiding leaking the delegation used into the other
tests. Additionally, replacement check is removed to be tested in its
own test case.

**Expected behavior** 
The expected behavior of the authorizations' validation shall be as
follows:

![image](https://github.com/user-attachments/assets/dbde7a1f-9679-4691-94eb-c197a0cbb823)
Notice that replacement shall be allowed, and behavior shall remain
coherent with the table, according to the replaced transaction.

---------

Co-authored-by: lightclient <[email protected]>

Author: LuisPH3

core/txpool/blobpool/blobpool.go

@@ -299,7 +299,7 @@ func newBlobTxMeta(id uint64, size uint64, storageSize uint32, tx *types.Transac
 //     and leading up to the first no-change.
 type BlobPool struct {
 	config         Config                    // Pool configuration
-	reserver       *txpool.Reserver          // Address reserver to ensure exclusivity across subpools
+	reserver       txpool.Reserver           // Address reserver to ensure exclusivity across subpools
 	hasPendingAuth func(common.Address) bool // Determine whether the specified address has a pending 7702-auth
 
 	store  billy.Database // Persistent data store for the tx metadata and blobs
@@ -355,7 +355,7 @@ func (p *BlobPool) Filter(tx *types.Transaction) bool {
 // Init sets the gas price needed to keep a transaction in the pool and the chain
 // head to allow balance / nonce checks. The transaction journal will be loaded
 // from disk and filtered based on the provided starting settings.
-func (p *BlobPool) Init(gasTip uint64, head *types.Header, reserver *txpool.Reserver) error {
+func (p *BlobPool) Init(gasTip uint64, head *types.Header, reserver txpool.Reserver) error {
 	p.reserver = reserver
 
 	var (

core/txpool/blobpool/blobpool_test.go

@@ -26,6 +26,7 @@ import (
 	"math/big"
 	"os"
 	"path/filepath"
+	"sync"
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -167,6 +168,44 @@ func (bc *testBlockChain) StateAt(common.Hash) (*state.StateDB, error) {
 	return bc.statedb, nil
 }
 
+// reserver is a utility struct to sanity check that accounts are
+// properly reserved by the blobpool (no duplicate reserves or unreserves).
+type reserver struct {
+	accounts map[common.Address]struct{}
+	lock     sync.RWMutex
+}
+
+func newReserver() txpool.Reserver {
+	return &reserver{accounts: make(map[common.Address]struct{})}
+}
+
+func (r *reserver) Hold(addr common.Address) error {
+	r.lock.Lock()
+	defer r.lock.Unlock()
+	if _, exists := r.accounts[addr]; exists {
+		panic("already reserved")
+	}
+	r.accounts[addr] = struct{}{}
+	return nil
+}
+
+func (r *reserver) Release(addr common.Address) error {
+	r.lock.Lock()
+	defer r.lock.Unlock()
+	if _, exists := r.accounts[addr]; !exists {
+		panic("not reserved")
+	}
+	delete(r.accounts, addr)
+	return nil
+}
+
+func (r *reserver) Has(address common.Address) bool {
+	r.lock.RLock()
+	defer r.lock.RUnlock()
+	_, exists := r.accounts[address]
+	return exists
+}
+
 // makeTx is a utility method to construct a random blob transaction and sign it
 // with a valid key, only setting the interesting fields from the perspective of
 // the blob pool.
@@ -405,10 +444,6 @@ func verifyBlobRetrievals(t *testing.T, pool *BlobPool) {
 	}
 }
 
-func newReserver() *txpool.Reserver {
-	return txpool.NewReservationTracker().NewHandle(42)
-}
-
 // Tests that transactions can be loaded from disk on startup and that they are
 // correctly discarded if invalid.
 //

core/txpool/legacypool/legacypool.go

@@ -237,7 +237,7 @@ type LegacyPool struct {
 	currentHead   atomic.Pointer[types.Header] // Current head of the blockchain
 	currentState  *state.StateDB               // Current state in the blockchain head
 	pendingNonces *noncer                      // Pending state tracking virtual nonces
-	reserver      *txpool.Reserver             // Address reserver to ensure exclusivity across subpools
+	reserver      txpool.Reserver              // Address reserver to ensure exclusivity across subpools
 
 	pending map[common.Address]*list     // All currently processable transactions
 	queue   map[common.Address]*list     // Queued but non-processable transactions
@@ -302,7 +302,7 @@ func (pool *LegacyPool) Filter(tx *types.Transaction) bool {
 // Init sets the gas price needed to keep a transaction in the pool and the chain
 // head to allow balance / nonce checks. The internal
 // goroutines will be spun up and the pool deemed operational afterwards.
-func (pool *LegacyPool) Init(gasTip uint64, head *types.Header, reserver *txpool.Reserver) error {
+func (pool *LegacyPool) Init(gasTip uint64, head *types.Header, reserver txpool.Reserver) error {
 	// Set the address reserver to request exclusive access to pooled accounts
 	pool.reserver = reserver
 
@@ -640,11 +640,18 @@ func (pool *LegacyPool) validateAuth(tx *types.Transaction) error {
 	if err := pool.checkDelegationLimit(tx); err != nil {
 		return err
 	}
-	// Authorities must not conflict with any pending or queued transactions,
-	// nor with addresses that have already been reserved.
+	// For symmetry, allow at most one in-flight tx for any authority with a
+	// pending transaction.
 	if auths := tx.SetCodeAuthorities(); len(auths) > 0 {
 		for _, auth := range auths {
-			if pool.pending[auth] != nil || pool.queue[auth] != nil {
+			var count int
+			if pending := pool.pending[auth]; pending != nil {
+				count += pending.Len()
+			}
+			if queue := pool.queue[auth]; queue != nil {
+				count += queue.Len()
+			}
+			if count > 1 {
 				return ErrAuthorityReserved
 			}
 			// Because there is no exclusive lock held between different subpools
@@ -1907,9 +1914,14 @@ func (pool *LegacyPool) Clear() {
 	// The transaction addition may attempt to reserve the sender addr which
 	// can't happen until Clear releases the reservation lock. Clear cannot
 	// acquire the subpool lock until the transaction addition is completed.
-	for _, tx := range pool.all.txs {
-		senderAddr, _ := types.Sender(pool.signer, tx)
-		pool.reserver.Release(senderAddr)
+
+	for addr := range pool.pending {
+		if _, ok := pool.queue[addr]; !ok {
+			pool.reserver.Release(addr)
+		}
+	}
+	for addr := range pool.queue {
+		pool.reserver.Release(addr)
 	}
 	pool.all = newLookup()
 	pool.priced = newPricedList(pool.all)