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

feat: Separate aggregation protocol #2736

Merged
merged 6 commits into from
Oct 10, 2023
Merged

Conversation

ledwards2225
Copy link
Contributor

@ledwards2225 ledwards2225 commented Oct 7, 2023

This PR moves the Goblin ECC op queue transcript aggregation protocol from the main Honk protocol to its own separate mini-protocol, referred to as "Merge" (based on Zac's original Goblin doc). Zac pointed out that this was likely the right approach once we go to incorporate folding.

This is also a necessary step for completing integration of ZeroMorph (and deprecation of Gemini/Shplonk) because the univariate evaluation claims related to this merge protocol would have otherwise needed to be incorporated via Shplonk.

This work automatically resolves one of the issues previously described in bberg 723 related to the size of the transcript polynomials being tied to the size of the present circuit.

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

  • If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
  • I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
  • Every change is related to the PR description.
  • I have linked this pull request to relevant issues (if any exist).

@ledwards2225 ledwards2225 self-assigned this Oct 7, 2023
@ledwards2225 ledwards2225 changed the title Lde/aggregation protocol feat: Separate aggregation protocol Oct 7, 2023
@ledwards2225 ledwards2225 force-pushed the lde/aggregation_protocol branch from 7cf2ebf to 492fee9 Compare October 9, 2023 16:04
@ledwards2225 ledwards2225 requested a review from Rumata888 October 9, 2023 16:06
@ledwards2225 ledwards2225 marked this pull request as ready for review October 9, 2023 22:09
auto honk_verified = construct_and_verify_honk_proof(composer, builder);
EXPECT_TRUE(honk_verified);

// Construct and verify Goblin ECC op queue Merge its proof
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you want to write "Merge its proof"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected, thanks

Copy link
Contributor

@Rumata888 Rumata888 left a comment

Choose a reason for hiding this comment

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

A very clean PR and easy to understand, thank you. I think there is a small typo, but that can be just merged with another PR

@ledwards2225 ledwards2225 force-pushed the lde/aggregation_protocol branch from 492fee9 to 3857a65 Compare October 10, 2023 14:00
@ledwards2225 ledwards2225 merged commit ad16937 into master Oct 10, 2023
2 checks passed
@ledwards2225 ledwards2225 deleted the lde/aggregation_protocol branch October 10, 2023 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants