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

Align draft with POC for multiproof changes #305

Merged
merged 1 commit into from
Nov 4, 2023

Conversation

albertpl
Copy link
Contributor

Update draft per multiproof POC

  • Add new Prio3 specific parameter, i.e. Prio3.PROOFS
  • Update Prio3 methods for sharing, preparation, aggregation to align with POC codes.

draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@cjpatton cjpatton left a comment

Choose a reason for hiding this comment

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

Great start! I still need to double check that the reference code was copied properly (will do so in my next pass). In the meantime, the text needs a little work:

  1. I think we need a gentler introduction to the idea of multiple proofs. In the overview section, I think we should start by describing things in terms of a single proof, then have a paragraph where we describe the multi-proof feature and, just as importantly, why we need it
  2. Terms like "proofs share" and "verifiers share" make sense as variable names in code, but they don't read very well in English. My advice would be to read through the Prio3.
  3. We're going to need a section in Security Considerations about how changing PROOFS impacts security, in particular when we go for a smaller field for a given circuit. Feel free to leave as a TODO for now, but we should at least stub it out so we remember to get to it later.

draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
@albertpl albertpl force-pushed the draft-prio3-multiproof branch from 3a5a21c to 6572a2b Compare October 27, 2023 22:12
@albertpl albertpl requested a review from cjpatton October 27, 2023 22:15
Copy link
Collaborator

@cjpatton cjpatton left a comment

Choose a reason for hiding this comment

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

Looking good, almost there.

draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
@@ -4561,6 +4621,10 @@ We also stress that even if the Idpf is not extractable, Poplar1 guarantees
that every client can contribute to at most one prefix among the ones being
evaluated by the helpers.

## Considerations for multiple proofs (`PROOFS`) {#security-multiproof}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a newline between the title and the first paragraph

Suggested change
## Considerations for multiple proofs (`PROOFS`) {#security-multiproof}
## Considerations for multiple proofs (`PROOFS`) {#security-multiproof}

@@ -4561,6 +4621,10 @@ We also stress that even if the Idpf is not extractable, Poplar1 guarantees
that every client can contribute to at most one prefix among the ones being
evaluated by the helpers.

## Considerations for multiple proofs (`PROOFS`) {#security-multiproof}
> TODO on how changing `PROOFS` ({{multiproofs}}) impacts security, in particular when we go
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • "TODO"s should be imperative, similarly to how we write commit messages. (E.g., TODO Add guidance for ..."
  • Add a reference to the open issue.
  • Break lines at 80 characters.

@albertpl albertpl force-pushed the draft-prio3-multiproof branch from 7ece83c to f1e0533 Compare October 31, 2023 16:12
@albertpl albertpl requested a review from cjpatton October 31, 2023 16:18
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-vdaf.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@cjpatton cjpatton left a comment

Choose a reason for hiding this comment

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

Thank you! Please squash and I'll merge.

@albertpl albertpl force-pushed the draft-prio3-multiproof branch from d8fc8fe to 974e24c Compare November 3, 2023 01:59
@cjpatton cjpatton merged commit ccbb6a0 into cfrg:main Nov 4, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants