-
Notifications
You must be signed in to change notification settings - Fork 188
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
Zeromorph and HyperKZG improvement (Arecibo backports) #301
base: main
Are you sure you want to change the base?
Conversation
deffd05
to
70979e6
Compare
70979e6
to
837396b
Compare
Hi @adr1anh, @emmorais, @huitseeker, @mpenciak, @storojs72 and @tchataigner, Thanks for the PR! For simplicity, could we please unfold this PR into several PRs? (1) Remaining improvements to HyperKZG (i.e., those not covered by #299 and #300) + PCS benchmark. From my quick skim at this PR, this appears to be improvements to KZG verifier to compute a multipairing instead of two separate pairings. Hopefully, we can merge this quickly as it should be a self-contained and localized change. (2) Refactoring (univariate) KZG out of HyperKZG On items (2) and (3), I'm not yet sure of the exact shape of (2) or (3) because ZeroMorph and HyperKZG have different SRS requirements. Also, it is not yet clear whether it makes sense to maintain both ZeroMorph and HyperKZG because of the following reasons:
In the absence of ZM, it may actually be simpler to have HyperKZG in the current form (i.e., we may not need item (2)), for easier auditability, simpler code, etc. |
To provide more clarity on the SRS: the PR already demonstrates how to use the existing Nova traits to overcome this conceptual difficulty. The KZG However, ZM and HyperKZG do not implement the In other words, ZM does not carry an incremental maintenance cost beyond the specifics of its |
Thanks for the clarifications @huitseeker! It makes sense that Item#3 is not significantly more complex. If ZM's benefits aren't significant (needs more examination!), then we also can avoid the (conceptual) complexity introduced from factoring out (univariate) KZG (item#2), which unlike the current hyperkzg.rs, spreads the code over multiple modules and some abstractions may be unnecessary if there's only going to be one KZG-based scheme. To give more time to understand these trade-offs, it may still make it easy to unfold item#1. What do you think? |
1c1cd63
to
23b44c5
Compare
I've done a pass to minimize disturbance to the Nova module organization, there no longer is a Item (1) as asked above seems to consist of:
|
Thanks @huitseeker! For (1), It think it may be easy to create a new lean PR with just the following changes:
|
* feat: Initial Shplonk prover/verifier as EvaluationEngine * feat: More optimal pairing check * feat: Avoid including C_P to the proof/transcript * feat: Check R / evals correlation on verification * feat: Verify correctness of P_i polynomials computing * chore: Add TODOs about avoiding operations with constant polynomial * chore: Fix clippy issues * chore: Review iteration fixes * feat: Include Shplonk PCS into the benchmark
Is this still being worked on? I ask because I worry about divergence with main, making it harder to merge this down the line. |
03ef48d
to
ba41769
Compare
* Schoolbook adapted KZG PoC Remove toxic waste from setup parameters Refactorings ZeroMorph skeleton * feat: provide a non-hiding variant of KZG Drafts a non-hiding variant of the KZG PCS on univariate Dense polynomial representations Details: - Changed access levels and ordering for various modules and functions like `sumcheck`, `cpu_best_multiexp`, and `mod kzg;`. - Upgraded `halo2curves` version and added new dependencies like `pairing`, `anyhow`, `rand`, and `group`. - Leveraged the UniPoly in sumcheck.rs - Created `non_hiding_kzg.rs` file, introducing new structures and functionalities like `UVUniversalKZGParam`, `UVKZGProverKey`, `UVKZGPoly`, and `UVKZGPCS` along with their implementation and tests. - Modified the import of `Engine` in `kzg.rs` from `halo2curves::pairing::Engine` to `pairing::Engine` (reflecting the new version of halo2curves). feat: add batch commit / open / prove * Small subtasks: params, phi, un function, EE Minor adjustments * chore: setup zeromorph refactor: Remove kzg and zeromorph provider modules * feat: add openings for Zeromorph - Added new data structures and their related functions including the `ZMProverKey`, `ZMVerifierKey`, `ZMCommitment`, `ZMEvaluation`, `ZMProof` and `ZMPCS` in `non_hiding_zeromorph.rs` - Implemented new functionalities in `non_hiding_zeromorph.rs` and `spartan/polynomial.rs` to provide better handling of polynomials, including fetching commitments for a polynomial, computing the quotient polynomials of an existing polynomial. - Enhanced the `UniPoly` struct in `spartan/sumcheck.rs` with `Index` and `IndexMut` for better coefficient access, and `AddAssign` and `MulAssign` for scalar and self types. - Removed and replaced certain elements in `UVUniversalKZGParam` struct in `non_hiding_kzg.rs` for improved flexibility, and included import and implementation of `TranscriptReprTrait`. draft ZM verify set up test structure defer test_quo * Include commits into ZMProof and adjust RO * feat: Refactor polynomial evaluation methods - Temporarily disabled some assertions in both `non_hiding_zeromorph.rs` and `polynomial.rs` for debugging purposes. - Introduced `evaluate_opt`, `fix_variables`, and `fix_one_variable_helper` functions in `polynomial.rs` to support multilinear polynomial evaluation and partial evaluation. fix: add domain separators - Integrated additional functionality into the ZMPCS implementation in `non_hiding_zeromorph.rs` by adding a protocol name function. - Improved security in `non_hiding_zeromorph.rs` by integrating domain separators into transcript's `open` and `verify` functions. refactor: Refactor code and test performance for NHZM - Implement `AsRef` trait for `UniPoly` structure in `src/spartan/sumcheck.rs` to facilitate direct access to its coefficients. - Refactor code in `src/provider/non_hiding_zeromorph.rs` to directly use `into_iter` in map function when creating `quotients_polys`, avoiding a large clone - Enhance test performance in `src/provider/non_hiding_zeromorph.rs` by pre-generating a `universal_setup` for tests, introducing a `max_vars` variable and RNG in the `commit_open_verify_with` test function, and adjusting the range of num_vars used for testing. fix: adjust APIs & Serialize impls for the Nova traits * feat: set up serde & abomonation bounds * add generic PCS errors * pp wip * fix: adjust type parameters for nontrivial_with_zm test fix: ignore panicking test fix: add doctest for evaluate_opt fix: remove obsolete comments chore: move UniPoly methods where they should be test: make clear current zeromorph operates in monomial basis - Added an additional test `test_dense_evaliations()` to provide more comprehensive testing for the `evaluate()` and `evaluate_opt()` functions in `MultilinearPolynomial`. refactor TranscriptReprTrait impl for compat with Commitments * feat: Implement KZG commitment trait and serialization features - Added serialization, deserialization, and Abomonation traits to UVUniversalKZGParam struct in `non_hiding_kzg.rs` file along with implementing comparison and length evaluation traits. - Created a new file `kzg_commitment.rs` which implements KZG Commitment Engine and setup, commit functions. - Integrated `kzg_commitment` in module `provider` and set up conversions between Commitment and UVKZGCommitment. - Enhanced assertion in `minroot_serde.rs` file from clone comparison to dereferenced comparison in MinRoot delay case. * Use the ZMPCS Evaluation Engine and the KZG Commitment Engine in tests. feat: Improve `prove` and `verify` methods in `ZMPCS` struct - Updated `prove` and `verify` methods in `ZMPCS<E>` struct within `non_hiding_zeromorph.rs` with actual logic for commitment, evaluation, and verification. - Adjusted the construction of `ZMCommitment` and `ZMEvaluation` within `prove` and `verify` methods. - Commented on portions of the code in `non_hiding_zeromorph.rs` that need further modification and refinement. - Modified test value for `test_pp_digest_with` in the `test_supernova_pp_digest` test case to match the new expected output. * fix evaluation reversal bug fix: remove superfluous eval functions fix: remove endianness shenanigans test: add evaluation unit test * fix: parallellize pp generation * feat: Enhance KZG commitment SRS generation efficiency using parallel computation - Introduced a new module `util` within the `provider` module, implementing fixed-base MSM, - New trait constraint has been imposed for `E::Fr: PrimeFieldBits` within the `non_hiding_zeromorph.rs` file, and usages have been adjusted in the `test` module. - Adding the `PrimeFieldBits` import from the `ff` crate and importing `provider::util` from the local crate. refactor: Refactor utility functions for elliptic curve operations - Renamed and moved `util.rs` to `fb_msm.rs` under the `provider` directory. - documented the FB MSM * Apply comments from code review Co-authored-by: Adrian Hamelink <[email protected]> * Improve documentation and testing in non_hiding_zeromorph.rs - Enhanced the clarity and accuracy of code comments and documentation in `non_hiding_zeromorph.rs`, specifically within the `ZMPCS` struct's methods. - Enriched the `quotients` function's documentation, detailing its mathematical underpinnings - Implemented a new rigorous test, `test_quotients`, to ensure the `quotients` function's output satisfies the polynomial identity. * test: refactor random ML poly generation * Finish documenting / testing Zeromorph (microsoft#142) * fix: remove a TODO using RefCast * fix: remove unsightly clone * doc: comment intermediate ZM steps * refactor: Refactor `open` function in zeromorph provider Extracted the computation of `q_hat` in the `open` function into a new function `batched_lifted_degree_quotient` for more modular code. * test: test batched_lifted_degree_quotient Addition of a new test `test_batched_lifted_degree_quotient` to validate batched degree quotient lifting for polynomials. * refactor: Refine computations in non_hiding_zeromorph.rs - Overhauled the 'eval_and_quotient_scalars' function with a revised return type with pair of vectors rather than scalar and a vector, - correspondingly split scalars `degree_check_q_scalars` and `zmpoly_q_scalars` within the `open` method of `non_hiding_zeromorph.rs`. - Adjusted the 'verify' method to accommodate the modified 'eval_and_quotient_scalars' function output changes. * test: test partially evaluated quotient \zeta_x - Introduced a new test case `test_partially_evaluated_quotient_zeta` to validate `zeta_x` construction. - correct an off-by-one error in the y_k terms * test: test partially evaluated quotient - Created a new function `phi` for evaluating using an inefficient formula in `non_hiding_zeromorph.rs` - Implemented a new test function `test_partially_evaluated_quotient_z` for validation of `Z_x` computation method in `non_hiding_zeromorph.rs` * fix: refine q_hat comment * fix: organize code more sparsely after rebase * refactor: Improve code readability and error handling in zeromorph - Updated and enhanced clarity and consistency of mathematical notation in comments across `non_hiding_kzg.rs` and `non_hiding_zeromorph.rs` files. - Implemented error handling in the `ZMPCS::verify` function within the `non_hiding_zeromorph.rs` file. * fix: compute quotient commitments in parallel * refactor: clean up --------- Co-authored-by: emmorais <[email protected]> Co-authored-by: Matej Penciak <[email protected]> Co-authored-by: Adrian Hamelink <[email protected]>
* Support for multilinear KZG commitments (microsoft#269) * multilinear KZG PCS as a provider; builds * fix two tests * fix third test; cut duplicate code * Tidy up source code comments Signed-off-by: Greg Zaverucha <[email protected]> * impl PairingGroup for bn256 * remove unneeded imports * simplify CommitmentKey * fix build; migrate G1Affine * fmt * checkpoint * migrate G2Affine and pairing * fix clippy; use unimplemented! * switch to affine form for compressed commitments * add a test with mlkzg * cargo fmt * cleanup * go back to compressed group * address clippy * rename * cleanup * add an alias * deduplicate * Revert "add an alias" This reverts commit 97cade6. * Use an alias for PreprocessedGroupElements Signed-off-by: Greg Zaverucha <[email protected]> * cargo fmt * update README.md --------- Signed-off-by: Greg Zaverucha <[email protected]> Co-authored-by: Greg Zaverucha <[email protected]> * refactor: clean up the needed scaffolding in MLKZG Summary: - THe MLKZG implementation re-implements some group traits, so as to give it maximum generality and depende maximally on the Nova traits. - However, the way in which it imports a pairing (using pairing::Engine) already implicitly constrains perfrectly usable group implementations to be available on the same types. This commit therefore removes the boilerplate and uses those external traits. - Finally, so as to mutualize part of the pairing implementation, this commit also leverages the MultiMillerLoop trait, a subtrait of `pairing::Engine`. - In sum, this commit only moves types - no actual data was harmed in its making. In detail: - Removed the `PairingGroup` trait and its related implementations from the `traits.rs` and `bn256_grumpkin.rs` files. - Simplified the imports from `halo2curves::bn256` in `bn256_grumpkin.rs` and removed unused types such as `pairing`, `G2Affine`, `G2Compressed`, `Gt`, and `G2`. - Deleted substantial amount of code associated with `G2` from `bn256_grumpkin.rs`. * make Minroot example generic over the supported curve cycles (microsoft#272) * make Minroot example generic over the supported curve cycles * upgrade version * refactor: Refactor and enhance point infinity handling in `to_transcript_bytes` - Enhanced the functionality of `to_transcript_bytes` method in `TranscriptReprTrait` for `Affine` in both `pasta.rs` and `traits.rs`. - Combined the x and y coordinates with the `is_infinity_byte` into a single byte stream for ease of handling. - Integrated additional checks for 'infinity' conditions to ensure accurate extractions of coordinate values. * refactor: Relocate multi-scalar multiplication module - Restructure the `provider` module by moving `msm` to the `util` subdirectory. * chore: Rename UV(KZG{ProverKey, VerifierKey}|UniversalKZGParam) -> \1 * refactor: Apply univariate polynomial evaluation - chore: move comment - fix: standardize power sequences computation - fix: parallelize several poly computations refactor: Refactor `EvaluationArgument` struct in mlkzg.rs - Renamed several fields in `EvaluationArgument` struct within `src/provider/mlkzg.rs` for increased clarity. - Adjusted the `prove` and `verify` methods in `src/provider/mlkzg.rs` to reflect these name changes. - Modified test code to align with the updates in the `EvaluationArgument` structure. --------- Signed-off-by: Greg Zaverucha <[email protected]> Co-authored-by: Srinath Setty <[email protected]> Co-authored-by: Greg Zaverucha <[email protected]>
…t#206) * chore: Remove redundant data absorptions while 'q' computing * chore: Remove redundant absorptions to transcript while 'd_0' computing * chore: Remove redundant absorptions to transcript while 'r' computing * chore: Apply rustfmt * chore: Absorb 'com' while 'r' computing
* chore: Parallelise 'Pi' computing * chore: Put 'x' out of brackets
* test(ipa): proof verified random seed test(ipa): proof verified test(ipa): switch from keccak to grumpkin transcript test(ipa): wip test IPA prove & verify chore: added Jetbrain config to gitignore * test(ipa): using generic fn over mlkzg, ipa & zm - Implemented test_fail_bad_proof that generates a test case from a seed to try and verify a non valid proof. - Implement both test_fail_bad_proof & test_from_seed for IPA over Grumpkin, MLKZG over Bn256 and ZM over Bn256 * ci(ipa): fixed xclippy * feat(ipa): wip bench pcs * feat(ipa): bench proof generation and verification - Created a benchmark for proof generation and verification for IPA over Grumpkin, MLKZG over Bn and ZM over Bn - multilinear.rs to public for benchmark purposes - Refactored generic test methods per @adr1anh comment * feat(ipa): proper polynomial sizes for test & sample size to 10 * feat(ipa): benchmark groups * ci(ipa): fix xclippy * test(ipa): added more polynomial sizes * test(ipa): renamed test method * refactor(ipa): revamp based on review * refactor(ipa): Flat -> Auto bench * refactor(ipa): random_with_eval inline * ci(ipa): fix xclippy
…LKZG (microsoft#242) * feat: Avoid computing final 'Pi' polynomial which is constant * chore: Cleanup * chore: Latest suggestions
…module - Removal of the redundant `src/provider/util/fb_msm.rs` file which included functions for scalar multiplication - Update of `kzg_commitment.rs` with the removal of `ff::PrimeFieldBits` dependency, transitional update of requirements, and improved method setups.
- Moved `UniversalParams` and several dependent structures for the KZG10 scheme in the `kzg_commitment.rs` file. - Deleted the `non_hiding_kzg.rs` file, - Consolidated KZG related structs under the `kzg_commitment` module, - Updated `mod.rs` to reflect the removal of the `non_hiding_kzg` module.
ba41769
to
2b58942
Compare
What's in this PR?
This implements the Zeromorph PCS as a Nova PCS, along with :
This is the work of a team at Lurk Lab, including @adr1anh, @emmorais, @huitseeker, @mpenciak, @storojs72 and @tchataigner.
Absolute Benchmarks
Complete bench output at https://gist.github.com/huitseeker/8401e4be3ef1430277d3fee6cd84efc4
Comparative benchmark
To gauge the impact of this PR on HyperKZG (a component, but not the major point of this PR), we can ask: how does this PR (at e34f0aa) compare to main (at 55a55dd) or main before #299 and #300 (at eab053b)?
Provenance
This is a port of several Arecibo PRs (click to unfold)
mlkzg::EvaluationArgument
fields argumentcomputer/arecibo#207Pi
polynomials argumentcomputer/arecibo#233Pi
polynomial in MLKZG argumentcomputer/arecibo#242Footnotes
PRs Rename mlkzg and remove redundant transcript absorbs #299 and Optimizations to KZG commitment scheme #300 also include work initially developed in Arecibo as part of this effort. ↩