Add initial experimental HyperNova multifolding support #175

oskarth · 2023-06-02T04:48:53Z

Update: PR now includes multifolding work we've done

First cut of experimental HyperNova multifolding support.

Main issue: privacy-scaling-explorations#13

cargo test --features hypernova ccs

Has the following tests:

test ccs::cccs::tests::test_compute_q ... ok
test ccs::cccs::tests::test_compute_Q ... ok
test ccs::lcccs::tests::test_lcccs_v_j ... ok
test ccs::util::tests::test_fix_variables ... ok
test ccs::util::tests::test_compute_all_sum_Mz_evals ... ok
test ccs::util::virtual_poly::test::test_eq_xr ... ok
test ccs::lcccs::tests::test_bad_v_j ... ok
test ccs::lcccs::tests::satisfied_ccs_is_satisfied_lcccs ... ok
test ccs::multifolding::tests::test_compute_g ... ok
test ccs::util::tests::test_compute_sum_Mz_over_boolean_hypercube ... ok
test ccs::cccs::tests::test_Q_against_q ... ok
test ccs::multifolding::tests::test_compute_sigmas_and_thetas ... ok
test ccs::util::virtual_poly::test::test_virtual_polynomial_mul_by_mle ... ok
test ccs::multifolding::tests::test_lcccs_fold ... ok
test ccs::util::virtual_poly::test::test_virtual_polynomial_additions ... ok
test ccs::test::test_tiny_ccs ... ok

Code is written to be merged as-is if there's interest. All code is feature flagged under a hypernova flag that is off by default, so it has no impact on other parts of the code or users of the Nova library.

Logic roughly follows the structure set out in r1cs.rs and only changing the parts that are different for CCS.

