From dcd30bb7e378d8ab0d62a8ec0aa0b94dbf8d725c Mon Sep 17 00:00:00 2001 From: Jakub Pajek Date: Wed, 4 Oct 2023 11:35:54 +0900 Subject: [PATCH] miner, cmd, eth: revert require explicit etherbase address (#26413) / commit 2b44ef5f93cc7479a77890917a29684b56e9167a --- cmd/geth/les_test.go | 4 ++- cmd/utils/flags.go | 76 ++++++++++++++++++++++++++++++++++---------- eth/backend.go | 18 ++++++++++- miner/miner.go | 51 ++++++++++++++++++++++------- miner/miner_test.go | 58 +++++++++++++++++++++++++-------- miner/worker.go | 42 ++++++++++++++++-------- 6 files changed, 191 insertions(+), 58 deletions(-) diff --git a/cmd/geth/les_test.go b/cmd/geth/les_test.go index 607b454eadfb..9233f511ba04 100644 --- a/cmd/geth/les_test.go +++ b/cmd/geth/les_test.go @@ -146,7 +146,9 @@ 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, "--miner.etherbase=0x02f0d131f1f97aef08aec6e3291b957d9efe7105", "--mine", "--light.serve=100", "--light.maxpeers=1", "--nodiscover", "--nat=extip:127.0.0.1", "--verbosity=4") + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //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") + 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") return server } diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 08de71ee831b..c2f407cc3a22 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -21,7 +21,6 @@ import ( "bytes" "context" "crypto/ecdsa" - "encoding/hex" "errors" "fmt" "math" @@ -539,8 +538,12 @@ var ( Category: flags.MinerCategory, } MinerEtherbaseFlag = &cli.StringFlag{ - Name: "miner.etherbase", - Usage: "0x prefixed public address for block mining rewards", + Name: "miner.etherbase", + // MODIFIED by Jakub Pajek BEG (revert require explicit etherbase address) + //Usage: "0x prefixed public address for block mining rewards", + Usage: "Public address for block mining rewards (default = first account)", + Value: "0", + // MODIFIED by Jakub Pajek END (revert require explicit etherbase address) Category: flags.MinerCategory, } MinerExtraDataFlag = &cli.StringFlag{ @@ -1347,6 +1350,8 @@ func MakeAddress(ks *keystore.KeyStore, account string) (accounts.Account, error return accs[index], nil } +// MODIFIED by Jakub Pajek BEG (revert require explicit etherbase address) +/* // setEtherbase retrieves the etherbase from the directly specified command line flags. func setEtherbase(ctx *cli.Context, cfg *ethconfig.Config) { if !ctx.IsSet(MinerEtherbaseFlag.Name) { @@ -1363,6 +1368,31 @@ func setEtherbase(ctx *cli.Context, cfg *ethconfig.Config) { } cfg.Miner.Etherbase = common.BytesToAddress(b) } +*/ + +// 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 + 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") + } + } +} + +// MODIFIED by Jakub Pajek END (revert require explicit etherbase address) // MakePasswordList reads password lines from the file specified by the global --password flag. func MakePasswordList(ctx *cli.Context) []string { @@ -1738,7 +1768,14 @@ 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") } - setEtherbase(ctx, cfg) + // MODIFIED by Jakub Pajek BEG (revert require explicit etherbase address) + //setEtherbase(ctx, cfg) + var ks *keystore.KeyStore + if keystores := stack.AccountManager().Backends(keystore.KeyStoreType); len(keystores) > 0 { + ks = keystores[0].(*keystore.KeyStore) + } + setEtherbase(ctx, ks, cfg) + // MODIFIED by Jakub Pajek END (revert require explicit etherbase address) setGPO(ctx, &cfg.GPO, ctx.String(SyncModeFlag.Name) == "light") setTxPool(ctx, &cfg.TxPool) setEthash(ctx, cfg) @@ -1912,16 +1949,20 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { 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") - } + // COMMENTED by Jakub Pajek (revert require explicit etherbase address) + /* + // 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") + } + + // Figure out the dev account address. + */ - // Figure out the dev account address. // 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} @@ -1933,9 +1974,12 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { Fatalf("Failed to create developer account: %v", err) } } - // Make sure the address is configured as fee recipient, otherwise - // the miner will fail to start. - cfg.Miner.Etherbase = developer.Address + // COMMENTED by Jakub Pajek (revert require explicit etherbase address) + /* + // Make sure the address is configured as fee recipient, otherwise + // the miner will fail to start. + cfg.Miner.Etherbase = developer.Address + */ if err := ks.Unlock(developer, passphrase); err != nil { Fatalf("Failed to unlock developer account: %v", err) diff --git a/eth/backend.go b/eth/backend.go index 6368c0e03c56..6a82b3a3d916 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -329,6 +329,20 @@ func (s *Ethereum) Etherbase() (eb common.Address, err error) { if etherbase != (common.Address{}) { return etherbase, nil } + // ADDED by Jakub Pajek BEG (revert require explicit etherbase address) + 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 + } + } + // ADDED by Jakub Pajek END (revert require explicit etherbase address) return common.Address{}, fmt.Errorf("etherbase must be explicitly specified") } @@ -444,7 +458,9 @@ func (s *Ethereum) StartMining(threads int) error { // introduced to speed sync times. atomic.StoreUint32(&s.handler.acceptTxs, 1) - go s.miner.Start() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //go s.miner.Start() + go s.miner.Start(eb) } return nil } diff --git a/miner/miner.go b/miner/miner.go index c969aec73546..33f7246598ff 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -45,7 +45,9 @@ type Backend interface { // Config is the configuration parameters of mining. type Config struct { - Etherbase common.Address `toml:",omitempty"` // Public address for block mining rewards + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //Etherbase common.Address `toml:",omitempty"` // Public address for block mining rewards + Etherbase common.Address `toml:",omitempty"` // Public address for block mining rewards (default = first account) 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 @@ -73,11 +75,15 @@ var DefaultConfig = Config{ // Miner creates blocks and searches for proof-of-work values. type Miner struct { - mux *event.TypeMux - eth Backend - engine consensus.Engine - exitCh chan struct{} - startCh chan struct{} + mux *event.TypeMux + // ADDED by Jakub Pajek (revert require explicit etherbase address) + coinbase common.Address + eth Backend + engine consensus.Engine + exitCh chan struct{} + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //startCh chan struct{} + startCh chan common.Address stopCh chan struct{} worker *worker @@ -86,11 +92,13 @@ type Miner struct { func New(eth Backend, config *Config, chainConfig *params.ChainConfig, mux *event.TypeMux, engine consensus.Engine, isLocalBlock func(header *types.Header) bool) *Miner { miner := &Miner{ - mux: mux, - eth: eth, - engine: engine, - exitCh: make(chan struct{}), - startCh: make(chan struct{}), + mux: mux, + eth: eth, + engine: engine, + exitCh: make(chan struct{}), + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //startCh: make(chan struct{}), + startCh: make(chan common.Address), stopCh: make(chan struct{}), worker: newWorker(config, chainConfig, engine, eth, mux, isLocalBlock, true), } @@ -137,17 +145,25 @@ func (miner *Miner) update() { case downloader.FailedEvent: canStart = true if shouldStart { + // ADDED by Jakub Pajek (revert require explicit etherbase address) + miner.SetEtherbase(miner.coinbase) miner.worker.start() } case downloader.DoneEvent: canStart = true if shouldStart { + // ADDED by Jakub Pajek (revert require explicit etherbase address) + miner.SetEtherbase(miner.coinbase) miner.worker.start() } // Stop reacting to downloader events events.Unsubscribe() } - case <-miner.startCh: + // MODIFIED by Jakub Pajek BEG (revert require explicit etherbase address) + //case <-miner.startCh: + case addr := <-miner.startCh: + miner.SetEtherbase(addr) + // MODIFIED by Jakub Pajek END (revert require explicit etherbase address) if canStart { miner.worker.start() } @@ -162,9 +178,18 @@ func (miner *Miner) update() { } } +// MODIFIED by Jakub Pajek BEG (revert require explicit etherbase address) +/* func (miner *Miner) Start() { miner.startCh <- struct{}{} } +*/ + +func (miner *Miner) Start(coinbase common.Address) { + miner.startCh <- coinbase +} + +// MODIFIED by Jakub Pajek END (revert require explicit etherbase address) func (miner *Miner) Stop() { miner.stopCh <- struct{}{} @@ -219,6 +244,8 @@ func (miner *Miner) PendingBlockAndReceipts() (*types.Block, types.Receipts) { } func (miner *Miner) SetEtherbase(addr common.Address) { + // ADDED by Jakub Pajek (revert require explicit etherbase address) + miner.coinbase = addr miner.worker.setEtherbase(addr) } diff --git a/miner/miner_test.go b/miner/miner_test.go index 2e7682acd331..ed7bf81ce39d 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -87,7 +87,9 @@ func TestMiner(t *testing.T) { miner, mux, cleanup := createMiner(t) defer cleanup(false) - miner.Start() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //miner.Start() + miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) // Start the downloader mux.Post(downloader.StartEvent{}) @@ -116,7 +118,9 @@ func TestMinerDownloaderFirstFails(t *testing.T) { miner, mux, cleanup := createMiner(t) defer cleanup(false) - miner.Start() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //miner.Start() + miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) // Start the downloader mux.Post(downloader.StartEvent{}) @@ -149,7 +153,9 @@ func TestMinerStartStopAfterDownloaderEvents(t *testing.T) { miner, mux, cleanup := createMiner(t) defer cleanup(false) - miner.Start() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //miner.Start() + miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) // Start the downloader mux.Post(downloader.StartEvent{}) @@ -162,7 +168,9 @@ func TestMinerStartStopAfterDownloaderEvents(t *testing.T) { miner.Stop() waitForMiningState(t, miner, false) - miner.Start() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //miner.Start() + miner.Start(common.HexToAddress("0x678910")) waitForMiningState(t, miner, true) miner.Stop() @@ -173,13 +181,17 @@ func TestStartWhileDownload(t *testing.T) { miner, mux, cleanup := createMiner(t) defer cleanup(false) waitForMiningState(t, miner, false) - miner.Start() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //miner.Start() + miner.Start(common.HexToAddress("0x12345")) 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() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //miner.Start() + miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, false) } @@ -187,7 +199,9 @@ func TestStartStopMiner(t *testing.T) { miner, _, cleanup := createMiner(t) defer cleanup(false) waitForMiningState(t, miner, false) - miner.Start() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //miner.Start() + miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) miner.Stop() waitForMiningState(t, miner, false) @@ -197,7 +211,9 @@ func TestCloseMiner(t *testing.T) { miner, _, cleanup := createMiner(t) defer cleanup(true) waitForMiningState(t, miner, false) - miner.Start() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //miner.Start() + miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) // Terminate the miner and wait for the update loop to run miner.Close() @@ -209,22 +225,36 @@ func TestCloseMiner(t *testing.T) { func TestMinerSetEtherbase(t *testing.T) { miner, mux, cleanup := createMiner(t) defer cleanup(false) - miner.Start() + // MODIFIED by Jakub Pajek BEG (revert require explicit etherbase address) + //miner.Start() + // Start with a 'bad' mining address + miner.Start(common.HexToAddress("0xdead")) + // MODIFIED by Jakub Pajek END (revert require explicit etherbase address) 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() + // MODIFIED by Jakub Pajek (revert require explicit etherbase address) + //miner.Start() + miner.Start(common.HexToAddress("0x1337")) // Stop the downloader and wait for the update loop to run mux.Post(downloader.DoneEvent{}) waitForMiningState(t, miner, true) - coinbase := common.HexToAddress("0xdeedbeef") - miner.SetEtherbase(coinbase) - if addr := miner.worker.etherbase(); addr != coinbase { - t.Fatalf("Unexpected etherbase want %x got %x", coinbase, addr) + // MODIFIED by Jakub Pajek BEG (revert require explicit etherbase address) + /* + coinbase := common.HexToAddress("0xdeedbeef") + miner.SetEtherbase(coinbase) + if addr := miner.worker.etherbase(); addr != coinbase { + t.Fatalf("Unexpected etherbase want %x got %x", coinbase, addr) + } + */ + // 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) } + // MODIFIED by Jakub Pajek END (revert require explicit etherbase address) } // waitForMiningState waits until either diff --git a/miner/worker.go b/miner/worker.go index 49204f71a076..585349015e66 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -272,18 +272,20 @@ type worker struct { func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus.Engine, eth Backend, mux *event.TypeMux, isLocalBlock func(header *types.Header) bool, init bool) *worker { worker := &worker{ - config: config, - chainConfig: chainConfig, - engine: engine, - eth: eth, - 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, + config: config, + chainConfig: chainConfig, + engine: engine, + eth: eth, + 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), + // COMMENTED by Jakub Pajek BEG (revert require explicit etherbase address) + //coinbase: config.Etherbase, + //extra: config.ExtraData, + // COMMENTED by Jakub Pajek END (revert require explicit etherbase address) pendingTasks: make(map[common.Hash]*task), txsCh: make(chan core.NewTxsEvent, txChanSize), chainHeadCh: make(chan core.ChainHeadEvent, chainHeadChanSize), @@ -342,12 +344,15 @@ func (w *worker) setEtherbase(addr common.Address) { w.coinbase = addr } +// COMMENTED by Jakub Pajek (revert require explicit etherbase address) +/* // 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() @@ -1124,11 +1129,20 @@ 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() { - coinbase = w.etherbase() - if coinbase == (common.Address{}) { + // MODIFIED by Jakub Pajek BEG (revert require explicit etherbase address) + /* + coinbase = w.etherbase() + if coinbase == (common.Address{}) { + log.Error("Refusing to mine without etherbase") + return + } + */ + if w.coinbase == (common.Address{}) { log.Error("Refusing to mine without etherbase") return } + coinbase = w.coinbase // Use the preset address as the fee recipient + // MODIFIED by Jakub Pajek END (revert require explicit etherbase address) } work, err := w.prepareWork(&generateParams{ timestamp: uint64(timestamp),