proof-systems icon indicating copy to clipboard operation
proof-systems copied to clipboard

[easy] Changing array of 2 (zeta and omegaz) for `ConsecutiveEvals` inside `ProverProof`

Open querolita opened this issue 2 years ago • 2 comments

This PR addresses a TODO comment that was suggesting to create a type for the evals fiend inside ProverProof.

~~Because we were using the tuple structure to iterate over the zeta and omegazeta evaluations, I created a method that returns the array version of the struct. I don't love it, but I thought rephrasing the prover would be less concise.~~

I defined the type generically enough to be instantiated with single fields or vectors of fields. This is useful to reuse the combine() method of ProofEvaluations to replace 25 loc in prover.rs. ConsecutiveEvals is now used more in the code, substituting tuples of ProofEvaluations. This includes the methods inside expr.rs. I created three constants representing indices inside the struct (0 for zeta, 1 for zetaomega). Implemented Index for this type, because I didn't want to change the logics inside evaluate() for Variable.

Any suggestions are welcome.

querolita avatar Jun 08 '22 14:06 querolita

I think it's a good idea to use a strong type instead of a vec/array, but if we do use a strong type I think we should go all the way and avoid just iterating on it like it's an array :o

Also, I think there should be an associated PR on the mina side to make sure this works

mimoo avatar Jun 08 '22 15:06 mimoo

Answering @mimoo , I will write the mina side as soon as the mina side for RecursionChallenge is merged. I am having issues building the repo, so it is taking me a while to configure everything. In the meantime I can think of refactors for the prover code without relying so much on arrays.

querolita avatar Jun 08 '22 15:06 querolita

Stale pull request message

github-actions[bot] avatar Aug 22 '22 07:08 github-actions[bot]