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: Make commitment key generation configurable with an optional parameter #203

Merged

Conversation

huitseeker
Copy link
Contributor

@huitseeker huitseeker commented Jul 13, 2023

Summary

The size of the public parameters can vary depending on the method used to process them in a later SNARK. In particular, the memory checking techniques used for computational commitments (Spartan paper section 6) requires larger commitment keys.

However, because public parameters are created before any SNARK that might use them, their creation as a Rust object is harder to parametrize. So the size of the commitment key is currently set, for all public parameters, to the more conservative (larger) size required in spartan/ppsnark.rs.

The current PR allows parametrizing public parameter creation with an optional function of the R1CS shape of the circuits: the size of the parameters will be the min of the usual considerations (min of wire /constraint count) and its output. This function can then be provided by SNARKs as an associated function. It allows the mere type definition of the SNARK to convey its special needs in terms of public parameters.

The Spartan instance that does have those special needs now implements this function to convey its extra requirements, letting all other SNARKs enjoy smaller parameters.

In detail

  • Implemented the Len trait for CommitmentKey to allow length quantification in terms of group generators. Made ppsnark::RelaxedR1CSSNARK fail setup if given commitment key with insufficient length, as measured by its own commitment_key_floor() (see below)
  • Made RelaxedR1CSTrait include a fn commitment_key_floor() -> Box<dyn for<'a> Fn(&'a R1CSShape<G>) -> usize> with default implementation to quantify the Snark's additional commitment key size requirements, in the shape of a closure,
  • Made PublicParams take a &dyn for<'a> Fn(&'a R1CSShape<G>) -> usize parameter for each circuit's group, to parametrize the CommitmentKey creation.

Other implementation details:

  • defined type alias CommitmentKeyHint<G> = dyn Fn(&R1CSShape<G>) -> usize
  • Modified numerous function calls and parameter setups to include parameter CommitmentKeyHint that gives a more flexible commitment key generation.
  • Added the CommitmentKeyHint to the r1cs import list and expanded NovaShape trait to optionally accept it.

@CPerezz
Copy link
Contributor

CPerezz commented Jul 19, 2023

What's the purpose of the optional parameters passed to PublicParams creation?
What's the rationale behind this or what does it enable specifically to do that now is not possible?

@huitseeker
Copy link
Contributor Author

@CPerezz thanks for the question, I have added a summary to the PR, please let me know if it addresses your concerns fully

@CPerezz
Copy link
Contributor

CPerezz commented Jul 20, 2023

kk so I see why you need this. But unsure why you can't do it differently:

I'm just saying this as it looks a bit odd in the API to have this hinting fn when it's only used for this particular Spartan setup. But maybe there's no better way to actually fit this in..

@huitseeker
Copy link
Contributor Author

huitseeker commented Jul 20, 2023

@CPerezz

Why a trait associated const won't work here? You actually need some input to be able to estimate the lenght of the params needed by Spartan Sec.6? If so, isn't it possible to estimate that beforehand?

I would love to use a trait constant. But as the current implementation of the computation-committing Spartan should make clear (in this PR, that’s in the commitment_key_floor function), the parametrization depends on the circuit (specifically, it is a function of the R1CS matrices), and that precludes using a constant.

Note that this Spartan variant differs from (and improves on) the literature’s SPARK-based Spartan, but since this was previously implemented in this repo, we can see that even then, the commitment key floor depended on the R1CS matrices, and could not be conveyed by a constant:
https://github.com/microsoft/Nova/pull/152/files#diff-3e805e07997f3ae585e243d1aa4f1e03383e9f64bbad0d619e804f7b1ae7015dR78

I concur with your thinking that depending on unstable nightly features is unwise.

Note: the commitment_key_floor function is a new member of the RelaxedR1CSTrait, but that field has a default implementation. A SNARK that does not use computation commitments requiring a larger commitment key does not have to care about, or implement anything related to this field.

@srinathsetty
Copy link
Collaborator

@huitseeker I wonder if the following might be a better solution. Eventually, there will be a trait (e.g., CycleEngineTrait) that specifies G1, G2, etc., which will eliminate unneeded complexity with many type parameters. The same trait could also specify a CompressedSNARK that will be later used to compress the produced IVC proofs. In that case, the setup could just ask the compressedSNARK type to tell its size requirements. Would we still need the modifications in this PR in that world?

@huitseeker huitseeker force-pushed the comfigurable_commitment_key_lengths branch from ae999ec to a75a7c3 Compare August 19, 2023 01:34
@huitseeker
Copy link
Contributor Author

