From beb2954fa4da3310c7fb4c9824e5136580710f79 Mon Sep 17 00:00:00 2001 From: Ng Wei Han <47109095+weiihann@users.noreply.github.com> Date: Tue, 13 Feb 2024 17:10:11 +0800 Subject: [PATCH] core/txpool/legacypool: use uint256.Int instead of big.Int (#28606) This change makes the legacy transaction pool use of `uint256.Int` instead of `big.Int`. The changes are made primarily only on the internal functions of legacypool. --------- Co-authored-by: Martin Holst Swende --- core/txpool/blobpool/blobpool.go | 4 +- core/txpool/blobpool/blobpool_test.go | 10 ++--- core/txpool/legacypool/legacypool.go | 29 ++++++++------- core/txpool/legacypool/legacypool2_test.go | 8 ++-- core/txpool/legacypool/legacypool_test.go | 43 ++++++++++------------ core/txpool/legacypool/list.go | 31 ++++++++++------ core/txpool/legacypool/list_test.go | 19 +++++++++- core/txpool/subpool.go | 2 +- core/txpool/txpool.go | 2 +- eth/backend.go | 2 +- eth/protocols/eth/handler_test.go | 2 +- miner/miner_test.go | 2 +- miner/worker_test.go | 2 +- 13 files changed, 91 insertions(+), 65 deletions(-) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 41ec930d5..7f713d017 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -342,7 +342,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 *big.Int, head *types.Header, reserve txpool.AddressReserver) error { +func (p *BlobPool) Init(gasTip uint64, head *types.Header, reserve txpool.AddressReserver) error { p.reserve = reserve var ( @@ -420,7 +420,7 @@ func (p *BlobPool) Init(gasTip *big.Int, head *types.Header, reserve txpool.Addr basefeeGauge.Update(int64(basefee.Uint64())) blobfeeGauge.Update(int64(blobfee.Uint64())) - p.SetGasTip(gasTip) + p.SetGasTip(new(big.Int).SetUint64(gasTip)) // Since the user might have modified their pool's capacity, evict anything // above the current allowance diff --git a/core/txpool/blobpool/blobpool_test.go b/core/txpool/blobpool/blobpool_test.go index a71c452b7..58353e482 100644 --- a/core/txpool/blobpool/blobpool_test.go +++ b/core/txpool/blobpool/blobpool_test.go @@ -567,7 +567,7 @@ func TestOpenDrops(t *testing.T) { statedb: statedb, } pool := New(Config{Datadir: storage}, chain) - if err := pool.Init(big.NewInt(1), chain.CurrentBlock(), makeAddressReserver()); err != nil { + if err := pool.Init(1, chain.CurrentBlock(), makeAddressReserver()); err != nil { t.Fatalf("failed to create blob pool: %v", err) } defer pool.Close() @@ -686,7 +686,7 @@ func TestOpenIndex(t *testing.T) { statedb: statedb, } pool := New(Config{Datadir: storage}, chain) - if err := pool.Init(big.NewInt(1), chain.CurrentBlock(), makeAddressReserver()); err != nil { + if err := pool.Init(1, chain.CurrentBlock(), makeAddressReserver()); err != nil { t.Fatalf("failed to create blob pool: %v", err) } defer pool.Close() @@ -788,7 +788,7 @@ func TestOpenHeap(t *testing.T) { statedb: statedb, } pool := New(Config{Datadir: storage}, chain) - if err := pool.Init(big.NewInt(1), chain.CurrentBlock(), makeAddressReserver()); err != nil { + if err := pool.Init(1, chain.CurrentBlock(), makeAddressReserver()); err != nil { t.Fatalf("failed to create blob pool: %v", err) } defer pool.Close() @@ -868,7 +868,7 @@ func TestOpenCap(t *testing.T) { statedb: statedb, } pool := New(Config{Datadir: storage, Datacap: datacap}, chain) - if err := pool.Init(big.NewInt(1), chain.CurrentBlock(), makeAddressReserver()); err != nil { + if err := pool.Init(1, chain.CurrentBlock(), makeAddressReserver()); err != nil { t.Fatalf("failed to create blob pool: %v", err) } // Verify that enough transactions have been dropped to get the pool's size @@ -1270,7 +1270,7 @@ func TestAdd(t *testing.T) { statedb: statedb, } pool := New(Config{Datadir: storage}, chain) - if err := pool.Init(big.NewInt(1), chain.CurrentBlock(), makeAddressReserver()); err != nil { + if err := pool.Init(1, chain.CurrentBlock(), makeAddressReserver()); err != nil { t.Fatalf("test %d: failed to create blob pool: %v", i, err) } verifyPoolInternals(t, pool) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 624dafc60..275ddda35 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -37,6 +37,7 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/params" + "github.com/holiman/uint256" ) const ( @@ -202,7 +203,7 @@ type LegacyPool struct { config Config chainconfig *params.ChainConfig chain BlockChain - gasTip atomic.Pointer[big.Int] + gasTip atomic.Pointer[uint256.Int] txFeed event.Feed signer types.Signer mu sync.RWMutex @@ -287,12 +288,12 @@ func (pool *LegacyPool) Filter(tx *types.Transaction) bool { // head to allow balance / nonce checks. The transaction journal will be loaded // from disk and filtered based on the provided starting settings. The internal // goroutines will be spun up and the pool deemed operational afterwards. -func (pool *LegacyPool) Init(gasTip *big.Int, head *types.Header, reserve txpool.AddressReserver) error { +func (pool *LegacyPool) Init(gasTip uint64, head *types.Header, reserve txpool.AddressReserver) error { // Set the address reserver to request exclusive access to pooled accounts pool.reserve = reserve // Set the basic pool parameters - pool.gasTip.Store(gasTip) + pool.gasTip.Store(uint256.NewInt(gasTip)) // Initialize the state with head block, or fallback to empty one in // case the head state is not available(might occur when node is not @@ -433,11 +434,13 @@ func (pool *LegacyPool) SetGasTip(tip *big.Int) { pool.mu.Lock() defer pool.mu.Unlock() - old := pool.gasTip.Load() - pool.gasTip.Store(new(big.Int).Set(tip)) - + var ( + newTip = uint256.MustFromBig(tip) + old = pool.gasTip.Load() + ) + pool.gasTip.Store(newTip) // If the min miner fee increased, remove transactions below the new threshold - if tip.Cmp(old) > 0 { + if newTip.Cmp(old) > 0 { // pool.priced is sorted by GasFeeCap, so we have to iterate through pool.all instead drop := pool.all.RemotesBelowTip(tip) for _, tx := range drop { @@ -445,7 +448,7 @@ func (pool *LegacyPool) SetGasTip(tip *big.Int) { } pool.priced.Removed(len(drop)) } - log.Info("Legacy pool tip threshold updated", "tip", tip) + log.Info("Legacy pool tip threshold updated", "tip", newTip) } // Nonce returns the next nonce of an account, with all transactions executable @@ -532,7 +535,7 @@ func (pool *LegacyPool) Pending(enforceTips bool) map[common.Address][]*txpool.L // If the miner requests tip enforcement, cap the lists now if enforceTips && !pool.locals.contains(addr) { for i, tx := range txs { - if tx.EffectiveGasTipIntCmp(pool.gasTip.Load(), pool.priced.urgent.baseFee) < 0 { + if tx.EffectiveGasTipIntCmp(pool.gasTip.Load().ToBig(), pool.priced.urgent.baseFee) < 0 { txs = txs[:i] break } @@ -594,7 +597,7 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction, local bool) erro 1< gasLimit || tx.Cost().Cmp(costLimit) > 0 + return tx.Gas() > gasLimit || tx.Cost().Cmp(costLimit.ToBig()) > 0 }) if len(removed) == 0 { @@ -456,7 +462,10 @@ func (l *list) LastElement() *types.Transaction { // total cost of all transactions. func (l *list) subTotalCost(txs []*types.Transaction) { for _, tx := range txs { - l.totalcost.Sub(l.totalcost, tx.Cost()) + _, underflow := l.totalcost.SubOverflow(l.totalcost, uint256.MustFromBig(tx.Cost())) + if underflow { + panic("totalcost underflow") + } } } diff --git a/core/txpool/legacypool/list_test.go b/core/txpool/legacypool/list_test.go index b5cd34b23..67256f63b 100644 --- a/core/txpool/legacypool/list_test.go +++ b/core/txpool/legacypool/list_test.go @@ -21,8 +21,10 @@ import ( "math/rand" "testing" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" + "github.com/holiman/uint256" ) // Tests that transactions can be added to strict lists and list contents and @@ -51,6 +53,21 @@ func TestStrictListAdd(t *testing.T) { } } +// TestListAddVeryExpensive tests adding txs which exceed 256 bits in cost. It is +// expected that the list does not panic. +func TestListAddVeryExpensive(t *testing.T) { + key, _ := crypto.GenerateKey() + list := newList(true) + for i := 0; i < 3; i++ { + value := big.NewInt(100) + gasprice, _ := new(big.Int).SetString("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", 0) + gaslimit := uint64(i) + tx, _ := types.SignTx(types.NewTransaction(uint64(i), common.Address{}, value, gaslimit, gasprice, nil), types.HomesteadSigner{}, key) + t.Logf("cost: %x bitlen: %d\n", tx.Cost(), tx.Cost().BitLen()) + list.Add(tx, DefaultConfig.PriceBump) + } +} + func BenchmarkListAdd(b *testing.B) { // Generate a list of transactions to insert key, _ := crypto.GenerateKey() @@ -60,7 +77,7 @@ func BenchmarkListAdd(b *testing.B) { txs[i] = transaction(uint64(i), 0, key) } // Insert the transactions in a random order - priceLimit := big.NewInt(int64(DefaultConfig.PriceLimit)) + priceLimit := uint256.NewInt(DefaultConfig.PriceLimit) b.ResetTimer() for i := 0; i < b.N; i++ { list := newList(true) diff --git a/core/txpool/subpool.go b/core/txpool/subpool.go index 2722174d7..7ae760729 100644 --- a/core/txpool/subpool.go +++ b/core/txpool/subpool.go @@ -86,7 +86,7 @@ type SubPool interface { // These should not be passed as a constructor argument - nor should the pools // start by themselves - in order to keep multiple subpools in lockstep with // one another. - Init(gasTip *big.Int, head *types.Header, reserve AddressReserver) error + Init(gasTip uint64, head *types.Header, reserve AddressReserver) error // Close terminates any background processing threads and releases any held // resources. diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index d03e025a9..ee2f774e8 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -79,7 +79,7 @@ type TxPool struct { // New creates a new transaction pool to gather, sort and filter inbound // transactions from the network. -func New(gasTip *big.Int, chain BlockChain, subpools []SubPool) (*TxPool, error) { +func New(gasTip uint64, chain BlockChain, subpools []SubPool) (*TxPool, error) { // Retrieve the current head so that all subpools and this main coordinator // pool will have the same starting state, even if the chain moves forward // during initialization. diff --git a/eth/backend.go b/eth/backend.go index aff23a910..0a0813aaf 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -229,7 +229,7 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) { } legacyPool := legacypool.New(config.TxPool, eth.blockchain) - eth.txPool, err = txpool.New(new(big.Int).SetUint64(config.TxPool.PriceLimit), eth.blockchain, []txpool.SubPool{legacyPool, blobPool}) + eth.txPool, err = txpool.New(config.TxPool.PriceLimit, eth.blockchain, []txpool.SubPool{legacyPool, blobPool}) if err != nil { return nil, err } diff --git a/eth/protocols/eth/handler_test.go b/eth/protocols/eth/handler_test.go index 08882faa7..897e317b9 100644 --- a/eth/protocols/eth/handler_test.go +++ b/eth/protocols/eth/handler_test.go @@ -117,7 +117,7 @@ func newTestBackendWithGenerator(blocks int, shanghai bool, generator func(int, txconfig.Journal = "" // Don't litter the disk with test journals pool := legacypool.New(txconfig, chain) - txpool, _ := txpool.New(new(big.Int).SetUint64(txconfig.PriceLimit), chain, []txpool.SubPool{pool}) + txpool, _ := txpool.New(txconfig.PriceLimit, chain, []txpool.SubPool{pool}) return &testBackend{ db: db, diff --git a/miner/miner_test.go b/miner/miner_test.go index 411d6026c..016732f36 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -317,7 +317,7 @@ func createMiner(t *testing.T) (*Miner, *event.TypeMux, func(skipMiner bool)) { blockchain := &testBlockChain{bc.Genesis().Root(), chainConfig, statedb, 10000000, new(event.Feed)} pool := legacypool.New(testTxPoolConfig, blockchain) - txpool, _ := txpool.New(new(big.Int).SetUint64(testTxPoolConfig.PriceLimit), blockchain, []txpool.SubPool{pool}) + txpool, _ := txpool.New(testTxPoolConfig.PriceLimit, blockchain, []txpool.SubPool{pool}) backend := NewMockBackend(bc, txpool) // Create event Mux diff --git a/miner/worker_test.go b/miner/worker_test.go index 675b8d55b..0420eeb29 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -135,7 +135,7 @@ func newTestWorkerBackend(t *testing.T, chainConfig *params.ChainConfig, engine t.Fatalf("core.NewBlockChain failed: %v", err) } pool := legacypool.New(testTxPoolConfig, chain) - txpool, _ := txpool.New(new(big.Int).SetUint64(testTxPoolConfig.PriceLimit), chain, []txpool.SubPool{pool}) + txpool, _ := txpool.New(testTxPoolConfig.PriceLimit, chain, []txpool.SubPool{pool}) return &testWorkerBackend{ db: db,