R1CS: benchmark with `bench_function_over_inputs`

[rubdos]

Hi! Cool work on R1CS! Experiments only, I promise :-)

I was looking at the benchmarks, and noticed it was run for multiple values without using criterion's bench_function_over_inputs. Is there a particular reason for this? If not, I can send a PR that refactors that.

[rubdos]

I also noticed the ConstraintSystem bindings are not mutable in the benchmarks, which means the benchmarks do not run in their current state. Shall I patch that too?

[cathieyun]

There's no good reason for why I used bench_function instead of bench_function_over_inputs, thanks for the catch.

Can you explain what you mean by "the ConstraintSystem bindings are not mutable in the benchmarks" please?

[rubdos]

diff --git a/benches/r1cs.rs b/benches/r1cs.rs
index 5b3711d..de30359 100644
--- a/benches/r1cs.rs
+++ b/benches/r1cs.rs
@@ -144,7 +144,7 @@ impl KShuffleGadget {
             .map(|v| prover.commit(*v, Scalar::random(&mut blinding_rng)))
             .unzip();

-        let cs = prover.finalize_inputs();
+        let mut cs = prover.finalize_inputs();

         Self::fill_cs(&mut cs, &input_vars, &output_vars);

@@ -178,7 +178,7 @@ impl KShuffleGadget {
             .map(|commitment| verifier.commit(*commitment))
             .collect();

-        let cs = verifier.finalize_inputs();
+        let mut cs = verifier.finalize_inputs();

         Self::fill_cs(&mut cs, &input_vars, &output_vars);

Self::fill_cs(&mut cs ...) mutually borrows non-mutable bindings. I find it weird that this somehow passes tests?

[rubdos]

To elaborate more:

error[E0596]: cannot borrow immutable local variable `cs` as mutable
   --> benches/r1cs.rs:149:28
    |
147 |         let cs = prover.finalize_inputs();
    |             -- help: make this binding mutable: `mut cs`
148 |
149 |         Self::fill_cs(&mut cs, &input_vars, &output_vars);
    |                            ^^ cannot borrow mutably

error[E0596]: cannot borrow immutable local variable `cs` as mutable
   --> benches/r1cs.rs:183:28
    |
181 |         let cs = verifier.finalize_inputs();
    |             -- help: make this binding mutable: `mut cs`
182 |
183 |         Self::fill_cs(&mut cs, &input_vars, &output_vars);
    |                            ^^ cannot borrow mutably

[rubdos]

I've fixed this in 4fde6f3

[cathieyun]

Makes sense, thanks for catching that! The PR that we've been working on merging into develop (https://github.com/dalek-cryptography/bulletproofs/pull/232) fixes the mutable constraint system issue, but using bench_function_over_inputs is still a helpful change.

I merged in https://github.com/dalek-cryptography/bulletproofs/pull/241.