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

Upgrade x25519-dalek to 2.0.0-pre.1 #81

Closed
wants to merge 1 commit into from

Conversation

jplatte
Copy link
Contributor

@jplatte jplatte commented Aug 11, 2022

The changes from the currently-used version are trivial, but they're required to upgrade to the latest version of zeroize in dependent crates.

@jplatte
Copy link
Contributor Author

jplatte commented Aug 11, 2022

Oh, looks like rand is part of the public API of that crate. I'll look into updating rand usage.

@poljar
Copy link
Collaborator

poljar commented Aug 11, 2022

It is, also that tag/release has been in the pre state for quite a while: https://github.com/dalek-cryptography/x25519-dalek/releases/tag/2.0.0-pre.1.

@jplatte
Copy link
Contributor Author

jplatte commented Aug 11, 2022

Yeah it has, but I think we're better off with a release that's stable¹ but declared "pre" rathen than a proper release that's constantly causing issues (albeit only at build time).

¹ I would declare it as such based on the trivial diff from the stable release

@poljar
Copy link
Collaborator

poljar commented Aug 11, 2022

What kind of issues are we talking about?

@jplatte
Copy link
Contributor Author

jplatte commented Aug 11, 2022

Not being able to upgrade other dependencies because their latest version requires zeroize >=1.4. Also cargo outdated doesn't work inside the matrix-sdk workspace, probably because of one such dependency.

@jplatte
Copy link
Contributor Author

jplatte commented Aug 11, 2022

sigh looks like we'd also need to use dalek-cryptography/ed25519-dalek#160 (through our own fork, I guess). I would still prefer that to the current state of things, but I understand if you don't want to go that route. Let me know if I should continue.

@poljar
Copy link
Collaborator

poljar commented Aug 11, 2022

sigh looks like we'd also need to use dalek-cryptography/ed25519-dalek#160 (through our own fork, I guess). I would still prefer that to the current state of things, but I understand if you don't want to go that route. Let me know if I should continue.

I think curve25519-dalek will need to get the same treatment.

I would let that simmer a bit longer, maybe something happens: WebAssembly/wasi-crypto#63.

@jplatte
Copy link
Contributor Author

jplatte commented Aug 11, 2022

curve25519-dalek seems to also have a pre-release using the latest version of rand_core. So we'd only need to have one fork.

@jplatte
Copy link
Contributor Author

jplatte commented Aug 11, 2022

Alright, let's see where this goes: RustCrypto/elliptic-curves#497 (comment)

I'll close this for now.

@jplatte jplatte closed this Aug 11, 2022
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.

None yet

2 participants