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

Test P-521 #349

Merged
merged 3 commits into from
Nov 16, 2023
Merged

Test P-521 #349

merged 3 commits into from
Nov 16, 2023

Conversation

daxpedda
Copy link
Contributor

@daxpedda daxpedda commented Nov 10, 2023

This adds testing of P-521 through the p521 crate.

Still requires:

See #290 for a previously similar addition.
See facebook/voprf#127 for the same PR in voprf, which isn't required to merge this.

@daxpedda
Copy link
Contributor Author

CI fails because of RustCrypto/elliptic-curves#965, which we are going to wait for anyway.

@daxpedda
Copy link
Contributor Author

This is ready to merge now.

@kevinlewi
Copy link
Contributor

It seems like the maintenance for full_test.rs is getting more and more cumbersome now... any thoughts for how we can make this simpler?

And maybe for now, can you also add (in a comment above each new set of constants in full_test.rs) instructions for the cargo command to re-generate those constants?

@daxpedda
Copy link
Contributor Author

Last two commits should address both concerns.
WDYT?

Copy link
Contributor

@kevinlewi kevinlewi left a comment

Choose a reason for hiding this comment

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

Oh wow, this is really awesome -- thank you for that refactor! Looks good to me

@kevinlewi kevinlewi merged commit 56cdf9a into facebook:main Nov 16, 2023
57 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.

None yet

3 participants