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

mask splitting polys for zk #76

Merged
merged 3 commits into from
Jul 1, 2022
Merged

mask splitting polys for zk #76

merged 3 commits into from
Jul 1, 2022

Conversation

alxiong
Copy link
Contributor

@alxiong alxiong commented Jun 30, 2022

Description

Sync with the recent bug fix of Plonk paper on masking the splitting polynomials to ensure successful ZK simulator.

image

The slight modification in our implementation is that our splitting step degree is n+2 instead of n. (since our splitting polynomials are of degree = n+1, see here and here)
@chancharles92 Do you remember why we decide to do this instead of split_lo + split_mid * X^n + ... as in the paper?


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 (main)
  • 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

@alxiong alxiong added the bug label Jun 30, 2022
@alxiong alxiong self-assigned this Jun 30, 2022
@CLAassistant
Copy link

CLAassistant commented Jun 30, 2022

CLA assistant check
All committers have signed the CLA.

@chancharles92
Copy link
Contributor

chancharles92 commented Jun 30, 2022

Possibly this is because we are using TurboPlonk and the total degree of the quotient poly is different. I believe (n+1) is the best parameter to make each polynomial's degree balanced. If we use n, the last splitted polynomial will have much higher degree.

Also note that we've masked witness polynomials and Z polynomials at the beginning (so their degrees are increased by 1 or 2 initially), the paper also does the same thing.

@chancharles92
Copy link
Contributor

@alxiong Do you know why it's insecure even if we've masked both witness polynomials and Z(X)? Also I saw this from Ariel's post:

Since this seems to not be an exploitable issue, and it seems infeasible to use this to extract any user information;

chancharles92
chancharles92 previously approved these changes Jul 1, 2022
Copy link
Contributor

@chancharles92 chancharles92 left a comment

Choose a reason for hiding this comment

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

LGTM

@alxiong alxiong merged commit 706eb8d into main Jul 1, 2022
@alxiong alxiong deleted the alex-mask-split-polys branch July 1, 2022 16:38
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.

4 participants