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

Add Hyrax multilinear PCS #130

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

Antonio95
Copy link
Contributor

@Antonio95 Antonio95 commented Oct 25, 2023

Description

This PR implements the Hyrax polynomial commitment scheme: a multilinear PCS based on the hardness of the discrete logarithm problem introduced as part of the Hyrax zkSNARK in this 2017 article.

The PCS described therein is interactive. When implementing the Fiat-Shamir transform, this paper was consulted.

Modification note

In the PCS contained in the cited article, the verifier never learns the actual evaluation of the polynomial at the requested point, but is instead convinced that a previously received Pedersen commitment is indeed a commitment to said evaluation - this is what the SNARK proposed therein necessitates. However, the Arkworks framework requies the verifier to actually learn that value, which is why we have added the opening of the commitment at the end of the protocol. This likely does not result in an optimal non-hiding PCS, but we feel it is the most faithful adaptation of the original PCS that can be implemented with the current restrictions.

Future optimisations

Some natural optimisations to the scheme which are not part of the current PR, but would make sensible follow-up work, are the following:

  • Deal with the modification described above: either modify the PCS trait to encompass hiding PCSs (in terms of the actual evaluation, not only the polynomial), or turn this scheme into a non-hiding one by removing unnecessary work (which would probably involve non-trivial theoretical work).
  • Add parallelisation. There is at least one natural place where parallelisation could bring performance gains: in essence, the prover commits to the polynomial by expressing it as an evaluation matrix and Pederson-multi-committing to each row. Each of this commitments can be computed independently from the rest, and therefore, in parallel. It is still to be seen how much of an improvement this would entail, since each Pederson multi-commitment boils down to a multi-exponentiation and this operation is itself parallelised.
  • Due to the homomorphic nature of Pedersen commitments, it is likely some of the following methods can be designed more efficiently than their default implementations: batch_open, batch_check, open_combinations, check_combinations. This is not discussed in the reference article, but the IPA and KZG modules might be a good starting point.
  • On a related note to the previous point, there might be a more efficient way to open several polynomials at a single point (this is the functionality of the open method) than the currently implemented technique, where only the computation of the vectors L and R is shared across polynomials.
  • The cited article proposes an optimisation in the section Reducing the cost of proof-of-dot-prod. It allows for non-square matrices (and hence removes the requirement for the number of variables to be even) and introduces a tradeoff between proof size and verifier time. It is probably worth pursuing.

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@Antonio95 Antonio95 requested a review from a team as a code owner October 25, 2023 08:19
@Antonio95 Antonio95 requested review from Pratyush, mmagician and weikengchen and removed request for a team October 25, 2023 08:19
@mmagician
Copy link
Member

This PR relies on arkworks-rs/algebra#691, so we temporarily expect CI to fail until that's merged.

mmagician and others added 2 commits November 13, 2023 11:21
* Enable parallel commitment in hyrax

amend

* make `rand` optional

* remove dead code
* removed evaluation randomness from proof and ignored claimed value in check to make scheme hiding

* fmt

* removed unnecessary usage of argument  in check, added _
@mmagician mmagician changed the title Added the Hyrax multilinear PCS Add Hyrax multilinear PCS Jan 15, 2024
@Pratyush
Copy link
Member

Sorry for the late update on this, but happy to merge this as-is, once it's updated wrt master.

autquis and others added 2 commits January 18, 2024 12:04
* Add the trait bounds

* Add `CommitmentState`

* Update benches for the new type

* Fix the name of local variable

* Merge `PCCommitmentState` with `PCRandomness`

* Update `README.md`

* Fix a bug

* Change `Randomness` to `CommitmentState`

* Maybe `empty` not return `Self`

* Make `empty` return `Self`

* Rename `rand` to `state`

* Partially integrate the new design into Hyrax

* Update Hyrax with the shared state

* Rename nonnative to emulated, as in `r1cs-std` (arkworks-rs#137)

* Rename nonnative to emulated, as in `r1cs-std`

* Run `fmt`

* Temporarily change `Cargo.toml`

* Revert `Cargo.toml`

* Refactor `FoldedPolynomialStream` partially

* Substitute `ChallengeGenerator` by the generic sponge (arkworks-rs#139)

* Rename nonnative to emulated, as in `r1cs-std`

* Run `fmt`

* Temporarily change `Cargo.toml`

* Substitute `ChallengeGenerator` with the generic sponge

* Run `fmt`

* Remove the extra file

* Update modules

* Delete the unnecessary loop

* Revert `Cargo.toml`

* Refactor `FoldedPolynomialStream` partially

* Update README

* Make the diff more readable

* Bring the whitespace back

* Make diff more readable, 2

* Fix according to breaking changes in `ark-ec` (arkworks-rs#141)

* Fix for KZG10

* Fix the breaking changes in `ark-ec`

* Remove the extra loop

* Fix the loop range

* re-use the preprocessing table

* also re-use the preprocessing table for multilinear_pc

---------

Co-authored-by: mmagician <[email protected]>

* Auxiliary opening data (arkworks-rs#134)

* Add the trait bounds

* Add `CommitmentState`

* Update benches for the new type

* Fix the name of local variable

* Merge `PCCommitmentState` with `PCRandomness`

* Update `README.md`

* Fix a bug

* Put `Randomness` in `CommitmentState`

* Add a comment

* Remove the extra loop

* Update the comment for `CommitmentState`

Co-authored-by: Marcin <[email protected]>

* cargo fmt

---------

Co-authored-by: Marcin <[email protected]>

* `batch_mul_with_preprocessing` no longer takes `self` as argument (arkworks-rs#142)

* batch_mul_with_preprocessing no longer takes `self` as argument

* Apply suggestions from code review

Co-authored-by: Pratyush Mishra <[email protected]>

* fix variable name

---------

Co-authored-by: Pratyush Mishra <[email protected]>

* Remove ChallengeGenerator for Ligero (#56)

* Squash and merge `delete-chalgen` onto here

* Fix for `ChallengeGenerator`

* Delete `IOPTranscript` for Hyrax (#55)

* Use the sponge generic and rearrange `use`s

* Use sponge instead of `IOPTransript`

* Fix benches

* Remove the extra loop

---------

Co-authored-by: mmagician <[email protected]>
Co-authored-by: Pratyush Mishra <[email protected]>
@autquis
Copy link
Contributor

autquis commented Jan 18, 2024

It is ready for review and to be merged.
@Pratyush If you are merging these three PRs, please do this PR first. Then, I will resolve the conflicts of the other two and let you know when they are ready.

poly-commit/src/utils.rs Outdated Show resolved Hide resolved
poly-commit/src/utils.rs Outdated Show resolved Hide resolved
@autquis
Copy link
Contributor

autquis commented Jan 29, 2024

Ping to all :) I would be happy to resolve any remaining issues.

@Antonio95
Copy link
Contributor Author

I am happy with the current state of things! Let us see if @Pratyush would like any further tweaks regarding the open threads of this discussion.

@autquis
Copy link
Contributor

autquis commented Feb 6, 2024

A gentle reminder @Pratyush :)

@autquis
Copy link
Contributor

autquis commented Mar 11, 2024

Gentle reminder @Pratyush. (It would be much simpler if we go ahead and merge, and then, we can always improve the code.)

@mmagician mmagician added this pull request to the merge queue Mar 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 25, 2024
@mmagician mmagician added this pull request to the merge queue Mar 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 25, 2024
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.

None yet

5 participants