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

Remove kzg point and blob validations in gossip #3217

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

pawanjay176
Copy link
Contributor

The c-kzg library has moved to all bytes interface after ethereum/c-kzg-4844#62 which also removes the bytes_to_g1 and bytes_to_bls_field from the public interface.

My understanding is that the conversion function was removed from the public interface since verify_aggregate_kzg_proof will contain the checks for if the commitments and proofs are valid g1 points and each blob < BLS_MODULUS so we do not need to make a separate check.

@dankrad
Copy link
Contributor

dankrad commented Jan 20, 2023

Yes, this makes sense. One question is if we should address this:

The KZG commitments of the blobs are all correctly encoded compressed BLS G1 points

Without this check, for any blobs out of the data retention window, there will be no check whether they are valid KZG commitments. We could add a function to check this in the BLS library.

As far as I can see this adds no additional security benefits but wanted to raise it.

@ralexstokes
Copy link
Member

Without this check, for any blobs out of the data retention window, there will be no check whether they are valid KZG commitments.

my assumption was that we would not gossip any blobs outside of the availability period, and if so, this would not be a concern as a gossip validation

@dankrad
Copy link
Contributor

dankrad commented Jan 21, 2023

Without this check, for any blobs out of the data retention window, there will be no check whether they are valid KZG commitments.

my assumption was that we would not gossip any blobs outside of the availability period, and if so, this would not be a concern as a gossip validation

Blocks are gossiped, and their KZG commitments will not be verified to be G1 points outside this window. As I mentioned, I do not think this has any security impact so I think it's fine to leave it.

@djrtwo
Copy link
Contributor

djrtwo commented Jan 24, 2023

Blocks are gossiped

blocks aren't gossiped outside of a 32 slot window which is << the pruning window.
so all good

@djrtwo djrtwo merged commit dd5c5af into ethereum:dev Jan 24, 2023
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.

4 participants