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.

Sign up for GitHub

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

Jump to bottom

Prio3: Add support for multiple proofs #850

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Prio3: Add support for multiple proofs #850

merged 1 commit into from
Nov 28, 2023

Conversation

cjpatton
Copy link
Collaborator

@cjpatton cjpatton commented Nov 27, 2023 •
edited
Loading

Partially addresses #839.
Stacked on #851.

As of draft-irtf-cfrg-vdaf-08 it's possible to configure to Prio3 so that the Client generates (and the Aggregators verify) multiple FLPs for the same measurement. This commit adds support for this feature. While at it, this commit de-duplicates some of the code used to derive field elements from seeds.

@cjpatton cjpatton force-pushed the cjpatton/839-prio3-multiproof branch 2 times, most recently from 618e840 to bb975ce Compare November 27, 2023 22:36
@cjpatton cjpatton force-pushed the cjpatton/839-prio3-multiproof branch from bb975ce to ea0286d Compare November 27, 2023 22:39
@cjpatton cjpatton marked this pull request as ready for review November 27, 2023 23:32
@cjpatton cjpatton requested a review from a team as a code owner November 27, 2023 23:32
@cjpatton
Copy link
Collaborator Author

I'll rebase on top of #851.

@cjpatton cjpatton force-pushed the cjpatton/839-prio3-multiproof branch from ea0286d to 11b6111 Compare November 28, 2023 18:35
@cjpatton
Copy link
Collaborator Author

Rebased!

@cjpatton cjpatton changed the base branch from main to david/turboshake November 28, 2023 18:36
inahga
inahga reviewed Nov 28, 2023
View reviewed changes
src/vdaf/prio3.rs Outdated Show resolved Hide resolved
src/vdaf/prio3.rs Outdated Show resolved Hide resolved
src/vdaf/prio3.rs Outdated Show resolved Hide resolved
src/vdaf/prio3.rs Outdated Show resolved Hide resolved
@cjpatton cjpatton requested a review from inahga November 28, 2023 19:14
divergentdave
divergentdave approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@divergentdave divergentdave left a comment

Choose a reason for hiding this comment

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

Looks good! I checked benchmark performance, and all differences were within the noise threshold, so no worries there.

src/vdaf/prio3.rs Outdated Show resolved Hide resolved
Base automatically changed from david/turboshake to main November 28, 2023 19:41
inahga
inahga approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@inahga inahga left a comment

Choose a reason for hiding this comment

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

🎉

@divergentdave
Copy link
Contributor

FYI, I was working through re-enabling Prio3 test vectors, and I noticed that the Prio3SumVecWithMultiproof_0.json file was generated using an algorithm ID of 0xFFFFFFFF. That may or may not be an oversight in the spec, but note that this implementation keeps using the same Prio3SumVec algorithm ID passed through from the SumVec Type impl. (We don't need to block merging this PR for this issue, and can fix the algorithm ID or test vector file in a followup)

@cjpatton
Copy link
Collaborator Author

FYI, I was working through re-enabling Prio3 test vectors, and I noticed that the Prio3SumVecWithMultiproof_0.json file was generated using an algorithm ID of 0xFFFFFFFF. That may or may not be an oversight in the spec, but note that this implementation keeps using the same Prio3SumVec algorithm ID passed through from the SumVec Type impl. (We don't need to block merging this PR for this issue, and can fix the algorithm ID or test vector file in a followup)

We didn't intend to allocate codepoints for these, so the algorithm ID is fine. In fact, generating test vectors for these was an oversight on my part (cfrg/draft-irtf-cfrg-vdaf#315). I suggest we remove them.

@cjpatton
Prio3: Add support for multiple proofs
1d95adf 
As of draft-irtf-cfrg-vdaf-08 it's possible to configure to Prio3 so
that the Client generates (and the Aggregators verify) multiple FLPs for
the same measurement. This commit adds support for this feature. While
at it, this commit de-duplicates some of the code used to derive field
elements from seeds.
@cjpatton cjpatton force-pushed the cjpatton/839-prio3-multiproof branch from c9c4f46 to 1d95adf Compare November 28, 2023 21:00
@cjpatton
Copy link
Collaborator Author

Rebased and squashed.

@cjpatton cjpatton merged commit 08840bb into main Nov 28, 2023
6 checks passed
@cjpatton cjpatton deleted the cjpatton/839-prio3-multiproof branch November 28, 2023 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@divergentdave divergentdave divergentdave approved these changes

@inahga inahga inahga approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cjpatton @divergentdave @inahga