Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

miner, cmd, eth: require explicit etherbase address #26413

Merged
merged 2 commits into from
Jan 20, 2023
Merged

miner, cmd, eth: require explicit etherbase address #26413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d53e38
miner, cmd, eth: require explicit etherbase address
rjl493456442 Jan 4, 2023
4270e24
miner: add test for setEtherbase
rjl493456442 Jan 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/geth/les_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func startLightServer(t *testing.T) *gethrpc {
t.Logf("Importing keys to geth")
runGeth(t, "account", "import", "--datadir", datadir, "--password", "./testdata/password.txt", "--lightkdf", "./testdata/key.prv").WaitExit()
account := "0x02f0d131f1f97aef08aec6e3291b957d9efe7105"
server := startGethWithIpc(t, "lightserver", "--allow-insecure-unlock", "--datadir", datadir, "--password", "./testdata/password.txt", "--unlock", account, "--mine", "--light.serve=100", "--light.maxpeers=1", "--nodiscover", "--nat=extip:127.0.0.1", "--verbosity=4")
server := startGethWithIpc(t, "lightserver", "--allow-insecure-unlock", "--datadir", datadir, "--password", "./testdata/password.txt", "--unlock", account, "--miner.etherbase=0x02f0d131f1f97aef08aec6e3291b957d9efe7105", "--mine", "--light.serve=100", "--light.maxpeers=1", "--nodiscover", "--nat=extip:127.0.0.1", "--verbosity=4")
return server
}

