The digest traits were introduced recently (#1078) and haven't seen much use yet. While working on #1493 we noticed several issues of the traits. One issue, that finish didn't consume self (#1496) is fixed in #1493. However, there are more problems with the APIs.
Digest traits take const OUTPUT_LEN:usize as input generic parameter
Because the OUTPUT_LEN is a const generic parameter to the trait, there could be multiple implementations of the trait for different lengths for one hasher type. I.e., that following would be possible:
struct Sha3_224;
pub trait DigestIncremental<const OUTPUT_LEN: usize> {
fn finish(state: &mut (), digest: &mut [u8; OUTPUT_LEN]);
}
impl DigestIncremental<14> for Sha3_224 {
fn finish(state: &mut (), digest: &mut [u8; 28]) { todo!() }
}
impl DigestIncremental<16> for Sha3_224 {
fn finish(state: &mut (), digest: &mut [u8; 32]) { todo!() }
}Playground
Of course, we would not actually implement the trait in this non-sensical way. The problem is, that we could. Because the compiler can't necessarily know that there is only one implementation, there can be cases where we would need to explicitly specify which trait instantiation we want to call. I don't have a reproducer for such inference issues at the moment, but I know they can occur.
Additionally to trait inference problems, this design makes it hard to write code that is generic over hash functions with different output sizes. Everything that wants to operate on a generic hash needs to add a generic const OUTPUT_LEN:usize parameter.
Sadly, this isn't easy to resolve with current stable Rust. Ideally, we'd write something like
trait Digest {
const OUTPUT_LEN: usize;
fn finish(self) -> [u8; Self::OUTPUT_LEN];
}Playground
But this isn't possible, because generic parameters can't be used in const contexts.
To work around this, the rust-crypto crates use hybrid-array (and previously used generic-array). If we wanted to get rid of the generic const parameter, we'd have to also solve this problem. Maybe there is a solution that solves the problem like hybrid-array, but is more easy to understand. Though my guess is, that it would look similar.
arrayref, owned, and slice variants of traits
Currently, there are 3 modules with 2 traits each for a total of 6 traits. The difference is the shape of the datatype into which the digest is written/returned as. I propose to merge the arrayref and owned traits and remove the slice variant. The result would look something like this:
trait Digest<const OUTPUT_LEN: usize> {
fn finish(self) -> [u8; OUTPUT_LEN];
fn finish_into(self, digest: &mut [u8; OUTPUT_LEN]);
}Playground
I'd argue there is no need to provide a fallible API that writes the digest to a slice. If a caller only has a slice available, they can perform the conversion to an array reference and error handling themselves.
Unclear interaction between digest traits and Hasher new type
Currently, the digest traits operate on a state that is an associated type of the traits and is passed into the corresponding methods. This state is then stored inside the Hasher new-type which then provides a more convenient (not trait-based) API on top of the digest traits. It is unclear to me why there is is split between the traits and the hasher new type. Using the Hasher new-type can easily hide missing trait implementations as we saw in #1493.
The digest traits were introduced recently (#1078) and haven't seen much use yet. While working on #1493 we noticed several issues of the traits. One issue, that
finishdidn't consume self (#1496) is fixed in #1493. However, there are more problems with the APIs.Digest traits take
const OUTPUT_LEN:usizeas input generic parameterBecause the
OUTPUT_LENis a const generic parameter to the trait, there could be multiple implementations of the trait for different lengths for one hasher type. I.e., that following would be possible:Playground
Of course, we would not actually implement the trait in this non-sensical way. The problem is, that we could. Because the compiler can't necessarily know that there is only one implementation, there can be cases where we would need to explicitly specify which trait instantiation we want to call. I don't have a reproducer for such inference issues at the moment, but I know they can occur.
Additionally to trait inference problems, this design makes it hard to write code that is generic over hash functions with different output sizes. Everything that wants to operate on a generic hash needs to add a generic
const OUTPUT_LEN:usizeparameter.Sadly, this isn't easy to resolve with current stable Rust. Ideally, we'd write something like
Playground
But this isn't possible, because generic parameters can't be used in const contexts.
To work around this, the rust-crypto crates use hybrid-array (and previously used generic-array). If we wanted to get rid of the generic const parameter, we'd have to also solve this problem. Maybe there is a solution that solves the problem like hybrid-array, but is more easy to understand. Though my guess is, that it would look similar.
arrayref,owned, andslicevariants of traitsCurrently, there are 3 modules with 2 traits each for a total of 6 traits. The difference is the shape of the datatype into which the digest is written/returned as. I propose to merge the
arrayrefandownedtraits and remove the slice variant. The result would look something like this:Playground
I'd argue there is no need to provide a fallible API that writes the digest to a slice. If a caller only has a slice available, they can perform the conversion to an array reference and error handling themselves.
Unclear interaction between digest traits and
Hashernew typeCurrently, the digest traits operate on a state that is an associated type of the traits and is passed into the corresponding methods. This state is then stored inside the
Hashernew-type which then provides a more convenient (not trait-based) API on top of the digest traits. It is unclear to me why there is is split between the traits and the hasher new type. Using theHashernew-type can easily hide missing trait implementations as we saw in #1493.