-
Notifications
You must be signed in to change notification settings - Fork 21.2k
eth/catalyst,cmd/geth: improve devnet experience #31316
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
eth/catalyst,cmd/geth: improve devnet experience #31316
Conversation
cmd/utils/flags.go
Outdated
@@ -1730,20 +1730,21 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { | |||
// setEtherbase has been called above, configuring the miner address from command line flags. | |||
if cfg.Miner.PendingFeeRecipient != (common.Address{}) { | |||
developer = accounts.Account{Address: cfg.Miner.PendingFeeRecipient} | |||
} else if accs := ks.Accounts(); len(accs) > 0 { |
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.
Apart from removing the call to unlock the keystore, the changes in this file seem like a no-op.
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 think the changes to automatically set the fee recipient are okay and should be kept, but the keystore changes are not correct.
Do you mean that you need a dev account even if an address is given? |
Yes, we should always attempt to unlock the first account in the keystore if it exists. Dev mode can be operated non-ephemerally, and when restarted the dev-mode account should be unlocked. Other cases, (e.g. where a user wants to swap to a different keystore, and don't necessarily want the first account unlocked) don't really matter for dev mode IMO. |
updated. |
Just revert all the changes except for setting the fee recipient to what is specified by the miner flag. |
if I don't have any accounts in keystore, the unlock will fail. I have to create a keystore first to start |
My use case is not to use the keystore to start
or I have to create a keystore first.
My PR is to avoid the chores, and lemme manage the keys in my apps. |
edit maybe not. |
yes, the keystore.unlock alwasy gets called, that's the problem. |
this adds 2 features to improve `geth --dev` experience. 1. we don't need to use `dev_SetFeeRecipient` to set initial coinbase address. it was a pain. 2. we don't need to unlock keystore if we don't use it. we had it because of clique.
this adds 2 features to improve `geth --dev` experience. 1. we don't need to use `dev_SetFeeRecipient` to set initial coinbase address. it was a pain. 2. we don't need to unlock keystore if we don't use it. we had it because of clique.
this adds 2 features to improve `geth --dev` experience. 1. we don't need to use `dev_SetFeeRecipient` to set initial coinbase address. it was a pain. 2. we don't need to unlock keystore if we don't use it. we had it because of clique.
This PR fixes a bug in dev mode. When dev mode is started together with the
--miner.pending.feeRecipient
it will use that account as the "developer account". I.e. that address will be prefunded.However this feature was buggy, in that geth would fail to boot up when that flag was present.
Additionally: the same address will be set as the default fee recipient when a block is mined. Previously it was only possible to update fee recipient in dev mode via
dev_setFeeRecipient
.