dalek-cryptography / bulletproofs

A pure-Rust implementation of Bulletproofs using Ristretto.
MIT License
1.02k stars 218 forks source link

ConstraintSystem::constrain(): check validity of LinearCombinations. #278

Open rubdos opened 5 years ago

rubdos commented 5 years ago

I found these lines in prover.rs:

fn constrain(&mut self, lc: LinearCombination) {
// TODO: check that the linear combinations are valid
// (e.g. that variables are valid, that the linear combination evals to 0 for prover, etc).
//[..]
}

I tried implementing this, since I have some issues debugging a proof system I built. I've written:

    fn constrain(&mut self, lc: LinearCombination) {
        let sum: Scalar = lc.terms.iter().map(|(v, s)| {
            let l = match v {
                Variable::One() => Scalar::one(),
                Variable::Committed(i) => self.v[*i],
                Variable::MultiplierLeft(i) => self.a_L[*i],
                Variable::MultiplierRight(i) => self.a_R[*i],
                Variable::MultiplierOutput(i) => self.a_O[*i],
            };
            l * s
        }).sum();
        debug_assert_eq!(sum, Scalar::zero());
        // TODO: check that the linear combinations are valid
        // (e.g. that variables are valid, that the linear combination evals to 0 for prover, etc).
        self.constraints.push(lc);
    }
}

Which works but it changes the behaviour of the method. Certain tests (e.g. example_gadget_test()) started failing, because now the prove system does not return Err on validation, but panics earlier.


Another option would be to return Result<(), R1CSError> in fn constrain, but that alters the signature for ConstraintSystem (and thus for verifier, who will always return Ok(()) anyhow). It would also not allow to disable this behaviour on --release builds.


What kind of implementation for this TODO would you consider best? Maybe even something different from my proposals?