arkworks-rs / r1cs-std

R1CS constraints for bits, fields, and elliptic curves
https://www.arkworks.rs
Apache License 2.0
133 stars 58 forks source link

Convenient method for context-based variable/constant allocation #141

Open winderica opened 6 months ago

winderica commented 6 months ago

Summary

Add a new method to AllocVar that creates a witness variable or constant in the constraint system depending on the type of cs.

Problem Definition

When feeding non-deterministic advice for some computation $f$ to a circuit, it is often desirable to call new_variable with a mode based on the context. That is, when the inputs to $f$ are all constant, we can choose AllocationMode::Constant for the advice to save some constraints for enforcing its validity. Otherwise, we need to choose AllocationMode::Witness.

However, doing so manually introduces redundancy. We can find many duplicated code fragments for this purpose in this repo, e.g.,

https://github.com/arkworks-rs/r1cs-std/blob/4020fbc22625621baa8125ede87abaeac3c1ca26/src/groups/curves/twisted_edwards/mod.rs#L120-L138 https://github.com/arkworks-rs/r1cs-std/blob/4020fbc22625621baa8125ede87abaeac3c1ca26/src/fields/quadratic_extension.rs#L279-L291

Furthermore, taking care for every advice allocation increases the mental burden of a circuit developer, while failing to do so may result in a larger circuit.

Proposal

This proposal aims to address the above problem by introducing a new method (tentatively dubbed new_hint) to AllocVar, which creates a constant if cs is None and a witness variable otherwise. The logic of new_hint should be similar as below:

pub trait AllocVar<V: ?Sized, F: Field>: Sized {
    // ...
    #[tracing::instrument(target = "r1cs", skip(cs, f))]
    fn new_hint<T: Borrow<V>>(
        cs: impl Into<Namespace<F>>,
        f: impl FnOnce() -> Result<T, SynthesisError>,
    ) -> Result<Self, SynthesisError> {
        let ns: Namespace<F> = cs.into();
        let cs = ns.cs();
        let mode = if cs.is_none() {
            AllocationMode::Constant
        } else {
            AllocationMode::Witness
        };
        Self::new_variable(cs, f, mode)
    }
    // ...
}

Please let me know what you think, and I can create a PR if this proposal looks good to you :)


For Admin Use

Pratyush commented 6 months ago

This looks like a good idea to me! Maybe we can call it new_variable_with_inferred_mode? Or some variant.

winderica commented 6 months ago

Sure, new_variable_with_inferred_mode also sounds better to me! I agree that naming it by functionality is better than by usage, as users may use it in some other cases.