make nonnative gadget params configurable

[slumber]

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

pub enum NonNativeFieldVar<
    TargetField: PrimeField,
    BaseField: PrimeField,
    P: Params = DefaultParams, // <-- opt in to define your own.
> {
    /// Constant
    Constant(TargetField),
    /// Allocated gadget
    Var(AllocatedNonNativeFieldVar<TargetField, BaseField, P>),
}

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.

• Targeted PR against correct branch (master)
• Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
• Wrote unit tests
• Updated relevant documentation in the code
• Added a relevant changelog entry to the Pending section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer

[slumber]

@Pratyush @weikengchen can you please take a look?

[mmagician]

@slumber Could you also provide the corresponding fix for crypto-primitives?

[Pratyush]

This seems a bit too close to Config; maybe we should rename it to something like ConfigBuilder?

Also, does it make sense to make TargetField and BaseField associated types on this trait? This would reduce the number of type parameters everywhere else.

[Pratyush]

@weikengchen what do you think?

[slumber]

This seems a bit too close to Config; maybe we should rename it to something like ConfigBuilder?

*Builder is usually used for builder pattern, like

// build runtime
let runtime = Builder::new_multi_thread()
    .worker_threads(4)
    .thread_name("my-custom-name")
    .thread_stack_size(3 * 1024 * 1024)
    .build()
    .unwrap();

Params name is indeed very generic, it could be FindConfig or ComputeParams or whatever. I chose Params because you can always do

use ark_r1cs_std::fields::nonnative::params::Params as FindNonNativeConfig;

But I'm OK with changing it to any other name.

[slumber]

Also, does it make sense to make TargetField and BaseField associated types on this trait? This would reduce the number of type parameters everywhere else.

This would cause DefaultParams to become generic, as you'll need DefaultParamsF1F2 for every field pair, because the trait can only be implemented once.

It's a question of whether Params trait is defined for any pair of fields or for one particular. Given the original design, I believe it's the former, and this is reflected in the code.

[slumber]

@slumber Could you also provide the corresponding fix for crypto-primitives?

I will once we're done with this PR.

[slumber]

@weikengchen @Pratyush can we have another iteration on this one?

[Pratyush]

@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]

@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 (#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:

1. It's the most flexible option: you can hardcode, use default, use anything custom like I did above.
2. Keeps the type safety. You can't multiply Var<F1, F2, P1> by Var<F1, F2, P2>.

[Pratyush]

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:

r1cs-std/src/groups/mod.rs

Line 51 in 4020fbc

+ Mul<EmulatedFpVar<C::ScalarField, ConstraintF>, Output = Self>

.
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?

[slumber]

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:

r1cs-std/src/groups/mod.rs

Line 51 in 4020fbc

+ Mul<EmulatedFpVar<C::ScalarField, ConstraintF>, Output = Self>

.

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 a Box<dyn EmulationConfig> that's stored within the struct?

partially:

• this won't work with AllocVar trait (we can introduce new_variable_with_config similar to new_witness_with_le_bits?)
• external methods should be implemented and used with care, for instance bits_le_to_emulated or squeeze_emulated_field_elements_with_sizes etc. don't account for configs. This might create a pitfall for bugs. Runtime assert_eq could help in some cases.

If you think it's OK then I'll reimplement it with suggested change.

[slumber]

@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

[slumber]

@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