Merge pull request ethereum#46 from rjl493456442/miner-stuff

miner, eth: various fixes

eth/backend.go

@@ -248,7 +248,7 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
 		return nil, err
 	}
 
-	eth.miner = miner.New(eth, &config.Miner, eth.engine)
+	eth.miner = miner.New(eth, config.Miner, eth.engine)
 	eth.miner.SetExtra(makeExtraData(config.Miner.ExtraData))
 
 	eth.APIBackend = &EthAPIBackend{stack.Config().ExtRPCEnabled(), stack.Config().AllowUnprotectedTxs, eth, nil}
@@ -413,7 +413,7 @@ func (s *Ethereum) SetEtherbase(etherbase common.Address) {
 	s.miner.SetEtherbase(etherbase)
 }
 
-func (s *Ethereum) IsMining() bool      { return s.miner.Mining() }
+func (s *Ethereum) IsMining() bool      { return true }
 func (s *Ethereum) Miner() *miner.Miner { return s.miner }
 
 func (s *Ethereum) AccountManager() *accounts.Manager  { return s.accountManager }
@@ -480,7 +480,6 @@ func (s *Ethereum) Stop() error {
 	s.bloomIndexer.Close()
 	close(s.closeBloomHandler)
 	s.txPool.Close()
-	s.miner.Close()
 	s.blockchain.Stop()
 	s.engine.Close()
 

miner/miner.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"math/big"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -48,8 +47,7 @@ type Config struct {
 	ExtraData hexutil.Bytes  `toml:",omitempty"` // Block extra data set by the miner
 	GasCeil   uint64         // Target gas ceiling for mined blocks.
 	GasPrice  *big.Int       // Minimum gas price for mining a transaction
-
-	Recommit time.Duration // The time interval for miner to re-create mining work.
+	Recommit  time.Duration  // The time interval for miner to re-create mining work.
 }
 
 // DefaultConfig contains default settings for miner.
@@ -64,90 +62,75 @@ var DefaultConfig = Config{
 	Recommit: 2 * time.Second,
 }
 
-// Update the pending block at most every second.
-const pendingTimeout = 1 * time.Second
-
 // Miner is the main object which takes care of submitting new work to consensus engine
 // and gathering the sealing result.
 type Miner struct {
-	confMu      sync.RWMutex // The lock used to protect the config
-	config      *Config
-	chainConfig *params.ChainConfig
-	engine      consensus.Engine
-	txpool      *txpool.TxPool
-	chain       *core.BlockChain
-
-	pendingMu    sync.Mutex // The lock used to protect the pending cache
-	pendingCache *newPayloadResult
-	cacheTime    time.Time
-
-	// Feeds
+	confMu          sync.RWMutex // The lock used to protect the config
+	config          *Config
+	chainConfig     *params.ChainConfig
+	engine          consensus.Engine
+	txpool          *txpool.TxPool
+	chain           *core.BlockChain
+	pending         *pending
 	pendingLogsFeed event.Feed
-
-	running atomic.Bool
 }
 
-// New creates a new miner.
-func New(eth Backend, config *Config, engine consensus.Engine) *Miner {
-	if config == nil {
-		config = &DefaultConfig
-	}
-	worker := &Miner{
-		config:      config,
+// New creates a new miner with provided config.
+func New(eth Backend, config Config, engine consensus.Engine) *Miner {
+	return &Miner{
+		config:      &config,
 		chainConfig: eth.BlockChain().Config(),
 		engine:      engine,
 		txpool:      eth.TxPool(),
 		chain:       eth.BlockChain(),
+		pending:     &pending{},
 	}
-	worker.running.Store(true)
-	return worker
 }
 
-func (miner *Miner) Close() {
-	miner.running.Store(false)
+// Pending returns the currently pending block and associated state. The returned
+// values can be nil in case the pending block is not initialized
+func (miner *Miner) Pending() (*types.Block, *state.StateDB) {
+	pending := miner.getPending()
+	if pending == nil {
+		return nil, nil
+	}
+	return pending.block, pending.stateDB.Copy()
 }
 
-func (miner *Miner) Mining() bool {
-	return miner.running.Load()
+// PendingBlockAndReceipts returns the currently pending block and corresponding receipts.
+// The returned values can be nil in case the pending block is not initialized.
+func (miner *Miner) PendingBlockAndReceipts() (*types.Block, types.Receipts) {
+	pending := miner.getPending()
+	if pending == nil {
+		return nil, nil
+	}
+	return pending.block, pending.receipts
 }
 
-// setExtra sets the content used to initialize the block extra field.
+// SetExtra sets the content used to initialize the block extra field.
 func (miner *Miner) SetExtra(extra []byte) error {
 	if uint64(len(extra)) > params.MaximumExtraDataSize {
 		return fmt.Errorf("extra exceeds max length. %d > %v", len(extra), params.MaximumExtraDataSize)
 	}
 	miner.confMu.Lock()
-	defer miner.confMu.Unlock()
 	miner.config.ExtraData = extra
+	miner.confMu.Unlock()
 	return nil
 }
 
-// Pending returns the currently pending block and associated state. The returned
-// values can be nil in case the pending block is not initialized
-func (miner *Miner) Pending() (*types.Block, *state.StateDB) {
-	block := miner.pending()
-	return block.block, block.stateDB
-}
-
-// PendingBlockAndReceipts returns the currently pending block and corresponding receipts.
-// The returned values can be nil in case the pending block is not initialized.
-func (miner *Miner) PendingBlockAndReceipts() (*types.Block, types.Receipts) {
-	block := miner.pending()
-	return block.block, block.receipts
-}
-
+// SetEtherbase sets the address of fee recipient.
 func (miner *Miner) SetEtherbase(addr common.Address) {
 	miner.confMu.Lock()
-	defer miner.confMu.Unlock()
 	miner.config.Etherbase = addr
+	miner.confMu.Unlock()
 }
 
 // SetGasCeil sets the gaslimit to strive for when mining blocks post 1559.
 // For pre-1559 blocks, it sets the ceiling.
 func (miner *Miner) SetGasCeil(ceil uint64) {
 	miner.confMu.Lock()
-	defer miner.confMu.Unlock()
 	miner.config.GasCeil = ceil
+	miner.confMu.Unlock()
 }
 
 // SubscribePendingLogs starts delivering logs from pending transactions
@@ -161,35 +144,30 @@ func (miner *Miner) BuildPayload(args *BuildPayloadArgs) (*Payload, error) {
 	return miner.buildPayload(args)
 }
 
-// pending returns the pending state and corresponding block. The returned
-// values can be nil in case the pending block is not initialized.
-func (miner *Miner) pending() *newPayloadResult {
-	// Read config
+// getPending retrieves the pending block based on the current head block.
+// The result might be nil if pending generation is failed.
+func (miner *Miner) getPending() *newPayloadResult {
 	miner.confMu.RLock()
 	coinbase := miner.config.Etherbase
 	miner.confMu.RUnlock()
-	// Lock pending block
-	miner.pendingMu.Lock()
-	defer miner.pendingMu.Unlock()
-	if time.Since(miner.cacheTime) < pendingTimeout && coinbase == miner.pendingCache.block.Coinbase() {
-		return miner.pendingCache
+
+	header := miner.chain.CurrentHeader()
+	if cached := miner.pending.resolve(header, coinbase); cached != nil {
+		return cached
 	}
-	pending := miner.generateWork(&generateParams{
+	ret := miner.generateWork(&generateParams{
 		timestamp:   uint64(time.Now().Unix()),
 		forceTime:   true,
-		parentHash:  miner.chain.CurrentBlock().Hash(),
+		parentHash:  header.Hash(),
 		coinbase:    coinbase,
 		random:      common.Hash{},
 		withdrawals: nil,
 		beaconRoot:  nil,
 		noTxs:       false,
 	})
-	if pending.err != nil {
-		// force subsequent calls to recreate pending block
-		miner.cacheTime = time.Time{}
-		return &newPayloadResult{}
+	if ret.err != nil {
+		return nil
 	}
-	miner.pendingCache = pending
-	miner.cacheTime = time.Now()
-	return pending
+	miner.pending.update(header, coinbase, ret)
+	return ret
 }

miner/miner_test.go

@@ -168,8 +168,8 @@ func createMiner(t *testing.T) *Miner {
 	pool := legacypool.New(testTxPoolConfig, blockchain)
 	txpool, _ := txpool.New(new(big.Int).SetUint64(testTxPoolConfig.PriceLimit), blockchain, []txpool.SubPool{pool})
 
-	backend := NewMockBackend(bc, txpool)
 	// Create Miner
-	miner := New(backend, &config, engine)
+	backend := NewMockBackend(bc, txpool)
+	miner := New(backend, config, engine)
 	return miner
 }

miner/payload_building.go

@@ -45,7 +45,6 @@ type BuildPayloadArgs struct {
 
 // Id computes an 8-byte identifier by hashing the components of the payload arguments.
 func (args *BuildPayloadArgs) Id() engine.PayloadID {
-	// Hash
 	hasher := sha256.New()
 	hasher.Write(args.Parent[:])
 	binary.Write(hasher, binary.BigEndian, args.Timestamp)
@@ -175,7 +174,7 @@ func (payload *Payload) ResolveFull() *engine.ExecutionPayloadEnvelope {
 }
 
 // buildPayload builds the payload according to the provided parameters.
-func (w *Miner) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
+func (miner *Miner) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
 	// Build the initial version with no transaction included. It should be fast
 	// enough to run. The empty payload can at least make sure there is something
 	// to deliver for not missing slot.
@@ -189,7 +188,7 @@ func (w *Miner) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
 		beaconRoot:  args.BeaconRoot,
 		noTxs:       true,
 	}
-	empty := w.generateWork(emptyParams)
+	empty := miner.generateWork(emptyParams)
 	if empty.err != nil {
 		return nil, empty.err
 	}
@@ -225,11 +224,11 @@ func (w *Miner) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
 			select {
 			case <-timer.C:
 				start := time.Now()
-				r := w.generateWork(fullParams)
+				r := miner.generateWork(fullParams)
 				if r.err == nil {
 					payload.update(r, time.Since(start))
 				}
-				timer.Reset(w.config.Recommit)
+				timer.Reset(miner.config.Recommit)
 			case <-payload.stop:
 				log.Info("Stopping work on payload", "id", payload.id, "reason", "delivery")
 				return

miner/payload_building_test.go

@@ -57,7 +57,7 @@ var (
 	pendingTxs []*types.Transaction
 	newTxs     []*types.Transaction
 
-	testConfig = &Config{
+	testConfig = Config{
 		Recommit: time.Second,
 		GasCeil:  params.GenesisGasLimit,
 	}

miner/pending.go

@@ -0,0 +1,70 @@
+// Copyright 2024 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+package miner
+
+import (
+	"sync"
+	"time"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/types"
+)
+
+// pendingTTL indicates the period of time a generated pending block should
+// exist to serve RPC requests before being discarded if the parent block
+// has not changed yet. The value is chosen to align with the recommit interval.
+const pendingTTL = 2 * time.Second
+
+// pending wraps a pending block with additional metadata.
+type pending struct {
+	created  time.Time
+	parent   *types.Header
+	coinbase common.Address
+	result   *newPayloadResult
+	lock     sync.Mutex
+}
+
+// resolve retrieves the cached pending result if it's available. Nothing will be
+// returned if the parent/coinbase is not matched or the result is already too old.
+//
+// Note, don't modify the returned payload result.
+func (p *pending) resolve(parent *types.Header, coinbase common.Address) *newPayloadResult {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+
+	if p.result == nil || p.parent == nil {
+		return nil
+	}
+	if parent.Hash() != p.parent.Hash() || p.coinbase != coinbase {
+		return nil
+	}
+	if time.Since(p.created) > pendingTTL {
+		return nil
+	}
+	return p.result
+}
+
+// update refreshes the cached pending block with newly created one.
+func (p *pending) update(parent *types.Header, coinbase common.Address, result *newPayloadResult) {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+
+	p.parent = parent
+	p.coinbase = coinbase
+	p.result = result
+	p.created = time.Now()
+}