@srinathsetty The question is a bit theoretical, since CycleEngineTrait doesn’t exist. In this PR, the commitment key sizing hint is passed through several « levels » from front-end to back-end (e.g. PublicParameters, NovaShape, R1CSInstance would be ordered from front to back).
At the top-level, you may be right that an all-inclusive trait may a way to pluck the necessary parameters. On the backend side, I would not advise using such a trait, as its use would prohibit using the defined types with anything but an entire Nova protocol defined. There, the parameters would still need to be passed piecemeal.

@huitseeker huitseeker force-pushed the comfigurable_commitment_key_lengths branch 3 times, most recently from 8891ce7 to 474da9c Compare November 3, 2023 13:20
huitseeker added a commit to argumentcomputer/arecibo that referenced this pull request Nov 3, 2023
…ommitment key size hint

- Converted `CommitmentKeyHint<G>` from a boxed dynamic trait object to a direct dynamic trait object in `r1cs/mod.rs`.
- Changed the `commitment_key` function to always require a commitment key floor, eliminating the need for default behavior when a floor function isn't provided.
- Updated the `r1cs_shape` function across various files to take in a `CommitmentKeyHint` instead of it being optional and introduce a closure as an argument.
- Relevant modifications and updates were made in the `r1cs_shape` and `commitment_key` function calls within the test functions for various modules.
- Ported use of commitment key hint to Supernova, closing #53.
- This PR puts Arecibo in line with microsoft/Nova#203
huitseeker added a commit to huitseeker/Nova that referenced this pull request Nov 3, 2023
…ommitment key size hint

- Converted `CommitmentKeyHint<G>` from a boxed dynamic trait object to a direct dynamic trait object in `r1cs/mod.rs`.
- Changed the `commitment_key` function to always require a commitment key floor, eliminating the need for default behavior when a floor function isn't provided.
- Updated the `r1cs_shape` function across various files to take in a `CommitmentKeyHint` instead of it being optional and introduce a closure as an argument.
- Relevant modifications and updates were made in the `r1cs_shape` and `commitment_key` function calls within the test functions for various modules.
- Ported use of commitment key hint to Supernova, closing microsoft#53.
- This PR puts Arecibo in line with microsoft#203
huitseeker added a commit to argumentcomputer/arecibo that referenced this pull request Nov 3, 2023
…ommitment key size hint

- Converted `CommitmentKeyHint<G>` from a boxed dynamic trait object to a direct dynamic trait object in `r1cs/mod.rs`.
- Changed the `commitment_key` function to always require a commitment key floor, eliminating the need for default behavior when a floor function isn't provided.
- Updated the `r1cs_shape` function across various files to take in a `CommitmentKeyHint` instead of it being optional and introduce a closure as an argument.
- Relevant modifications and updates were made in the `r1cs_shape` and `commitment_key` function calls within the test functions for various modules.
- Ported use of commitment key hint to Supernova, closing #53.
- This PR puts Arecibo in line with microsoft/Nova#203
@huitseeker huitseeker force-pushed the comfigurable_commitment_key_lengths branch from e479375 to 402afba Compare November 3, 2023 14:38
…rameter

- Implemented the `Len` trait within `CommitmentKey` to allow length quantification in terms of group generators. Made ppSnark fail setup if given commitment key with insufficient length,
  as measured by its own commitment_key_floor() (see below)
- Made RelaxedR1CSTrait include a fn commitment_key_floor() -> Box<dyn for<'a> Fn(&'a R1CSShape<G>) -> usize> with default implementation to quantify the Snark's commitment key size requirements
  in the shape of a closure,
- Made PublicParameters accept optional Box<dyn for<'a> Fn(&'a R1CSShape<G>) -> usize> parameters for each circuit's group, to parametrize the CommitmentKey creation.

Implementation details:
- defined type alias CommitmentKeyHint<G> = Box<dyn Fn(&R1CSShape<G>) -> usize>; (only used internally)
- Modified numerous function calls and parameter setups to include optional parameter `CommitmentKeyHint` that gives a more flexible commitment key generation.
- Added the `CommitmentKeyHint` to the `r1cs` import list and expanded `NovaShape` trait to optionally accept it.
…ommitment key size hint