Work will continue in the PSE Nova fork (hypernova branch: https://github.com/privacy-scaling-explorations/nova/tree/hypernova). We're also doing some experimental work in arkworks in a separate repo (https://github.com/privacy-scaling-explorations/multifolding-poc), with the goal of eventually integrating it into the Nova code base.

We are open to ideas for direction w.r.t. getting it integrated into the Nova code base. Comments welcome!

oskarth · 2023-06-02T06:18:09Z

@microsoft-github-policy-service agree

srinathsetty · 2023-06-02T18:03:33Z

Hi, @oskarth Thanks for the PR! We certainly welcome contributions in this direction. This looks like an exciting PR! I'll review the PR soon and provide any comments that might come up.

src/ccs.rs

srinathsetty · 2023-06-02T18:36:25Z

src/utils.rs

+#[cfg(test)]
+mod tests {
+  use super::*;
+  use pasta_curves::Fq;


There were recent PRs (e.g., #171, #165) to make tests generic over the curves. I wonder if it makes sense to continue that approach to allow library users to switch curves and still run these tests more easily.

Thanks, agree! Will adjust tests accordingly.

Done for this module. We did it w.r.t. PrimeField for these types as that's all they need.

(Will update other tests similarly once consensus on approach, see privacy-scaling-explorations#34)

Still quite rough and lots of bugs

Simplified in recent commit microsoft@b28aaf7

Also add docs to README

1. Add documentation and test for spartan/polynomial.rs. - EqPolynomial: $\tilde{eq}$ - MultilinearPolynomial: multilinear polynomial - SparsePolynomial 2. Remove duplicate get_bits in SparsePolynomial::evaluate.

Also add some comments

srinathsetty · 2023-07-18T17:07:36Z

src/ccs/multifolding.rs

@@ -0,0 +1,390 @@
+use super::cccs::{CCCSInstance, CCCSShape};


Should we just call this file nifs.rs to make it consistent with the existing code?

srinathsetty · 2023-07-18T17:09:14Z

src/ccs/multifolding.rs

+// instances. Then the rest of CCS, CCCS, LCCCS hold references to it.
+// Is our single source of data.
+#[derive(Debug)]
+pub struct Multifolding<G: Group> {


Could you please clarify the purpose of this type? In particular, it is not clear how this type will be used.

srinathsetty · 2023-07-18T17:10:48Z

src/ccs/multifolding.rs

+#[derive(Debug)]
+pub struct Multifolding<G: Group> {
+  ccs: CCSShape<G>,
+  ccs_mle: Vec<MultilinearPolynomial<G::Scalar>>,


If multilinear polynomials are represented over the Boolean hypercube, then there is no difference between the original representation and the MLE-encoded representation. Is it necessary to have two separate ways to storing the same data?

I wonder if we can define certain MLE-related traits and just implement them on CCSShape so there is no need for separately tracking MLE-encoded data.

srinathsetty · 2023-07-18T17:12:57Z

src/ccs/multifolding.rs

+  }
+
+  /// Compute g(x) polynomial for the given inputs.
+  pub fn compute_g(


As I noted above, I think there is no need to compute this polynomial g explicitly. In general, it is more efficient to run the sum-check protocol on an implicit representation of g (i.e., by using the multilinear polynomials that directly constitute g than explicitly materializing g). We can find an example of this in the src/spartan/ where the sum-check is run on degree-3 multivariate polynomials by just working with multilinear polynomials.

srinathsetty · 2023-07-18T17:13:53Z

src/ccs/multifolding.rs

+    sigmas: &[G::Scalar],
+    thetas: &[G::Scalar],
+    r_x_prime: Vec<G::Scalar>,
+    rho: G::Scalar,


Shouldn't \rho be generated from the RO (i.e., with the sponge)?

srinathsetty · 2023-07-18T17:14:27Z

src/ccs/multifolding.rs

+      .collect();
+
+    // XXX: There's no handling of r_w atm. So we will ingore until all folding is implemented,
+    // let r_w = w1.r_w + rho * w2.r_w;


What is r_w?

srinathsetty · 2023-07-18T17:15:19Z

src/ccs/multifolding.rs

+}
+
+/// Evaluate eq polynomial.
+pub fn eq_eval<F: PrimeField>(x: &[F], y: &[F]) -> F {


There is a type called EqPolynomial in src/spartan/polynomial.rs. Should we use that?

srinathsetty · 2023-07-18T17:18:10Z

src/ccs/util/virtual_poly.rs

@@ -0,0 +1,498 @@
+use crate::hypercube::BooleanHypercube;


We have a polynomial type in src/spartan/polynomial.rs, which was sufficient to implement the sum-check protocol for R1CS. I wonder if we can reuse that functionality rather than create an alternate type with overlapping functionality.

srinathsetty · 2023-07-18T17:19:08Z

src/hypercube.rs

@@ -0,0 +1,66 @@
+//! This module defines basic types related to Boolean hypercubes.


Since Nova does not require this functionality and is needed only for the multi-folding scheme, does it make sense to move this to the ccs module?

srinathsetty · 2023-07-18T17:23:02Z

src/hypercube.rs

+use itertools::Itertools;
+
+#[derive(Debug)]
+pub(crate) struct BooleanHypercube<F: PrimeField> {


This type might be unnecessary since multilinear polynomials are going to be stored in evaluation form over the Boolean hypercube. In particular, given a vector z that represents evaluations of multilinear polynomial \widetilde{z}, if we need the evaluation of \widetilde{z}(x) where x is an appropriate bit string represented with usize, we could simply return z[x]. In essence, we are treating bitstrings as indexes into the vector (we can see examples of this in src/spartan/polynomial.rs). This would avoid two separate representations for polynomials.

srinathsetty · 2023-07-18T17:23:42Z

src/lib.rs

+#[cfg(feature = "hypernova")]
+mod hypercube;
+#[cfg(feature = "hypernova")]
+mod utils;


Could we move this to the ccs module as well as this is used only by the hypernova code?

srinathsetty · 2023-07-18T17:27:05Z

src/spartan/polynomial.rs

@@ -69,10 +100,13 @@ impl<Scalar: PrimeField> MultilinearPolynomial<Scalar> {
    self.Z.len()
  }

+  // NOTE: this is equivalent to Espresso/HyperPlonk's 'fix_last_variables' method


Maybe remove this comment as it could cause confusion: Espresso/HyperPlonk came way after Spartan (the code below is inherited from Spartan) rather than the other way around :)

This wins as best comment of the review.

In fact, this was an internal note for internal clarity while fixing stuff, not to be send in a PR.

srinathsetty · 2023-07-18T21:36:05Z

src/spartan/polynomial.rs

  pub fn bound_poly_var_top(&mut self, r: &Scalar) {
    let n = self.len() / 2;

    let (left, right) = self.Z.split_at_mut(n);
+
+    // XXX: What does this do? Seems like nothing?


Good catch! I think in the past, we weren't resizing the Z vector, so this was necessary, but we do resize Z (in the code below), so this code is likely not needed (I'll double-check).

oskarth · 2023-07-20T06:05:06Z

@srinathsetty Thanks for the review of this still fairly rough code! Useful to get some feedback on direction to make sure we are on the right track and can prioritize work.

We went over comments and most things made sense to us; will comment as they are addressed. Some are quick fixes, others are TODO/WIP, and some will require a bit more work.

Also as a FYI we have a draft PR for changing API to make it more NIMFS-centric: privacy-scaling-explorations#41 - this should address several of the points raised.

Will mark this PR as draft again for now so you don't have to review it again until we've addressed most or all of your points.

What do you think about splitting this PR up into smaller chunks? It might make it easier to review and merge partial progress. Only where it makes sense and is a self-contained PR of course. Whatever you prefer!

- Move bit_decompose into hypercube.rs which is closer conceptually and only used there (except for a VirtualPolynomial test) - Remove unused bit_decompose imports from other files

) Fixes additional clippy errors I see when I upgraded my Rust toolchain (had some issues with clippy and Homebrew) ``` error: use of `default` to create a unit struct --> src/ccs/util/virtual_poly.rs:120:29 | 120 | phantom: PhantomData::default(), | ^^^^^^^^^^^ help: remove this call to `default` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_structs note: the lint level is defined here ```

Addresses #34 Should cover all files

srinathsetty · 2023-07-21T18:30:45Z

@srinathsetty Thanks for the review of this still fairly rough code! Useful to get some feedback on direction to make sure we are on the right track and can prioritize work.

We went over comments and most things made sense to us; will comment as they are addressed. Some are quick fixes, others are TODO/WIP, and some will require a bit more work.

Also as a FYI we have a draft PR for changing API to make it more NIMFS-centric: privacy-scaling-explorations#41 - this should address several of the points raised.

Thank you, @oskarth!

Will mark this PR as draft again for now so you don't have to review it again until we've addressed most or all of your points.

Sounds good!

What do you think about splitting this PR up into smaller chunks? It might make it easier to review and merge partial progress. Only where it makes sense and is a self-contained PR of course. Whatever you prefer!

Yes, that certainly sounds good!

This is a proposal for the new API of the Hypernova/multifolding current impl. The idea is that the API is [NIMFS](https://github.com/privacy-scaling-explorations/Nova/blob/change%2FNIMFS_centric_API/src/ccs/multifolding.rs#L39-L44)-centric. That means, the end user only needs to deal for now with `CCS`(this can be prevented) and the `NIMFS` object. The rest of interactions with `CCCS` & `LCCCS` have been "masked" or at the very least, is not necessary to import these structs to have full functionallity. The current workflow that this API brings can be clearly seen here: ```rust let ccs = CCS::<G>::from_r1cs(r1cs_shape); let mles = ccs.matrix_mles(); let mut nimfs = NIMFS::init( &mut rng, ccs, mles, // Note we constructed z on the fly with the previously-used witness. vec![ G::Scalar::ONE, G::Scalar::from(3u64), G::Scalar::from(35u64), ], ); // Now, the NIMFS should satisfy correctly as we have inputed valid starting inpuits for the first LCCCS contained instance: assert!(nimfs.is_sat().is_ok()); // Now let's create a valid CCCS instance and fold it: let valid_cccs = nimfs.gen_cccs(vec![ G::Scalar::ONE, G::Scalar::from(2u64), G::Scalar::from(15u64), ]); nimfs.fold(&mut rng, valid_cccs); // Since the instance was correct, the NIMFS should still be satisfied. assert!(nimfs.is_sat().is_ok()); // Now let's create am invalid CCCS instance and fold it: let invalid_cccs = nimfs.gen_cccs(vec![ G::Scalar::ONE, G::Scalar::from(5u64), G::Scalar::from(55u64), ]); nimfs.fold(&mut rng, invalid_cccs); // Since the instance was wrong, the NIMFS should not be satisfied correctly. assert!(nimfs.is_sat().is_err()); ``` This is part of a test, located in https://github.com/privacy-scaling-explorations/Nova/blob/change%2FNIMFS_centric_API/tests/nimfs.rs The idea is that the user needs to type as less as possible in order to get a fold done.

- Remove custom eq_poly and use EqPolynomial instead - Update polynomial docs - Ensures order of evaluation is consistent between for VirtualPoly and EqPolynomial - Remove old build_eq_x_r_vec and build_eq_x_r_helper - Tests around boolean hypercube and other introduced functions to make endianness explicit etc Addresses #19

The PR introduces usage from the associated trait/type of `Group` called `TranscriptEngineTrait` to perform the Fiat-Shamir inside the multifolding for the Sigmas and Thetas computations as well as gammas. This should be rebased on the top of `main` once #41 is merged and then reviewed. As there could be places where FS should be applied but it isn't.

Since the Verifier in the protocol does not have access to the witnesses, we should not have `z`s inside of the LCCCS and CCCS instances and instead, work with the witness sepparatedly. This PR implements this sort of behaviour. Resolves: #46

Sync upstream --------- Co-authored-by: François Garillot <[email protected]> Co-authored-by: Srinath Setty <[email protected]>

oskarth · 2023-08-21T07:58:24Z

Update: Since the last post some of the comments above have been addressed. However, there's still a lot more work to be done get this to a mergeable state.

Work on HyperNova was a timeboxed bet as part of a "Team Novi" effort. As that concluded a few weeks ago, we decided to pause our effort to integrate HyperNova upstream. Future work on Nova and related things will continue but likely in other forms for the time being.

Outside of comments above, here are some issues that remain: https://github.com/privacy-scaling-explorations/Nova/issues (e.g. privacy-scaling-explorations#50) and privacy-scaling-explorations#28

It is very likely that this PR and other work can be cut up further for easier integration (e.g. like #215), perhaps under an experimental hypernova feature flag.

If someone wants to take this work over and run with it that'd be great! Hopefully it is useful to someone and not a complete waste :) Feel free to take commits above and go for it!

microsoft#175) * refactor circuit_index dependence * add check on circuit arities

srinathsetty self-requested a review June 2, 2023 18:09

srinathsetty reviewed Jun 2, 2023

View reviewed changes

src/ccs.rs Outdated Show resolved Hide resolved

srinathsetty reviewed Jun 2, 2023

View reviewed changes

src/ccs.rs Outdated Show resolved Hide resolved

srinathsetty reviewed Jun 2, 2023

View reviewed changes

oskarth and others added 24 commits July 11, 2023 13:17

Add utils for hadamard and matrix vector product

789354a

utils: Add more utils, sparse matrix vector product etc

5a025c6

feat: First cut of CCS (#14)

a3e2182

Still quite rough and lots of bugs

refactor: Remove digest

5d4b110

Simplified in recent commit microsoft@b28aaf7

feat(CCS): Add from_r1cs and get rid of hardcoded constants

60bbec2

fix(CCS): Add commitment key and CCS is_sat test

4f5e784

style(ccs): Remove old comments and unused code, clarify todos

f72c49b

refactor(ccs): Add experimental hypernova feature flag (on by default)

4470d92

Also add docs to README

chore(ccs): Turn off hypernova feature flag by default

9182cc3

chore: update dep

db6add0

chore: Cargo.toml

2a59d80

add doc and tests for spartan/polynomial.rs

85f4e22

1. Add documentation and test for spartan/polynomial.rs. - EqPolynomial: $\tilde{eq}$ - MultilinearPolynomial: multilinear polynomial - SparsePolynomial 2. Remove duplicate get_bits in SparsePolynomial::evaluate.

use pasta instead of defining new fp

5223fea

wrap link with <> in comment

6a45489

fmt(spartan): cargo fmt

9598d1d

Remove: Clippy unused fn lint

64b7bd7

change: Impl SparseMatrix type

8db47a5

change: Adopt SparseMatrix type and make pad recieve &mut

ac57cb6

Fix: Update tests to match new trait bounds

d373753

feat(ccs): Sketch out basic CCCS/LCCCS associated data structures

ed36b4a

Also add some comments

feat(utils): Add SparseMatrix n_rows/n_cols utils

0262dc6

fix(ccs): Calculcate m n_rows correctly for a set of sparse matrices

f1a8834

feat(ccs): Add m, n, s and s_prime to CCS struct

d1e63a0

WIP CCCS

0192d63

srinathsetty reviewed Jul 18, 2023

View reviewed changes

oskarth marked this pull request as draft July 20, 2023 06:06

arnaucube and others added 3 commits July 20, 2023 09:35

Update bit_decompose def:

6fa448f

- Move bit_decompose into hypercube.rs which is closer conceptually and only used there (except for a VirtualPolynomial test) - Remove unused bit_decompose imports from other files

refactor: Make tests generic w.r.t. curve/group (#40)

9a508b2

Addresses #34 Should cover all files

CPerezz and others added 4 commits July 31, 2023 18:44

oskarth mentioned this pull request Aug 6, 2023

feat(spartan/polynomial): Update polynomial with docs and new methods #215

Merged

Chore/sync upstream (#51)

a18afe0

Sync upstream --------- Co-authored-by: François Garillot <[email protected]> Co-authored-by: Srinath Setty <[email protected]>

oskarth force-pushed the feat/basic-ccs branch from 16f260b to a18afe0 Compare August 21, 2023 07:47

oskarth mentioned this pull request Aug 21, 2023

HyperNova PoC privacy-scaling-explorations/Nova#13

Open

huitseeker pushed a commit to huitseeker/Nova that referenced this pull request Dec 18, 2023

Remove circuit_index dependence of supernova::RecursiveSNARK::verify (

5e45c6b

microsoft#175) * refactor circuit_index dependence * add check on circuit arities

sjudson mentioned this pull request Feb 13, 2024

CCS implementation nexus-xyz/nexus-zkvm#52

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial experimental HyperNova multifolding support #175

Add initial experimental HyperNova multifolding support #175

oskarth commented Jun 2, 2023 •

edited

Loading

oskarth commented Jun 2, 2023

srinathsetty commented Jun 2, 2023

srinathsetty Jun 2, 2023

oskarth Jun 5, 2023

oskarth Jul 14, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

srinathsetty Jul 18, 2023

CPerezz Jul 19, 2023

arnaucube Jul 19, 2023

srinathsetty Jul 18, 2023

oskarth commented Jul 20, 2023

srinathsetty commented Jul 21, 2023

oskarth commented Aug 21, 2023

		@@ -0,0 +1,390 @@
		use super::cccs::{CCCSInstance, CCCSShape};

		@@ -0,0 +1,66 @@
		//! This module defines basic types related to Boolean hypercubes.

Add initial experimental HyperNova multifolding support #175

Are you sure you want to change the base?

Add initial experimental HyperNova multifolding support #175

Conversation

oskarth commented Jun 2, 2023 • edited Loading

oskarth commented Jun 2, 2023

srinathsetty commented Jun 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oskarth commented Jul 20, 2023

srinathsetty commented Jul 21, 2023

oskarth commented Aug 21, 2023

oskarth commented Jun 2, 2023 •

edited

Loading