diff --git a/eth/catalyst/queue.go b/eth/catalyst/queue.go index ff8edc1201c4..dcdb77edbb75 100644 --- a/eth/catalyst/queue.go +++ b/eth/catalyst/queue.go @@ -23,6 +23,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/beacon" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/miner" ) // maxTrackedPayloads is the maximum number of prepared payloads the execution @@ -42,7 +43,7 @@ type payload struct { done bool empty *types.Block block *types.Block - result chan *types.Block + result chan *miner.Work } // resolve extracts the generated full block from the given channel if possible @@ -53,15 +54,13 @@ func (req *payload) resolve() *beacon.ExecutableDataV1 { req.lock.Lock() defer req.lock.Unlock() - // Try to resolve the full block first if it's not obtained - // yet. The returned block can be nil if the generation fails. - if !req.done { timeout := time.NewTimer(500 * time.Millisecond) defer timeout.Stop() select { - case req.block = <-req.result: + case res := <-req.result: + req.block = res.Resolve() req.done = true case <-timeout.C: // TODO(rjl49345642, Marius), should we keep this @@ -69,7 +68,6 @@ func (req *payload) resolve() *beacon.ExecutableDataV1 { // default and then fallback to empty directly? } } - if req.block != nil { return beacon.BlockToExecutableData(req.block) } diff --git a/miner/miner.go b/miner/miner.go index 1e9607a76ad9..2d1f97f48823 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -245,7 +245,7 @@ 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) { +func (miner *Miner) GetSealingBlockAsync(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, noTxs bool) (chan *Work, error) { resCh, _, err := miner.worker.getSealingBlock(parent, timestamp, coinbase, random, noTxs) if err != nil { return nil, err @@ -261,5 +261,23 @@ func (miner *Miner) GetSealingBlockSync(parent common.Hash, timestamp uint64, co if err != nil { return nil, err } - return <-resCh, <-errCh + res := <-resCh + return res.Resolve(), <-errCh +} + +type Work struct { + b *types.Block + mu *sync.RWMutex +} + +func (w *Work) Resolve() *types.Block { + w.mu.RLock() + defer w.mu.RUnlock() + return w.b +} + +func (w *Work) set(b *types.Block) { + w.mu.Lock() + defer w.mu.Unlock() + w.b = b } diff --git a/miner/worker.go b/miner/worker.go index 93fb6288bb45..00ecd9613672 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -170,7 +170,8 @@ type newWorkReq struct { // getWorkReq represents a request for getting a new sealing work with provided parameters. type getWorkReq struct { params *generateParams - result chan *types.Block // non-blocking channel + result *Work + resCh chan *Work // non-blocking channel err chan error } @@ -535,12 +536,21 @@ func (w *worker) mainLoop() { case req := <-w.getWorkCh: block, err := w.generateWork(req.params) - if err != nil { + res := &Work{b: block, mu: new(sync.RWMutex)} + if req.result == nil { + req.result = res + req.resCh <- res req.err <- err - req.result <- nil - } else { - req.err <- nil - req.result <- block + } else if err != nil { + // Update the block if no error occurred + req.result.set(block) + } + + if time.Since(time.Unix(int64(req.params.timestamp), 0)) < maxRecommitInterval { + go func() { + time.Sleep(minRecommitInterval) + w.getWorkCh <- req + }() } case ev := <-w.chainSideCh: // Short circuit for duplicate side blocks @@ -1170,10 +1180,10 @@ 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) (chan *Work, chan error, error) { var ( - resCh = make(chan *types.Block, 1) - errCh = make(chan error, 1) + result = make(chan *Work, 1) + errCh = make(chan error, 1) ) req := &getWorkReq{ params: &generateParams{ @@ -1186,12 +1196,13 @@ func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase noExtra: true, noTxs: noTxs, }, - result: resCh, - err: errCh, + resCh: result, + err: errCh, } + select { case w.getWorkCh <- req: - return resCh, errCh, nil + return result, errCh, nil case <-w.exitCh: return nil, nil, errors.New("miner closed") } diff --git a/miner/worker_test.go b/miner/worker_test.go index 2f1939f75981..30d5829461ac 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -644,7 +644,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co if err != nil { t.Errorf("Unexpected error %v", err) } - assertBlock(block, c.expectNumber, c.coinbase, c.random) + assertBlock(block.Resolve(), c.expectNumber, c.coinbase, c.random) } } @@ -662,7 +662,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co if err != nil { t.Errorf("Unexpected error %v", err) } - assertBlock(block, c.expectNumber, c.coinbase, c.random) + assertBlock(block.Resolve(), c.expectNumber, c.coinbase, c.random) } } }