- Converted `CommitmentKeyHint<G>` from a boxed dynamic trait object to a direct dynamic trait object in `r1cs/mod.rs`.
- Changed the `commitment_key` function to always require a commitment key floor, eliminating the need for default behavior when a floor function isn't provided.
- Updated the `r1cs_shape` function across various files to take in a `CommitmentKeyHint` instead of it being optional and introduce a closure as an argument.
- Relevant modifications and updates were made in the `r1cs_shape` and `commitment_key` function calls within the test functions for various modules.
- Introduced a new function `default_commitment_key_hint` in `nova_snark` package to provide a clearer default sizing hint for the commitment key,
- Replaced hard-coded zero values throughout the codebase with calls to `default_commitment_key_hint` function,
@huitseeker huitseeker force-pushed the comfigurable_commitment_key_lengths branch from e62132f to a3fdbe6 Compare November 3, 2023 16:31
huitseeker added a commit to argumentcomputer/arecibo that referenced this pull request Nov 3, 2023
…ommitment key size hint

- Converted `CommitmentKeyHint<G>` from a boxed dynamic trait object to a direct dynamic trait object in `r1cs/mod.rs`.
- Changed the `commitment_key` function to always require a commitment key floor, eliminating the need for default behavior when a floor function isn't provided.
- Updated the `r1cs_shape` function across various files to take in a `CommitmentKeyHint` instead of it being optional and introduce a closure as an argument.
- Relevant modifications and updates were made in the `r1cs_shape` and `commitment_key` function calls within the test functions for various modules.
- Ported use of commitment key hint to Supernova, closing #53.
- This PR puts Arecibo in line with microsoft/Nova#203
github-merge-queue bot pushed a commit to argumentcomputer/arecibo that referenced this pull request Nov 3, 2023
…ommitment key size hint (#95)

* refactor: Refactor public parameters setup to remove optionality of commitment key size hint

- Converted `CommitmentKeyHint<G>` from a boxed dynamic trait object to a direct dynamic trait object in `r1cs/mod.rs`.
- Changed the `commitment_key` function to always require a commitment key floor, eliminating the need for default behavior when a floor function isn't provided.
- Updated the `r1cs_shape` function across various files to take in a `CommitmentKeyHint` instead of it being optional and introduce a closure as an argument.
- Relevant modifications and updates were made in the `r1cs_shape` and `commitment_key` function calls within the test functions for various modules.
- Ported use of commitment key hint to Supernova, closing #53.
- This PR puts Arecibo in line with microsoft/Nova#203

* doc: document Supernova Public Parameters

- Enhanced the `PublicParams` struct within `./src/supernova/mod.rs` with detailed comments

* feat: Refactor to use `default_commitment_key_hint` across codebase

- Introduced a new function `default_commitment_key_hint` in `nova_snark` package to provide a clearer default sizing hint for the commitment key,
- Replaced hard-coded zero values throughout the codebase with calls to `default_commitment_key_hint` function,
github-merge-queue bot pushed a commit to argumentcomputer/arecibo that referenced this pull request Nov 3, 2023
…ommitment key size hint (#95)

* refactor: Refactor public parameters setup to remove optionality of commitment key size hint

- Converted `CommitmentKeyHint<G>` from a boxed dynamic trait object to a direct dynamic trait object in `r1cs/mod.rs`.
- Changed the `commitment_key` function to always require a commitment key floor, eliminating the need for default behavior when a floor function isn't provided.
- Updated the `r1cs_shape` function across various files to take in a `CommitmentKeyHint` instead of it being optional and introduce a closure as an argument.
- Relevant modifications and updates were made in the `r1cs_shape` and `commitment_key` function calls within the test functions for various modules.
- Ported use of commitment key hint to Supernova, closing #53.
- This PR puts Arecibo in line with microsoft/Nova#203

* doc: document Supernova Public Parameters

- Enhanced the `PublicParams` struct within `./src/supernova/mod.rs` with detailed comments

* feat: Refactor to use `default_commitment_key_hint` across codebase

- Introduced a new function `default_commitment_key_hint` in `nova_snark` package to provide a clearer default sizing hint for the commitment key,
- Replaced hard-coded zero values throughout the codebase with calls to `default_commitment_key_hint` function,
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/r1cs/mod.rs Outdated Show resolved Hide resolved
src/spartan/ppsnark.rs Outdated Show resolved Hide resolved
- Implement marking of `NovaError` enum as non-exhaustive in `errors.rs`.
- Introduce a new error case `InvalidCommitmentKeyLength` in `NovaError` enum that checks the length of the provided commitment key.
- Improve code readability by renaming `generators_hint` variable to `ck_hint` in the `commitment_key` function.
@srinathsetty srinathsetty merged commit ded51b5 into microsoft:main Nov 9, 2023
6 checks passed
huitseeker pushed a commit to huitseeker/Nova that referenced this pull request Dec 21, 2023
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.

3 participants