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

Update digest crate to 0.10 for traits-preview feature #215

Merged
merged 1 commit into from Dec 30, 2021

Conversation

neocturne
Copy link
Contributor

@neocturne neocturne commented Dec 22, 2021

Adjust to the following changes that happened in digest:

  • The crypto-mac crate has been merged into digest (with "mac" feature
    enabled)
  • Various traits have been split up
  • The Digest and Mac traits now share their update/finalize/reset
    implementations
  • The BlockInput trait was dropped without replacement apparently (as
    long as the low-level core API is not used)

Related issue: #212 (not marked as closed by this PR, as discussion regarding the use the low-level API is still ongoing)

Adjust to the following changes that happened in digest:

- The crypto-mac crate has been merged into digest (with "mac" feature
  enabled)
- Various traits have been split up
- The Digest and Mac traits now share their update/finalize/reset
  implementations
- The BlockInput trait was dropped without replacement apparently (as
  long as the low-level core API is not used)
@@ -85,8 +85,7 @@ arrayvec = { version = "0.7.0", default-features = false }
constant_time_eq = "0.1.5"
rayon = { version = "1.2.1", optional = true }
cfg-if = "1.0.0"
digest = { version = "0.9.0", optional = true }
crypto-mac = { version = "0.11.0", optional = true }
digest = { version = "0.10.1", features = [ "mac" ], optional = true }
Copy link
Member

Choose a reason for hiding this comment

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

Not a problem with this PR, but I think we have an issue where std = ["digest/std"] on line 26 unfortunately enables the digest feature all the time. So optional = true here isn't really behaving like we'd expect it to behave. Do you know of any way to solve this? Maybe we should just get rid of std = ["digest/std"]?

Copy link
Member

Choose a reason for hiding this comment

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

Oops never mind. I just played with it, and while it's true that the digest dependency does get pulled in by default (no biggie), our traits-preview feature is not enabled by default. So callers still can't rely on these trait implementations without explicitly asking for the feature, which is what we really care about for (in)stability purposes.

//! RustCrypto [`signature`] crate.)
//! RustCrypto [`digest`] crate, and re-exports those crates as
//! `traits::digest`. However, the traits aren't stable, and they're expected to
//! change in incompatible ways before those crates reach 1.0. For that reason,
Copy link
Member

Choose a reason for hiding this comment

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

s/those crates/that crate/

@@ -901,8 +899,7 @@ fn parent_node_output(
///
/// When the `traits-preview` Cargo feature is enabled, this type implements
/// several commonly used traits from the
/// [`digest`](https://crates.io/crates/digest) and
/// [`crypto_mac`](https://crates.io/crates/crypto-mac) crates. However, those
/// [`digest`](https://crates.io/crates/digest) crate. However, those
/// traits aren't stable, and they're expected to change in incompatible ways
/// before those crates reach 1.0. For that reason, this crate makes no SemVer
Copy link
Member

Choose a reason for hiding this comment

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

same

@oconnor663
Copy link
Member

oconnor663 commented Dec 30, 2021

Thank you! This will save me a lot of time looking through the change notes :)

The comment tweaks suggested above aren't blocking. Feel free to make them, or I can make them myself after I land this. (EDIT: Done with c7b5881.)

@oconnor663 oconnor663 merged commit 61d6621 into BLAKE3-team:master Dec 30, 2021
@neocturne neocturne deleted the digest-0.10 branch December 30, 2021 19:49
@newpavlov
Copy link

The BlockInput trait was dropped without replacement apparently

It was replaced with the digest::crypto_common::BlockSizeUser trait. The hmac::SimpleHmac type uses it to get the block size used in HMAC. Note that you can no longer use hmac::Hmac with BLAKE2/BLAKE3 since it now works only with hashes which expose block-level interface and consume blocks eagerly.

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

3 participants