Add batching commit support

[veljkovranic]

This PR has been in the works for months now.

The CI should be passing as well.

The batch committing to the multiple polynomials is a requirement for ZK WHIR

[veljkovranic]

Moving the review conversation from Slack to Github for visibility:

@WizardOfMenlo comment:

We are doing something weird?
Why are we first committing to rt_1, ..., rt_n and then (after the batching randomness) doing another commitment to the RLC?
We should instead commit (before the batching randomness) to a single MT rt with leaves of size n instead.

@yswami-tfh response:

I think Giacomo has a more general model in mind for batching. The current code is sufficient for ZK. (Supporting the more general model will require substantial refactoring.)
The current implementation assumes that the user has f_1, f_2, …, f_n polynomials and the weight for each polynomial is the same, say w(x1, … , x_n).
The code works as follows:
Computes NTT of each f1, …, fn and commits the Merkle root R1, …, Rn separately.
It then samples batching_randomness (let’s call it B.)
It then computes the random linear combination (RLC) of f_i’s as h := f_1 + B*f_2 + B^2*f_3 + …
It then runs the normal WHIR sumcheck over h(x_1,…,x_m)*w(x_1,…,x_m)
On the verifier side, since each polynomial is committed individually in the first round, the verifier checks the consistency by using individual Merkle tree.
Supporting the more general use case where weight are different for different f_i will requires some changes to last round of WHIR and requires major refactoring efforts.

Lets continue the review in this PR, for the sake of visibilitiy

[WizardOfMenlo]

Supporting the more general use case where weight are different for different f_i will requires some changes to last round of WHIR and requires major refactoring efforts.

I don't really care about the case where the f_i have different weights, I am happy with testing the RLC at the same weight. My objection is in how f_1, ..., f_n are committed.
Roughly speaking, there are two approaches that make sense (and there is a tradeoff):

• commit f_1, ..., f_n as a single Merkle tree with leaves of size n, absorb the root and squeeze batching randomness and commit to the RLC f^* of f_1, ..., f_n as a Merkle tree with leaves of size 2^k (note you could also domain shift this). Then sample x_1, ..., x_t and add as claims when testing f^* that f^*(x_i) = RLC((f_1, ..., f_n), x_i). In terms of queries to the initial oracles, you read something like t * (n + 2^k) field elements and 2t authentication paths (one for each tree).
• commit f_1, ..., f_n as a single Merkle tree with leaves of size n * 2^k, absorb the root and squeeze batching randomness, and whenever you query this tree you first RLC and the do the other operation. This requires reading t *n * 2^k field elements and t authentication paths.

Since in our case n = 2, probably the second option is the most beneficial. The one currently implement commits to (n + 1) Merkle trees with leaves 2^k which leads to reading t * n * 2^k field elements and (n+1) * t authentication paths, which is strictly worse than either of the above.

[yswami-tfh]

I agree, option 2 is far more efficient, but not sure if anyone has free cycles to implement it.

@veljkovranic The high level idea is that right now commit_batch calls commit_single which computes both the NTT as well as separate Merkle trees for each batching polynomial. Instead, it will be better for commit_single to just compute the NTTs and then commit_batch can concatenate all the NTT from all the polynomials to create a single Merkle tree. (Most likely, it will also require changes to the transcript and verifier's consistency checks in the Verifier code.)

[veljkovranic]

@recmo what is your opinion here, how should we proceed from here?

Should I implement this on top of the codebase that we currently have, or do we want to leave it for the planned rewrite of WHIR?

[WizardOfMenlo]

Left a few comments, happy to merge after some cleanpu

[WizardOfMenlo]

How about just changing the add_ood to take as input batch size?

[WizardOfMenlo]

Why change to ProverFSState? Keep as ProverState for consistency

[veljkovranic]

yeah, good catch, this is just too old of a branch :D