Open slumber opened 1 year ago
@Pratyush @weikengchen can you please take a look?
@slumber Could you also provide the corresponding fix for crypto-primitives?
@slumber Could you also provide the corresponding fix for crypto-primitives?
I will once we're done with this PR.
@weikengchen @Pratyush can we have another iteration on this one?
@slumber, sorry for the delay on this! Could you provide some example alternative instantiations of Params
? That way we can get a sense of the why the new API looks the way it does. Thanks!
@slumber, sorry for the delay on this! Could you provide some example alternative instantiations of
Params
? That way we can get a sense of the why the new API looks the way it does. Thanks!
@Pratyush thank you for getting back to the PR.
An example is computing poseidon hash. I'd like to minimize the number of limbs in emulated variables, but keep "constraints" optimizations target.
This is a sketch, but the idea should be clear; Knowing that the minimum legal number of limbs is 3 (https://github.com/arkworks-rs/r1cs-std/issues/128):
const MIN_SUPPORTED_LIMBS: usize = 3;
pub struct MinLimbsParams;
impl Params for MinLimbsParams {
fn get<TargetField: PrimeField, BaseField: PrimeField>(
_optimization_type: OptimizationType, // always minimize
) -> NonNativeFieldConfig {
let target_bits = TargetField::MODULUS_BIT_SIZE;
let base_bits = BaseField::MODULUS_BIT_SIZE;
let num_limbs = if target_bits >= base_bits {
MIN_SUPPORTED_LIMBS
} else {
(base_bits / target_bits).max(MIN_SUPPORTED_LIMBS)
};
let bits_per_limb = (base_bits / num_limbs).next_power_of_two();
NonNativeFieldConfig {
num_limbs: num_limbs as usize,
bits_per_limb: bits_per_limb as usize,
}
}
}
Why put it as a generic parameter:
Var<F1, F2, P1>
by Var<F1, F2, P2>
.The primary concern I have is that users will need to specify an additional type parameter whenever they want to use EmulatedFpVar
generically.
This happens, for example, here: https://github.com/arkworks-rs/r1cs-std/blob/4020fbc22625621baa8125ede87abaeac3c1ca26/src/groups/mod.rs#L51 .
In this case, we want to support scalar multiplication by any EmulatedFpVar
as long as it is for the right field.
Would it be possible to have a different way to track the parameters? E.g. via a Box<dyn EmulationConfig>
that's stored within the struct?
The primary concern I have is that users will need to specify an additional type parameter whenever they want to use
EmulatedFpVar
generically.This happens, for example, here:
.
Yes, this bound wasn't present at the time of writing this PR. Being required to specify config in a trait definition doesn't appeal to me either.
In this case, we want to support scalar multiplication by any
EmulatedFpVar
as long as it is for the right field. Would it be possible to have a different way to track the parameters? E.g. via aBox<dyn EmulationConfig>
that's stored within the struct?
partially:
AllocVar
trait (we can introduce new_variable_with_config
similar to new_witness_with_le_bits?)assert_eq
could help in some cases.If you think it's OK then I'll reimplement it with suggested change.
@Pratyush on second thought, I don't think it's going to work because of object safety rules, at least I couldn't make it work in the playground
@Pratyush what do you think about storing configuration directly in the struct:
num_limbs: usize,
bits_per_limb: usize,
And whenever you want to use non-default one you provide generic EmulatedConfig
, for example to squeeze_non_native
or new_variable_with_config
Depending on a goal, some developers would want to have a specific configuration perhaps with a smaller number of limbs.
This change is almost backwards compatible and actually breaks some code in
ark-crypto-primitives
, but it tries to follow the pattern from Rust std like "Vec<T, A: Allocator = Global>"Hence the new definition
Likewise for
AllocatedNonNativeFieldVar
,NonNativeFieldMulResultVar
,AllocatedNonNativeFieldMulResultVar
.Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why.
Pending
section inCHANGELOG.md
Files changed
in the Github PR explorer