[feat] Make selector compression optional #212
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
han0110
approved these changes
Sep 20, 2023
Ah yea I noticed this too when I was reviewing. Let me see if I can fix quickly. I fixed the |
iquerejeta
pushed a commit
to input-output-hk/halo2
that referenced
this pull request
May 8, 2024
…ns#212) * feat: add `keygen_vk_custom` function to turn off selector compression * feat(vkey): add `version` and `compress_selectors` bytes to serialization * feat: do not serialize selectors when `compress_selectors = false` * feat: make `keygen_vk` have `compress_selectors = true` by default * chore: fix clippy * fix: pass empty poly to fake_selectors * fix: byte_length calculation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Selector compression is an optimization, but often it can lead to unexpected behavior / makes life complicated when doing more complicated proof composability like with SNARK recursion or universal verifiers such as in
snark-verifierandhalo2-solidity-verifier. As such, it is useful to be able to optionally turn it off.This PR does the following:
keygen_vk_customfunction where user can choose whether to turn on selector compression(=combination) or not.keygen_vkwill be set so the default is with selector compression, so it matches current behavior.VerifyingKeydoes not need to store anything about selectors. Afterkeygen_vk_custom, theConstraintSystemhas entirely replaced selectors with fixed columns, and fixed commitments have been updated. Theverify_prooffunction does not use selectors in any way. Inkeygen_pkyou still need to know about selectors, but it reconstructs theConstraintSystemfrom scratch anyways.VerifyingKey::readandwritefunctions accordingly, where there is now a new byte recording whether selector compression is on or off. When off, I skip serializing selectors since they are unnecessary for reasons mentioned above.I also did not realize until now how inefficient
keygen_vkandkeygen_pkare: it re-does the FFTs for all fixed columns twice, once inkeygen_vkand again inkeygen_pkbecauseVerifyingKeycan't store the results of the FFT. The fact that you need to re-generate the constraint system to handle selectors is also a bad design pattern in many ways, but for now I opted for the path of least code changes.