Feat(Zeromorph PCS): Add Zeromorph Multilinear PCS Scheme #339
Conversation
There was a problem hiding this comment.
This is huge, thank you @PatStiles !!
I'm going to read the zeromorph paper and do another pass tomorrow, but in the mean time here are some nits –– also, it looks like tests are broken after the recent git merge, could you fix those (and the other CI failures)?
jolt-core/src/poly/unipoly.rs
Outdated
| #[cfg(feature = "multicore")] | ||
| let iter = self.coeffs.par_iter_mut(); | ||
| #[cfg(not(feature = "multicore"))] | ||
| let iter = self.coeffs.iter_mut(); |
There was a problem hiding this comment.
the multicore feature is a vestigial thing, we should get rid of it entirely
| #[cfg(feature = "multicore")] | |
| let iter = self.coeffs.par_iter_mut(); | |
| #[cfg(not(feature = "multicore"))] | |
| let iter = self.coeffs.iter_mut(); | |
| let iter = self.coeffs.par_iter_mut(); |
There was a problem hiding this comment.
This AddAssign impl is a bit unintuitive, I'd expect adding a scalar to a univariate polynomial to only change the degree-0 coefficient. Maybe move this function to the struct impl and rename it shift_coefficients or something?
jolt-core/src/poly/commitment/kzg.rs
Outdated
| } | ||
|
|
||
| #[derive(Debug, Clone, Eq, PartialEq, Default)] | ||
| pub struct UVKZGPCS<P: Pairing> { |
There was a problem hiding this comment.
nit:
| pub struct UVKZGPCS<P: Pairing> { | |
| pub struct UnivariateKZG<P: Pairing> { |
jolt-core/src/poly/commitment/kzg.rs
Outdated
| fn kzg_verify<P: Pairing>( | ||
| vk: &KZGVerifierKey<P>, | ||
| commitment: &P::G1Affine, | ||
| point: &P::ScalarField, | ||
| proof: &P::G1Affine, | ||
| evaluation: &P::ScalarField, | ||
| ) -> Result<bool, ProofVerifyError> { | ||
| let lhs = P::pairing( | ||
| commitment.into_group() - vk.g1.into_group() * evaluation, | ||
| vk.g2, | ||
| ); | ||
| let rhs = P::pairing(proof, vk.beta_g2.into_group() - (vk.g2 * point)); | ||
| Ok(lhs == rhs) | ||
| } |
There was a problem hiding this comment.
let's move this to the UVKZGPCS impl
jolt-core/src/poly/commitment/kzg.rs
Outdated
| let scalars = poly.as_vec(); | ||
| let bases = pk.g1_powers(); | ||
| let c = <P::G1 as VariableBaseMSM>::msm( | ||
| &bases[offset..scalars.len()], | ||
| &poly.as_vec()[offset..], |
There was a problem hiding this comment.
Let's just make the coeffs field on UniPoly pub, so we can avoid these as_vec() calls
jolt-core/src/poly/commitment/kzg.rs
Outdated
| &pk.g1_powers()[..poly.as_vec().len()], | ||
| &poly.as_vec().as_slice(), |
jolt-core/src/poly/commitment/kzg.rs
Outdated
| &pk.g1_powers()[..witness_poly.as_vec().len()], | ||
| &witness_poly.as_vec().as_slice(), |
jolt-core/src/poly/unipoly.rs
Outdated
| @@ -50,6 +55,49 @@ impl<F: JoltField> UniPoly<F> { | |||
| gaussian_elimination(&mut vandermonde) | |||
| } | |||
|
|
|||
| /// Divide self by another polynomial, and returns the | |||
| /// quotient and remainder. | |||
| pub fn divide_with_q_and_r(&self, divisor: &Self) -> Option<(Self, Self)> { | |||
There was a problem hiding this comment.
nit
| pub fn divide_with_q_and_r(&self, divisor: &Self) -> Option<(Self, Self)> { | |
| pub fn divide_with_remainder(&self, divisor: &Self) -> Option<(Self, Self)> { |
| #[cfg(feature = "ark-msm")] | ||
| use ark_ec::VariableBaseMSM; |
There was a problem hiding this comment.
we got rid of this feature
| #[cfg(feature = "ark-msm")] | |
| use ark_ec::VariableBaseMSM; |
| .take(num_vars + 1) | ||
| .collect(); | ||
|
|
||
| // offsets of x = |
| .zip(offsets_of_x) | ||
| .zip(squares_of_x) | ||
| .zip(&vs) | ||
| .zip(&vs[1..]) | ||
| .zip(challenges.iter().rev()) |
| transcript: &mut ProofTranscript, | ||
| ) -> Self::Proof { | ||
| let eval = poly.evaluate(&opening_point); | ||
| Zeromorph::<P>::open(&setup.0, &poly, &opening_point, &eval, transcript).unwrap() |
There was a problem hiding this comment.
is there any reason to have this CommitmentScheme impl wrap the struct impl? If not, I'd prefer to get rid of the struct impl
PatStiles
force-pushed
the
feat/zeromorph_pcs
branch
from
198c252
to
830cf41
Compare
PatStiles
force-pushed
the
feat/zeromorph_pcs
branch
from
830cf41
to
8ece39b
Compare
Refactors #66 for new Jolt CommitmentScheme Trait and addresses #249.