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

Deprecate BASEPOINT_ORDER from pub API consts #581

Merged
merged 4 commits into from
Sep 18, 2023
Merged

Deprecate BASEPOINT_ORDER from pub API consts #581

merged 4 commits into from
Sep 18, 2023

Conversation

pinkforest
Copy link
Contributor

Deprecates BASEPOINT_ORDER from pub API constants

Redirect internally to avoid deprecation errors

tarcieri
tarcieri reviewed Sep 12, 2023
View reviewed changes
curve25519-dalek/src/constants.rs Outdated
@@ -94,6 +78,9 @@ pub const BASEPOINT_ORDER: Scalar = Scalar {
],
};

#[allow(deprecated)]
pub(crate) const BASEPOINT_ORDER_PRIVATE: Scalar = BASEPOINT_ORDER;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe reverse this relationship so BASEPOINT_ORDER_PRIVATE defines the constant and BASEPOINT_ORDER points to BASEPOINT_ORDER_PRIVATE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ofcourse, thanks 💜

tarcieri
tarcieri requested changes Sep 12, 2023
View reviewed changes
Copy link
Contributor

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

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

It seems like there are a lot of unrelated changes in this PR now?

@pinkforest pinkforest force-pushed the deprecate-exposed-consts branch from 2b07c1d to 0218b77 Compare September 12, 2023 17:43
@pinkforest
Copy link
Contributor Author

Fixed - Forgot to rebase

@pinkforest pinkforest mentioned this pull request Sep 12, 2023
Merged
@rozbb rozbb merged commit 533b53a into dalek-cryptography:main Sep 18, 2023
@marshallpierce
Copy link

marshallpierce commented Oct 10, 2023 •
edited
Loading

I have a use for this constant (I need to exponentiate a scalar by order - 2) -- would it be objectionable to un-deprecate this? Otherwise I'll need to construct my own copy of a constant, which would be a shame.

Edit: I found another way, never mind!

@trevor-crypto
Copy link

@marshallpierce what was the solution? 🤔 Just making your own constant?

@marshallpierce
Copy link

It turned out I was doing curve operations in a needlessly complex way. I don't have the details in front of me, but tl;dr I found a way to do what I needed to do using other public parts of the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tarcieri tarcieri tarcieri 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.
5 participants
@pinkforest @marshallpierce @trevor-crypto @tarcieri @rozbb