-
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
consensus/ethash, core: implement Metropolis EIP 100 #14733
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 in the context that I think this won't break pre-metro, I think the only thing that can make an impact is the change in chainmakers.
@@ -218,6 +218,7 @@ func makeHeader(config *params.ChainConfig, parent *types.Block, state *state.St | |||
Number: parent.Number(), | |||
Time: new(big.Int).Sub(time, big.NewInt(10)), | |||
Difficulty: parent.Difficulty(), | |||
UncleHash: parent.UncleHash(), |
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.
What is the effect of this change ?
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.
This is only used in our tests when we create artificial chains to test various corner cases. The chain maker is messing around with incomplete blocks/headers to get around certain validity checks (to evoke failures), so here we don't have a "real" parent block to calculate the difficulty of the child's based on, but instead make one up on the fly. The difficulty algo didn't require access to the uncles until now, so we just skipped copying that field over. It's however needed in metro.
This PR polishes the EIP 100 difficulty adjustment algorithm to match the same mechanisms as the Homestead was implemented to keep the code uniform. It also avoids a few memory allocs by reusing big1 and big2, pulling it out of the common package and into ethash. The commit also fixes chain maker to forward the uncle hash when creating a simulated chain (it wasn't needed until now so we just skipped a copy there).
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
Implements ethereum/EIPs#100.