From 78212c881677ccb05a3216af42b407ab92418e86 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Mon, 9 Aug 2021 11:26:39 +0800 Subject: [PATCH] miner: address comments --- miner/worker.go | 70 ++++++++++++++++++++++++-------------------- miner/worker_test.go | 4 +-- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/miner/worker.go b/miner/worker.go index 5cd351d7da9e1..5120e58571a2a 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -82,30 +82,28 @@ const ( type environment struct { signer types.Signer - state *state.StateDB // apply state changes here - ancestors mapset.Set // ancestor set (used for checking uncle parent validity) - family mapset.Set // family set (used for checking uncle invalidity) - uncleHashes mapset.Set // uncle set - tcount int // tx count in cycle - gasPool *core.GasPool // available gas used to pack transactions + state *state.StateDB // apply state changes here + ancestors mapset.Set // ancestor set (used for checking uncle parent validity) + family mapset.Set // family set (used for checking uncle invalidity) + tcount int // tx count in cycle + gasPool *core.GasPool // available gas used to pack transactions header *types.Header txs []*types.Transaction receipts []*types.Receipt - uncles []*types.Header + uncles map[common.Hash]*types.Header } // copy creates a deep copy of environment. func (env *environment) copy() *environment { cpy := &environment{ - signer: env.signer, - state: env.state.Copy(), - ancestors: env.ancestors.Clone(), - family: env.family.Clone(), - uncleHashes: env.uncleHashes.Clone(), - tcount: env.tcount, - header: types.CopyHeader(env.header), - receipts: copyReceipts(env.receipts), + signer: env.signer, + state: env.state.Copy(), + ancestors: env.ancestors.Clone(), + family: env.family.Clone(), + tcount: env.tcount, + header: types.CopyHeader(env.header), + receipts: copyReceipts(env.receipts), } if env.gasPool != nil { cpy.gasPool = &(*env.gasPool) @@ -114,11 +112,22 @@ func (env *environment) copy() *environment { // to do the expensive deep copy for them. cpy.txs = make([]*types.Transaction, len(env.txs)) copy(cpy.txs, env.txs) - cpy.uncles = make([]*types.Header, len(env.uncles)) - copy(cpy.uncles, env.uncles) + cpy.uncles = make(map[common.Hash]*types.Header) + for hash, uncle := range env.uncles { + cpy.uncles[hash] = uncle + } return cpy } +// unclelist returns the contained uncles as the list format. +func (env *environment) unclelist() []*types.Header { + var uncles []*types.Header + for _, uncle := range env.uncles { + uncles = append(uncles, uncle) + } + return uncles +} + // discard terminates the background prefetcher go-routine. It should // always be called for all created environment instances otherwise // the go-routine leak can happen. @@ -534,7 +543,7 @@ func (w *worker) mainLoop() { // If our sealing block contains less than 2 uncle blocks, // add the new uncle block if valid and regenerate a new // sealing block for higher profit. - if w.isRunning() && w.current != nil && w.current.uncleHashes.Cardinality() < 2 { + if w.isRunning() && w.current != nil && len(w.current.uncles) < 2 { start := time.Now() if err := w.commitUncle(w.current, ev.Block.Header()); err == nil { w.commit(w.current.copy(), nil, true, start) @@ -732,12 +741,12 @@ func (w *worker) makeEnv(parent *types.Block, header *types.Header) (*environmen state.StartPrefetcher("miner") env := &environment{ - signer: types.MakeSigner(w.chainConfig, header.Number), - state: state, - ancestors: mapset.NewSet(), - family: mapset.NewSet(), - uncleHashes: mapset.NewSet(), - header: header, + signer: types.MakeSigner(w.chainConfig, header.Number), + state: state, + ancestors: mapset.NewSet(), + family: mapset.NewSet(), + header: header, + uncles: make(map[common.Hash]*types.Header), } // when 08 is processed ancestors contain 07 (quick block) for _, ancestor := range w.chain.GetBlocksFromHash(parent.Hash(), 7) { @@ -755,7 +764,7 @@ func (w *worker) makeEnv(parent *types.Block, header *types.Header) (*environmen // commitUncle adds the given block to uncle block set, returns error if failed to add. func (w *worker) commitUncle(env *environment, uncle *types.Header) error { hash := uncle.Hash() - if env.uncleHashes.Contains(hash) { + if _, exist := env.uncles[hash]; exist { return errors.New("uncle not unique") } if env.header.ParentHash == uncle.ParentHash { @@ -767,8 +776,7 @@ func (w *worker) commitUncle(env *environment, uncle *types.Header) error { if env.family.Contains(hash) { return errors.New("uncle already included") } - env.uncleHashes.Add(uncle.Hash()) - env.uncles = append(env.uncles, uncle) + env.uncles[hash] = uncle return nil } @@ -781,7 +789,7 @@ func (w *worker) updateSnapshot(env *environment) { w.snapshotBlock = types.NewBlock( env.header, env.txs, - env.uncles, + env.unclelist(), env.receipts, trie.NewStackTrie(nil), ) @@ -993,7 +1001,7 @@ func (w *worker) prepareWork(genParams *generateParams) (*environment, error) { if !genParams.noUncle { commitUncles := func(blocks map[common.Hash]*types.Block) { for hash, uncle := range blocks { - if env.uncleHashes.Cardinality() == 2 { + if len(env.uncles) == 2 { break } if err := w.commitUncle(env, uncle.Header()); err != nil { @@ -1054,7 +1062,7 @@ func (w *worker) generateWork(params *generateParams) (*types.Block, error) { if err := w.fillTransactions(nil, work); err != nil { return nil, err } - return w.engine.FinalizeAndAssemble(w.chain, work.header, work.state, work.txs, work.uncles, work.receipts) + return w.engine.FinalizeAndAssemble(w.chain, work.header, work.state, work.txs, work.unclelist(), work.receipts) } // commitWork generates several new sealing tasks based on the parent block @@ -1094,7 +1102,7 @@ func (w *worker) commit(env *environment, interval func(), update bool, start ti interval() } // Deep copy receipts here to avoid interaction between different tasks. - block, err := w.engine.FinalizeAndAssemble(w.chain, env.header, env.state, env.txs, env.uncles, env.receipts) + block, err := w.engine.FinalizeAndAssemble(w.chain, env.header, env.state, env.txs, env.unclelist(), env.receipts) if err != nil { return err } diff --git a/miner/worker_test.go b/miner/worker_test.go index 00f46faa37c71..c9b2564d1c3d9 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -593,7 +593,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co }, } - // This API should work even the automatic sealing is not enabled + // This API should work even when the automatic sealing is not enabled for _, c := range cases { block, err := w.getSealingBlock(c.parent, timestamp) if c.expectErr { @@ -608,7 +608,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co } } - // This API should work when the automatic sealing is enabled + // This API should work even when the automatic sealing is enabled w.start() for _, c := range cases { block, err := w.getSealingBlock(c.parent, timestamp)