search

ZK-Garage / plonk

A pure Rust PLONK implementation using arkworks as a backend.
https://discord.gg/XWJdhVf37F
Mozilla Public License 2.0
294 stars 76 forks source link

PlonkUp #123

Closed davidnevadoc closed 2 years ago

davidnevadoc commented 2 years ago

The purpose of this PR is to include lookup gates to achieve a combination of the original Plonk + Plookup (known as PlonkUp). Closes #41

CPerezz commented 2 years ago

As discussed in private, I suggest to add some benchmarks to check not only the overhead that lookups add but also, the increase in size of ProverKey structure.

CPerezz commented 2 years ago

Should we go for the merge @davidnevadoc @lopeetall @bhgomes and file issues of low prio for the const-generics adventure?

lopeetall commented 2 years ago

I think so. The remaining tasks are non-trivial and should be their own PRs with their own discussions.

CPerezz commented 2 years ago

Nice! There're still some nits on my review @davidnevadoc .

Re-request one once addressed and will approve! :smile: Also, I think we should file the tech-debt and improvevent issues before closing this.

bhgomes commented 2 years ago

@LukePearson1 or whoever has the right to edit branch protection: we should not be able to merge when there are changes requested. Please select the Require approvals in the branch protection for master and put at least 2 approving reviews (I say 2 because we have a large working group and also lots of different people potentially will depend on this code). Also, make sure that Require status checks to pass before merging and Require conversation resolution before merging are selected.