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

Verify terminal PoW block after call to state_transition #2595

Merged
merged 1 commit into from
Sep 15, 2021

Conversation

mkalinin
Copy link
Collaborator

@mkalinin mkalinin commented Sep 10, 2021

What's done?

Switches the order of terminal PoW block verification and the state_transition call in the on_block handler of the fork choice.

This change has the following implications:

  • During the state_transition call either all dependencies of the execution payload must be resolved (meaning that ancestors of the execution payload must be pulled from the network and processed) or the block processing is delayed
  • After the state_transition call, when the terminal PoW block verification reached all the ancestors of the execution payload must be deemed as valid, otherwise, the state transition function would deem the beacon block as invalid
  • Consensus clients rely on an execution client pulling the data from the wire and processing the ancestors of the execution payload as a part of serving the engine_executePayload method
  • It simplifies the spec and implementations in the following way:
    • there is no need to take into account is_processed and is_valid statuses of the PoW block
    • eth_getBlockByHash JSON-RPC call is sufficiently enough to get the PoW block data, no need to implement extra method
    • no delay in requesting the PoW blocks information is expected other than the delay related to the request/response roundtrip of the JSON-RPC
  • There is a risk of wasting time to process a block (including the execution) that is invalid wrt transition process. The risk in the case if such a block appears has low probability and low impact on the system as this block will likely be either invalidated or not processed in time which both leads to a skipped slot which would also be the case before the introduction of this change

Thanks to @djrtwo and @MicahZoltu for the idea of this change

@mkalinin mkalinin requested a review from djrtwo September 10, 2021 10:41
@protolambda protolambda added the Bellatrix CL+EL Merge label Sep 10, 2021
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

excellent!

@djrtwo djrtwo merged commit c396ee1 into ethereum:dev Sep 15, 2021
Nashatyrev added a commit to Nashatyrev/teku that referenced this pull request Sep 24, 2021
Nashatyrev added a commit to Consensys/teku that referenced this pull request Sep 28, 2021
* Replace consts in all configs with TERMINAL_TOTAL_DIFFICULTY
* Remove TransitionStore and targetSecondsToMerge, minAnchorPowBlockDifficulty constants
* Adopt merge PR https://github.com/ethereum/consensus-specs/pull/2522/files
* Adopt merge PR ethereum/consensus-specs#2595
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bellatrix CL+EL Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants