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

Union type update #2445

Merged
merged 2 commits into from
May 28, 2021
Merged

Union type update #2445

merged 2 commits into from
May 28, 2021

Conversation

protolambda
Copy link
Collaborator

@protolambda protolambda commented May 25, 2021

  • Clarify missing Union type in SSZ spec
  • Use "selector" wording instead of "type", since there may be multiple options of the same type in rare cases.
  • Reduce selector to 1 byte, and reserve the top 128 values (most significant bit) for future backwards-compatible extensions
  • Fix null reference in python-ish spec. It's referenced as the native-none/nil/null of the implementation anyway.
  • Update serialization and merkleization to cover None case in the Union

Note: this change is assumed to be backwards-compatible, since Union is not used anywhere in the spec. Outside of the spec there is one known case in the DB format of Lighthouse, but the old serialization/deserialization can be kept around there.

TODO:

fixes #2270

@protolambda protolambda added scope:SSZ Simple Serialize Altair aka HF1 labels May 25, 2021
@protolambda
Copy link
Collaborator Author

Tagging this with the Altair label. It's not critical, nor used in consensus, but I like to ship the SSZ update before the old Union type gets more actual uses outside of the spec. And we are looking to use it in the Merge and Sharding specs.

Copy link
Member

@Nashatyrev Nashatyrev left a comment

Choose a reason for hiding this comment

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

LTGM

ssz/simple-serialize.md Show resolved Hide resolved
Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

looks great! now we can define Option types in the spec and i'm sure there will be other uses along the way

left one comment below for clarification

ssz/simple-serialize.md Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Altair aka HF1 scope:SSZ Simple Serialize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSZ Union improvement proposal
4 participants