arkworks-rs / crypto-primitives

Interfaces and implementations of cryptographic primitives, along with R1CS constraints for them
https://www.arkworks.rs
Apache License 2.0
176 stars 87 forks source link

Is `ParametersVar` necessary? #36

Open tsunrise opened 3 years ago

tsunrise commented 3 years ago

https://github.com/arkworks-rs/crypto-primitives/blob/1a71386a10ef627834e4c3b14ca982811f23192d/src/crh/constraints.rs#L9-L25

Is there any circumstance where user compute CRH parameters in circuit? Computing ParametersVar requires extra constraints so it should be better to allow an option to use native CRH parameters instead.

Relevant discussions: https://github.com/arkworks-rs/crypto-primitives/pull/30#discussion_r606352586

weikengchen commented 3 years ago

Based on what I know, the ParametersVar has always been a trivial copy of Parameters in our current implementations, so it does not add any new constraints?

It would still be helpful to separate them since it opens the opportunity for one-time precomputation for some parameters in case we want to verify CRH inside the circuits, though none of our current constructions need this...

Pratyush commented 3 years ago

Hmm generally we always invoke alloc_constant on these CRH parameters, so it never actually costs any constraints. That being said, we should evaluate How the code would change if we replaced ParametersVar with just Parameters in, say, PedersenCRH

weikengchen commented 3 years ago

In PedersenCRH it seems that this change would be trivial since the current ParametersVar simply invokes Parameters.

#[derive(Derivative)]
#[derivative(Clone(bound = "C: ProjectiveCurve, GG: CurveVar<C, ConstraintF<C>>"))]
pub struct CRHParametersVar<C: ProjectiveCurve, GG: CurveVar<C, ConstraintF<C>>>
where
    for<'a> &'a GG: GroupOpsBounds<'a, C, GG>,
{
    params: Parameters<C>,
    #[doc(hidden)]
    _group_g: PhantomData<GG>,
}

If this does not create new constraints, we could come back to this issue later, like when we do a refactoring of CRH in the future.