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

feat: switch to version 2.0 (pre) of the signature crate #217

Merged
merged 5 commits into from
Dec 5, 2022
Merged

feat: switch to version 2.0 (pre) of the signature crate #217

merged 5 commits into from
Dec 5, 2022

Conversation

lumag
Copy link
Contributor

@lumag lumag commented Oct 30, 2022

Rework the crate to implement traits from the preview of the signature crate. Use Vec as Self::Repr type.

Note: I'm yet to see how will this impact the code in my projects. But for the evaluation I'd first need ecdsa / p384 to be updated.

cc @tarcieri @dignifiedquire

@lumag lumag marked this pull request as draft October 30, 2022 10:37
@lumag
Copy link
Contributor Author

lumag commented Oct 30, 2022

Converted to draft to prevent merging the PR with [patch.crates-io] in Cargo.toml

@lumag lumag force-pushed the sign-2.0 branch 4 times, most recently from 0cd2049 to c28a2c1 Compare October 31, 2022 16:35
@tarcieri
Copy link
Member

FYI, I'm cutting a crate prerelease here shortly: RustCrypto/traits#1145

@lumag lumag marked this pull request as ready for review October 31, 2022 16:52
@lumag
Copy link
Contributor Author

lumag commented Oct 31, 2022

@dignifiedquire @tarcieri gracious ping for the review.

tarcieri
tarcieri reviewed Oct 31, 2022
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@tarcieri
Copy link
Member

@lumag can you rebase?

@lumag
Copy link
Contributor Author

lumag commented Oct 31, 2022

@tarcieri rebased

tarcieri
tarcieri approved these changes Oct 31, 2022
View reviewed changes
Copy link
Member

@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.

This looks fine now as a first pass.

Curious what @dignifiedquire thinks about another breaking release so soon.

We should coordinate with ed25519-dalek and the https://github.com/RustCrypto/signatures crates to have signature v2.0 releases ready at the same time.

tarcieri
tarcieri reviewed Nov 7, 2022
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@lumag lumag mentioned this pull request Nov 7, 2022
Closed
@lumag lumag requested review from tarcieri and removed request for dignifiedquire November 7, 2022 22:11
@dignifiedquire
Copy link
Member

Curious what @dignifiedquire thinks about another breaking release so soon.

  • I think we should put this out as a prerelease, and only fully release it once we are sure the other crates using the signature crates are going to be updated/are already updated to avoid fragmentation and pain in upstream use cases
  • we should gather the biggest users of the library and make sure they can easily upgrade (I only know about rpgp, but expect/hope there are others out there)

@tarcieri
Copy link
Member

@dignifiedquire yeah, the plan is to coordinate with ed25519-dalek so we can release all of the major crates which use signature at the same time.

I think everything on master has been released so we could potentially merge this (after a rebase) and cut a prerelease after that

@tarcieri
Copy link
Member

tarcieri commented Nov 18, 2022 •
edited
Loading

@lumag can you rebase please?

@lumag
Copy link
Contributor Author

lumag commented Nov 18, 2022

@lumag can you rebase please?

Done

tarcieri
tarcieri reviewed Nov 18, 2022
View reviewed changes
src/pkcs1v15.rs Outdated Show resolved Hide resolved
@tarcieri
Copy link
Member

@lumag looks good other than the one nit about the RNG trait bounds

@lumag
feat: switch to version 2.0 (pre) of the signature crate
b74c0d8 
Rework the crate to implement traits from the preview of the signature
crate. Use Vec<u8> as Self::Repr type.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
@lumag
feat: switch to implementing the Keypair trait
1a225a0 
Drop the hand-crafted From traits, replacing them with the
implementation of the Keypair trait.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
@lumag
Cargo.toml: bump version to -pre
840583f 
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
@lumag
feat: drop the hazmat guarding feature
da78856 
Now that the signature's hazmat-preview feature has matured, enable
corresponding interfaces by default and drop the 'hazmat' feature.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
@lumag
feat: allow generating random SigningKey's
1d34c81 
Allow generating random SigningKey structs directly.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
@lumag
Copy link
Contributor Author

lumag commented Nov 18, 2022 via email

@tarcieri
Copy link
Member

tarcieri commented Dec 1, 2022 •
edited
Loading

The PRs for updating the dsa, ecdsa, ed25519, ed25519-dalek, and k256/p256/p384 crates have all landed.

This is the only remaining one. I'd suggest we merge this soon so we can further iterate on signature 2.0 work.

@tarcieri tarcieri merged commit b39752d into RustCrypto:master Dec 5, 2022
@tarcieri tarcieri mentioned this pull request Jan 16, 2023
Merged
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

@dignifiedquire dignifiedquire Awaiting requested review from dignifiedquire

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lumag @tarcieri @dignifiedquire