Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protogalaxy based IVC #123

Merged

Conversation

winderica
Copy link
Contributor

@winderica winderica commented Jul 5, 2024

Depends on #120.

This is an implementation of the IVC scheme based on Protogalaxy, which is compiled from Protogalaxy "the folding scheme" by leveraging CycleFold.

Specifically, this PR implements:

  1. The augmented circuit AugmentedFCircuit built upon AugmentationGadget::prepare_and_fold_{primary, cyclefold}.
  2. The ProtoGalaxy struct that satisfies the FoldingScheme trait.
  3. The integration with CycleFold, in both the augmented circuit and the prove_step method, for off-loading EC operations to the secondary curve.
  4. The RelaxedR1CS trait converted from the original RelaxedR1CS struct, which can now check satisfiability of witness-instance pairs for both Nova and ProtoGalaxy.

Note that there are still two features missing in ProtoGalaxy-based IVC, but I will try to implement them in the incoming PRs.

  1. Enable multi-instances folding in IVC.P
  2. Support hiding commitments

This PR wouldn't be possible without the current Nova-based IVC. Thanks a lot for the amazing work from all of you 😇!

@winderica winderica mentioned this pull request Jul 8, 2024
13 tasks
@winderica winderica marked this pull request as ready for review July 23, 2024 21:56
@winderica winderica force-pushed the protogalaxy-based-ivc branch 2 times, most recently from 08d92cd to 21fd21e Compare August 5, 2024 15:07
@CPerezz
Copy link
Member

CPerezz commented Aug 6, 2024

@winderica is this indeed ready for review? If so, i'll go for it!

@winderica
Copy link
Contributor Author

@CPerezz Yes it is! Let's go ahead 😄

Copy link
Collaborator

@arnaucube arnaucube left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!! Sorry for the delay in the review, lately I've been a bit time-constrained and wanted to devote enough time to this PR since this is a big PR.

Nice implementation, also nice updates to the arith package make a lot of sense, also the differentiation between the running and incoming instances in ProtoGalaxy.

Left a question about the CycleFold instances logic which I'm not sure on, but very great work overall!

folding-schemes/src/folding/protogalaxy/folding.rs Outdated Show resolved Hide resolved
folding-schemes/src/folding/protogalaxy/folding.rs Outdated Show resolved Hide resolved
folding-schemes/src/folding/protogalaxy/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused with the list of things that are pub and exposed.
I think we should expose only ProtoGalaxy and the minimal associated stuff to it.

Otherwise, it would be complex for the user to know what to use.

For the rest, AWESOME WORK!!! 👏

folding-schemes/src/folding/protogalaxy/circuits.rs Outdated Show resolved Hide resolved
folding-schemes/src/folding/protogalaxy/circuits.rs Outdated Show resolved Hide resolved
folding-schemes/src/folding/protogalaxy/mod.rs Outdated Show resolved Hide resolved
folding-schemes/src/frontend/utils.rs Outdated Show resolved Hide resolved
@winderica
Copy link
Contributor Author

Hey, sorry for being inactive for a while, and thanks for the detailed comments @CPerezz!
Unfortunately I have had a lot to handle recently, but I will be back in about a week to address the comments!

@CPerezz
Copy link
Member

CPerezz commented Sep 1, 2024

@winderica take all the time you need. I can try to keep rebasing your branch such taht you don't have a ton of work in the future.

Copy link
Collaborator

@arnaucube arnaucube left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Very nice work, also the several abstractions and generalizations added make a lot of sense and are great improvements! Great work!

(there are some git-conflicts due recent updates in the main branch, but once they're fixed from my side we're good to merge ^^)

Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Awesome work!

@arnaucube arnaucube added this pull request to the merge queue Sep 12, 2024
Merged via the queue into privacy-scaling-explorations:main with commit 1322767 Sep 12, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants