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

feat: zk shplemini #9830

Merged
merged 10 commits into from
Nov 8, 2024
Merged

feat: zk shplemini #9830

merged 10 commits into from
Nov 8, 2024

Conversation

iakovenkos
Copy link
Contributor

@iakovenkos iakovenkos commented Nov 8, 2024

We achieve ZK in Shplemini as follows. Before batching the multilinear evaluation claims obtained as the sumcheck output, the Gemini prover

  • creates a random polynomial M of the circuit size;
  • commits to M using KZG/IPA, sends the commitment to the verifier;
  • evaluates M at the sumcheck challenge, sends the evaluation to the verifier.

The verifier simply adds this new commitment and the appropriate scalar multiplier to the BatchOpeningClaim.

@iakovenkos iakovenkos marked this pull request as ready for review November 8, 2024 14:50
@iakovenkos iakovenkos self-assigned this Nov 8, 2024
Copy link
Contributor

@codygunton codygunton left a comment

Choose a reason for hiding this comment

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

Looks good. Noted one style thing. Also: there is a typo in the PR description. "... the circuit size, sends" and then the next word is "commits"/"evaluates"

@@ -120,7 +120,8 @@ template <typename Curve> class GeminiProver_ {
const std::shared_ptr<CommitmentKey<Curve>>& commitment_key,
const std::shared_ptr<Transcript>& transcript,
RefSpan<Polynomial> concatenated_polynomials = {},
const std::vector<RefVector<Polynomial>>& groups_to_be_concatenated = {});
const std::vector<RefVector<Polynomial>>& groups_to_be_concatenated = {},
bool HasZK = false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to has_zk for style--this case is for classes and concepts.


// To achieve ZK, we mask the batched polynomial by a random polynomial of the same size
if (HasZK) {
batched_unshifted = Polynomial::random(n);
Copy link
Contributor

Choose a reason for hiding this comment

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

Before: initialize poly to zero then start to accumulate f_polynomials[0]:
More efficient: initialize with f_polynomials[0] and start the loop at index 1.

After: initialize poly to zero then start to accumulate f_polynomials[0] OR overwrite with random.
More efficient: conditionally initialize as in

Polynomial batched_unshifted =  has_zk ?  Polynomial::random(n) : f_polynomials[0];
size_t starting_idx = has_zk ? 0 : 1;
...
for (size_t i = starting_idx; i < f_polynomials.size(); i++) {
...

But then you have to similarly define a conditional starting point for the loop. Anyway, you're not making things worse so don't worry about it, just remarking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the suggestion! it breaks some edge case tests in Gemini though, namely when we open shifts without unshifted (quite unrealistic). adding another check (f_polynomials.size()>0) would be somewhat ugly

@iakovenkos iakovenkos enabled auto-merge (squash) November 8, 2024 19:18
Copy link
Contributor

github-actions bot commented Nov 8, 2024

Changes to public function bytecode sizes

Generated at commit: 4c1862ef63404543617a174b3967d41821aa9687, compared to commit: 90696cd0e126d7db3c4ef396ada4bddd3ac0de73

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
FPC::public_dispatch +1,034 ❌ +12.37%
FPC::constructor -96 ✅ -3.05%

Full diff report 👇
Program Bytecode size in bytes (+/-) %
FPC::public_dispatch 9,396 (+1,034) +12.37%
FPC::constructor 3,056 (-96) -3.05%

@iakovenkos iakovenkos merged commit 23ff518 into master Nov 8, 2024
46 checks passed
@iakovenkos iakovenkos deleted the si/zk-shplemini branch November 8, 2024 19:27
TomAFrench added a commit that referenced this pull request Nov 11, 2024
* master: (182 commits)
  feat(avm): mem specific range check (#9828)
  refactor: remove public kernel inner (#9865)
  chore: Revert "chore: Validate RPC inputs" (#9875)
  Revert "fix: deploy l2 contracts fails on 48 validator" (#9871)
  fix: deploy l2 contracts fails on 48 validator (#9860)
  chore: Validate RPC inputs (#9672)
  fix: fixing devcontainers to use the sandbox docker-compose file (#9782)
  fix: Revert changes to ci.yml (#9863)
  chore: Move epoch and slot durations to config (#9861)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  fix: tree heights that last past 3 days (#9760)
  fix(build): l1-contracts .rebuild_patterns did not cover test files (#9862)
  fix: bench prover test (#9856)
  fix: Fix mac build by calling `count` on durations (#9855)
  feat: zk shplemini (#9830)
  feat: domain separate block proposals and attestations (#9842)
  chore: bump runner cache disk size (#9849)
  ...
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.

2 participants