zksecurity / noname

Noname: a programming language to write zkapps
https://zksecurity.github.io/noname/
181 stars 47 forks source link

Can we remove `add_constant`? #34

Open mimoo opened 6 months ago

mimoo commented 6 months ago

We should audit the usage of add_constant function. I have a feeling that it should be abstracted away, and the backend should decide if constants need to be created by adding a constraint. That’s in the case of plonk, r1cs doesn’t seem to care about that.

For example, one use of add_constant is in the mul function:

pub fn mul<B: Backend>(
    compiler: &mut CircuitWriter<B>,
    lhs: &ConstOrCell<B::Field>,
    rhs: &ConstOrCell<B::Field>,
    span: Span,
) -> Var<B::Field> {
    let zero = B::Field::zero();
    let one = B::Field::one();

    match (lhs, rhs) {
        // 2 constants
        (ConstOrCell::Const(lhs), ConstOrCell::Const(rhs)) => Var::new_constant(*lhs * *rhs, span),

        // const and a var
        (ConstOrCell::Const(cst), ConstOrCell::Cell(cvar))
        | (ConstOrCell::Cell(cvar), ConstOrCell::Const(cst)) => {
            // if the constant is zero, we can ignore this gate
            if cst.is_zero() {
                let zero = compiler.backend.add_constant(
                    Some("encoding zero for the result of 0 * var"),
                    B::Field::zero(),
                    span,
                );
                return Var::new_var(zero, span);
            }

we need this at the moment because we return a Var which cannot represent a constant here. But would it make sense to return a ConstOrCell instead so that we can return a constant here?

Exploring if this is possible should be fairly easy hence the tag

katat commented 5 months ago

I think we would still need to add_constant to constraint constants.

For this case in the mul constraint:

let zero = compiler.backend.add_constant(
    Some("encoding zero for the result of 0 * var"),
    B::Field::zero(),
    span,
);

It is an "empty" constraint, as it constraint a zero constant and the purpose is to avoid creating a gate. So this constraint is ineffective and unnecessary.

But for the case in equal_cells:

let x1 = match x1 {
  ConstOrCell::Const(cst) => compiler.backend.add_constant(
      Some("encode the lhs constant of the equality check in the circuit"),
      *cst,
      span,
  ),
  ConstOrCell::Cell(cvar) => *cvar,
};

let x2 = match x2 {
  ConstOrCell::Const(cst) => compiler.backend.add_constant(
      Some("encode the rhs constant of the equality check in the circuit"),
      *cst,
      span,
  ),
  ConstOrCell::Cell(cvar) => *cvar,
};

It needs to enforce the equality with the constants, so the constraints for these constants need to be presented in the circuit.

katat commented 5 months ago

This add_constant does two things:

  1. store a constant var
  2. constraint the constant value

So if we want to remove this interface, we would need to replace it with

  1. manually store a var via new_internval_var
  2. constraint it with constraint_eq_const (this is a newly added backend interface without returns)