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

chore: Add more description for the kzg verify algorithm #3703

Merged
merged 2 commits into from
Apr 22, 2024

Conversation

kevaundray
Copy link
Contributor

This fleshes out the description for the KZG verify algorithm being used, similar to how the proving algorithm has been expanded.

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.

Nice work. I'm suggesting some improvements below.

This is done by checking if the following equation holds:
Q(x) Z(x) = f(X) - r(X)
Where:
f(X) is the polynomial that we want to show opens at `k` points to `k` values
Copy link
Contributor

Choose a reason for hiding this comment

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

"we want to show" seems like the wrong PoV here since we are documenting a verification function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to "we want to verify" let me know if that also sounds off

Where:
f(X) is the polynomial that we want to show opens at `k` points to `k` values
Q(X) is the quotient polynomial computed by the prover
r(X) is the degree `k-1` polynomial that agrees with f(x) at all `k` points
Copy link
Contributor

@asn-d6 asn-d6 Apr 22, 2024

Choose a reason for hiding this comment

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

"agrees with f(x)" seems like the wrong PoV here, since the verifier does not know if f(x) actually evaluates to the right values at the right points.

Pehraps "r(x) is the degree k-1 polynomial that evaluates to ys at all zs points"? Or something similar that indicates that r(x) is something that the verifier actually "trusts".

It seems a bit weird to not use the ys and zs inputs anywhere in the doc description.

Finally, what about renaming r(x) to I(x) standing for interpolation polynomial? No strong opinion about this one. I know Dankrad used this notation in his post and we also used it in ours.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will take your suggestion for the description and rename r(X) to I(X) -- I think r(X) describes what it is, while I(X) describes how it was made (by interpolating the values). In this case, to avoid confusion from folks reading multiple documents, using I(X) is the sound approach

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!

@asn-d6 asn-d6 merged commit 0ffd0ca into ethereum:dev Apr 22, 2024
28 checks passed
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.

2 participants