feat(eth): do not add failed tx to localTxTracker #31202

core/txpool/legacypool/legacypool.go

@@ -582,11 +582,11 @@ func (pool *LegacyPool) Pending(filter txpool.PendingFilter) map[common.Address]
 	return pending
 }
 
-// validateTxBasics checks whether a transaction is valid according to the consensus
+// ValidateTxBasics checks whether a transaction is valid according to the consensus
 // rules, but does not check state-dependent validation such as sufficient balance.
 // This check is meant as an early check which only needs to be performed once,
 // and does not require the pool mutex to be held.
-func (pool *LegacyPool) validateTxBasics(tx *types.Transaction) error {
+func (pool *LegacyPool) ValidateTxBasics(tx *types.Transaction) error {
 	opts := &txpool.ValidationOptions{
 		Config: pool.chainconfig,
 		Accept: 0 |
@@ -600,10 +600,7 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction) error {
 			return pool.isSigner != nil && !pool.isSigner(from)
 		},
 	}
-	if err := txpool.ValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts); err != nil {
-		return err
-	}
-	return nil
+	return txpool.ValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts)
 }
 
 // validateTx checks whether a transaction is valid according to the consensus
@@ -1070,7 +1067,7 @@ func (pool *LegacyPool) Add(txs []*types.Transaction, sync bool) []error {
 		// Exclude transactions with basic errors, e.g invalid signatures and
 		// insufficient intrinsic gas as soon as possible and cache senders
 		// in transactions before obtaining lock
-		if err := pool.validateTxBasics(tx); err != nil {
+		if err := pool.ValidateTxBasics(tx); err != nil {
 			errs[i] = err
 			invalidTxMeter.Mark(1)
 			continue

core/txpool/legacypool/legacypool_test.go

@@ -688,7 +688,7 @@ func TestValidateTransactionEIP2681(t *testing.T) {
 				GasPrice: tt.gasPrice,
 			})
 			signedTx, _ := types.SignTx(tx, types.HomesteadSigner{}, key)
-			err := pool.validateTxBasics(signedTx)
+			err := pool.ValidateTxBasics(signedTx)
 
 			if tt.wantErr == nil && err != nil {
 				t.Errorf("expected nil, got %v", err)

core/txpool/locals/tx_tracker.go

@@ -74,16 +74,24 @@ func New(journalPath string, journalTime time.Duration, chainConfig *params.Chai
 }
 
 // Track adds a transaction to the tracked set.
-func (tracker *TxTracker) Track(tx *types.Transaction) {
-	tracker.TrackAll([]*types.Transaction{tx})
+func (tracker *TxTracker) Track(tx *types.Transaction) error {
+	return tracker.TrackAll([]*types.Transaction{tx})[0]
 }
 
 // TrackAll adds a list of transactions to the tracked set.
-func (tracker *TxTracker) TrackAll(txs []*types.Transaction) {
+func (tracker *TxTracker) TrackAll(txs []*types.Transaction) []error {
 	tracker.mu.Lock()
 	defer tracker.mu.Unlock()
 
+	var errors []error
 	for _, tx := range txs {
+		// Ignore the transactions which are failed for fundamental
+		// validation such as invalid parameters.
+		if err := tracker.pool.ValidateTxBasics(tx); err != nil {
+			log.Debug("Invalid transaction submitted", "hash", tx.Hash(), "err", err)
+			errors = append(errors, err)
+			continue
+		}
 		// If we're already tracking it, it's a no-op
 		if _, ok := tracker.all[tx.Hash()]; ok {
 			continue
@@ -94,8 +102,11 @@ func (tracker *TxTracker) TrackAll(txs []*types.Transaction) {
 		// Therefore, the error is still checked just in case.
 		addr, err := types.Sender(tracker.signer, tx)
 		if err != nil { // Ignore this tx
+			errors = append(errors, err)
 			continue
 		}
+		errors = append(errors, nil)
+
 		tracker.all[tx.Hash()] = tx
 		if tracker.byAddr[addr] == nil {
 			tracker.byAddr[addr] = legacypool.NewSortedMap()
@@ -107,6 +118,7 @@ func (tracker *TxTracker) TrackAll(txs []*types.Transaction) {
 		}
 	}
 	localGauge.Update(int64(len(tracker.all)))
+	return errors
 }
 
 // recheck checks and returns any transactions that needs to be resubmitted.

core/txpool/subpool.go

@@ -114,6 +114,12 @@ type SubPool interface {
 	// Get returns a transaction if it is contained in the pool, or nil otherwise.
 	Get(hash common.Hash) *types.Transaction
 
+	// ValidateTxBasics checks whether a transaction is valid according to the consensus
+	// rules, but does not check state-dependent validation such as sufficient balance.
+	// This check is meant as a static check which can be performed without holding the
+	// pool mutex.
+	ValidateTxBasics(tx *types.Transaction) error
+
 	// Add enqueues a batch of transactions into the pool if they are valid. Due
 	// to the large transaction churn, add may postpone fully integrating the tx
 	// to a later point to batch multiple ones together.

core/txpool/txpool.go

@@ -68,8 +68,8 @@ type TxPool struct {
 // LocalTracker is the minimal local transaction tracking functionality used by
 // TxPool local submission helpers.
 type LocalTracker interface {
-	Track(tx *types.Transaction)
-	TrackAll(txs []*types.Transaction)
+	Track(tx *types.Transaction) error
+	TrackAll(txs []*types.Transaction) []error
 }
 
 // New creates a new transaction pool to gather, sort and filter inbound
@@ -274,6 +274,17 @@ func (p *TxPool) Get(hash common.Hash) *types.Transaction {
 	return nil
 }
 
+// ValidateTxBasics checks whether a transaction is valid according to the consensus
+// rules, but does not check state-dependent validation such as sufficient balance.
+func (p *TxPool) ValidateTxBasics(tx *types.Transaction) error {
+	for _, subpool := range p.subpools {
+		if subpool.Filter(tx) {
+			return subpool.ValidateTxBasics(tx)
+		}
+	}
+	return fmt.Errorf("%w: received type %d", core.ErrTxTypeNotSupported, tx.Type())
+}
+
 // Add enqueues a batch of transactions into the pool if they are valid. Due
 //
 // Note, if sync is set the method will block until all internal maintenance

core/txpool/txpool_local_test.go

@@ -29,17 +29,18 @@ type testLocalTracker struct {
 	tracked []common.Hash
 }
 
-func (t *testLocalTracker) Track(tx *types.Transaction) {
-	t.TrackAll([]*types.Transaction{tx})
+func (t *testLocalTracker) Track(tx *types.Transaction) error {
+	return t.TrackAll([]*types.Transaction{tx})[0]
 }
 
-func (t *testLocalTracker) TrackAll(txs []*types.Transaction) {
+func (t *testLocalTracker) TrackAll(txs []*types.Transaction) []error {
 	t.mu.Lock()
 	defer t.mu.Unlock()
 	*t.events = append(*t.events, "track")
 	for _, tx := range txs {
 		t.tracked = append(t.tracked, tx.Hash())
 	}
+	return make([]error, len(txs))
 }
 
 type testSubPool struct {
@@ -63,6 +64,8 @@ func (s *testSubPool) Has(hash common.Hash) bool { return false }
 
 func (s *testSubPool) Get(hash common.Hash) *types.Transaction { return nil }
 
+func (s *testSubPool) ValidateTxBasics(tx *types.Transaction) error { return nil }
+
 func (s *testSubPool) Add(txs []*types.Transaction, sync bool) []error {
 	*s.events = append(*s.events, "add")
 	s.lastAdd = txs

eth/api_backend.go

@@ -301,10 +301,20 @@ func (b *EthAPIBackend) SubscribeLogsEvent(ch chan<- []*types.Log) event.Subscri
 }
 
 func (b *EthAPIBackend) SendTx(ctx context.Context, signedTx *types.Transaction) error {
-	if locals := b.eth.localTxTracker; locals != nil {
-		locals.Track(signedTx)
+	locals := b.eth.localTxTracker
+	if locals != nil {
+		if err := locals.Track(signedTx); err != nil {
+			return err
+		}
+	}
+	// No error will be returned to user if the transaction fails stateful
+	// validation (e.g., no available slot), as the locally submitted transactions
+	// may be resubmitted later via the local tracker.
+	err := b.eth.txPool.Add([]*types.Transaction{signedTx}, false)[0]
+	if err != nil && locals == nil {
+		return err
 	}
-	return b.eth.txPool.Add([]*types.Transaction{signedTx}, false)[0]
+	return nil
 }
 
 func (b *EthAPIBackend) GetPoolTransactions() (types.Transactions, error) {