From d9867ea87d4c00e531c6ae9aec0d6b5cd4c1e269 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Mon, 15 Dec 2025 16:34:23 +0800 Subject: [PATCH] core/txpool: move some validation to outside of mutex #27006 (#1858) --- core/txpool/txpool.go | 123 +++++++++++++++++++------------------ core/txpool/txpool_test.go | 60 ++++++++++++------ 2 files changed, 104 insertions(+), 79 deletions(-) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 8c606d0cda..a27b52ccfc 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -272,12 +272,12 @@ type TxPool struct { signer types.Signer mu sync.RWMutex - eip2718 bool // Fork indicator whether we are using EIP-2718 type transactions. - eip1559 bool // Fork indicator whether we are using EIP-1559 type transactions. + eip2718 atomic.Bool // Fork indicator whether we are using EIP-2718 type transactions. + eip1559 atomic.Bool // Fork indicator whether we are using EIP-1559 type transactions. currentState *state.StateDB // Current state in the blockchain head pendingNonces *noncer // Pending state tracking virtual nonces - currentMaxGas uint64 // Current gas limit for transaction caps + currentMaxGas atomic.Uint64 // Current gas limit for transaction caps locals *accountSet // Set of local transaction to exempt from eviction rules journal *journal // Journal of local transaction to back up to disk @@ -620,15 +620,17 @@ func (pool *TxPool) GetSender(tx *types.Transaction) (common.Address, error) { return from, 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 (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { +// 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 *TxPool) validateTxBasics(tx *types.Transaction, local bool) error { // Accept only legacy transactions until EIP-2718/2930 activates. - if !pool.eip2718 && tx.Type() != types.LegacyTxType { + if !pool.eip2718.Load() && tx.Type() != types.LegacyTxType { return core.ErrTxTypeNotSupported } // Reject dynamic fee transactions until EIP-1559 activates. - if !pool.eip1559 && tx.Type() == types.DynamicFeeTxType { + if !pool.eip1559.Load() && tx.Type() == types.DynamicFeeTxType { return core.ErrTxTypeNotSupported } // Reject transactions over defined size to prevent DOS attacks @@ -636,31 +638,16 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { return ErrOversizedData } // Check whether the init code size has been exceeded. - if pool.eip1559 && tx.To() == nil && len(tx.Data()) > params.MaxInitCodeSize { + if pool.eip1559.Load() && tx.To() == nil && len(tx.Data()) > params.MaxInitCodeSize { return fmt.Errorf("%w: code size %v limit %v", core.ErrMaxInitCodeSizeExceeded, len(tx.Data()), params.MaxInitCodeSize) } - // Get current block number - var number *big.Int = nil - if pool.chain.CurrentHeader() != nil { - number = pool.chain.CurrentHeader().Number - } - if number == nil || number.Uint64() >= common.BlackListHFNumber { - // check if sender is in black list - if common.IsInBlacklist(tx.From()) { - return fmt.Errorf("reject transaction with sender in black-list: %v", tx.From().Hex()) - } - // check if receiver is in black list - if common.IsInBlacklist(tx.To()) { - return fmt.Errorf("reject transaction with receiver in black-list: %v", tx.To().Hex()) - } - } // Transactions can't be negative. This may never happen using RLP decoded // transactions but may occur if you create a transaction using the RPC. if tx.Value().Sign() < 0 { return ErrNegativeValue } // Ensure the transaction doesn't exceed the current block limit gas. - if pool.currentMaxGas < tx.Gas() { + if pool.currentMaxGas.Load() < tx.Gas() { return ErrGasLimit } // Sanity check for extremely large numbers @@ -696,6 +683,30 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { return ErrUnderpriced } } + // Stop checking for special transactions + if tx.IsSpecialTransaction() { + return nil + } + // Ensure the transaction has more gas than the basic tx fee. + intrGas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, true, pool.eip1559.Load()) + if err != nil { + return err + } + if tx.Gas() < intrGas { + return core.ErrIntrinsicGas + } + // Check zero gas price. + if tx.GasPrice().Sign() == 0 { + return ErrZeroGasPrice + } + return 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 (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { + // Signature has been checked already, this cannot error. + from, _ := types.Sender(pool.signer, tx) // Ensure the transaction adheres to nonce ordering if pool.currentState.GetNonce(from) > tx.Nonce() { return core.ErrNonceTooLow @@ -703,13 +714,26 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { if pool.pendingNonces.get(from)+common.LimitThresholdNonceInQueue < tx.Nonce() { return core.ErrNonceTooHigh } + // Get current block number + var number *big.Int = nil + if pool.chain.CurrentHeader() != nil { + number = pool.chain.CurrentHeader().Number + } + if number == nil || number.Uint64() >= common.BlackListHFNumber { + // check if sender is in black list + if common.IsInBlacklist(tx.From()) { + return fmt.Errorf("reject transaction with sender in black-list: %v", tx.From().Hex()) + } + // check if receiver is in black list + if common.IsInBlacklist(tx.To()) { + return fmt.Errorf("reject transaction with receiver in black-list: %v", tx.To().Hex()) + } + } // Transactor should have enough funds to cover the costs // cost == V + GP * GL balance := pool.currentState.GetBalance(from) cost := tx.Cost() - minGasPrice := common.GetMinGasPrice(number) feeCapacity := big.NewInt(0) - if tx.To() != nil { if value, ok := pool.trc21FeeCapacity[*tx.To()]; ok { feeCapacity = value @@ -739,34 +763,13 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { } if !tx.IsSpecialTransaction() { - // Ensure the transaction has more gas than the basic tx fee. - intrGas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, true, pool.eip1559) - if err != nil { - return err - } - // Exclude check smart contract sign address. - if tx.Gas() < intrGas { - return core.ErrIntrinsicGas - } - - // Check zero gas price. - if tx.GasPrice().Sign() == 0 { - return ErrZeroGasPrice - } - // under min gas price + minGasPrice := common.GetMinGasPrice(number) if tx.GasPrice().Cmp(minGasPrice) < 0 { return ErrUnderMinGasPrice } } - /* - minGasDeploySMC := new(big.Int).Mul(new(big.Int).SetUint64(10), new(big.Int).SetUint64(params.Ether)) - if tx.To() == nil && (tx.Cost().Cmp(minGasDeploySMC) < 0 || tx.GasPrice().Cmp(new(big.Int).SetUint64(10000*params.Shannon)) < 0) { - return ErrMinDeploySMC - } - */ - // validate minFee slot for XDCZ if tx.IsXDCZApplyTransaction() { copyState := pool.currentState.Copy() @@ -1120,12 +1123,12 @@ func (pool *TxPool) addTxs(txs []*types.Transaction, local, sync bool) []error { knownTxMeter.Mark(1) continue } - // Exclude transactions with invalid signatures as soon as - // possible and cache senders in transactions before - // obtaining lock - _, err := types.Sender(pool.signer, tx) - if err != nil { - errs[i] = ErrInvalidSender + // 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, local); err != nil { + errs[i] = err invalidTxMeter.Mark(1) continue } @@ -1514,7 +1517,7 @@ func (pool *TxPool) reset(oldHead, newHead *types.Header) { pool.currentState = statedb pool.trc21FeeCapacity = statedb.GetTRC21FeeCapacityFromStateWithCache(newHead.Root) pool.pendingNonces = newNoncer(statedb) - pool.currentMaxGas = newHead.GasLimit + pool.currentMaxGas.Store(newHead.GasLimit) // Inject any transactions discarded due to reorgs log.Debug("Reinjecting stale transactions", "count", len(reinject)) @@ -1523,8 +1526,8 @@ func (pool *TxPool) reset(oldHead, newHead *types.Header) { // Update all fork indicator by next pending block number. next := new(big.Int).Add(newHead.Number, big.NewInt(1)) - pool.eip2718 = pool.chainconfig.IsEIP1559(next) - pool.eip1559 = pool.chainconfig.IsEIP1559(next) + pool.eip2718.Store(pool.chainconfig.IsEIP1559(next)) + pool.eip1559.Store(pool.chainconfig.IsEIP1559(next)) } // promoteExecutables moves transactions that have become processable from the @@ -1557,7 +1560,7 @@ func (pool *TxPool) promoteExecutables(accounts []common.Address) []*types.Trans if pool.chain.CurrentHeader() != nil { number = pool.chain.CurrentHeader().Number } - drops, _ := list.Filter(pool.currentState.GetBalance(addr), pool.currentMaxGas, pool.trc21FeeCapacity, number) + drops, _ := list.Filter(pool.currentState.GetBalance(addr), pool.currentMaxGas.Load(), pool.trc21FeeCapacity, number) for _, tx := range drops { hash := tx.Hash() pool.all.Remove(hash) @@ -1758,7 +1761,7 @@ func (pool *TxPool) demoteUnexecutables() { if pool.chain.CurrentHeader() != nil { number = pool.chain.CurrentHeader().Number } - drops, invalids := list.Filter(pool.currentState.GetBalance(addr), pool.currentMaxGas, pool.trc21FeeCapacity, number) + drops, invalids := list.Filter(pool.currentState.GetBalance(addr), pool.currentMaxGas.Load(), pool.trc21FeeCapacity, number) for _, tx := range drops { hash := tx.Hash() log.Trace("Removed unpayable pending transaction", "hash", hash) diff --git a/core/txpool/txpool_test.go b/core/txpool/txpool_test.go index aa39d0269e..d8e7145c1c 100644 --- a/core/txpool/txpool_test.go +++ b/core/txpool/txpool_test.go @@ -309,28 +309,29 @@ func TestInvalidTransactions(t *testing.T) { tx := transaction(0, 100, key) from, _ := deriveSender(tx) + // Intrinsic gas too low testAddBalance(pool, from, big.NewInt(1)) - if err := pool.AddRemote(tx); err != core.ErrInsufficientFunds { - t.Error("expected", core.ErrInsufficientFunds) + if err, want := pool.AddRemote(tx), core.ErrIntrinsicGas; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) } - balance := new(big.Int).Add(tx.Value(), new(big.Int).Mul(new(big.Int).SetUint64(tx.Gas()), tx.GasPrice())) - testAddBalance(pool, from, balance) - if err := pool.AddRemote(tx); err != core.ErrIntrinsicGas { - t.Error("expected", core.ErrIntrinsicGas, "got", err) + // Insufficient funds + tx = transaction(0, 100000, key) + if err, want := pool.AddRemote(tx), core.ErrInsufficientFunds; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) } testSetNonce(pool, from, 1) testAddBalance(pool, from, big.NewInt(0xffffffffffffff)) tx = transaction(0, 100000, key) - if err := pool.AddRemote(tx); err != core.ErrNonceTooLow { - t.Error("expected", core.ErrNonceTooLow) + if err, want := pool.AddRemote(tx), core.ErrNonceTooLow; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) } tx = transaction(1, 100000, key) pool.gasPrice = big.NewInt(1000) - if err := pool.AddRemote(tx); err != ErrUnderpriced { - t.Error("expected", ErrUnderpriced, "got", err) + if err, want := pool.AddRemote(tx), ErrUnderpriced; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) } if err := pool.AddLocal(tx); err != nil { t.Error("expected", nil, "got", err) @@ -430,9 +431,30 @@ func TestValidateTransactionEIP2681(t *testing.T) { gasPrice *big.Int wantErr error }{ - {"nonce 0", 0, big.NewInt(1e18), 21000, big.NewInt(2e10), nil}, - {"nonce 1", 1, big.NewInt(1e18), 21000, big.NewInt(2e10), nil}, - {"EIP-2681 overflow", math.MaxUint64, big.NewInt(1e18), 21000, big.NewInt(2e10), core.ErrNonceMax}, + { + "nonce 0", + 0, + big.NewInt(1e18), + 21000, + big.NewInt(2e10), + nil, + }, + { + "nonce 1", + 1, + big.NewInt(1e18), + 21000, + big.NewInt(2e10), + nil, + }, + { + "EIP-2681 overflow", + math.MaxUint64, + big.NewInt(1e18), + 21000, + big.NewInt(2e10), + core.ErrNonceMax, + }, } for _, tt := range tests { @@ -445,7 +467,7 @@ func TestValidateTransactionEIP2681(t *testing.T) { GasPrice: tt.gasPrice, }) signedTx, _ := types.SignTx(tx, types.HomesteadSigner{}, key) - err := pool.validateTx(signedTx, true) + err := pool.validateTxBasics(signedTx, true) if tt.wantErr == nil && err != nil { t.Errorf("expected nil, got %v", err) @@ -1286,22 +1308,22 @@ func TestAllowedTxSize(t *testing.T) { // All those fields are summed up to at most 213 bytes. baseSize := uint64(213) dataSize := txMaxSize - baseSize - + maxGas := pool.currentMaxGas.Load() // Try adding a transaction with maximal allowed size - tx := pricedDataTransaction(0, pool.currentMaxGas, big.NewInt(1), key, dataSize) + tx := pricedDataTransaction(0, pool.currentMaxGas.Load(), big.NewInt(1), key, dataSize) if err := pool.addRemoteSync(tx); err != nil { t.Fatalf("failed to add transaction of size %d, close to maximal: %v", int(tx.Size()), err) } // Try adding a transaction with random allowed size - if err := pool.addRemoteSync(pricedDataTransaction(1, pool.currentMaxGas, big.NewInt(1), key, uint64(rand.Intn(int(dataSize))))); err != nil { + if err := pool.addRemoteSync(pricedDataTransaction(1, maxGas, big.NewInt(1), key, uint64(rand.Intn(int(dataSize))))); err != nil { t.Fatalf("failed to add transaction of random allowed size: %v", err) } // Try adding a transaction of minimal not allowed size - if err := pool.addRemoteSync(pricedDataTransaction(2, pool.currentMaxGas, big.NewInt(1), key, txMaxSize)); err == nil { + if err := pool.addRemoteSync(pricedDataTransaction(2, maxGas, big.NewInt(1), key, txMaxSize)); err == nil { t.Fatalf("expected rejection on slightly oversize transaction") } // Try adding a transaction of random not allowed size - if err := pool.addRemoteSync(pricedDataTransaction(2, pool.currentMaxGas, big.NewInt(1), key, dataSize+1+uint64(rand.Intn(int(10*txMaxSize))))); err == nil { + if err := pool.addRemoteSync(pricedDataTransaction(2, maxGas, big.NewInt(1), key, dataSize+1+uint64(rand.Intn(int(10*txMaxSize))))); err == nil { t.Fatalf("expected rejection on oversize transaction") } // Run some sanity checks on the pool internals