Skip to content

Conversation

@asn-d6
Copy link
Contributor

@asn-d6 asn-d6 commented Sep 22, 2023

In a similar vein to #3507, this PR is adding some valid test vectors for valid edge-case blobs whose proof or commitment is the point at infinity.

In particular:

  • Proof being the point at infinity is a valid case for when the blob is all zeroes. A blob being all zeroes is basically the zero polynomial -- its commitment will be the point at infinity and the corresponding proof will also be the point at infinity.
  • A related edge case can happen for a blob full of twos (or any other constant number): A blob full of twos would correspond to the polynomial p(x) = 2 and it's corresponding KZG proof would be the point at infinity. It's corresponding commitment would be 2G (not the point at infinity).

As part of this commit, I also edited the verify_kzg_proof_case_incorrect_proof_point_at_infinity() test to loop over all valid field elements for the evaluation point (lmk if you want me to split it to another commit).

Also, I didn't modify the verify_blob_kzg_proof_batch() test vectors because the edge-case will be tested automatically by adding BLOB_ALL_TWOS to VALID_BLOBS and the test verify_blob_kzg_proof_batch_case_*().

/cc @jtraglia

@hwwhww hwwhww force-pushed the testgen_kzg_4844_positive_g1_at_inf branch from 3f354fd to 0052e68 Compare September 25, 2023 09:46
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

I fixed the minor linter errors.

It's great to add BLOB_ALL_TWOS test cases! 👍

@jtraglia
The kzg test vectors with this PR: kzg.tar.gz

Could you help test it against c-kzg before we merge it? 🙏

@hwwhww hwwhww added the testing CI, actions, tests label Sep 25, 2023
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 👍 Can confirm these tests pass in c-kzg. Thanks @asn-d6!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants