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

Ad/IPA uses Fiat-Shamir #244

Closed
wants to merge 11 commits into from
Closed

Conversation

arijitdutta67
Copy link
Contributor

@arijitdutta67 arijitdutta67 commented Mar 15, 2023

Description

This PR mainly add transcript to ipa so that it can use the implicit hash function to generate Fiat-Shamir challenges. Followings are the main changes made:

  1. We generate a Fiat-Shamir challenge aux_challenge which is computed as aux_challenge = Hash(Commit, challenge, evaluation). This aux_challenge is used to generate aux_generator as aux_generator = srs_element[poly_degree] * aux_challenge.
  2. log(n) number of round challenges are generated as u_j = Hash(L_j, R_j) in the jth round where n is the degree of the polynomial.
  3. These challenges are added to the transcript in the reduce_prove method. These challenges are accessed from the transcript in the reduce_verify method.
  4. We remove G_vec_local from reduce_verify method as it was unnecessary.
  5. We have created a mock_manifest in ipa.test.cpp to test the transcript functionality related only to ipa.
  6. Some cleanup that was due in PR IPA uses srs #205.

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • There are no circuit changes, OR specifications in /markdown/specs have been updated.
  • There are no circuit changes, OR a cryptographer has been assigned for review.
  • I've updated any terraform that needs updating (e.g. environment variables) for deployment.
  • The branch has been rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.
  • New functions, classes, etc. have been documented according to the doxygen comment format. Classes and structs must have @brief describing the intended functionality.
  • If existing code has been modified, such documentation has been added or updated.

@arijitdutta67 arijitdutta67 changed the title Ad/IPA uses Fiat Shamir Ad/IPA uses Fiat-Shamir Mar 17, 2023
@arijitdutta67 arijitdutta67 linked an issue Mar 17, 2023 that may be closed by this pull request
@arijitdutta67 arijitdutta67 marked this pull request as ready for review March 17, 2023 13:06
*
* @return true/false depending on if the proof verifies
*/
static bool reduce_verify(std::shared_ptr<VK> vk, const Proof& proof, const PubInput& pub_input)
static bool reduce_verify(std::shared_ptr<VK> vk,
Copy link
Collaborator

Choose a reason for hiding this comment

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

To follow the idiom established in KZG, we would use a class ipa::VerificationKey to avoid passing the vk as an argument here.. I see you defined this and tested it but that you don't use it. Apologies if I forgot a discussion on this--did you explain why you chose this approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This problem stems from the fact that ipa::VerificationKey class does not take ownership of SRS and pippenger_runtime_state (we have an issue for this here

* TODO(#218)(Adrian): This class should take ownership of the SRS, and handle reading the file from disk.
). These two parameters are needed in ipa::reduce_verify for MSM unlike other PCSs. These parameter are set by calling a constructor in commitment_key.test.cpp here. Under this settings, we access these parameters by passing the vk shared ptr as argument to ipa::reduce_verify in ipa_test.cpp here.
This was a due point in PR #205, and we decided to go with this keeping this as an issue (#236). In the current PR, we somehow solved the issue with reduce_prove by using the transcript. But I could not see a path to resolve it for reduce_verify. Please let me know if this makes sense. I shall record this in the issue #236.

@arijitdutta67 arijitdutta67 mentioned this pull request Mar 21, 2023
11 tasks
@maramihali
Copy link
Contributor

done by #367

@maramihali maramihali closed this May 3, 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.

Use Fiat-Shamir in IPA
4 participants