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

Update the endianness of the polynomial commitments to be big endian #3354

Merged
merged 4 commits into from
May 23, 2023

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented May 12, 2023

Update the endianess of polynomal commitments to big endian in conjuction with

post ACD may 11 call discussion (Ref: https://discord.com/channels/595666850260713488/745077610685661265/1106250714415648849)

Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
@g11tech g11tech changed the title Update the endianess of the polynomial commitments to be big endian Update the endianness of the polynomial commitments to be big endian May 15, 2023
@etan-status
Copy link
Contributor

In CL, represent these as ByteVector - they will retain endianness across layers this way.

CL shouldn't have to see into these commitments, it only has to pass the opaque data to the crypto lib

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

hwwhww commented May 18, 2023

KZG test vectors: general-kzg-4844-20230518.tar.gz

It would be nice if we could verify the KZG update correctness before the spec release. 🙏

/cc @jtraglia @asn-d6 @kevaundray

@jtraglia
Copy link
Member

It would be nice if we could verify the KZG update correctness before the spec release. 🙏

Just to confirm, these test vectors work in c-kzg-4844 after making the required changes.

Copy link
Contributor

@asn-d6 asn-d6 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

Copy link
Member

@jtraglia jtraglia left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kevaundray
Copy link
Contributor

I'm encountering errors on the consensus-specs test in go-kzg-4844 -- Going to check in a minute to see if I made a wrong change

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.

6 participants