Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eth: do not add failed tx to localTxTracker #31202

Merged
merged 6 commits into from
Mar 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions core/txpool/blobpool/blobpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -1085,19 +1085,24 @@ func (p *BlobPool) SetGasTip(tip *big.Int) {
p.updateStorageMetrics()
}

// validateTx checks whether a transaction is valid according to the consensus
// rules and adheres to some heuristic limits of the local node (price and size).
func (p *BlobPool) validateTx(tx *types.Transaction) error {
// Ensure the transaction adheres to basic pool filters (type, size, tip) and
// consensus rules
baseOpts := &txpool.ValidationOptions{
// 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 (p *BlobPool) ValidateTxBasics(tx *types.Transaction) error {
opts := &txpool.ValidationOptions{
Config: p.chain.Config(),
Accept: 1 << types.BlobTxType,
MaxSize: txMaxSize,
MinTip: p.gasTip.ToBig(),
}
return txpool.ValidateTransaction(tx, p.head, p.signer, opts)
}

if err := p.txValidationFn(tx, p.head, p.signer, baseOpts); err != nil {
// validateTx checks whether a transaction is valid according to the consensus
// rules and adheres to some heuristic limits of the local node (price and size).
func (p *BlobPool) validateTx(tx *types.Transaction) error {
if err := p.ValidateTxBasics(tx); err != nil {
return err
}
// Ensure the transaction adheres to the stateful pool filters (nonce, balance)
Expand Down
11 changes: 4 additions & 7 deletions core/txpool/legacypool/legacypool.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,11 +558,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 |
Expand All @@ -573,10 +573,7 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction) error {
MaxSize: txMaxSize,
MinTip: pool.gasTip.Load().ToBig(),
}
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
Expand Down Expand Up @@ -929,7 +926,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
log.Trace("Discarding invalid transaction", "hash", tx.Hash(), "err", err)
invalidTxMeter.Mark(1)
Expand Down
26 changes: 22 additions & 4 deletions core/txpool/locals/tx_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,28 +74,45 @@ 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 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
}
errors = append(errors, nil)

tracker.all[tx.Hash()] = tx
if tracker.byAddr[addr] == nil {
tracker.byAddr[addr] = legacypool.NewSortedMap()
Expand All @@ -107,6 +124,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.
Expand Down
6 changes: 6 additions & 0 deletions core/txpool/subpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ type SubPool interface {
// retrieve blobs from the pools directly instead of the network.
GetBlobs(vhashes []common.Hash) ([]*kzg4844.Blob, []*kzg4844.Proof)

// 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.
Expand Down
11 changes: 11 additions & 0 deletions core/txpool/txpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,17 @@ func (p *TxPool) GetBlobs(vhashes []common.Hash) ([]*kzg4844.Blob, []*kzg4844.Pr
return nil, 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
// to the large transaction churn, add may postpone fully integrating the tx
// to a later point to batch multiple ones together.
Expand Down
16 changes: 13 additions & 3 deletions eth/api_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,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) {
Expand Down