-
Notifications
You must be signed in to change notification settings - Fork 998
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
Add block_to_light_client_header
helper
#3149
Conversation
Introduce `block_to_light_client_header` helper function to enable future forks to override it with additional info (e.g., execution), without having to change the general light client logic. Likewise, update existing light client data creation flow to use `block_to_light_client_header` and default-initialize empty fields. Furthermore, generalize `create_update` helper to streamline test code using `block_to_light_client_header`. Note: In Altair spec, LC header is the same as `BeaconBlockHeader`. however; future forks will extend it with more information.
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.
nice work! 👍
return LightClientUpdate( | ||
attested_header=attested_header, | ||
next_sync_committee=next_sync_committee, | ||
next_sync_committee_branch=next_sync_committee_branch, | ||
finalized_header=finalized_header, | ||
finality_branch=finality_branch, | ||
sync_aggregate=block.message.body.sync_aggregate, | ||
signature_slot=block.message.slot, | ||
) |
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.
I feel the old return LightClientUpdate(...)
is more mistake-proofing, i.e., less likely to miss one field. However, I like how your refactoring reduced the variables. 👍
else: | ||
finalized_header = BeaconBlockHeader() | ||
finality_branch = [Bytes32() for _ in range(floorlog2(FINALIZED_ROOT_INDEX))] |
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.
Note to reviewers: removed because they are the default value of update.finalized_header
and update.finality_branch
.
Anything still blocking on this one? |
Introduce
block_to_light_client_header
helper function to enable future forks to override it with additional info (e.g., execution), without having to change the general light client logic.Likewise, update existing light client data creation flow to use
block_to_light_client_header
and default-initialize empty fields.Furthermore, generalize
create_update
helper to streamline test code usingblock_to_light_client_header
.Note: In Altair spec, LC header is the same as
BeaconBlockHeader
. however; future forks will extend it with more information.