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

feat: ssz write schemas for all containers #520

Merged
merged 29 commits into from
Jan 23, 2024

Conversation

f3r10
Copy link
Contributor

@f3r10 f3r10 commented Dec 12, 2023

Motivation

Closes #513

@f3r10 f3r10 requested a review from a team as a code owner December 12, 2023 19:19
@f3r10 f3r10 marked this pull request as draft December 12, 2023 19:19
@f3r10 f3r10 marked this pull request as ready for review December 14, 2023 16:30
Comment on lines 237 to 242
{:validators, {:list, SszTypes.Validator, 1_099_511_627_776}},
{:balances, {:list, {:int, 64}, 1_099_511_627_776}},
{:randao_mixes, {:list, {:bytes, 32}, 65_536}},
{:slashings, {:list, {:int, 64}, 8192}},
{:previous_epoch_participation, {:list, {:int, 8}, 1_099_511_627_776}},
{:current_epoch_participation, {:list, {:int, 8}, 1_099_511_627_776}},
Copy link
Collaborator

@MegaRedHand MegaRedHand Jan 2, 2024

Choose a reason for hiding this comment

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

A lot of these constants are in the configuration, so we should grab them from there. For example, 1_099_511_627_776 is VALIDATOR_REGISTRY_LIMIT.

Copy link
Collaborator

@Arkenan Arkenan left a comment

Choose a reason for hiding this comment

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

Hi, this is not a full review, but I wanted to provide quick feedback and request a way to provide type aliases for less type repetition, and also fix the bytes apparitions, as they are not ints.

lib/types/beacon_chain/fork.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/fork.ex Outdated Show resolved Hide resolved
MegaRedHand
MegaRedHand previously approved these changes Jan 8, 2024
Copy link
Collaborator

@Arkenan Arkenan left a comment

Choose a reason for hiding this comment

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

Thanks for the PR Fernando! I requested some changes.

lib/ssz_types/beacon_chain/beacon_block_body.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/withdrawal.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/deposit.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/beacon_block_body.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/beacon_state.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/execution_payload.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/execution_payload.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/execution_payload_header.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/execution_payload_header.ex Outdated Show resolved Hide resolved
lib/types/type_aliases.ex Show resolved Hide resolved
@f3r10 f3r10 requested a review from Arkenan January 18, 2024 12:00
Copy link
Collaborator

@Arkenan Arkenan left a comment

Choose a reason for hiding this comment

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

There are some mistakes regarding byte lists. Let's also fix the logs_bloom as you said.

test/unit/ssz_ex_test.exs Show resolved Hide resolved
lib/types/type_aliases.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/execution_payload.ex Outdated Show resolved Hide resolved
lib/types/beacon_chain/execution_payload_header.ex Outdated Show resolved Hide resolved
@f3r10 f3r10 requested a review from Arkenan January 23, 2024 12:07
Copy link
Collaborator

@Arkenan Arkenan left a comment

Choose a reason for hiding this comment

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

LGTM

@Arkenan Arkenan merged commit 363420f into lambdaclass:main Jan 23, 2024
11 checks passed
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.

[SSZ] Write schemas for all containers
3 participants