Expand Down
41 changes: 17 additions & 24 deletions cmd/utils/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,7 @@ var (
}
MinerEtherbaseFlag = &cli.StringFlag{
Name: "miner.etherbase",
Usage: "Public address for block mining rewards (default = first account)",
Value: "0",
Usage: "0x prefixed public address for block mining rewards",
Category: flags.MinerCategory,
}
MinerExtraDataFlag = &cli.StringFlag{
Expand Down Expand Up @@ -1359,25 +1358,15 @@ func MakeAddress(ks *keystore.KeyStore, account string) (accounts.Account, error
return accs[index], nil
}

// setEtherbase retrieves the etherbase either from the directly specified
// command line flags or from the keystore if CLI indexed.
func setEtherbase(ctx *cli.Context, ks *keystore.KeyStore, cfg *ethconfig.Config) {
// Extract the current etherbase
var etherbase string
// setEtherbase retrieves the etherbase from the directly specified command line flags.
func setEtherbase(ctx *cli.Context, cfg *ethconfig.Config) {
if ctx.IsSet(MinerEtherbaseFlag.Name) {
etherbase = ctx.String(MinerEtherbaseFlag.Name)
}
// Convert the etherbase into an address and configure it
if etherbase != "" {
if ks != nil {
account, err := MakeAddress(ks, etherbase)
if err != nil {
Fatalf("Invalid miner etherbase: %v", err)
}
cfg.Miner.Etherbase = account.Address
} else {
Fatalf("No etherbase configured")
b, err := hexutil.Decode(ctx.String(MinerEtherbaseFlag.Name))
if err != nil || len(b) != common.AddressLength {
log.Info("Failed to decode etherbase", "err", err)
return
}
cfg.Miner.Etherbase = common.BytesToAddress(b)
}
}

Expand Down Expand Up @@ -1757,11 +1746,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
if ctx.IsSet(LightServeFlag.Name) && ctx.Uint64(TxLookupLimitFlag.Name) != 0 {
log.Warn("LES server cannot serve old transaction status and cannot connect below les/4 protocol version if transaction lookup index is limited")
}
var ks *keystore.KeyStore
if keystores := stack.AccountManager().Backends(keystore.KeyStoreType); len(keystores) > 0 {
ks = keystores[0].(*keystore.KeyStore)
}
setEtherbase(ctx, ks, cfg)
setEtherbase(ctx, cfg)
setGPO(ctx, &cfg.GPO, ctx.String(SyncModeFlag.Name) == "light")
setTxPool(ctx, &cfg.TxPool)
setEthash(ctx, cfg)
Expand Down Expand Up @@ -1945,6 +1930,14 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
// when we're definitely concerned with only one account.
passphrase = list[0]
}
// Unlock the developer account by local keystore.
var ks *keystore.KeyStore
if keystores := stack.AccountManager().Backends(keystore.KeyStoreType); len(keystores) > 0 {
ks = keystores[0].(*keystore.KeyStore)
}
if ks == nil {
Fatalf("Keystore is not available")
}
// setEtherbase has been called above, configuring the miner address from command line flags.
if cfg.Miner.Etherbase != (common.Address{}) {
developer = accounts.Account{Address: cfg.Miner.Etherbase}
Expand Down
14 changes: 1 addition & 13 deletions eth/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,18 +332,6 @@ func (s *Ethereum) Etherbase() (eb common.Address, err error) {
if etherbase != (common.Address{}) {
return etherbase, nil
}
if wallets := s.AccountManager().Wallets(); len(wallets) > 0 {
if accounts := wallets[0].Accounts(); len(accounts) > 0 {
etherbase := accounts[0].Address

s.lock.Lock()
s.etherbase = etherbase
s.lock.Unlock()

log.Info("Etherbase automatically configured", "address", etherbase)
return etherbase, nil
}
}
return common.Address{}, fmt.Errorf("etherbase must be explicitly specified")
}

Expand Down Expand Up @@ -459,7 +447,7 @@ func (s *Ethereum) StartMining(threads int) error {
// introduced to speed sync times.
atomic.StoreUint32(&s.handler.acceptTxs, 1)

go s.miner.Start(eb)
go s.miner.Start()
}
return nil
}
Expand Down
31 changes: 13 additions & 18 deletions miner/miner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type Backend interface {

// Config is the configuration parameters of mining.
type Config struct {
Etherbase common.Address `toml:",omitempty"` // Public address for block mining rewards (default = first account)
Etherbase common.Address `toml:",omitempty"` // Public address for block mining rewards
Notify []string `toml:",omitempty"` // HTTP URL list to be notified of new work packages (only useful in ethash).
NotifyFull bool `toml:",omitempty"` // Notify with pending block headers instead of work packages
ExtraData hexutil.Bytes `toml:",omitempty"` // Block extra data set by the miner
Expand Down Expand Up @@ -73,25 +73,24 @@ var DefaultConfig = Config{

// Miner creates blocks and searches for proof-of-work values.
type Miner struct {
mux *event.TypeMux
worker *worker
coinbase common.Address
eth Backend
engine consensus.Engine
exitCh chan struct{}
startCh chan common.Address
stopCh chan struct{}
mux *event.TypeMux
eth Backend
engine consensus.Engine
exitCh chan struct{}
startCh chan struct{}
stopCh chan struct{}
worker *worker

wg sync.WaitGroup
}

func New(eth Backend, config *Config, chainConfig *params.ChainConfig, mux *event.TypeMux, engine consensus.Engine, isLocalBlock func(header *types.Header) bool) *Miner {
miner := &Miner{
eth: eth,
mux: mux,
eth: eth,
engine: engine,
exitCh: make(chan struct{}),
startCh: make(chan common.Address),
startCh: make(chan struct{}),
stopCh: make(chan struct{}),
worker: newWorker(config, chainConfig, engine, eth, mux, isLocalBlock, true),
}
Expand Down Expand Up @@ -138,20 +137,17 @@ func (miner *Miner) update() {
case downloader.FailedEvent:
canStart = true
if shouldStart {
miner.SetEtherbase(miner.coinbase)
miner.worker.start()
}
case downloader.DoneEvent:
canStart = true
if shouldStart {
miner.SetEtherbase(miner.coinbase)
miner.worker.start()
}
// Stop reacting to downloader events
events.Unsubscribe()
}
case addr := <-miner.startCh:
miner.SetEtherbase(addr)
case <-miner.startCh:
if canStart {
miner.worker.start()
}
Expand All @@ -166,8 +162,8 @@ func (miner *Miner) update() {
}
}

func (miner *Miner) Start(coinbase common.Address) {
miner.startCh <- coinbase
func (miner *Miner) Start() {
miner.startCh <- struct{}{}
}

func (miner *Miner) Stop() {
Expand Down Expand Up @@ -223,7 +219,6 @@ func (miner *Miner) PendingBlockAndReceipts() (*types.Block, types.Receipts) {
}

func (miner *Miner) SetEtherbase(addr common.Address) {
miner.coinbase = addr
miner.worker.setEtherbase(addr)
}

Expand Down
33 changes: 18 additions & 15 deletions miner/miner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ func (bc *testBlockChain) SubscribeChainHeadEvent(ch chan<- core.ChainHeadEvent)
func TestMiner(t *testing.T) {
miner, mux, cleanup := createMiner(t)
defer cleanup(false)
miner.Start(common.HexToAddress("0x12345"))

miner.Start()
waitForMiningState(t, miner, true)
// Start the downloader
mux.Post(downloader.StartEvent{})
Expand Down Expand Up @@ -114,7 +115,8 @@ func TestMiner(t *testing.T) {
func TestMinerDownloaderFirstFails(t *testing.T) {
miner, mux, cleanup := createMiner(t)
defer cleanup(false)
miner.Start(common.HexToAddress("0x12345"))

miner.Start()
waitForMiningState(t, miner, true)
// Start the downloader
mux.Post(downloader.StartEvent{})
Expand Down Expand Up @@ -146,7 +148,8 @@ func TestMinerDownloaderFirstFails(t *testing.T) {
func TestMinerStartStopAfterDownloaderEvents(t *testing.T) {
miner, mux, cleanup := createMiner(t)
defer cleanup(false)
miner.Start(common.HexToAddress("0x12345"))

miner.Start()
waitForMiningState(t, miner, true)
// Start the downloader
mux.Post(downloader.StartEvent{})
Expand All @@ -159,7 +162,7 @@ func TestMinerStartStopAfterDownloaderEvents(t *testing.T) {
miner.Stop()
waitForMiningState(t, miner, false)

miner.Start(common.HexToAddress("0x678910"))
miner.Start()
waitForMiningState(t, miner, true)

miner.Stop()
Expand All @@ -170,21 +173,21 @@ func TestStartWhileDownload(t *testing.T) {
miner, mux, cleanup := createMiner(t)
defer cleanup(false)
waitForMiningState(t, miner, false)
miner.Start(common.HexToAddress("0x12345"))
miner.Start()
waitForMiningState(t, miner, true)
// Stop the downloader and wait for the update loop to run
mux.Post(downloader.StartEvent{})
waitForMiningState(t, miner, false)
// Starting the miner after the downloader should not work
miner.Start(common.HexToAddress("0x12345"))
miner.Start()
waitForMiningState(t, miner, false)
}

func TestStartStopMiner(t *testing.T) {
miner, _, cleanup := createMiner(t)
defer cleanup(false)
waitForMiningState(t, miner, false)
miner.Start(common.HexToAddress("0x12345"))
miner.Start()
waitForMiningState(t, miner, true)
miner.Stop()
waitForMiningState(t, miner, false)
Expand All @@ -194,7 +197,7 @@ func TestCloseMiner(t *testing.T) {
miner, _, cleanup := createMiner(t)
defer cleanup(true)
waitForMiningState(t, miner, false)
miner.Start(common.HexToAddress("0x12345"))
miner.Start()
waitForMiningState(t, miner, true)
// Terminate the miner and wait for the update loop to run
miner.Close()
Expand All @@ -206,21 +209,21 @@ func TestCloseMiner(t *testing.T) {
func TestMinerSetEtherbase(t *testing.T) {
miner, mux, cleanup := createMiner(t)
defer cleanup(false)
// Start with a 'bad' mining address
miner.Start(common.HexToAddress("0xdead"))
miner.Start()
waitForMiningState(t, miner, true)
// Start the downloader
mux.Post(downloader.StartEvent{})
waitForMiningState(t, miner, false)
// Now user tries to configure proper mining address
miner.Start(common.HexToAddress("0x1337"))
miner.Start()
// Stop the downloader and wait for the update loop to run
mux.Post(downloader.DoneEvent{})

waitForMiningState(t, miner, true)
// The miner should now be using the good address
if got, exp := miner.coinbase, common.HexToAddress("0x1337"); got != exp {
t.Fatalf("Wrong coinbase, got %x expected %x", got, exp)

coinbase := common.HexToAddress("0xdeedbeef")
miner.SetEtherbase(coinbase)
if addr := miner.worker.etherbase(); addr != coinbase {
t.Fatalf("Unexpected etherbase want %x got %x", coinbase, addr)
}
}

Expand Down
17 changes: 13 additions & 4 deletions miner/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,14 @@ func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus
chainConfig: chainConfig,
engine: engine,
eth: eth,
mux: mux,
chain: eth.BlockChain(),
mux: mux,
isLocalBlock: isLocalBlock,
localUncles: make(map[common.Hash]*types.Block),
remoteUncles: make(map[common.Hash]*types.Block),
unconfirmed: newUnconfirmedBlocks(eth.BlockChain(), sealingLogAtDepth),
coinbase: config.Etherbase,
extra: config.ExtraData,
pendingTasks: make(map[common.Hash]*task),
txsCh: make(chan core.NewTxsEvent, txChanSize),
chainHeadCh: make(chan core.ChainHeadEvent, chainHeadChanSize),
Expand All @@ -290,8 +292,8 @@ func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus
getWorkCh: make(chan *getWorkReq),
taskCh: make(chan *task),
resultCh: make(chan *types.Block, resultQueueSize),
exitCh: make(chan struct{}),
startCh: make(chan struct{}, 1),
exitCh: make(chan struct{}),
resubmitIntervalCh: make(chan time.Duration),
resubmitAdjustCh: make(chan *intervalAdjust, resubmitAdjustChanSize),
}
Expand Down Expand Up @@ -340,6 +342,13 @@ func (w *worker) setEtherbase(addr common.Address) {
w.coinbase = addr
}

// etherbase retrieves the configured etherbase address.
func (w *worker) etherbase() common.Address {
w.mu.RLock()
defer w.mu.RUnlock()
return w.coinbase
}

func (w *worker) setGasCeil(ceil uint64) {
w.mu.Lock()
defer w.mu.Unlock()
Expand Down Expand Up @@ -1114,11 +1123,11 @@ func (w *worker) commitWork(interrupt *int32, noempty bool, timestamp int64) {
// Set the coinbase if the worker is running or it's required
var coinbase common.Address
if w.isRunning() {
if w.coinbase == (common.Address{}) {
coinbase = w.etherbase()
if coinbase == (common.Address{}) {
Comment on lines +1126 to +1127
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But - this still requires an etherbase, right? If the work-package from CL contains an etherbase, this shouldn't be needed, should it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commitWork is only for legacy block generation.
generateWork is for post-merge mode.

log.Error("Refusing to mine without etherbase")
return
}
coinbase = w.coinbase // Use the preset address as the fee recipient
}
work, err := w.prepareWork(&generateParams{
timestamp: uint64(timestamp),
Expand Down