From c63c4ec3474f0b5153def324987e68849892b945 Mon Sep 17 00:00:00 2001 From: setunapo Date: Fri, 18 Nov 2022 22:55:17 +0800 Subject: [PATCH] worker: always drop on new block imported. When new block is imported, there is no need to commit the current work, even the new imported block is offturn and itself is inturn. That is because when offturn block is received, the inturn block is already later to broadcast block, deliver the later block will cause many reorg, which is not reasonable. And also make sure all useless work can be discarded, to avoid goroutine leak. --- consensus/beacon/consensus.go | 4 ---- consensus/clique/clique.go | 5 ---- consensus/consensus.go | 6 ----- consensus/ethash/consensus.go | 4 ---- consensus/parlia/parlia.go | 5 ---- miner/worker.go | 44 +++++++++++++++++++++-------------- 6 files changed, 27 insertions(+), 41 deletions(-) diff --git a/consensus/beacon/consensus.go b/consensus/beacon/consensus.go index 4f4c272a0a..8282ed7cb4 100644 --- a/consensus/beacon/consensus.go +++ b/consensus/beacon/consensus.go @@ -363,10 +363,6 @@ func (beacon *Beacon) SetThreads(threads int) { } } -func (p *Beacon) DropOnNewBlock(*types.Header) bool { - return true -} - // IsTTDReached checks if the TotalTerminalDifficulty has been surpassed on the `parentHash` block. // It depends on the parentHash already being stored in the database. // If the parentHash is not stored in the database a UnknownAncestor error is returned. diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index 11287e74ef..a258f1fe5f 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -705,11 +705,6 @@ func (c *Clique) APIs(chain consensus.ChainHeaderReader) []rpc.API { }} } -func (p *Clique) DropOnNewBlock(header *types.Header) bool { - // drop the block if it is not in turn. - return header.Difficulty.Cmp(diffNoTurn) == 0 -} - // SealHash returns the hash of a block prior to it being sealed. func SealHash(header *types.Header) (hash common.Hash) { hasher := sha3.NewLegacyKeccak256() diff --git a/consensus/consensus.go b/consensus/consensus.go index 367a703678..87632a9d0d 100644 --- a/consensus/consensus.go +++ b/consensus/consensus.go @@ -130,12 +130,6 @@ type Engine interface { // Close terminates any background threads maintained by the consensus engine. Close() error - - // DropOnNewBlock determine the action of mining when it is interrupted by new imported block. - // Return - // true: the mining result will be dropped - // false: the mining result will be kept and move on to the next mine step. - DropOnNewBlock(header *types.Header) bool } // PoW is a consensus engine based on proof-of-work. diff --git a/consensus/ethash/consensus.go b/consensus/ethash/consensus.go index 0bf77a8ae0..12a69c127a 100644 --- a/consensus/ethash/consensus.go +++ b/consensus/ethash/consensus.go @@ -647,10 +647,6 @@ var ( big32 = big.NewInt(32) ) -func (p *Ethash) DropOnNewBlock(*types.Header) bool { - return true -} - // AccumulateRewards credits the coinbase of the given block with the mining // reward. The total reward consists of the static block reward and rewards for // included uncles. The coinbase of each uncle block is also rewarded. diff --git a/consensus/parlia/parlia.go b/consensus/parlia/parlia.go index c16b36ac12..2e544803ef 100644 --- a/consensus/parlia/parlia.go +++ b/consensus/parlia/parlia.go @@ -976,11 +976,6 @@ func (p *Parlia) CalcDifficulty(chain consensus.ChainHeaderReader, time uint64, return CalcDifficulty(snap, p.val) } -func (p *Parlia) DropOnNewBlock(header *types.Header) bool { - // drop the block if it is not in turn. - return header.Difficulty.Cmp(diffNoTurn) == 0 -} - // CalcDifficulty is the difficulty adjustment algorithm. It returns the difficulty // that a new block should have based on the previous blocks in the chain and the // current signer. diff --git a/miner/worker.go b/miner/worker.go index d6593d7384..11c65199d6 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -1084,8 +1084,6 @@ func (w *worker) commitWork(interruptCh chan int32, timestamp int64) { // validator can try several times to get the most profitable block, // as long as the timestamp is not reached. - var bestWork *environment - bestReward := new(big.Int) param := generateParams{ timestamp: uint64(timestamp), coinbase: coinbase, @@ -1095,6 +1093,8 @@ func (w *worker) commitWork(interruptCh chan int32, timestamp int64) { if err != nil { return } + var bestWork *environment + bestReward := new(big.Int) LOOP: for { // StateDB can not be shared, create for each time. @@ -1102,9 +1102,6 @@ LOOP: if err != nil { return } - if bestWork == nil { - bestWork = work - } delay := w.engine.Delay(w.chain, work.header, &w.config.DelayLeftOver) if delay == nil { @@ -1112,6 +1109,9 @@ LOOP: stopTimer = nil } else if *delay <= 0 { log.Debug("Not enough time for commitWork") + if bestWork == nil { + bestWork = work + } break } else { log.Debug("commitWork stopTimer", "block", work.header.Number, @@ -1130,26 +1130,35 @@ LOOP: err = w.fillTransactions(interruptCh, work, stopTimer) fillDuration := time.Since(fillStart) - // To get the most profitable work - curReward := work.state.GetBalance(consensus.SystemAddress) - log.Debug("Get the most profitable work", "curReward", curReward, "bestReward", bestReward) - if curReward.Cmp(bestReward) > 0 { - bestWork.discard() + if bestWork == nil { + // it is the first work, update bestReward bestWork = work - bestReward = curReward + bestReward = work.state.GetBalance(consensus.SystemAddress) + log.Debug("The first work", "bestReward", bestReward) + } else { + // To get the most profitable work + curReward := work.state.GetBalance(consensus.SystemAddress) + log.Debug("The more profitable work", "curReward", curReward, "bestReward", bestReward) + if curReward.Cmp(bestReward) < 0 { + // this work will not be selected, discard it. + work.discard() + } else { + // get a better one, update and discard the previous one + bestWork.discard() + bestWork = work + bestReward = curReward + } } switch { case errors.Is(err, errBlockInterruptedByNewHead): - // For Parlia, it will drop the work on receiving new block if it is not inturn. - if w.engine.DropOnNewBlock(work.header) { - log.Debug("drop the block, when new block is imported") - return - } + log.Debug("commitWork abort", "err", err) + bestWork.discard() + return case errors.Is(err, errBlockInterruptedByTimeout): case errors.Is(err, errBlockInterruptedByOutOfGas): // break the loop to get the best work - log.Debug("commitWork abort", "err", err) + log.Debug("commitWork finish", "reason", err) break LOOP } @@ -1171,6 +1180,7 @@ LOOP: break LOOP case <-interruptCh: log.Debug("commitWork interruptCh closed, new block imported or resubmit triggered") + bestWork.discard() return case ev := <-txsCh: delay := w.engine.Delay(w.chain, work.header, &w.config.DelayLeftOver)