Skip to content

Don't implicitly enable zeroize for ed25519-dalek alloc feature #761

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

Merged
merged 1 commit into from
Jul 7, 2025

Conversation

stevefan1999-personal
Copy link
Contributor

I tried to compile ed25519-dalek using the new https://github.com/Rust-GPU/Rust-CUDA reboot and they mentioned they fixed a lot of issue, and I tried to include curve25519-dalek and it does miraculously compile, except it has to be without zeroize which is complaining about atomic fence not supported on PTX, and I have to use the equivalent intrinsic in cuda_std instead.

As a quick demo for an internal finite field arithmetic + SIMD project and I rather need performance, I don't need zeroize at all. So a quick solution is to just disable it.

Anyway, I noticed with ed25519-dalek, when you enable the alloc feature, zeroize is implicitly enabled. This PR just add a single character to fix that

@rozbb
Copy link
Contributor

rozbb commented May 28, 2025

Thank you! @tarcieri is this technically breaking? It could make existing code cease to zeroize structs

@stevefan1999-personal
Copy link
Contributor Author

https://doc.dalek.rs/merlin/index.html
Uhhhh you also need to fix merlin for that...

@stevefan1999-personal
Copy link
Contributor Author

wait...did someone took over merlin crate? despite it is for crypto but it is not from dalek...

@tarcieri
Copy link
Contributor

@tarcieri
Copy link
Contributor

@rozbb it might technically be considered breaking, yes. Would be safer to do at a major version boundary

@rozbb
Copy link
Contributor

rozbb commented May 29, 2025

Ok agreed. I labeled this so we can come back to it next major release

@rozbb rozbb merged commit 0736088 into dalek-cryptography:main Jul 7, 2025
14 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.

3 participants