From ac044caa1bd60bd7f36ec152f7681df92ec607a3 Mon Sep 17 00:00:00 2001 From: HAOYUatHZ <37070449+HAOYUatHZ@users.noreply.github.com> Date: Mon, 29 Jul 2024 19:07:43 +0800 Subject: [PATCH] feat(miner): account fetch limit (#939) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(worker): try to limit the number of txns miner has to deal with (#745) to reduce the effect of having a huge backlog on performance * fix(worker): set default account fetch limit (#756) * fix * update miner/worker.go * fix --------- Co-authored-by: Ömer Faruk Irmak Co-authored-by: Péter Garamvölgyi --- cmd/geth/main.go | 1 + cmd/utils/flags.go | 14 ++++++++++-- core/txpool/blobpool/blobpool.go | 17 +++++++++++++-- core/txpool/legacypool/legacypool.go | 13 ++++++++++++ core/txpool/legacypool/legacypool_test.go | 26 +++++++++++++++++++++++ core/txpool/subpool.go | 2 ++ core/txpool/txpool.go | 10 +++++++++ miner/miner.go | 1 + miner/miner_test.go | 4 +++- miner/scroll_worker.go | 9 +++++++- miner/worker.go | 8 ++++++- miner/worker_test.go | 6 ++++-- 12 files changed, 102 insertions(+), 9 deletions(-) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 4df3edd78e..2e117c4833 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -124,6 +124,7 @@ var ( utils.MinerRecommitIntervalFlag, utils.MinerNewPayloadTimeout, utils.MinerStoreSkippedTxTracesFlag, + utils.MinerMaxAccountsNumFlag, utils.NATFlag, utils.NoDiscoverFlag, utils.DiscoveryV4Flag, diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index fecf2ccf14..216ef29cf4 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -524,8 +524,15 @@ var ( Category: flags.MinerCategory, } MinerStoreSkippedTxTracesFlag = &cli.BoolFlag{ - Name: "miner.storeskippedtxtraces", - Usage: "Store the wrapped traces when storing a skipped tx", + Name: "miner.storeskippedtxtraces", + Usage: "Store the wrapped traces when storing a skipped tx", + Category: flags.MinerCategory, + } + MinerMaxAccountsNumFlag = &cli.IntFlag{ + Name: "miner.maxaccountsnum", + Usage: "Maximum number of accounts that miner will fetch the pending transactions of when building a new block", + Value: math.MaxInt, + Category: flags.MinerCategory, } // Account settings @@ -1697,6 +1704,9 @@ func setMiner(ctx *cli.Context, cfg *miner.Config) { if ctx.IsSet(MinerStoreSkippedTxTracesFlag.Name) { cfg.StoreSkippedTxTraces = ctx.Bool(MinerStoreSkippedTxTracesFlag.Name) } + if ctx.IsSet(MinerMaxAccountsNumFlag.Name) { + cfg.MaxAccountsNum = ctx.Int(MinerMaxAccountsNumFlag.Name) + } } func setRequiredBlocks(ctx *cli.Context, cfg *ethconfig.Config) { diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 39cda23787..84c62b3401 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -29,6 +29,8 @@ import ( "sync" "time" + "github.com/holiman/billy" + "github.com/holiman/uint256" "github.com/scroll-tech/go-ethereum/common" "github.com/scroll-tech/go-ethereum/consensus/misc/eip1559" "github.com/scroll-tech/go-ethereum/consensus/misc/eip4844" @@ -42,8 +44,6 @@ import ( "github.com/scroll-tech/go-ethereum/params" "github.com/scroll-tech/go-ethereum/rlp" "github.com/scroll-tech/go-ethereum/rollup/fees" - "github.com/holiman/billy" - "github.com/holiman/uint256" ) const ( @@ -1381,6 +1381,16 @@ func (p *BlobPool) drop() { // Pending retrieves all currently processable transactions, grouped by origin // account and sorted by nonce. func (p *BlobPool) Pending(enforceTips bool) map[common.Address][]*txpool.LazyTransaction { + return p.pendingWithMax(enforceTips, math.MaxInt) +} + +// PendingWithMax works similar to Pending but allows setting an upper limit on how many +// accounts to return +func (p *BlobPool) PendingWithMax(enforceTips bool, maxAccountsNum int) map[common.Address][]*txpool.LazyTransaction { + return p.pendingWithMax(enforceTips, maxAccountsNum) +} + +func (p *BlobPool) pendingWithMax(enforceTips bool, maxAccountsNum int) map[common.Address][]*txpool.LazyTransaction { // Track the amount of time waiting to retrieve the list of pending blob txs // from the pool and the amount of time actually spent on assembling the data. // The latter will be pretty much moot, but we've kept it to have symmetric @@ -1411,6 +1421,9 @@ func (p *BlobPool) Pending(enforceTips bool) map[common.Address][]*txpool.LazyTr if len(lazies) > 0 { pending[addr] = lazies } + if len(pending) >= maxAccountsNum { + break + } } return pending } diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index f5fd931698..97f820ed75 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -523,6 +523,16 @@ func (pool *LegacyPool) ContentFrom(addr common.Address) ([]*types.Transaction, // transactions and only return those whose **effective** tip is large enough in // the next pending execution environment. func (pool *LegacyPool) Pending(enforceTips bool) map[common.Address][]*txpool.LazyTransaction { + return pool.pendingWithMax(enforceTips, math.MaxInt) +} + +// PendingWithMax works similar to Pending but allows setting an upper limit on how many +// accounts to return +func (pool *LegacyPool) PendingWithMax(enforceTips bool, maxAccountsNum int) map[common.Address][]*txpool.LazyTransaction { + return pool.pendingWithMax(enforceTips, maxAccountsNum) +} + +func (pool *LegacyPool) pendingWithMax(enforceTips bool, maxAccountsNum int) map[common.Address][]*txpool.LazyTransaction { pool.mu.Lock() defer pool.mu.Unlock() @@ -554,6 +564,9 @@ func (pool *LegacyPool) Pending(enforceTips bool) map[common.Address][]*txpool.L } } pending[addr] = lazies + if len(pending) >= maxAccountsNum { + break + } } } return pending diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index 97b513f4ca..d6ce8c1e09 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -29,6 +29,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/scroll-tech/go-ethereum/common" "github.com/scroll-tech/go-ethereum/core" "github.com/scroll-tech/go-ethereum/core/rawdb" @@ -2628,3 +2630,27 @@ func BenchmarkMultiAccountBatchInsert(b *testing.B) { pool.addRemotesSync([]*types.Transaction{tx}) } } + +func TestPoolPending(t *testing.T) { + // Generate a batch of transactions to enqueue into the pool + pool, _ := setupPool() + defer pool.Close() + numTxns := 100 + batches := make(types.Transactions, numTxns) + for i := 0; i < numTxns; i++ { + key, _ := crypto.GenerateKey() + account := crypto.PubkeyToAddress(key.PublicKey) + pool.currentState.AddBalance(account, big.NewInt(1000000)) + tx := transaction(uint64(0), 100000, key) + batches[i] = tx + } + // Benchmark importing the transactions into the queue + for _, tx := range batches { + pool.addRemotesSync([]*types.Transaction{tx}) + } + + assert.Len(t, pool.Pending(false), numTxns) + + maxAccounts := 10 + assert.Len(t, pool.PendingWithMax(false, maxAccounts), maxAccounts) +} diff --git a/core/txpool/subpool.go b/core/txpool/subpool.go index 7c2838a218..4843b97e87 100644 --- a/core/txpool/subpool.go +++ b/core/txpool/subpool.go @@ -110,6 +110,8 @@ type SubPool interface { // account and sorted by nonce. Pending(enforceTips bool) map[common.Address][]*LazyTransaction + PendingWithMax(enforceTips bool, maxAccountsNum int) map[common.Address][]*LazyTransaction + // SubscribeTransactions subscribes to new transaction events. The subscriber // can decide whether to receive notifications only for newly seen transactions // or also for reorged out ones. diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 47ff4b3cb5..b132b8b98d 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -318,6 +318,16 @@ func (p *TxPool) Pending(enforceTips bool) map[common.Address][]*LazyTransaction return txs } +func (p *TxPool) PendingWithMax(enforceTips bool, maxAccountsNum int) map[common.Address][]*LazyTransaction { + txs := make(map[common.Address][]*LazyTransaction) + for _, subpool := range p.subpools { + for addr, set := range subpool.PendingWithMax(enforceTips, maxAccountsNum) { + txs[addr] = set + } + } + return txs +} + // SubscribeTransactions registers a subscription for new transaction events, // supporting feeding only newly seen or also resurrected transactions. func (p *TxPool) SubscribeTransactions(ch chan<- core.NewTxsEvent, reorgs bool) event.Subscription { diff --git a/miner/miner.go b/miner/miner.go index fdb1956959..e870232ef7 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -59,6 +59,7 @@ type Config struct { NewPayloadTimeout time.Duration // The maximum time allowance for creating a new payload StoreSkippedTxTraces bool // Whether store the wrapped traces when storing a skipped tx + MaxAccountsNum int // Maximum number of accounts that miner will fetch the pending transactions of when building a new block } // DefaultConfig contains default settings for miner. diff --git a/miner/miner_test.go b/miner/miner_test.go index 8729261350..49bf802dca 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -19,6 +19,7 @@ package miner import ( "errors" + "math" "math/big" "testing" "time" @@ -301,7 +302,8 @@ func minerTestGenesisBlock(period uint64, gasLimit uint64, faucet common.Address func createMiner(t *testing.T) (*Miner, *event.TypeMux, func(skipMiner bool)) { // Create Ethash config config := Config{ - Etherbase: common.HexToAddress("123456789"), + Etherbase: common.HexToAddress("123456789"), + MaxAccountsNum: math.MaxInt, } // Create chainConfig chainDB := rawdb.NewMemoryDatabase() diff --git a/miner/scroll_worker.go b/miner/scroll_worker.go index 500acc0967..c7e6249ccd 100644 --- a/miner/scroll_worker.go +++ b/miner/scroll_worker.go @@ -19,6 +19,7 @@ package miner import ( "bytes" "errors" + "math" "math/big" "sync" "sync/atomic" @@ -162,6 +163,12 @@ func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus // Subscribe events for blockchain worker.chainHeadSub = eth.BlockChain().SubscribeChainHeadEvent(worker.chainHeadCh) + // Sanitize account fetch limit. + if worker.config.MaxAccountsNum == 0 { + log.Warn("Sanitizing miner account fetch limit", "provided", worker.config.MaxAccountsNum, "updated", math.MaxInt) + worker.config.MaxAccountsNum = math.MaxInt + } + worker.wg.Add(1) go worker.mainLoop() @@ -382,7 +389,7 @@ func (w *worker) startNewPipeline(timestamp int64) { tidyPendingStart := time.Now() // Fill the block with all available pending transactions. - pending := w.eth.TxPool().Pending(false) + pending := w.eth.TxPool().PendingWithMax(false, w.config.MaxAccountsNum) // Split the pending transactions into locals and remotes localTxs, remoteTxs := make(map[common.Address][]*txpool.LazyTransaction), pending for _, account := range w.eth.TxPool().Locals() { diff --git a/miner/worker.go b/miner/worker.go index 7420c44ac7..f80a999de5 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -353,6 +353,12 @@ func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus } worker.newpayloadTimeout = newpayloadTimeout + // Sanitize account fetch limit. + if worker.config.MaxAccountsNum == 0 { + log.Warn("Sanitizing miner account fetch limit", "provided", worker.config.MaxAccountsNum, "updated", math.MaxInt) + worker.config.MaxAccountsNum = math.MaxInt + } + worker.wg.Add(4) go worker.mainLoop() go worker.newWorkLoop(recommit) @@ -1420,7 +1426,7 @@ func (w *worker) fillTransactions(interrupt *atomic.Int32, env *environment) err } tidyPendingStart := time.Now() - pending := w.eth.TxPool().Pending(true) + pending := w.eth.TxPool().PendingWithMax(true, w.config.MaxAccountsNum) // Split the pending transactions into locals and remotes. localTxs, remoteTxs := make(map[common.Address][]*txpool.LazyTransaction), pending diff --git a/miner/worker_test.go b/miner/worker_test.go index da2c8a86f8..5f40ab88ae 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -17,6 +17,7 @@ package miner import ( + "math" "math/big" "testing" "time" @@ -67,8 +68,9 @@ var ( newTxs []*types.Transaction testConfig = &Config{ - Recommit: time.Second, - GasCeil: params.GenesisGasLimit, + Recommit: time.Second, + GasCeil: params.GenesisGasLimit, + MaxAccountsNum: math.MaxInt, } )