-
Notifications
You must be signed in to change notification settings - Fork 135
1875 remv miner cli options #1882
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.
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: m4sterbunny <harrie.bickle-ext@consensys.net>
Signed-off-by: m4sterbunny <harrie.bickle-ext@consensys.net>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
macfarla
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added some comments.
| Returns the client coinbase address. The coinbase address is the account to pay mining rewards to. | ||
| To set a coinbase address, start Besu with the `--miner-coinbase` option set to a valid Ethereum account address. You can get the Ethereum account address from a client such as MetaMask or Etherscan. For example: | ||
| :::note |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eth_coinbase RPC has also been removed so can we delete this whole section
| ::: | ||
| Mining required a recipient address which was set using the, now deprecated, `--miner-coinbase`. This is now non-configurable and systems which still maintain `miner_start` auto assign this address. | ||
| ::: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be inclined to remove miner_start and miner_stop RPCs as well since they are likely to be removed from the codebase shortly
| To start a single node for testing with metrics enabled, run the following command: | ||
|
|
||
| <!-- can I remove the --miner-enabled --miner-coinbase flags --> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes please
| ## Non-configurable defaults | ||
|
|
||
| - `miner-enabled` defaults to `true`. | ||
| - `miner-coinbase` defaults to the local node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI Clique mining (block production) is also deprecated, which means you'll be able to sync existing clique networks but not be a validator or start new Clique networks.
I don't think you need to add these lines about the non-configurable defaults but if you want to include it, I'd use words rather than the CLI option names since they have actually been removed.
Clique uses the local node's address for the coinbase, and mining is enabled by default.
Description
Implements no-brainers and adds comments where need to discuss per #1875
Issue(s) fixed
Fixes #1875
Preview