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

zk-token-sdk: Fix incorrect mention of OsRng in docs #33774

Conversation

ripatel-fd
Copy link
Contributor

Problem

The documentation of BatchedGroupedCiphertext2HandlesValidityProof::verify mentions use of OsRng.

This function is used in Zero-Knowledge Token Proof Program instruction processing. Therefore use of OsRng would break consensus.

Summary of Changes

It looks like the problematic comment was copy pasted from somewhere else as is not valid. Please double check that this is actually the case.

Removes the problematic comment.

Fixes #

@mergify mergify bot added community Community contribution need:merge-assist labels Oct 19, 2023
@mergify mergify bot requested a review from a team October 19, 2023 16:20
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks for noticing and fixing! It looks like the comment really belongs with BatchedGroupedCiphertext2HandlesValidityProof::new, since GroupedCiphertext2HandlesValidityProof::new uses OsRng.

Do you mind moving the comment up under new?

@ripatel-fd ripatel-fd force-pushed the ripatel/zero-knowledge-token-sdk-sigma-proof-batched-grouped-ciphertext-2-handles-validity-proof-verify-incorrect-documentation branch from 5cbca50 to 939900f Compare October 19, 2023 22:51
@ripatel-fd
Copy link
Contributor Author

@joncinque Done. I'm glad it was just a bug in the documentation 🎉

Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks a bunch!

@joncinque
Copy link
Contributor

Merging without buildkite CI since there's no functional change

@joncinque joncinque merged commit fb80288 into solana-labs:master Oct 19, 2023
16 checks passed
@ripatel-fd ripatel-fd deleted the ripatel/zero-knowledge-token-sdk-sigma-proof-batched-grouped-ciphertext-2-handles-validity-proof-verify-incorrect-documentation branch October 19, 2023 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution need:merge-assist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants