Is ParametersVar necessary?

[tsunrise]

crypto-primitives/src/crh/constraints.rs

Lines 9 to 25 in 1a71386

	pub trait FixedLengthCRHGadget<H: FixedLengthCRH, ConstraintF: Field>: Sized {
	type OutputVar: EqGadget<ConstraintF>
	+ ToBytesGadget<ConstraintF>
	+ CondSelectGadget<ConstraintF>
	+ AllocVar<H::Output, ConstraintF>
	+ R1CSVar<ConstraintF>
	+ Debug
	+ Clone
	+ Sized;
	type ParametersVar: AllocVar<H::Parameters, ConstraintF> + Clone;
	fn evaluate(
	parameters: &Self::ParametersVar,
	input: &[UInt8<ConstraintF>],
	) -> Result<Self::OutputVar, SynthesisError>;
	}

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: #30 (comment)

[weikengchen]

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]

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]

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.