Add Relation trait
#348
Add Relation trait
#348
Conversation
snark/src/lib.rs
Outdated
| circuit_pk: &Self::ProvingKey, | ||
| circuit: C, | ||
| rng: &mut R, | ||
| instance: &R::Instance, | ||
| witness: &R::Witness, |
There was a problem hiding this comment.
Matches the API from the formal definition better.
| /// Takes in a description of a computation (specified in R1CS constraints), | ||
| /// and samples proving and verification keys for that circuit. | ||
| fn circuit_specific_setup<C: ConstraintSynthesizer<F>, R: RngCore + CryptoRng>( | ||
| circuit: C, | ||
| rng: &mut R, | ||
| ) -> Result<(Self::ProvingKey, Self::VerifyingKey), Self::Error>; | ||
|
|
There was a problem hiding this comment.
I moved this algorithm to CircuitSpecificSetupSNARK; it doesn't really belong here.
| } | ||
|
|
||
| /// A helper type for universal-setup SNARKs, which must infer their computation | ||
| /// size bounds. | ||
| pub enum UniversalSetupIndexError<Bound, E> { | ||
| pub enum IndexingError<Bound, E> { |
There was a problem hiding this comment.
Indexing is an operation that happens only for UniversalSetupSNARKs, so we don't need to specify that separately here.
| /// Specifies how to bound the size of public parameters required to | ||
| /// generate the index proving and verification keys for a given | ||
| /// circuit. | ||
| type ComputationBound: Clone + Default + Debug; | ||
| type IndexBound: Clone + Default + Debug; |
There was a problem hiding this comment.
Slightly more descriptive name.
| /// Specifies the type of universal public parameters. | ||
| type PublicParameters: Clone + Debug; | ||
|
|
||
| /// Specifies the bound size that is necessary and sufficient to | ||
| /// generate public parameters for `index`. | ||
| fn bound_for_index(index: &R::Index) -> Self::IndexBound; |
There was a problem hiding this comment.
New API that enables computing the bound for a given index.
| pp: &Self::PublicParameters, | ||
| circuit: C, | ||
| rng: &mut R, |
There was a problem hiding this comment.
Indexing is now deterministic, as per the formal definition.
snark/src/lib.rs
Outdated
| impl<R: NPRelation, S> CircuitSpecificSetupSNARK<R> for S | ||
| where | ||
| S: UniversalSetupSNARK<R> | ||
| { |
There was a problem hiding this comment.
Every UniversalSetupSNARK is also a CircuitSpecificSetupSNARK, so I added this blanket impl here.
snark/src/r1cs.rs
Outdated
| @@ -0,0 +1,70 @@ | |||
| use ark_relations::r1cs::{R1CS, ConstraintSynthesizer}; | |||
There was a problem hiding this comment.
These are specialized APIs for working with the existing ConstraintSystem-based API
There was a problem hiding this comment.
Instead of having these segregated APIs, another idea would be to add a RelationProvider trait that is a generalization of RelationProvider.
pub trait RelationProvider<R: Relation> {
// These are all optional because they might not exist always.
// For example, in indexing phase, there is no assignment,
// while during proving there may be no index.
fn index(&self) -> Option<R::Index>; // Maybe this should be `Rc<RefCell<R::Index>>`? to avoid copying.
fn instance(&self) -> Option<R::Instance>;
fn witness(&self) -> Option<R::Witness>;
}
pub trait SNARK<R: Relation> {
fn prove<RP: RelationProvider<R>, ...>(pk, rp: RP)
// similar for other methods.
}
impl<F: Field> RelationProvider<R1CS<F>> for ConstraintSynthesizer<F> {
// do stuff.
}
snark/src/r1cs.rs
Outdated
| /// specified as R1CS constraints. | ||
| fn verify_with_processed_vk<C: ConstraintSynthesizer<F>>( | ||
| circuit_pvk: &Self::ProcessedVerifyingKey, | ||
| public_input_generator: C, |
There was a problem hiding this comment.
The verifier now takes in a ConstraintSynthesizer that outputs the assignment to instance variables.
| @@ -20,7 +20,7 @@ use ark_std::{ | |||
| // TODO: Think: should we replace this with just a closure? | |||
| pub trait ConstraintSynthesizer<F: Field> { | |||
There was a problem hiding this comment.
Probably we should rename this trait and add new methods like generate_instance_assignment and generate_witness_assignment
Alternatively, we could replace the trait with three closures that do these tasks, as the TODO suggests.
| /// Generate inputs for the SNARK prover from [`cs`]. | ||
| fn prover_inputs<CS: ConstraintSynthesizer<F>>(cs: &CS) -> (Instance<F>, Witness<F>); | ||
|
|
||
| /// Generate inputs for the SNARK verifier from [`cs`]. |
There was a problem hiding this comment.
| /// Generate inputs for the SNARK verifier from [`cs`]. | |
| /// Generate inputs for the SNARK verifier from [`cs`]. | |
| /// Assumes the verifier has a version of the matrices from elsewhere. | |
| /// (Either preprocessed, or a direct copy of the matrices) |
|
Accidentally just reviewed the last few commits lol |
relations/src/r1cs/mod.rs
Outdated
| } | ||
|
|
||
| /// An R1CS instance consists of variable assignments to the instance variables. | ||
| /// The first variable must be assigned a value of `F::one()`. |
There was a problem hiding this comment.
I wonder if we should do the converse, and make the second variable onwards what goes into the struct?
There was a problem hiding this comment.
what do other sources of R1CS do, like zkinterface?
|
Updated the PR further, I think it's now in the final form and is ready for merging as is (modulo the CHANGELOG) |
Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Dev Ojha <[email protected]>
|
So the So I'm wondering if it's a good time to try to do, or at least plan for, a draft migration of the halo2 code, although @daira seemed to be saying its not exactly ready for that. But from my perspective, it maybe would help put an extra pair of eyes on zcash's code and help arkworks advance to being yet more featured and flexible. I'll post an issue soon regarding an approach, also outlining potential friction areas and incompatibilities, and ideas for how to solve them. I'll also try to see if there are any tweaks that could be made regarding this PR. I'm also wondering if relatedly, a common "plonk description language" ought to be established to make at the very least, relations, which can consist of multiple sub-relations, compatible between various libraries. The description language can also contain meta data, such as subcircuit names and component names, or not. If R1CS is a general-purpose processor, plonk/plookup is a sort of FPGA/ASIC, with reuseable components, lookup tables, wiring, fan-in/fan-out and locality constraints, custom gates. The glue that holds it all together is the wiring (permutation argument). There are also subtle tradeoffs between the maximum relation multiplicative depth and proof size/proving time. Just like the differences between programming a CPU and programming an FPGA are huge, there is a need to manage, and hide, a lot more complexity when it comes to Plonk/Plookup. |
|
When should we merge this one (and plan for the next/next release)? And how many code changes to the other repos should we expect? |
|
I still have to make corresponding PRs for Groth16 and Marlin; let's push merging this until after 0.3 (so in 0.4). |
Description
Adds two traits:
RelationandNPRelation. Modifies theSNARKtrait to use these, instead of theConstraintSysteminfrastructure.TBD: How to integrate the old
ConstraintSystem-based APIs into this. See #348 (comment) for an idea.closes #323
closes #321
closes #341
closes #342
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
Pendingsection inCHANGELOG.mdFiles changedin the Github PR explorer