cleanup commitment scheme bounds #39
Conversation
| @@ -19,7 +19,7 @@ where | |||
| impl<G: CurveGroup, PC: PolyCommitmentScheme<G>> CommitmentScheme<G> for PolyVectorCommitment<G, PC> | |||
| where | |||
| G: CurveGroup, | |||
| PC::Commitment: Copy, | |||
| PC::Commitment: Copy + Into<G> + From<G>, | |||
There was a problem hiding this comment.
I believe we still have to carry these 2 bounds, as we can't put it into PC::Commitment definition.
There was a problem hiding this comment.
this seems reasonable to me. adding these bounds to PolyCommitmentScheme in spartan would break the hyrax implementation (commitments are sqrt-sized vecs of G elements). the hyrax implementation was mostly a way for me to develop this trait before implementing zeromorph, but we might as well keep it.
There was a problem hiding this comment.
arkworks-rs/poly-commit#130 might get merged soon
| @@ -90,7 +93,7 @@ where | |||
| G: CurveGroup, | |||
| G::BaseField: PrimeField + Absorb, | |||
| G::Affine: Absorb, | |||
| C: CommitmentScheme<G, Commitment = G>, | |||
There was a problem hiding this comment.
good catch - i guess this impl is only used for the secondary circuit, so the compiler didn't catch this one.
| @@ -19,7 +19,7 @@ where | |||
| impl<G: CurveGroup, PC: PolyCommitmentScheme<G>> CommitmentScheme<G> for PolyVectorCommitment<G, PC> | |||
| where | |||
| G: CurveGroup, | |||
| PC::Commitment: Copy, | |||
| PC::Commitment: Copy + Into<G> + From<G>, | |||
There was a problem hiding this comment.
this seems reasonable to me. adding these bounds to PolyCommitmentScheme in spartan would break the hyrax implementation (commitments are sqrt-sized vecs of G elements). the hyrax implementation was mostly a way for me to develop this trait before implementing zeromorph, but we might as well keep it.
I noticed an image is missing in README. I'm adding it.
Basically, get rid of
, Commitment = Projective<G>bound everywhere + moveInto + Frominto theCommitmenttrait.