Skip to content
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/clique: add some missing checks #22836

Merged
merged 1 commit into from
May 7, 2021

Conversation

holiman
Copy link
Contributor

@holiman holiman commented May 7, 2021

This PR adds some checks from ethash header verification that were missing on clique.

@holiman holiman requested a review from karalabe as a code owner May 7, 2021 06:50
Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@karalabe
Copy link
Member

karalabe commented May 7, 2021

These should be handled commonly between all consensus engines. Are you sure it's needed?

@karalabe karalabe added this to the 1.10.4 milestone May 7, 2021
@holiman
Copy link
Contributor Author

holiman commented May 7, 2021

Are you sure it's needed?

This is not strictly needed. The state transition ensures that these values are correct, however, that is technically not correct.

State transition should not (need to) verify header fields, it should simply assume that headers are correct. In fast, snap and light sync mode, headerchains are verified without state transition. So we can't have header-checks that are done in state transition only.

@holiman holiman merged commit 8a070e8 into ethereum:master May 7, 2021
atif-konasl pushed a commit to frozeman/pandora-execution-engine that referenced this pull request Oct 15, 2021
wanwiset25 added a commit to XinFinOrg/XDPoSChain that referenced this pull request Mar 20, 2024
wanwiset25 added a commit to XinFinOrg/XDPoSChain that referenced this pull request Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants