scipr-lab / libsnark

C++ library for zkSNARKs
Other
1.81k stars 579 forks source link

Convert assertions to exceptions #64

Open tromer opened 7 years ago

tromer commented 7 years ago

Currently libsnark makes generous use of assert()s.

Any abnormal condition that may be triggered by bad external inputs should cause an (informative) exception, not an assert.

Likewise for abnormal conditions that can be triggered by violation of supposed invariants/preconditions/postconditions within the libsnark code, if those cannot be convincingly argued and maintained.

Because of their catastrophic failure mode, we should use assertions only for invariants/preconditions/postconditions that we can convincingly argued will always hold and be maintained.

(Common wisdom also says assertions should be used when the check is expensive, because they can be removed in release mode. But in practice, it seems everybody is being conservative and uses libsnark with assertions enabled.)

Tasks:

  1. Decide on an assert convention (typing, information strings, etc.)
  2. Convert asserts to throws, by the above guideline
  3. Make sure our code is exception-safe in the superficial sense of memory leaks, locks etc.
  4. Analyze and document how exception-safe are different classes, in terms of semantics (e.g., for an object of class foo, if one of its method raised an exception, is it safe to continue using it?)
  5. In pertinent high-level functions (e.g., main()), add a default exception handler that informatively displays caught exceptions and aborts.

1+5 should make sure we don't lose valuable debug information (what's the error? what's the stacktrace?) compared to the current assertions.

One complication is the OpenMP parallelized loops. What's the right way to handle exceptions in these? We can do a catch within the parallelized loop, store the exception info into variables, skip the payload of remaining iterations via continue, and when out of the loop - throw a new exception (losing type info...). That's tricky and tedious. Ditto for critical sections. Any better patterns?

mvdbos commented 4 years ago

@tromer , are there any plans to move forward with this? Currently, this assertion terminates proof generation when a gadget I use receives invalid input: https://github.com/scipr-lab/libsnark/blob/master/libsnark/reductions/r1cs_to_qap/r1cs_to_qap.tcc#L216

Is there any way to get around this? I would expect that proof generation would succeed, and just result in an invalid proof.