Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
4 tasks
winderica opened this issue Mar 11, 2024 · 2 comments · Fixed by #143
Closed
4 tasks

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

winderica opened this issue Mar 11, 2024 · 2 comments · Fixed by #143

Comments

@winderica
Copy link
Contributor

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

let mode = if cs.is_none() {
AllocationMode::Constant
} else {
AllocationMode::Witness
};
// Compute u = x / y
let u = F::new_variable(
ark_relations::ns!(cs, "u"),
|| {
let y_inv = self
.y
.value()?
.inverse()
.ok_or(SynthesisError::DivisionByZero)?;
Ok(self.x.value()? * &y_inv)
},
mode,
)?;

let mode = if self.is_constant() {
AllocationMode::Constant
} else {
AllocationMode::Witness
};
let inverse = Self::new_variable(
self.cs(),
|| {
self.value()
.map(|f| f.inverse().unwrap_or_else(QuadExtField::zero))
},
mode,
)?;

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

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@Pratyush
Copy link
Member

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

@winderica
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants