From c748bf6201cd93ea463a649d9e53dc6ee24ba15d Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Tue, 25 Feb 2025 20:55:40 +0800 Subject: [PATCH] core/txpool/local, eth: return no error if transaction fails stateful check --- core/txpool/locals/tx_tracker.go | 21 ++++++++++++++++----- eth/api_backend.go | 15 ++++++++------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/core/txpool/locals/tx_tracker.go b/core/txpool/locals/tx_tracker.go index f46cb38f8a..aa0256fd72 100644 --- a/core/txpool/locals/tx_tracker.go +++ b/core/txpool/locals/tx_tracker.go @@ -74,31 +74,41 @@ func New(journalPath string, journalTime time.Duration, chainConfig *params.Chai // Track adds a transaction to the tracked set. // Note: blob-type transactions are ignored. -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. // Note: blob-type transactions are ignored. -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 { if tx.Type() == types.BlobTxType { + errors = append(errors, nil) continue } // Ignore the transactions which are failed for fundamental // validation such as invalid parameters. - if tracker.pool.ValidateTxBasics(tx) != nil { + 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 { + errors = append(errors, nil) continue } + // Theoretically, checking the error here is unnecessary since sender recovery + // is already part of basic validation. However, retrieving the sender address + // from the transaction cache is effectively a no-op if it was previously verified. + // Therefore, the error is still checked just in case. addr, err := types.Sender(tracker.signer, tx) - if err != nil { // Ignore this tx + if err != nil { + errors = append(errors, err) continue } tracker.all[tx.Hash()] = tx @@ -112,6 +122,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. diff --git a/eth/api_backend.go b/eth/api_backend.go index 3aa90f0a12..4c79b7b1bb 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -273,14 +273,15 @@ 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) + if err := locals.Track(signedTx); err != nil { + return err + } } - // TODO(rjl493456442): If a transaction fails stateful validation (e.g., no - // available slot), an error will be returned to the user. However, locally - // submitted transactions may be resubmitted later via the local tracker, which - // could cause confusion that a previously "rejected" transaction is later - // accepted. - return b.eth.txPool.Add([]*types.Transaction{signedTx}, false)[0] + // 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. + b.eth.txPool.Add([]*types.Transaction{signedTx}, false) + return nil } func (b *EthAPIBackend) GetPoolTransactions() (types.Transactions, error) {