-
Notifications
You must be signed in to change notification settings - Fork 281
Instantly enable all upgrades on all network types #7
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
Conversation
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.
LGTM
@mask-pp Could you take a look? |
params/config.go
Outdated
@@ -57,21 +57,21 @@ var ( | |||
// MainnetChainConfig is the chain parameters to run a node on the main network. | |||
MainnetChainConfig = &ChainConfig{ |
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.
That constant fields is not necessary to change. We can start geth and allocate those by config file.
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.
We can genesis geth node by genesis.json like this:
{
"commit": "0000000000000000000000000000000000000000",
"config": {
"chainId": 420,
"homesteadBlock": 0,
"eip150Block": 0,
"eip155Block": 0,
"eip158Block": 0,
"byzantiumBlock": 0,
"constantinopleBlock": 0,
"petersburgBlock": 0,
"istanbulBlock": 0,
"muirGlacierBlock": 0,
"clique": {
"period": 0,
"epoch": 30000
}
},
...
}
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.
Correct, though I figured this may be a bit easier since I believe these constant parameters are loaded in if no config.json is provided. So it may help us ensure that we always run with the correct kind of configuration. If you feel it is redundant though, we can simply close the PR.
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.
Try not to change if possible, so that others know the difference clearly, and it's convenient for us to upgrade and maintain.
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.
Agree with @mask-pp . It'd be good practice to avoid unnecessary changes. We can just add a default genesis config that enables all EIPs.
d396756
to
4df9e82
Compare
genesis.json
Outdated
}, | ||
"nonce": "0x0", | ||
"timestamp": "0x61bc0b0a", | ||
"extraData": "0x0000000000000000000000000000000000000000000000000000000000000000cf281c8a32fb8d1a3bb4a64055c85cf20a1ed33e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", |
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 that it's necessary to add 0xcf281c8a32fb8d1a3bb4a64055c85cf20a1ed33e's genesis balance and the address keystore file, so we can use init node directly and use the addr send tx.
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.
Good catch, I will add it!
4d46650
to
7c0bf5e
Compare
Just so it is traceable later, the current keystore password is |
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.
LGTM
This reverts commit 80940fa.
Fixes #4