Closed bhgomes closed 2 years ago
Will review this during the weekend.
Thanks for the amazing PR description BTW!!!
@davidnevadoc @LukePearson1 I can merge this whenever you're ready.
@bhgomes, does the benches/plonk.rs
compile on your machine?
1) Why removing the black_boxes from the benchmarks? They're useful to prevent compiler optimizations.
Black boxes are called around functions not function arguments and the "benchmark with input" automatically calls black box for you. See the criterion book/source.
My mistake, it appears that you do want to call black_box
on some inputs (typically if the inputs themselves can be optimized away), but in this case, criterion
handles it for us (but not needed).
2) The diff is much larger than what it should be. There are a lot of formatting changes which I'm not sure where are comming from. Are you using the same toolchain??
If you're referring to the permutation files, this is just some corner case of the git diff algorithm I guess, because it's not just a rename but also some code is changed. I've had this happen before, not sure how to set it up differently so that the diff is more reasonable.
If you're referring to the code formatting changes, I did that formatting by hand because rustfmt doesn't have handlers for those patterns. The patterns I use are more standard, more readable, easier to extend, and more diff-friendly. One common formatting pattern is for generics:
impl<E,P> StandardComposer<E,P>
where
E: PairingEngine,
P: TEModelParameters<BaseField = E::Fr>,
{}
is preferred over
impl<E: PairingEngine, P: TEModelParameters<BaseField = E::Fr>> StandardComposer<E,P> {}
So I changed the declarations over the code base, hopefully I didn't miss any.
This PR addresses some refactors and design changes to improve the development experience and extensibility of the library. Closes #14. Closes #48.
Design
This PR adds a new trait with the following signature:
where
GateValues<F>
is astruct
of values available to a custom gate:GateConstraint
provides a default implementation for the following methods:which use the
GateConstraint::constraints
method to compute these.To build a custom gate, say the
Range
gate, one should do the following:GateConstraint<F>
for that struct:range_selector
toProverKey
range_selector_commitment
toVerifierKey
range_separation_challenge
during proving/verifying using the transcript.range_selector_commitment
to the transcript during proving/verifying.which uses the default
GateConstraint::quotient_term
implementation.which uses the default
GateConstraint::linearisation_term
implementation.StandardComposer
.Now you have a custom gate! This process can be streamlined even more and we can perform the above nine steps automatically, but this is left for future PRs.
Refactors
The new
trait
replaces theProverKey
/VerifierKey
structs and files for every gate with a single file, singletrait
implementation. Each custom gate subdirectory in thesrc/proof_system/widget
directory is now replaced with a single file (or a file-per-topic as in theecc
case) which implements the constraints as above.Also, this PR tries to unify the formatting/documentation used across the library and reduce unnecessary complexity and noise.
Implementation Note
Because the
arithmetic
gate is treated differently than custom gates, it cannot take advantage of the abovetrait
. This may not be a necessary restriction on the protocol and it could be possible to treat it as all other gates, but this PR does not attempt to address this.Reviewing
To review this PR see the following list of changes and use the
Files changed
menu to ensure that these changes correspond to the list below and that they are satisfactory.Notable Changes: 9, 22, 24, 25, 26, 28, 30, 34, 38, 44, 48, 51, 58
Cargo.toml
:derivative
dependencysrc/circuit.rs
:PublicInputValue
andVerifierData
(NB: Should add an issue to build better abstractions for these constructions.)src/constraint_system/arithmetic.rs
:src/constraint_system/boolean.rs
:src/constraint_system/composer.rs
:derive
semantics usingderivative::Derivative
src/constraint_system/ecc/curve_addition/fixed_base_gate.rs
:derive
semanticssrc/constraint_system/ecc/curve_addition/mod.rs
:src/constraint_system/ecc/curve_addition/variable_base_gate.rs
:src/constraint_system/ecc/mod.rs
:derive
semanticsassert_equal_point
src/constraint_system/ecc/scalar_mul/fixed_base.rs
:src/constraint_system/ecc/scalar_mul/mod.rs
:src/constraint_system/ecc/scalar_mul/variable_base.rs
:src/constraint_system/helper.rs
:src/constraint_system/logic.rs
:src/constraint_system/mod.rs
:src/constraint_system/range.rs
:src/constraint_system/variable.rs
:src/lib.rs
:src/permutation/mod.rs
:src/permutation/permutation.rs
heresrc/permutation/permutation.rs
:src/permutation/mod.rs
src/prelude.rs
:src/proof_system/linearisation_poly.rs
:derive
semanticsPairingEngine
constraint where unnecessaryGateConstraint
modelsrc/proof_system/mod.rs
:src/proof_system/permutation.rs
:ProverKey
andVerifierKey
into one file, eventually, we can share some code between them to reduce repetitionderive
semanticssrc/proof_system/preprocess.rs
:range
should always come beforelogic
to match the transcript)src/proof_system/proof.rs
:derive
semanticsbeta != alpha
assertion to correct part of protocolGateConstraint
modelsrc/proof_system/prover.rs
:src/proof_system/quotient_poly.rs
:GateConstraint
modelsrc/proof_system/verifier.rs
:src/proof_system/widget/arithmetic.rs
:ProverKey
andVerifierKey
into the same filederive
semanticssrc/proof_system/widget/arithmetic/mod.rs
:src/proof_system/widget/arithmetic.rs
src/proof_system/widget/arithmetic/proverkey.rs
:src/proof_system/widget/arithmetic.rs
src/proof_system/widget/arithmetic/verifierkey.rs
:src/proof_system/widget/arithmetic.rs
src/proof_system/widget/ecc/curve_addition.rs
:GateConstraint
modelsrc/proof_system/widget/ecc/curve_addition/mod.rs
:src/proof_system/widget/ecc/curve_addition.rs
src/proof_system/widget/ecc/curve_addition/proverkey.rs
:src/proof_system/widget/ecc/curve_addition.rs
src/proof_system/widget/ecc/curve_addition/verifierkey.rs
:src/proof_system/widget/ecc/curve_addition.rs
src/proof_system/widget/ecc/fixed_base_scalar_mul.rs
:GateConstraint
modelsrc/proof_system/widget/ecc/mod.rs
:src/proof_system/widget/ecc/scalar_mul/fixed_base/mod.rs
:src/proof_system/widget/ecc/fixed_base_scalar_mul.rs
src/proof_system/widget/ecc/scalar_mul/fixed_base/proverkey.rs
:src/proof_system/widget/ecc/fixed_base_scalar_mul.rs
src/proof_system/widget/ecc/scalar_mul/fixed_base/verifierkey.rs
:src/proof_system/widget/ecc/fixed_base_scalar_mul.rs
src/proof_system/widget/ecc/scalar_mul/mod.rs
:src/proof_system/widget/ecc/fixed_base_scalar_mul.rs
src/proof_system/widget/logic.rs
:GateConstraint
modelsrc/proof_system/widget/logic/mod.rs
:src/proof_system/widget/logic.rs
src/proof_system/widget/logic/proverkey.rs
:src/proof_system/widget/logic.rs
src/proof_system/widget/logic/verifierkey.rs
:src/proof_system/widget/logic.rs
src/proof_system/widget/mod.rs
:GateValues
andGateConstraint
ProverKey
andVerifierKey
derive
semanticsrange
should always come beforelogic
to match the transcript)src/proof_system/widget/permutation/mod.rs
:src/proof_system/permutation.rs
src/proof_system/widget/permutation/verifierkey.rs
:src/proof_system/permutation.rs
src/proof_system/widget/range.rs
:GateConstraint
modelsrc/proof_system/widget/range/mod.rs
:src/proof_system/widget/range.rs
src/proof_system/widget/range/proverkey.rs
:src/proof_system/widget/range.rs
src/proof_system/widget/range/verifierkey.rs
:src/proof_system/widget/range.rs
src/test.rs
:src/tests.rs
:src/test.rs
to match the rest of the librarysrc/transcript.rs
:derive
semanticssrc/util.rs
:powers_of
to return an iterator instead of a vector to improve its interface safety