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

Bellatrix: pass justified as a safe block #2858

Merged
merged 9 commits into from
Mar 30, 2022

Conversation

mkalinin
Copy link
Collaborator

@mkalinin mkalinin commented Mar 22, 2022

Introduces a get_safe_block_hash function that returns a block hash of a safe execution block. This function may be redefined post-Merge. The current definition uses the most recent justified block as a safe block.

An alternative approach is in having generally defined get_safe_block function (as an extension outside of the main fork-choice spec) utilized by get_safe_block_hash. But it can be easily done later on once get_safe_block will represent more complicated logic than it currently is, or I can be convinced that it should be done within this PR.

TODO

specs/bellatrix/validator.md Outdated Show resolved Hide resolved
specs/bellatrix/validator.md Outdated Show resolved Hide resolved
@hwwhww hwwhww added the Bellatrix CL+EL Merge label Mar 24, 2022
@mkalinin
Copy link
Collaborator Author

I have added a /fork_choice/safe-block.md document with the function definition. I am free to debate on the naming and place of this small piece of the spec. Once this is done it can be included in the spec build and testing

fork_choice/safe-block.md Show resolved Hide resolved
MUST be set to the return value of the following function:

```python
def get_safe_block_hash(store: Store) -> Hash32:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should likely got in the safe_block helper file

@mkalinin
Copy link
Collaborator Author

Applied the following review suggestions:

  • get_safe_beacon_block -> get_safe_beacon_block_root
  • get_safe_block_hash -> get_safe_execution_payload_hash
  • get_safe_execution_payload_hash moved to safe-block.md

@@ -0,0 +1,46 @@
# Fork Choice -- Safe Block
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this file is cross forks. Maybe note that it is only valid post-Bellatrix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In general, safe block algorithm can be implemented with Phase0 structures. I would add a note that get_safe_execution_payload_hash helper is based on Bellatrix

specs/bellatrix/fork-choice.md Outdated Show resolved Hide resolved
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.

looks good to me! opened #2865 to track needing some unit tests here

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