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

Remove ring dependency #161

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Remove ring dependency #161

merged 1 commit into from
Nov 15, 2023

Conversation

xfbs
Copy link
Contributor

@xfbs xfbs commented Nov 3, 2023

This PR removes the ring crate, and replaces it with the sha2 crate from the awesome RustCrypto ecosystem.

Pros:

  • The ring crate is quite heavyweight, it is basically a replacement for the likes of OpenSSL and BoringSSL. If we only need to hash data with a specific algorithm (this is what we use it for), then it should make a lot more sense to import a crate that only does that algorithm, rather than importing the swiss army knife but only using the toothpick.
  • The ring crate uses C (ported from BoringSSL) to implement a lot of things. Not really terrible, it is well-tested. I have run into an issue before where it would not build on RISC-V.
  • I love using things that are written in pure Rust, which sha2 is.

Cons:

  • We have a dependency on rustls, which uses the ring crate anyways.
  • Normally, throwing out ring should mean a reduction in binary size, but due to our dependency on rustls this is not the case. I have tested it using cargo bloat and there is no meaningful difference.

Feel free to close this PR if we don't agree that it makes sense!

Cheers!

@xfbs xfbs added complexity::low Issues or ideas with a low implementation cost component::cli Everything related to the buffrs cli type::idea Rough idea or proposal for buffrs priority::low Please dont work on this if you can contribute something with a higher priority labels Nov 3, 2023
@xfbs xfbs requested review from asmello and mara-schulke November 3, 2023 08:35
@xfbs xfbs self-assigned this Nov 3, 2023
@xfbs xfbs merged commit 7cd372d into main Nov 15, 2023
7 checks passed
@xfbs xfbs deleted the pe/remove-ring branch November 15, 2023 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity::low Issues or ideas with a low implementation cost component::cli Everything related to the buffrs cli priority::low Please dont work on this if you can contribute something with a higher priority type::idea Rough idea or proposal for buffrs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants