-
Notifications
You must be signed in to change notification settings - Fork 129
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
improve: apply tachyon optimizations(1) #342
Changes from all commits
5af7ac4
efbfcac
630fe7d
43093a1
e26dfef
c102941
4628c24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -13,7 +13,7 @@ use crate::{ | |||||
plonk::{self, permutation::ProvingKey, ChallengeBeta, ChallengeGamma, ChallengeX, Error}, | ||||||
poly::{ | ||||||
commitment::{Blind, Params}, | ||||||
Coeff, ExtendedLagrangeCoeff, LagrangeCoeff, Polynomial, ProverQuery, | ||||||
Coeff, LagrangeCoeff, Polynomial, ProverQuery, | ||||||
}, | ||||||
transcript::{EncodedChallenge, TranscriptWrite}, | ||||||
}; | ||||||
|
@@ -25,25 +25,15 @@ use halo2_middleware::poly::Rotation; | |||||
|
||||||
pub(crate) struct CommittedSet<C: CurveAffine> { | ||||||
pub(crate) permutation_product_poly: Polynomial<C::Scalar, Coeff>, | ||||||
pub(crate) permutation_product_coset: Polynomial<C::Scalar, ExtendedLagrangeCoeff>, | ||||||
permutation_product_blind: Blind<C::Scalar>, | ||||||
} | ||||||
|
||||||
pub(crate) struct Committed<C: CurveAffine> { | ||||||
pub(crate) sets: Vec<CommittedSet<C>>, | ||||||
} | ||||||
|
||||||
pub(crate) struct ConstructedSet<C: CurveAffine> { | ||||||
permutation_product_poly: Polynomial<C::Scalar, Coeff>, | ||||||
permutation_product_blind: Blind<C::Scalar>, | ||||||
} | ||||||
|
||||||
pub(crate) struct Constructed<C: CurveAffine> { | ||||||
sets: Vec<ConstructedSet<C>>, | ||||||
} | ||||||
|
||||||
pub(crate) struct Evaluated<C: CurveAffine> { | ||||||
constructed: Constructed<C>, | ||||||
constructed: Committed<C>, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for suggestion! |
||||||
} | ||||||
|
||||||
#[allow(clippy::too_many_arguments)] | ||||||
|
@@ -177,39 +167,20 @@ pub(in crate::plonk) fn permutation_commit< | |||||
.commit_lagrange(&engine.msm_backend, &z, blind) | ||||||
.to_affine(); | ||||||
let permutation_product_blind = blind; | ||||||
let z = domain.lagrange_to_coeff(z); | ||||||
let permutation_product_poly = z.clone(); | ||||||
|
||||||
let permutation_product_coset = domain.coeff_to_extended(z); | ||||||
ed255 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
let permutation_product_poly = domain.lagrange_to_coeff(z); | ||||||
|
||||||
// Hash the permutation product commitment | ||||||
transcript.write_point(permutation_product_commitment)?; | ||||||
|
||||||
sets.push(CommittedSet { | ||||||
permutation_product_poly, | ||||||
permutation_product_coset, | ||||||
permutation_product_blind, | ||||||
}); | ||||||
} | ||||||
|
||||||
Ok(Committed { sets }) | ||||||
} | ||||||
|
||||||
impl<C: CurveAffine> Committed<C> { | ||||||
pub(in crate::plonk) fn construct(self) -> Constructed<C> { | ||||||
Constructed { | ||||||
sets: self | ||||||
.sets | ||||||
.iter() | ||||||
.map(|set| ConstructedSet { | ||||||
permutation_product_poly: set.permutation_product_poly.clone(), | ||||||
permutation_product_blind: set.permutation_product_blind, | ||||||
}) | ||||||
.collect(), | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
impl<C: CurveAffine> super::ProvingKey<C> { | ||||||
pub(in crate::plonk) fn open( | ||||||
&self, | ||||||
|
@@ -236,7 +207,7 @@ impl<C: CurveAffine> super::ProvingKey<C> { | |||||
} | ||||||
} | ||||||
|
||||||
impl<C: CurveAffine> Constructed<C> { | ||||||
impl<C: CurveAffine> Committed<C> { | ||||||
pub(in crate::plonk) fn evaluate<E: EncodedChallenge<C>, T: TranscriptWrite<C, E>>( | ||||||
self, | ||||||
pk: &plonk::ProvingKey<C>, | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After removing this, how is
CommittedSet
different fromConstructedSet
?This part of the code is quite poorly documented so it's a bit hard to follow the function of each struct...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function
construct()
forCommitted
may be simplified now to a simpleclone()
(not sure, some types may need to be fixed).In fact, after this change I think we should re-evaluate the need of
Commited
andConstructed
, as these may be redundant.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree with you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this update? 4628c24
I am sure that we don't need
Constructed
struct any more.It also removes unnecessary conversion, which needs
.clone()
.