-
Notifications
You must be signed in to change notification settings - Fork 131
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
feat!: remove the compress_selectors
field from VerifyingKey
#310
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From this PR, my only worry is breaking the API for create_proof
; I wrote an idea that could resolve that.
@ed255 From my end, I believe we should break the API for This is kinda side effect. |
Oh how unfortunate, I had written it as a review comment, but for some reason it was not submitted.
My idea was to break the
This way we don't break the API, and the resulting API looks consistent. |
Wow, thanks for the idea! @ed255 Done 56f10ac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
43e2ceb
to
8b3bdf5
Compare
compress_selectors
field from VerifyingKey
[draft]compress_selectors
field from VerifyingKey
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
Remove the
compress_selectors
field fromVerifyingKey
Related issues
Changes
keygen_pk_custom
utilkeygen_pk
,keygen_pk_custom
,keygen_vk
&keygen_vk_custom
addcompress_selectors
field tohalo2_proofs::plonk::create_proof
funccreate_proof_custom_with_engine
func for both compatibility & new APIContext
While working in PR #304, I found that
compress_selectors
field is essentially not needed inVerifyingKey
.It was there because @ed255 didn't find appropriate solution while doing frontend-backend split work.
But, after frontend-backend split, it is clear that
compress_selectors
is for frontend(compile circuit), andVerifyingKey
is only for backend stuff.Regarding this issue, I had a bit of discussion with @ed255
He gave some good ideas:
compress_selectors
fromVerifyingKey
compress_selectors
param to legacykeygen_pk
(safe, but break API)keygen_pk_custom
with assumption ofkeygen_pk
havingcompress_selectors: true
(compatible, but user could misuse)He recommended to create draft PR for discussing the possible solutions with other members.
This PR serves the purpose of discussing the better solutions regarding the
compress_selectors
inVerifyingKey