Circuit: Prefer `Default` values over `Option`

[adr1anh]

A circuit/gadget needs to be written so that it can be run in many different configurations (constraint, witness, shape synthesis). In some cases, it may be impractical to generate a valid set of inputs in order to actually call the synthesize function.

To facilitate synthesis when we are not interested in computing any witness data, it may be practical to wrap the input value with Option so that allocated variables can be created by passing None as the native value being allocated. Another reason why gadgets may chose to accept Option inputs is to facilitate the allocation of variables whose value depends on an existing allocated variable, which may not have a value because the ConstraintSystem is explicitly not allocating values.

This unfortunately leads to a lot of boilerplate and unnecessary clones when handling potentially empty witness values and checking if a value is indeed Some. Moreover, it leads to an awkward pattern where each input needs to be checked not to be None when in theory, either all the inputs should be None, or all should be Some.

For some circuits, it can actually be very straight-forward to generate valid default inputs without much overhead. It also makes circuits easier to read and can prevent non-uniformity bugs from the implicit branches involved in unwrapping an Option.

For example in the CyclefoldCircuit, we can make the following observations:

• It would be incorrect for any of the fields to not all be the same variant of Option.
• Even if we were not interested in generating witnesses, it is easy and cheap to generate "dummy" inputs (e.g. two points at infinity and an array of false) to pass to the synthesize method, even if they are ignored by the allocation closures.

Another example would be the SuperNovaAugmentedCircuitInputs.

• Given the concrete size of the witness, it would not be too much overhead to actually copy the incoming inputs directly into the struct
• A "dummy" version could be generated where
  • z0 and zi are allocated as E::Base::ZERO,
  • Each U is just a "default" where all values are zero/point at infinity
  • T is the point at infinity (valid if all Us are trivial)
• This struct is also unnecessarily wrapped as an Option in SuperNovaAugmentedCircuit.

Overall, we should be more careful about wrapping all circuit inputs in Option, and prefer allowing default or easy-to-generate "dummy" values, as this removes boilerplate, and prevents the risk of non-uniformity bugs.

[adr1anh]

Example NovaAugmentedCircuit

The inputs here should not be an Option. The None variant is only used to facilitate the generation of public parameters. However, it is easy to add a dummy method to NovaAugmentedCircuitInputs that generates an appropriate set of inputs (even if most of these are None, and if the inputs are not actually valid). The setup function the public parameters would simply create a set of dummy inputs with the following function:

impl<E: Engine> NovaAugmentedCircuitInputs<E> {
  /// Create dummy inputs/witness for the verification circuit, to be used for constraint synthesis
  pub fn dummy(
    arity: usize
  ) -> Self {
    Self {
      params: E::Base::ZERO,
      I: E::Base::ZERO,
      z0: vec![E::Base::ZERO; arity],
      zi: None,
      U: None,
      u: None,
      T: None,
    }
  }
}

In general for circuits, we should be able to generate "dummy" input which allow the circuit to be called even for constraint generation. These are often cheap to generate, and avoid having to call self.inputs.get()? over every item in the struct.

For gadgets, it would be nice to have equivalent alloc_infallible methods that take as input a value rather than an Option wrapped one.

[huitseeker]

One of the issues with this is conveying that the gadget is not populated with a valid value. The posterchild for this issue is https://github.com/lurk-lab/bellpepper/pull/88