Plonk: error handling

[vesselinux]

Issue description (bullet 8): When throwing an exception, it should not be up to the function that throws it to decide how it should be handled. Catch and handle must be done by the caller instead, who may wish to handle it in some way other than exiting the process, like retrying. See #49 (comment) , #49 (comment) , #49 (comment)

[vesselinux]

I am facing the following problem addressing this issue:

Consider https://github.com/clearmatics/libsnark/blob/plonk/libsnark/zk_proof_systems/plonk/prover.tcc#L446-L453

    try {
        bool b_zero_remainder = libfqfft::_is_zero(remainder);
        if (!b_zero_remainder) {
            throw std::logic_error("Non-zero remainder in polynomial division");
        }
    } catch (const std::logic_error &e) {
        std::cout << "Error: " << e.what() << "\n";
    }

I transform this into

    try {
        bool b_zero_remainder = libfqfft::_is_zero(remainder);
        if (!b_zero_remainder) {
            throw std::logic_error("Non-zero remainder in polynomial division");
        }
    }

Next I catch the exception in the caller https://github.com/clearmatics/libsnark/blob/plonk/libsnark/zk_proof_systems/plonk/prover.tcc#L1039-L1040 where instead of

    round_three_out_t<ppT> round_three_out = plonk_prover::round_three(
        alpha, beta, gamma, round_zero_out, round_one_out, round_two_out, srs);

I do

    round_three_out_t<ppT> round_three_out;
    try {
        round_three_out = plonk_prover::round_three(
            alpha,
            beta,
            gamma,
            round_zero_out,
            round_one_out,
            round_two_out,
            srs);
    } catch (const std::logic_error &e) {
        std::cout << "Error: " << e.what() << "\n";
    }

As can be seen, for the above to work we need to declare an empty constructor for the struct round_three_out_t. However this opens the door to having uninitialized instances of this struct, which is not desirable.

Do you have a better suggestion how to catch the exception by the caller without having the above problem?

[dtebbs]

As discussed elsewhere, please remove all try and catch from this code (unless there is some reason you need to handle an error). The user of the plonk code will determine what to do when there is an exception, so it is them who should catch.

Regarding the first transformation, you appear to have a try with no catch.

Regarding the problem of scope and the variable initialization (if you hypothetically wanted to catch exceptions here, which I don't think we do) you would probably want to enclose a larger block of code in the try and deal with any exception at any step.

Let's discuss if this doesn't mkae sense or if I've missed something.

[vesselinux]

Yes, that makes sense. Thanks for the clarification!

I'll remove all try-catch clauses then and I'll just be throwing exceptions when needed.

For example, https://github.com/clearmatics/libsnark/blob/plonk/libsnark/zk_proof_systems/plonk/prover.tcc#L446-L453

    try {
        bool b_zero_remainder = libfqfft::_is_zero(remainder);
        if (!b_zero_remainder) {
            throw std::logic_error("Non-zero remainder in polynomial division");
        }
    } catch (const std::logic_error &e) {
        std::cout << "Error: " << e.what() << "\n";
    }

will be replaced by

        bool b_zero_remainder = libfqfft::_is_zero(remainder);
        if (!b_zero_remainder) {
            throw std::logic_error("Non-zero remainder in polynomial division");
        }

[vesselinux]

That change was applied and committed.