-
Notifications
You must be signed in to change notification settings - Fork 20.3k
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
cmd/utils: require TTD and difficulty to be zero at genesis for dev mode #29579
Conversation
Alternatively, to get rid of any ambiguity, since it is already the case that if genesis.Config.TerminalTotalDifficulty.Cmp(big.NewInt(0)) != 0 {
Fatalf("Bad developer-mode genesis configuration: terminalTotalDifficulty must be 0 in developer mode")
}
if genesis.Difficulty.Cmp(big.NewInt(0)) != 0 {
Fatalf("Bad developer-mode genesis configuration: difficulty must be 0 in developer mode")
} This seems to be reasonable and probably the best UX out of the two options. Unless there's a good reason to allow arbitrary TTD and difficulty in dev mode. It seems like the difficulty will have to be reset to |
I added the same validation in #29566 but opted to close it. My thinking is that there is no valid reason for a user to change these. If we go the route of trying to catch every possible misconfiguration that a user could make, we will end up with bloated validation logic that hurts readability. |
Ah, well. It's fairly difficult to discern what's going on currently if those values are set since even though edit: With a custom genesis, I don't believe it's possible to leave these values alone also |
ab25969
to
69fc89d
Compare
0
at genesis for dev mode
69fc89d
to
56bf23e
Compare
I would retain the |
1676e39
to
1477862
Compare
Changed to keep null pointer check 👍🏼 |
- No need to reiterate that this is for developer mode since it's in the error already.
1477862
to
0a0e1fe
Compare
0
at genesis for dev mode`go: upgraded github.com/ethereum/go-ethereum v1.14.0 => v1.14.2` Also update geth genesis difficulty to 0 as per [geth PR](ethereum/go-ethereum#29579). task: none
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.
Thx
Related to #29469 as that doesn't seem resolved if the user sets their own genesis config. I also discovered TTD can be negative and I could trick the correct state by setting
TTD = -1
anddifficulty = 0
since 0 difficulty is what triggers the correct EVM rules. This PR also validates that negative values are not accepted.If genesis config is set by the user,
difficulty
must be > TTD. However, if TTD is0
and we are post-merge, settingdifficulty > 0
will use the wrong EVM fork rules and raise issues like:I'm not sure if this is the best way to resolve it but if the user can set both to
0
at genesis, that makes sense that we'd be in a state where the difficulty is0
and we are at TTD - which seems to be required for correct EVM rules. Perhaps a better message is needed for when TTD is set to0
and difficulty is>0
. As of right now, that would just end up using incorrect EVM rules which might be confusing to the user.I can confirm this works as expected for me. Setting
TTD = 0
anddifficulty = 0
, I get correct EVM rules. I can also no longer setTTD = -1
anddifficulty = 0
to trick the EVM rules to trigger.Thoughts?
edit: I think a better UX may be to only allow
TTD = 0
anddifficulty = 0
for dev mode (see my comment below). If that's the case I can force push the change here.Note: I've updated the code to be this latter option instead of validating any other values for TTD and difficulty.