From db2240c27bcaf9e77fdf71cccfb8fde87cb6a297 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Tue, 26 Jul 2022 10:41:54 +0800 Subject: [PATCH 1/4] eth, miner: add timeout for building sealing block --- eth/catalyst/api.go | 7 +-- miner/miner.go | 8 ++-- miner/worker.go | 105 ++++++++++++++++++++++++++++--------------- miner/worker_test.go | 12 ++--- 4 files changed, 82 insertions(+), 50 deletions(-) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 754d8b214ca7..4ba7820eed83 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -281,14 +281,15 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa // might replace it arbitrarily many times in between. if payloadAttributes != nil { // Create an empty block first which can be used as a fallback - empty, err := api.eth.Miner().GetSealingBlockSync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, true) + empty, err := api.eth.Miner().GetSealingBlockSync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, true, 0) if err != nil { log.Error("Failed to create empty sealing payload", "err", err) return valid(nil), beacon.InvalidPayloadAttributes.With(err) } // Send a request to generate a full block in the background. - // The result can be obtained via the returned channel. - resCh, err := api.eth.Miner().GetSealingBlockAsync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, false) + // The result can be obtained via the returned channel. The + // timeout for building block is set to prevent some huge blocks. + resCh, err := api.eth.Miner().GetSealingBlockAsync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, false, time.Second*3) if err != nil { log.Error("Failed to create async sealing payload", "err", err) return valid(nil), beacon.InvalidPayloadAttributes.With(err) diff --git a/miner/miner.go b/miner/miner.go index 1e9607a76ad9..85c41ff9794a 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -245,8 +245,8 @@ func (miner *Miner) SubscribePendingLogs(ch chan<- []*types.Log) event.Subscript // there is always a result that will be returned through the result channel. // The difference is that if the execution fails, the returned result is nil // and the concrete error is dropped silently. -func (miner *Miner) GetSealingBlockAsync(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool) (chan *types.Block, error) { - resCh, _, err := miner.worker.getSealingBlock(parent, timestamp, coinbase, random, noTxs) +func (miner *Miner) GetSealingBlockAsync(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool, timeout time.Duration) (chan *types.Block, error) { + resCh, _, err := miner.worker.getSealingBlock(parent, timestamp, coinbase, random, noTxs, timeout) if err != nil { return nil, err } @@ -256,8 +256,8 @@ func (miner *Miner) GetSealingBlockAsync(parent common.Hash, timestamp uint64, c // GetSealingBlockSync creates a sealing block according to the given parameters. // If the generation is failed or the underlying work is already closed, an error // will be returned. -func (miner *Miner) GetSealingBlockSync(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool) (*types.Block, error) { - resCh, errCh, err := miner.worker.getSealingBlock(parent, timestamp, coinbase, random, noTxs) +func (miner *Miner) GetSealingBlockSync(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool, timeout time.Duration) (*types.Block, error) { + resCh, errCh, err := miner.worker.getSealingBlock(parent, timestamp, coinbase, random, noTxs, timeout) if err != nil { return nil, err } diff --git a/miner/worker.go b/miner/worker.go index 93fb6288bb45..1c1e3041842d 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -80,6 +80,7 @@ const ( var ( errBlockInterruptedByNewHead = errors.New("new head arrived while building block") errBlockInterruptedByRecommit = errors.New("recommit interrupt while building block") + errBlockInterruptedByTimeout = errors.New("timeout while building block") ) // environment is the worker's current environment and holds all @@ -158,6 +159,7 @@ const ( commitInterruptNone int32 = iota commitInterruptNewHead commitInterruptResubmit + commitInterruptTimeout ) // newWorkReq represents a request for new sealing work submitting with relative interrupt notifier. @@ -844,42 +846,26 @@ func (w *worker) commitTransactions(env *environment, txs *types.TransactionsByP var coalescedLogs []*types.Log for { - // In the following three cases, we will interrupt the execution of the transaction. - // (1) new head block event arrival, the interrupt signal is 1 - // (2) worker start or restart, the interrupt signal is 1 - // (3) worker recreate the sealing block with any newly arrived transactions, the interrupt signal is 2. - // For the first two cases, the semi-finished work will be discarded. - // For the third case, the semi-finished work will be submitted to the consensus engine. - if interrupt != nil && atomic.LoadInt32(interrupt) != commitInterruptNone { - // Notify resubmit loop to increase resubmitting interval due to too frequent commits. - if atomic.LoadInt32(interrupt) == commitInterruptResubmit { - ratio := float64(gasLimit-env.gasPool.Gas()) / float64(gasLimit) - if ratio < 0.1 { - ratio = 0.1 - } - w.resubmitAdjustCh <- &intervalAdjust{ - ratio: ratio, - inc: true, - } - return errBlockInterruptedByRecommit + // Check interruption signal and abort building if it's fired. + if interrupt != nil { + if signal := atomic.LoadInt32(interrupt); signal != commitInterruptNone { + return signalToErr(signal) } - return errBlockInterruptedByNewHead } - // If we don't have enough gas for any further transactions then we're done + // If we don't have enough gas for any further transactions then we're done. if env.gasPool.Gas() < params.TxGas { log.Trace("Not enough gas for further transactions", "have", env.gasPool, "want", params.TxGas) break } - // Retrieve the next transaction and abort if all done + // Retrieve the next transaction and abort if all done. tx := txs.Peek() if tx == nil { break } // Error may be ignored here. The error has already been checked // during transaction acceptance is the transaction pool. - // - // We use the eip155 signer regardless of the current hf. from, _ := types.Sender(env.signer, tx) + // Check whether the tx is replay protected. If we're not in the EIP155 hf // phase, start ignoring the sender until we do. if tx.Protected() && !w.chainConfig.IsEIP155(env.header.Number) { @@ -926,7 +912,6 @@ func (w *worker) commitTransactions(env *environment, txs *types.TransactionsByP txs.Shift() } } - if !w.isRunning() && len(coalescedLogs) > 0 { // We don't push the pendingLogsEvent while we are sealing. The reason is that // when we are sealing, the worker will regenerate a sealing block every 3 seconds. @@ -942,11 +927,6 @@ func (w *worker) commitTransactions(env *environment, txs *types.TransactionsByP } w.pendingLogsFeed.Send(cpy) } - // Notify resubmit loop to decrease resubmitting interval if current interval is larger - // than the user-specified one. - if interrupt != nil { - w.resubmitAdjustCh <- &intervalAdjust{inc: false} - } return nil } @@ -960,6 +940,7 @@ type generateParams struct { noUncle bool // Flag whether the uncle block inclusion is allowed noExtra bool // Flag whether the extra field assignment is allowed noTxs bool // Flag whether an empty block without any transaction is expected + timeout time.Duration // The time allowance for building block, 0 means no limit } // prepareWork constructs the sealing task according to the given parameters, @@ -986,15 +967,15 @@ func (w *worker) prepareWork(genParams *generateParams) (*environment, error) { } timestamp = parent.Time() + 1 } - // Construct the sealing block header, set the extra field if it's allowed - num := parent.Number() + // Construct the sealing block header. header := &types.Header{ ParentHash: parent.Hash(), - Number: num.Add(num, common.Big1), + Number: new(big.Int).Add(parent.Number(), common.Big1), GasLimit: core.CalcGasLimit(parent.GasLimit(), w.config.GasCeil), Time: timestamp, Coinbase: genParams.coinbase, } + // Set the extra field if it's allowed. if !genParams.noExtra && len(w.extra) != 0 { header.Extra = w.extra } @@ -1082,7 +1063,18 @@ func (w *worker) generateWork(params *generateParams) (*types.Block, error) { defer work.discard() if !params.noTxs { - w.fillTransactions(nil, work) + var interrupt *int32 + if params.timeout != 0 { + interrupt = new(int32) + timer := time.AfterFunc(params.timeout, func() { + atomic.StoreInt32(interrupt, commitInterruptTimeout) + }) + defer timer.Stop() + } + err := w.fillTransactions(interrupt, work) + if errors.Is(err, errBlockInterruptedByTimeout) { + log.Warn("Block building is interrupted", "allowance", common.PrettyDuration(params.timeout)) + } } return w.engine.FinalizeAndAssemble(w.chain, work.header, work.state, work.txs, work.unclelist(), work.receipts) } @@ -1113,13 +1105,36 @@ func (w *worker) commitWork(interrupt *int32, noempty bool, timestamp int64) { if !noempty && atomic.LoadUint32(&w.noempty) == 0 { w.commit(work.copy(), nil, false, start) } - - // Fill pending transactions from the txpool + // Fill pending transactions from the txpool into the block. err = w.fillTransactions(interrupt, work) - if errors.Is(err, errBlockInterruptedByNewHead) { + switch { + case err == nil: + // The entire block is filled, decrease resubmit interval in case + // of current interval is larger than the user-specified one. + w.resubmitAdjustCh <- &intervalAdjust{inc: false} + + case errors.Is(err, errBlockInterruptedByRecommit): + // Notify resubmit loop to increase resubmitting interval if the + // interruption is due to frequent commits. + gaslimit := work.header.GasLimit + ratio := float64(gaslimit-work.gasPool.Gas()) / float64(gaslimit) + if ratio < 0.1 { + ratio = 0.1 + } + w.resubmitAdjustCh <- &intervalAdjust{ + ratio: ratio, + inc: true, + } + + case errors.Is(err, errBlockInterruptedByNewHead): + // If the block building is interrupted by newhead event, discard it + // totally. Committing the interrupted block introduces unnecessary + // delay, and possibly causes miner to mine on the previous head, + // which could result in higher uncle rate. work.discard() return } + // Submit the generated block for consensus sealing. w.commit(work.copy(), w.fullTaskHook, true, start) // Swap out the old work with the new one, terminating any leftover @@ -1170,7 +1185,7 @@ func (w *worker) commit(env *environment, interval func(), update bool, start ti // getSealingBlock generates the sealing block based on the given parameters. // The generation result will be passed back via the given channel no matter // the generation itself succeeds or not. -func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool) (chan *types.Block, chan error, error) { +func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool, timeout time.Duration) (chan *types.Block, chan error, error) { var ( resCh = make(chan *types.Block, 1) errCh = make(chan error, 1) @@ -1185,6 +1200,7 @@ func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase noUncle: true, noExtra: true, noTxs: noTxs, + timeout: timeout, }, result: resCh, err: errCh, @@ -1231,3 +1247,18 @@ func totalFees(block *types.Block, receipts []*types.Receipt) *big.Float { } return new(big.Float).Quo(new(big.Float).SetInt(feesWei), new(big.Float).SetInt(big.NewInt(params.Ether))) } + +// signalToErr converts the interruption signal to a concrete error type for return. +// The given signal must be a valid interruption signal. +func signalToErr(signal int32) error { + switch signal { + case commitInterruptNewHead: + return errBlockInterruptedByNewHead + case commitInterruptResubmit: + return errBlockInterruptedByRecommit + case commitInterruptTimeout: + return errBlockInterruptedByTimeout + default: + panic(fmt.Errorf("undefined signal %d", signal)) + } +} diff --git a/miner/worker_test.go b/miner/worker_test.go index 2f1939f75981..b903e8db2102 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -523,21 +523,21 @@ func testAdjustInterval(t *testing.T, chainConfig *params.ChainConfig, engine co } func TestGetSealingWorkEthash(t *testing.T) { - testGetSealingWork(t, ethashChainConfig, ethash.NewFaker(), false) + testGetSealingWork(t, ethashChainConfig, ethash.NewFaker()) } func TestGetSealingWorkClique(t *testing.T) { - testGetSealingWork(t, cliqueChainConfig, clique.New(cliqueChainConfig.Clique, rawdb.NewMemoryDatabase()), false) + testGetSealingWork(t, cliqueChainConfig, clique.New(cliqueChainConfig.Clique, rawdb.NewMemoryDatabase())) } func TestGetSealingWorkPostMerge(t *testing.T) { local := new(params.ChainConfig) *local = *ethashChainConfig local.TerminalTotalDifficulty = big.NewInt(0) - testGetSealingWork(t, local, ethash.NewFaker(), true) + testGetSealingWork(t, local, ethash.NewFaker()) } -func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine, postMerge bool) { +func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine) { defer engine.Close() w, b := newTestWorker(t, chainConfig, engine, rawdb.NewMemoryDatabase(), 0) @@ -633,7 +633,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co // This API should work even when the automatic sealing is not enabled for _, c := range cases { - resChan, errChan, _ := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, false) + resChan, errChan, _ := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, false, 0) block := <-resChan err := <-errChan if c.expectErr { @@ -651,7 +651,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co // This API should work even when the automatic sealing is enabled w.start() for _, c := range cases { - resChan, errChan, _ := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, false) + resChan, errChan, _ := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, false, 0) block := <-resChan err := <-errChan if c.expectErr { From 8e49f8a732feeaf3a116a326a870822710f24997 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Mon, 8 Aug 2022 12:23:40 +0800 Subject: [PATCH 2/4] eth, cmd, miner: add newpayloadtimeout flag --- cmd/geth/main.go | 1 + cmd/utils/flags.go | 11 ++++++++++- eth/catalyst/api.go | 22 ++++++++++++++++++---- eth/catalyst/api_test.go | 36 ++++++++++++++++++------------------ eth/ethconfig/config.go | 7 ++++--- miner/miner.go | 2 ++ 6 files changed, 53 insertions(+), 26 deletions(-) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 70b354ae148b..c60ad3c9cf92 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -133,6 +133,7 @@ var ( utils.MinerExtraDataFlag, utils.MinerRecommitIntervalFlag, utils.MinerNoVerifyFlag, + utils.MinerNewPayloadTimeout, utils.NATFlag, utils.NoDiscoverFlag, utils.DiscoveryV5Flag, diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index fb2aa7c21587..de8b4b7282b6 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -560,6 +560,12 @@ var ( Usage: "Disable remote sealing verification", Category: flags.MinerCategory, } + MinerNewPayloadTimeout = &cli.DurationFlag{ + Name: "miner.newpayload_timeout", + Usage: "Specify the maximum time allowance for creating a new payload(pending block)", + Value: ethconfig.Defaults.Miner.NewPayloadTimeout, + Category: flags.MinerCategory, + } // Account settings UnlockedAccountFlag = &cli.StringFlag{ @@ -1649,6 +1655,9 @@ func setMiner(ctx *cli.Context, cfg *miner.Config) { if ctx.IsSet(MinerNoVerifyFlag.Name) { cfg.Noverify = ctx.Bool(MinerNoVerifyFlag.Name) } + if ctx.IsSet(MinerNewPayloadTimeout.Name) { + cfg.NewPayloadTimeout = ctx.Duration(MinerNewPayloadTimeout.Name) + } if ctx.IsSet(LegacyMinerGasTargetFlag.Name) { log.Warn("The generic --miner.gastarget flag is deprecated and will be removed in the future!") } @@ -2008,7 +2017,7 @@ func RegisterEthService(stack *node.Node, cfg *ethconfig.Config) (ethapi.Backend Fatalf("Failed to create the LES server: %v", err) } } - if err := ethcatalyst.Register(stack, backend); err != nil { + if err := ethcatalyst.Register(stack, backend, cfg); err != nil { Fatalf("Failed to register the Engine API service: %v", err) } stack.RegisterAPIs(tracers.APIs(backend.APIBackend)) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 4ba7820eed83..bbebeea0f8aa 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -33,18 +33,19 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/eth/downloader" + "github.com/ethereum/go-ethereum/eth/ethconfig" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/rpc" ) // Register adds the engine API to the full node. -func Register(stack *node.Node, backend *eth.Ethereum) error { +func Register(stack *node.Node, backend *eth.Ethereum, config *ethconfig.Config) error { log.Warn("Engine API enabled", "protocol", "eth") stack.RegisterAPIs([]rpc.API{ { Namespace: "engine", - Service: NewConsensusAPI(backend), + Service: NewConsensusAPI(backend, config.Miner.NewPayloadTimeout), Authenticated: true, }, }) @@ -86,6 +87,13 @@ type ConsensusAPI struct { remoteBlocks *headerQueue // Cache of remote payloads received localBlocks *payloadQueue // Cache of local payloads generated + // newpayloadTimeout is the maximum timeout allowance for creating payload. + // The default value is 2 seconds but node operator can set it to arbitrary + // large value. A large timeout allowance may cause Geth to fail to create + // a non-empty payload within the specified time and eventually miss the slot + // in case there are some computation expensive transactions in txpool. + newpayloadTimeout time.Duration + // The forkchoice update and new payload method require us to return the // latest valid hash in an invalid chain. To support that return, we need // to track historical bad blocks as well as bad tipsets in case a chain @@ -124,14 +132,20 @@ type ConsensusAPI struct { // NewConsensusAPI creates a new consensus api for the given backend. // The underlying blockchain needs to have a valid terminal total difficulty set. -func NewConsensusAPI(eth *eth.Ethereum) *ConsensusAPI { +func NewConsensusAPI(eth *eth.Ethereum, newpayloadTimeout time.Duration) *ConsensusAPI { if eth.BlockChain().Config().TerminalTotalDifficulty == nil { log.Warn("Engine API started but chain not configured for merge yet") } + // Sanitize the timeout config for creating payload. + if newpayloadTimeout < time.Millisecond*100 { + log.Warn("Sanitizing new payload timeout to default", "provided", newpayloadTimeout, "updated", ethconfig.Defaults.Miner.NewPayloadTimeout) + newpayloadTimeout = ethconfig.Defaults.Miner.NewPayloadTimeout + } api := &ConsensusAPI{ eth: eth, remoteBlocks: newHeaderQueue(), localBlocks: newPayloadQueue(), + newpayloadTimeout: newpayloadTimeout, invalidBlocksHits: make(map[common.Hash]int), invalidTipsets: make(map[common.Hash]*types.Header), } @@ -289,7 +303,7 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa // Send a request to generate a full block in the background. // The result can be obtained via the returned channel. The // timeout for building block is set to prevent some huge blocks. - resCh, err := api.eth.Miner().GetSealingBlockAsync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, false, time.Second*3) + resCh, err := api.eth.Miner().GetSealingBlockAsync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, false, api.newpayloadTimeout) if err != nil { log.Error("Failed to create async sealing payload", "err", err) return valid(nil), beacon.InvalidPayloadAttributes.With(err) diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index ae53462ff812..ea42f093d5d7 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -81,7 +81,7 @@ func TestEth2AssembleBlock(t *testing.T) { n, ethservice := startEthService(t, genesis, blocks) defer n.Close() - api := NewConsensusAPI(ethservice) + api := NewConsensusAPI(ethservice, 0) signer := types.NewEIP155Signer(ethservice.BlockChain().Config().ChainID) tx, err := types.SignTx(types.NewTransaction(uint64(10), blocks[9].Coinbase(), big.NewInt(1000), params.TxGas, big.NewInt(params.InitialBaseFee), nil), signer, testKey) if err != nil { @@ -105,7 +105,7 @@ func TestEth2AssembleBlockWithAnotherBlocksTxs(t *testing.T) { n, ethservice := startEthService(t, genesis, blocks[:9]) defer n.Close() - api := NewConsensusAPI(ethservice) + api := NewConsensusAPI(ethservice, 0) // Put the 10th block's tx in the pool and produce a new block api.eth.TxPool().AddRemotesSync(blocks[9].Transactions()) @@ -126,7 +126,7 @@ func TestSetHeadBeforeTotalDifficulty(t *testing.T) { n, ethservice := startEthService(t, genesis, blocks) defer n.Close() - api := NewConsensusAPI(ethservice) + api := NewConsensusAPI(ethservice, 0) fcState := beacon.ForkchoiceStateV1{ HeadBlockHash: blocks[5].Hash(), SafeBlockHash: common.Hash{}, @@ -146,7 +146,7 @@ func TestEth2PrepareAndGetPayload(t *testing.T) { n, ethservice := startEthService(t, genesis, blocks[:9]) defer n.Close() - api := NewConsensusAPI(ethservice) + api := NewConsensusAPI(ethservice, 0) // Put the 10th block's tx in the pool and produce a new block ethservice.TxPool().AddLocals(blocks[9].Transactions()) @@ -203,7 +203,7 @@ func TestInvalidPayloadTimestamp(t *testing.T) { n, ethservice := startEthService(t, genesis, preMergeBlocks) defer n.Close() var ( - api = NewConsensusAPI(ethservice) + api = NewConsensusAPI(ethservice, 0) parent = ethservice.BlockChain().CurrentBlock() ) tests := []struct { @@ -248,7 +248,7 @@ func TestEth2NewBlock(t *testing.T) { defer n.Close() var ( - api = NewConsensusAPI(ethservice) + api = NewConsensusAPI(ethservice, 0) parent = preMergeBlocks[len(preMergeBlocks)-1] // This EVM code generates a log when the contract is created. @@ -439,7 +439,7 @@ func TestFullAPI(t *testing.T) { } func setupBlocks(t *testing.T, ethservice *eth.Ethereum, n int, parent *types.Block, callback func(parent *types.Block)) { - api := NewConsensusAPI(ethservice) + api := NewConsensusAPI(ethservice, 0) for i := 0; i < n; i++ { callback(parent) @@ -475,7 +475,7 @@ func TestExchangeTransitionConfig(t *testing.T) { n, ethservice := startEthService(t, genesis, preMergeBlocks) defer n.Close() var ( - api = NewConsensusAPI(ethservice) + api = NewConsensusAPI(ethservice, 0) ) // invalid ttd config := beacon.TransitionConfigurationV1{ @@ -538,7 +538,7 @@ func TestNewPayloadOnInvalidChain(t *testing.T) { defer n.Close() var ( - api = NewConsensusAPI(ethservice) + api = NewConsensusAPI(ethservice, 0) parent = ethservice.BlockChain().CurrentBlock() // This EVM code generates a log when the contract is created. logCode = common.Hex2Bytes("60606040525b7f24ec1d3ff24c2f6ff210738839dbc339cd45a5294d85c79361016243157aae7b60405180905060405180910390a15b600a8060416000396000f360606040526008565b00") @@ -599,7 +599,7 @@ func TestNewPayloadOnInvalidChain(t *testing.T) { } func assembleBlock(api *ConsensusAPI, parentHash common.Hash, params *beacon.PayloadAttributesV1) (*beacon.ExecutableDataV1, error) { - block, err := api.eth.Miner().GetSealingBlockSync(parentHash, params.Timestamp, params.SuggestedFeeRecipient, params.Random, false) + block, err := api.eth.Miner().GetSealingBlockSync(parentHash, params.Timestamp, params.SuggestedFeeRecipient, params.Random, false, 0) if err != nil { return nil, err } @@ -612,7 +612,7 @@ func TestEmptyBlocks(t *testing.T) { defer n.Close() commonAncestor := ethservice.BlockChain().CurrentBlock() - api := NewConsensusAPI(ethservice) + api := NewConsensusAPI(ethservice, 0) // Setup 10 blocks on the canonical chain setupBlocks(t, ethservice, 10, commonAncestor, func(parent *types.Block) {}) @@ -732,8 +732,8 @@ func TestTrickRemoteBlockCache(t *testing.T) { } nodeA.Server().AddPeer(nodeB.Server().Self()) nodeB.Server().AddPeer(nodeA.Server().Self()) - apiA := NewConsensusAPI(ethserviceA) - apiB := NewConsensusAPI(ethserviceB) + apiA := NewConsensusAPI(ethserviceA, 0) + apiB := NewConsensusAPI(ethserviceB, 0) commonAncestor := ethserviceA.BlockChain().CurrentBlock() @@ -791,7 +791,7 @@ func TestInvalidBloom(t *testing.T) { defer n.Close() commonAncestor := ethservice.BlockChain().CurrentBlock() - api := NewConsensusAPI(ethservice) + api := NewConsensusAPI(ethservice, 0) // Setup 10 blocks on the canonical chain setupBlocks(t, ethservice, 10, commonAncestor, func(parent *types.Block) {}) @@ -810,15 +810,15 @@ func TestInvalidBloom(t *testing.T) { func TestNewPayloadOnInvalidTerminalBlock(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(100) - fmt.Println(genesis.Config.TerminalTotalDifficulty) + //fmt.Println(genesis.Config.TerminalTotalDifficulty) genesis.Config.TerminalTotalDifficulty = preMergeBlocks[0].Difficulty() //.Sub(genesis.Config.TerminalTotalDifficulty, preMergeBlocks[len(preMergeBlocks)-1].Difficulty()) - fmt.Println(genesis.Config.TerminalTotalDifficulty) + //fmt.Println(genesis.Config.TerminalTotalDifficulty) n, ethservice := startEthService(t, genesis, preMergeBlocks) defer n.Close() var ( - api = NewConsensusAPI(ethservice) + api = NewConsensusAPI(ethservice, 0) parent = preMergeBlocks[len(preMergeBlocks)-1] ) @@ -842,7 +842,7 @@ func TestNewPayloadOnInvalidTerminalBlock(t *testing.T) { Random: crypto.Keccak256Hash([]byte{byte(1)}), SuggestedFeeRecipient: parent.Coinbase(), } - empty, err := api.eth.Miner().GetSealingBlockSync(parent.Hash(), params.Timestamp, params.SuggestedFeeRecipient, params.Random, true) + empty, err := api.eth.Miner().GetSealingBlockSync(parent.Hash(), params.Timestamp, params.SuggestedFeeRecipient, params.Random, true, 0) if err != nil { t.Fatalf("error preparing payload, err=%v", err) } diff --git a/eth/ethconfig/config.go b/eth/ethconfig/config.go index a897294175ea..8065601c7cea 100644 --- a/eth/ethconfig/config.go +++ b/eth/ethconfig/config.go @@ -85,9 +85,10 @@ var Defaults = Config{ SnapshotCache: 102, FilterLogCacheSize: 32, Miner: miner.Config{ - GasCeil: 30000000, - GasPrice: big.NewInt(params.GWei), - Recommit: 3 * time.Second, + GasCeil: 30000000, + GasPrice: big.NewInt(params.GWei), + Recommit: 3 * time.Second, + NewPayloadTimeout: 2 * time.Second, }, TxPool: core.DefaultTxPoolConfig, RPCGasCap: 50000000, diff --git a/miner/miner.go b/miner/miner.go index 85c41ff9794a..8c8643bcd71a 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -53,6 +53,8 @@ type Config struct { GasPrice *big.Int // Minimum gas price for mining a transaction Recommit time.Duration // The time interval for miner to re-create mining work. Noverify bool // Disable remote mining solution verification(only useful in ethash). + + NewPayloadTimeout time.Duration // The maximum time allowance for creating a new payload } // Miner creates blocks and searches for proof-of-work values. From 5879b810bf5122b016ca0507fec9c06c2e02271c Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Wed, 31 Aug 2022 16:29:57 +0800 Subject: [PATCH 3/4] eth, miner, cmd: address comments --- cmd/utils/flags.go | 6 +++--- eth/catalyst/api.go | 24 +++++------------------- eth/catalyst/api_test.go | 35 +++++++++++++++++------------------ eth/ethconfig/config.go | 17 ++++++----------- miner/miner.go | 16 ++++++++++++---- miner/worker.go | 37 +++++++++++++++++++++++++------------ 6 files changed, 68 insertions(+), 67 deletions(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index de8b4b7282b6..450c5cbddcee 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -561,8 +561,8 @@ var ( Category: flags.MinerCategory, } MinerNewPayloadTimeout = &cli.DurationFlag{ - Name: "miner.newpayload_timeout", - Usage: "Specify the maximum time allowance for creating a new payload(pending block)", + Name: "miner.newpayload-timeout", + Usage: "Specify the maximum time allowance for creating a new payload", Value: ethconfig.Defaults.Miner.NewPayloadTimeout, Category: flags.MinerCategory, } @@ -2017,7 +2017,7 @@ func RegisterEthService(stack *node.Node, cfg *ethconfig.Config) (ethapi.Backend Fatalf("Failed to create the LES server: %v", err) } } - if err := ethcatalyst.Register(stack, backend, cfg); err != nil { + if err := ethcatalyst.Register(stack, backend); err != nil { Fatalf("Failed to register the Engine API service: %v", err) } stack.RegisterAPIs(tracers.APIs(backend.APIBackend)) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index bbebeea0f8aa..92a154a4f668 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -33,19 +33,18 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/eth/downloader" - "github.com/ethereum/go-ethereum/eth/ethconfig" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/rpc" ) // Register adds the engine API to the full node. -func Register(stack *node.Node, backend *eth.Ethereum, config *ethconfig.Config) error { +func Register(stack *node.Node, backend *eth.Ethereum) error { log.Warn("Engine API enabled", "protocol", "eth") stack.RegisterAPIs([]rpc.API{ { Namespace: "engine", - Service: NewConsensusAPI(backend, config.Miner.NewPayloadTimeout), + Service: NewConsensusAPI(backend), Authenticated: true, }, }) @@ -87,13 +86,6 @@ type ConsensusAPI struct { remoteBlocks *headerQueue // Cache of remote payloads received localBlocks *payloadQueue // Cache of local payloads generated - // newpayloadTimeout is the maximum timeout allowance for creating payload. - // The default value is 2 seconds but node operator can set it to arbitrary - // large value. A large timeout allowance may cause Geth to fail to create - // a non-empty payload within the specified time and eventually miss the slot - // in case there are some computation expensive transactions in txpool. - newpayloadTimeout time.Duration - // The forkchoice update and new payload method require us to return the // latest valid hash in an invalid chain. To support that return, we need // to track historical bad blocks as well as bad tipsets in case a chain @@ -132,20 +124,14 @@ type ConsensusAPI struct { // NewConsensusAPI creates a new consensus api for the given backend. // The underlying blockchain needs to have a valid terminal total difficulty set. -func NewConsensusAPI(eth *eth.Ethereum, newpayloadTimeout time.Duration) *ConsensusAPI { +func NewConsensusAPI(eth *eth.Ethereum) *ConsensusAPI { if eth.BlockChain().Config().TerminalTotalDifficulty == nil { log.Warn("Engine API started but chain not configured for merge yet") } - // Sanitize the timeout config for creating payload. - if newpayloadTimeout < time.Millisecond*100 { - log.Warn("Sanitizing new payload timeout to default", "provided", newpayloadTimeout, "updated", ethconfig.Defaults.Miner.NewPayloadTimeout) - newpayloadTimeout = ethconfig.Defaults.Miner.NewPayloadTimeout - } api := &ConsensusAPI{ eth: eth, remoteBlocks: newHeaderQueue(), localBlocks: newPayloadQueue(), - newpayloadTimeout: newpayloadTimeout, invalidBlocksHits: make(map[common.Hash]int), invalidTipsets: make(map[common.Hash]*types.Header), } @@ -295,7 +281,7 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa // might replace it arbitrarily many times in between. if payloadAttributes != nil { // Create an empty block first which can be used as a fallback - empty, err := api.eth.Miner().GetSealingBlockSync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, true, 0) + empty, err := api.eth.Miner().GetSealingBlockSync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, true) if err != nil { log.Error("Failed to create empty sealing payload", "err", err) return valid(nil), beacon.InvalidPayloadAttributes.With(err) @@ -303,7 +289,7 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa // Send a request to generate a full block in the background. // The result can be obtained via the returned channel. The // timeout for building block is set to prevent some huge blocks. - resCh, err := api.eth.Miner().GetSealingBlockAsync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, false, api.newpayloadTimeout) + resCh, err := api.eth.Miner().GetSealingBlockAsync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, false) if err != nil { log.Error("Failed to create async sealing payload", "err", err) return valid(nil), beacon.InvalidPayloadAttributes.With(err) diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index ea42f093d5d7..023e92ddef22 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -81,7 +81,7 @@ func TestEth2AssembleBlock(t *testing.T) { n, ethservice := startEthService(t, genesis, blocks) defer n.Close() - api := NewConsensusAPI(ethservice, 0) + api := NewConsensusAPI(ethservice) signer := types.NewEIP155Signer(ethservice.BlockChain().Config().ChainID) tx, err := types.SignTx(types.NewTransaction(uint64(10), blocks[9].Coinbase(), big.NewInt(1000), params.TxGas, big.NewInt(params.InitialBaseFee), nil), signer, testKey) if err != nil { @@ -105,7 +105,7 @@ func TestEth2AssembleBlockWithAnotherBlocksTxs(t *testing.T) { n, ethservice := startEthService(t, genesis, blocks[:9]) defer n.Close() - api := NewConsensusAPI(ethservice, 0) + api := NewConsensusAPI(ethservice) // Put the 10th block's tx in the pool and produce a new block api.eth.TxPool().AddRemotesSync(blocks[9].Transactions()) @@ -126,7 +126,7 @@ func TestSetHeadBeforeTotalDifficulty(t *testing.T) { n, ethservice := startEthService(t, genesis, blocks) defer n.Close() - api := NewConsensusAPI(ethservice, 0) + api := NewConsensusAPI(ethservice) fcState := beacon.ForkchoiceStateV1{ HeadBlockHash: blocks[5].Hash(), SafeBlockHash: common.Hash{}, @@ -146,7 +146,7 @@ func TestEth2PrepareAndGetPayload(t *testing.T) { n, ethservice := startEthService(t, genesis, blocks[:9]) defer n.Close() - api := NewConsensusAPI(ethservice, 0) + api := NewConsensusAPI(ethservice) // Put the 10th block's tx in the pool and produce a new block ethservice.TxPool().AddLocals(blocks[9].Transactions()) @@ -203,7 +203,7 @@ func TestInvalidPayloadTimestamp(t *testing.T) { n, ethservice := startEthService(t, genesis, preMergeBlocks) defer n.Close() var ( - api = NewConsensusAPI(ethservice, 0) + api = NewConsensusAPI(ethservice) parent = ethservice.BlockChain().CurrentBlock() ) tests := []struct { @@ -248,7 +248,7 @@ func TestEth2NewBlock(t *testing.T) { defer n.Close() var ( - api = NewConsensusAPI(ethservice, 0) + api = NewConsensusAPI(ethservice) parent = preMergeBlocks[len(preMergeBlocks)-1] // This EVM code generates a log when the contract is created. @@ -439,7 +439,7 @@ func TestFullAPI(t *testing.T) { } func setupBlocks(t *testing.T, ethservice *eth.Ethereum, n int, parent *types.Block, callback func(parent *types.Block)) { - api := NewConsensusAPI(ethservice, 0) + api := NewConsensusAPI(ethservice) for i := 0; i < n; i++ { callback(parent) @@ -474,10 +474,9 @@ func TestExchangeTransitionConfig(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(10) n, ethservice := startEthService(t, genesis, preMergeBlocks) defer n.Close() - var ( - api = NewConsensusAPI(ethservice, 0) - ) + // invalid ttd + api := NewConsensusAPI(ethservice) config := beacon.TransitionConfigurationV1{ TerminalTotalDifficulty: (*hexutil.Big)(big.NewInt(0)), TerminalBlockHash: common.Hash{}, @@ -538,7 +537,7 @@ func TestNewPayloadOnInvalidChain(t *testing.T) { defer n.Close() var ( - api = NewConsensusAPI(ethservice, 0) + api = NewConsensusAPI(ethservice) parent = ethservice.BlockChain().CurrentBlock() // This EVM code generates a log when the contract is created. logCode = common.Hex2Bytes("60606040525b7f24ec1d3ff24c2f6ff210738839dbc339cd45a5294d85c79361016243157aae7b60405180905060405180910390a15b600a8060416000396000f360606040526008565b00") @@ -599,7 +598,7 @@ func TestNewPayloadOnInvalidChain(t *testing.T) { } func assembleBlock(api *ConsensusAPI, parentHash common.Hash, params *beacon.PayloadAttributesV1) (*beacon.ExecutableDataV1, error) { - block, err := api.eth.Miner().GetSealingBlockSync(parentHash, params.Timestamp, params.SuggestedFeeRecipient, params.Random, false, 0) + block, err := api.eth.Miner().GetSealingBlockSync(parentHash, params.Timestamp, params.SuggestedFeeRecipient, params.Random, false) if err != nil { return nil, err } @@ -612,7 +611,7 @@ func TestEmptyBlocks(t *testing.T) { defer n.Close() commonAncestor := ethservice.BlockChain().CurrentBlock() - api := NewConsensusAPI(ethservice, 0) + api := NewConsensusAPI(ethservice) // Setup 10 blocks on the canonical chain setupBlocks(t, ethservice, 10, commonAncestor, func(parent *types.Block) {}) @@ -732,8 +731,8 @@ func TestTrickRemoteBlockCache(t *testing.T) { } nodeA.Server().AddPeer(nodeB.Server().Self()) nodeB.Server().AddPeer(nodeA.Server().Self()) - apiA := NewConsensusAPI(ethserviceA, 0) - apiB := NewConsensusAPI(ethserviceB, 0) + apiA := NewConsensusAPI(ethserviceA) + apiB := NewConsensusAPI(ethserviceB) commonAncestor := ethserviceA.BlockChain().CurrentBlock() @@ -791,7 +790,7 @@ func TestInvalidBloom(t *testing.T) { defer n.Close() commonAncestor := ethservice.BlockChain().CurrentBlock() - api := NewConsensusAPI(ethservice, 0) + api := NewConsensusAPI(ethservice) // Setup 10 blocks on the canonical chain setupBlocks(t, ethservice, 10, commonAncestor, func(parent *types.Block) {}) @@ -818,7 +817,7 @@ func TestNewPayloadOnInvalidTerminalBlock(t *testing.T) { defer n.Close() var ( - api = NewConsensusAPI(ethservice, 0) + api = NewConsensusAPI(ethservice) parent = preMergeBlocks[len(preMergeBlocks)-1] ) @@ -842,7 +841,7 @@ func TestNewPayloadOnInvalidTerminalBlock(t *testing.T) { Random: crypto.Keccak256Hash([]byte{byte(1)}), SuggestedFeeRecipient: parent.Coinbase(), } - empty, err := api.eth.Miner().GetSealingBlockSync(parent.Hash(), params.Timestamp, params.SuggestedFeeRecipient, params.Random, true, 0) + empty, err := api.eth.Miner().GetSealingBlockSync(parent.Hash(), params.Timestamp, params.SuggestedFeeRecipient, params.Random, true) if err != nil { t.Fatalf("error preparing payload, err=%v", err) } diff --git a/eth/ethconfig/config.go b/eth/ethconfig/config.go index 8065601c7cea..b5a7837ffda3 100644 --- a/eth/ethconfig/config.go +++ b/eth/ethconfig/config.go @@ -84,17 +84,12 @@ var Defaults = Config{ TrieTimeout: 60 * time.Minute, SnapshotCache: 102, FilterLogCacheSize: 32, - Miner: miner.Config{ - GasCeil: 30000000, - GasPrice: big.NewInt(params.GWei), - Recommit: 3 * time.Second, - NewPayloadTimeout: 2 * time.Second, - }, - TxPool: core.DefaultTxPoolConfig, - RPCGasCap: 50000000, - RPCEVMTimeout: 5 * time.Second, - GPO: FullNodeGPO, - RPCTxFeeCap: 1, // 1 ether + Miner: miner.DefaultConfig, + TxPool: core.DefaultTxPoolConfig, + RPCGasCap: 50000000, + RPCEVMTimeout: 5 * time.Second, + GPO: FullNodeGPO, + RPCTxFeeCap: 1, // 1 ether } func init() { diff --git a/miner/miner.go b/miner/miner.go index 8c8643bcd71a..0f644b200bcf 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -57,6 +57,14 @@ type Config struct { NewPayloadTimeout time.Duration // The maximum time allowance for creating a new payload } +// DefaultConfig contains default settings for miner. +var DefaultConfig = Config{ + GasCeil: 30000000, + GasPrice: big.NewInt(params.GWei), + Recommit: 3 * time.Second, + NewPayloadTimeout: 2 * time.Second, +} + // Miner creates blocks and searches for proof-of-work values. type Miner struct { mux *event.TypeMux @@ -247,8 +255,8 @@ func (miner *Miner) SubscribePendingLogs(ch chan<- []*types.Log) event.Subscript // there is always a result that will be returned through the result channel. // The difference is that if the execution fails, the returned result is nil // and the concrete error is dropped silently. -func (miner *Miner) GetSealingBlockAsync(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool, timeout time.Duration) (chan *types.Block, error) { - resCh, _, err := miner.worker.getSealingBlock(parent, timestamp, coinbase, random, noTxs, timeout) +func (miner *Miner) GetSealingBlockAsync(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool) (chan *types.Block, error) { + resCh, _, err := miner.worker.getSealingBlock(parent, timestamp, coinbase, random, noTxs) if err != nil { return nil, err } @@ -258,8 +266,8 @@ func (miner *Miner) GetSealingBlockAsync(parent common.Hash, timestamp uint64, c // GetSealingBlockSync creates a sealing block according to the given parameters. // If the generation is failed or the underlying work is already closed, an error // will be returned. -func (miner *Miner) GetSealingBlockSync(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool, timeout time.Duration) (*types.Block, error) { - resCh, errCh, err := miner.worker.getSealingBlock(parent, timestamp, coinbase, random, noTxs, timeout) +func (miner *Miner) GetSealingBlockSync(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool) (*types.Block, error) { + resCh, errCh, err := miner.worker.getSealingBlock(parent, timestamp, coinbase, random, noTxs) if err != nil { return nil, err } diff --git a/miner/worker.go b/miner/worker.go index 1c1e3041842d..bf9434eefe70 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -243,6 +243,13 @@ type worker struct { // non-stop and no real transaction will be included. noempty uint32 + // newpayloadTimeout is the maximum timeout allowance for creating payload. + // The default value is 2 seconds but node operator can set it to arbitrary + // large value. A large timeout allowance may cause Geth to fail creating + // a non-empty payload within the specified time and eventually miss the slot + // in case there are some computation expensive transactions in txpool. + newpayloadTimeout time.Duration + // External functions isLocalBlock func(header *types.Header) bool // Function used to determine whether the specified block is mined by local miner. @@ -290,6 +297,16 @@ func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus log.Warn("Sanitizing miner recommit interval", "provided", recommit, "updated", minRecommitInterval) recommit = minRecommitInterval } + // Sanitize the timeout config for creating payload. + newpayloadTimeout := worker.config.NewPayloadTimeout + if newpayloadTimeout == 0 { + log.Warn("Sanitizing new payload timeout to default", "provided", newpayloadTimeout, "updated", DefaultConfig.NewPayloadTimeout) + newpayloadTimeout = DefaultConfig.NewPayloadTimeout + } + if newpayloadTimeout < time.Millisecond*100 { + log.Warn("Low payload timeout may cause high amount of non-full blocks", "provided", newpayloadTimeout, "default", DefaultConfig.NewPayloadTimeout) + } + worker.newpayloadTimeout = newpayloadTimeout worker.wg.Add(4) go worker.mainLoop() @@ -940,7 +957,6 @@ type generateParams struct { noUncle bool // Flag whether the uncle block inclusion is allowed noExtra bool // Flag whether the extra field assignment is allowed noTxs bool // Flag whether an empty block without any transaction is expected - timeout time.Duration // The time allowance for building block, 0 means no limit } // prepareWork constructs the sealing task according to the given parameters, @@ -1063,17 +1079,15 @@ func (w *worker) generateWork(params *generateParams) (*types.Block, error) { defer work.discard() if !params.noTxs { - var interrupt *int32 - if params.timeout != 0 { - interrupt = new(int32) - timer := time.AfterFunc(params.timeout, func() { - atomic.StoreInt32(interrupt, commitInterruptTimeout) - }) - defer timer.Stop() - } + interrupt := new(int32) + timer := time.AfterFunc(w.newpayloadTimeout, func() { + atomic.StoreInt32(interrupt, commitInterruptTimeout) + }) + defer timer.Stop() + err := w.fillTransactions(interrupt, work) if errors.Is(err, errBlockInterruptedByTimeout) { - log.Warn("Block building is interrupted", "allowance", common.PrettyDuration(params.timeout)) + log.Warn("Block building is interrupted", "allowance", common.PrettyDuration(w.newpayloadTimeout)) } } return w.engine.FinalizeAndAssemble(w.chain, work.header, work.state, work.txs, work.unclelist(), work.receipts) @@ -1185,7 +1199,7 @@ func (w *worker) commit(env *environment, interval func(), update bool, start ti // getSealingBlock generates the sealing block based on the given parameters. // The generation result will be passed back via the given channel no matter // the generation itself succeeds or not. -func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool, timeout time.Duration) (chan *types.Block, chan error, error) { +func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool) (chan *types.Block, chan error, error) { var ( resCh = make(chan *types.Block, 1) errCh = make(chan error, 1) @@ -1200,7 +1214,6 @@ func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase noUncle: true, noExtra: true, noTxs: noTxs, - timeout: timeout, }, result: resCh, err: errCh, From 0fe45c278b8e79fba7d187d221f62657ab5f24f2 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Wed, 31 Aug 2022 16:40:21 +0800 Subject: [PATCH 4/4] eth, miner: minor fixes --- eth/catalyst/api.go | 3 +-- eth/catalyst/api_test.go | 2 -- miner/worker_test.go | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 92a154a4f668..754d8b214ca7 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -287,8 +287,7 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa return valid(nil), beacon.InvalidPayloadAttributes.With(err) } // Send a request to generate a full block in the background. - // The result can be obtained via the returned channel. The - // timeout for building block is set to prevent some huge blocks. + // The result can be obtained via the returned channel. resCh, err := api.eth.Miner().GetSealingBlockAsync(update.HeadBlockHash, payloadAttributes.Timestamp, payloadAttributes.SuggestedFeeRecipient, payloadAttributes.Random, false) if err != nil { log.Error("Failed to create async sealing payload", "err", err) diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 023e92ddef22..66da0bc18c3c 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -809,10 +809,8 @@ func TestInvalidBloom(t *testing.T) { func TestNewPayloadOnInvalidTerminalBlock(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(100) - //fmt.Println(genesis.Config.TerminalTotalDifficulty) genesis.Config.TerminalTotalDifficulty = preMergeBlocks[0].Difficulty() //.Sub(genesis.Config.TerminalTotalDifficulty, preMergeBlocks[len(preMergeBlocks)-1].Difficulty()) - //fmt.Println(genesis.Config.TerminalTotalDifficulty) n, ethservice := startEthService(t, genesis, preMergeBlocks) defer n.Close() diff --git a/miner/worker_test.go b/miner/worker_test.go index b903e8db2102..0cba7ff9955f 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -633,7 +633,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co // This API should work even when the automatic sealing is not enabled for _, c := range cases { - resChan, errChan, _ := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, false, 0) + resChan, errChan, _ := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, false) block := <-resChan err := <-errChan if c.expectErr { @@ -651,7 +651,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co // This API should work even when the automatic sealing is enabled w.start() for _, c := range cases { - resChan, errChan, _ := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, false, 0) + resChan, errChan, _ := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, false) block := <-resChan err := <-errChan if c.expectErr {