Skip to content

Conversation

@ankeleralph
Copy link
Contributor

This PR fixes issue #82 by batching multiple client queries for the DLEQ ZKPs following the pseudo code from draft-irtf-cfrg-voprf.

@ankeleralph ankeleralph requested a review from a team as a code owner February 17, 2023 09:19
Run `cargo fmt` to clean up whitespace and make ci happy.
Copy link
Contributor

@rillian rillian left a comment

Choose a reason for hiding this comment

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

Not commenting on the algorithm, here, just some suggestions for code cleanup. I ran cargo fmt to address ci lints from there. Please address the cargo clippy lints. I've added comments near a few which hopefully explain a bit what's going on with the lint. If you don't want the practice, let me know and I can address them quickly. You can also use cargo clippy --fix or the fix action in your editor if you have that set up.

The rust String type is explicitly for UTF-8 text and can't be used for constructing binary data. Look for equivalents of python list methods rather than format string equivalents, convenient as they are!

Copy link
Member

@claucece claucece left a comment

Choose a reason for hiding this comment

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

It looks good! Need some changes and testing ;)

rillian
rillian previously approved these changes Mar 6, 2023
Copy link
Contributor

@rillian rillian left a comment

Choose a reason for hiding this comment

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

Thanks for addressing earlier comments, @ankeleralph . I think it's ready to go as far as the implementation with the remaining nits addressed. That just leaves the question from @claucece about whether the ristretto mod is necessary.

Copy link
Member

@claucece claucece left a comment

Choose a reason for hiding this comment

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

I think we can simplify this PR by quite a lot. There is a lot of code repetition here. We can basically use the same code of the proof generation without batching (https://github.com/brave/sta-rs/blob/main/ppoprf/src/ppoprf.rs#L48) and the verification of the proof without batching (https://github.com/brave/sta-rs/blob/main/ppoprf/src/ppoprf.rs#L71). Perhaps, even just extending those functions to take a param to batch when it is needed. Else, the two places are basically doing the same but calling different functions. Happy to have a call to explain.

@ankeleralph ankeleralph changed the title Draft: Batch ZK proofs for PPOPRF Batch ZK proofs for PPOPRF Mar 9, 2023
Copy link
Member

@claucece claucece left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@ankeleralph ankeleralph merged commit b087013 into main Apr 11, 2023
@ankeleralph ankeleralph deleted the batch-dleq-proofs branch April 11, 2023 10:32
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.

4 participants