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

[KZG] Serialize roots of unity as hex as well #3357

Closed
wants to merge 3 commits into from
Closed

[KZG] Serialize roots of unity as hex as well #3357

wants to merge 3 commits into from

Conversation

mratsim
Copy link
Contributor

@mratsim mratsim commented May 15, 2023

This serializes the roots_of_unity as hex as well.

Most JSON readers would fail to deserialize a bigint encoded as int and while most cryptographic libraries have a hex->internal_repr converter, many don't have a decimal->internal_repr converter, in particular BLST.

@hwwhww
Copy link
Contributor

hwwhww commented May 15, 2023

bigint encoded

In CL specs, we use SSZ to define ROOTS_OF_UNITY as a Vector[BLSFieldElement, FIELD_ELEMENTS_PER_BLOB] type. Although I support that polynomial-commitments.md should actually be independent of SSZ as it's laid in a cryptography black box, I feel it's more consistent to use SSZ serialization for preset in this case at the current stage.

^^^ However, it may be messier per #3354. 🫨

@hwwhww hwwhww added the Deneb was called: eip-4844 label May 15, 2023
@CarlBeek
Copy link
Contributor

I agree with @mratsim that these should be hex-encoded. On top of that, I'm not convinced these should be a JSON file either. As was discussed at Edelweiss, most crypto libraries won't have a JSON parser, so it seems prudent to have an even flatter (text) file.

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.

Where do we stand here @CarlBeek ?

@hwwhww
Copy link
Contributor

hwwhww commented Oct 19, 2023

closing via #3521

@hwwhww hwwhww closed this Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deneb was called: eip-4844
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants