-
Notifications
You must be signed in to change notification settings - Fork 232
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
Add support for Ristretto255 #646
base: main
Are you sure you want to change the base?
Conversation
We need some docs for the ristretto class, but this approach generally looks correct. |
Just a ping to see if there's a desire to finish the docs here so we can merge. |
There is and it's on my ToDo short list :-) |
I'd like to have this feature land in PyNaCl 1.5.0 so I'm holding that release a bit hoping we can get this in there first 😄 |
(Ping) |
08f49e1
to
2f658ff
Compare
Pong! |
61b8b13
to
2d0f7e9
Compare
Readthedocs needs a version update. Then docs build too: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small comments and one big one: Why does Ristretto255Scalar allow native Python types to do scalar math in the field? From my perspective that seems to have very little advantage in exchange for a lot of dangerous complexity. Do you have a specific use case that makes this desirable?
return ffi.buffer(z, crypto_core_ristretto255_SCALAR_BYTES)[:] | ||
|
||
|
||
if has_crypto_core_ristretto25519: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not put this up with the other check?
crypto_core_ristretto255_SCALAR_BYTES = 0 | ||
crypto_core_ristretto255_NONREDUCED_SCALAR_BYTES = 0 | ||
|
||
if has_crypto_core_ristretto25519: # pragma: no branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't need a pragma since we do coverage on both minimal and non-minimal builds, did you see a coverage issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I run tox
, minimal build isn't checked. How should I run tox to check both?
I feel that it's actually useful. But looking at the examples that I already wrote, somehow I don't use it. |
I'm pretty negative on allowing other Python types to interact via infix operators with points (or scalars) absent a compelling counterargument. Why not just have users create additional points and use those points to perform mathematical operations? |
This adds support for https://doc.libsodium.org/advanced/point-arithmetic/ristretto / https://ristretto.group/
I'll finish the docstrings and add documentation once the public interface is stable.