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

Fixes from Common Prefix audit of the Plonk verifier contract #1962

Merged
merged 24 commits into from
Sep 4, 2024

Conversation

philippecamacho
Copy link
Contributor

Closes #1727

This PR:

Implements of the changes requested by Common Prefix.

This PR does not:

Note that the code in this PR still points to a branch of Jellyfish for Plonk. See

jf-plonk = { git = "https://github.com/EspressoSystems/jellyfish", branch = "commonprefix-patch", features = [

Key places to review:

This is a quite big diff. Most changes though are related to the Plonk verifier contract.

alxiong and others added 24 commits July 3, 2024 10:55
Fix the edge case when zeta is one of the evaluation domain elements, and our lagrange coefficient (and also pi_poly_eval) is computed incorrectly.
Tests are also added for these rare edge cases.
* Update script to run gas benchmarks. Store gas benchmarks for this commit.
* Update lint.yml as CI is complaining.
* fix transcript and tests

* fix computeChallenges in verifier

* update gas benchmark

* further improve and add comment to inline assembly
* Fix comment in _linearizationScalarsAndBases.

* Update contracts/src/libraries/PlonkVerifier.sol

Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>

---------

Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>
* add G2 from SRS to VK and append to Transcript

* update with jf
* Remove unused function _batchVerifyOpeningProofs.

* Remove redundant variable sumEval.

* attempt: free disk space to avoid ci failure

---------

Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>
* Gas optimization for evaluatePiPoly

* add credit

---------

Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>
* use state-approach for squeeze domain-separation

* memory align word size

* update with merged jf
* rewrite computeChallenges in pure assembly

* add more comments
* change publicInputs from dynamic to fixed array
* Remove redundant field EvalDomain.groupGenInv

* Remove redundant field EvalDomain.size.

* address comments

---------

Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>
* Use of constants COSET_K1,...,COSET_K4.
* Put Transcript.sol and Transcript.t.sol files in some legacy folder.
* Remove unused domain sizes.
* Hardcode domain elements.
* removed domain.groupGen and localDomainElements.

---------

Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>
* avoid carrying around commScalars and commBases

* address jakov comment on L01
Generate bindings.
Copy link
Contributor

@alxiong alxiong left a comment

Choose a reason for hiding this comment

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

I'd prefer to wait until jellyfish cut a new tag, and we repoint to that tag instead of commonprefix-patch branch, before merging.

@philippecamacho philippecamacho merged commit 1443f0a into main Sep 4, 2024
16 checks passed
@philippecamacho philippecamacho deleted the commonprefix-patch branch September 4, 2024 13:19
@philippecamacho
Copy link
Contributor Author

I'd prefer to wait until jellyfish cut a new tag, and we repoint to that tag instead of commonprefix-patch branch, before merging.

Ticket created so we do not forget: #1973

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.

Common Prefix audit
2 participants