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

Add block_to_light_client_header helper #3149

Merged
merged 2 commits into from
Jan 5, 2023
Merged

Conversation

etan-status
Copy link
Contributor

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.

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.
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

nice work! 👍

specs/altair/light-client/full-node.md Outdated Show resolved Hide resolved
specs/altair/light-client/full-node.md Outdated Show resolved Hide resolved
specs/altair/light-client/full-node.md Show resolved Hide resolved
Comment on lines -133 to -141
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,
)
Copy link
Contributor

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. 👍

Comment on lines -129 to -131
else:
finalized_header = BeaconBlockHeader()
finality_branch = [Bytes32() for _ in range(floorlog2(FINALIZED_ROOT_INDEX))]
Copy link
Contributor

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.

@etan-status
Copy link
Contributor Author

Anything still blocking on this one?

@hwwhww hwwhww merged commit 0777a52 into ethereum:dev Jan 5, 2023
@etan-status etan-status deleted the lc-toheader branch January 5, 2023 09:53
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.

2 participants