remove per-hash digest type

[Stebalien]

Remove the per-hasher digest type. Instead, store hash digests inside the hashers and "borrow" it. In all cases, we're going to copy it into a Multihash<S> anyways.

This:

1. Removes bunch of code.
2. Means that hashers don't need to be generic over the size (unless they actually support multiple sizes). This fixes the UX issue introduced in the const generics PR.
3. Avoids some copying.

BREAKING CHANGE

1. Hasher.digest no longer exists. Users should use Code::SomeCode.digest where possible.
2. The hasher digests no longer exist.

TODO: Do we want this? From what I can tell, most users won't really be impacted by it.

[vmx]

I like how much simpler the code becomes.

Though if I understand this change correctly, you no longer have the guarantee that you create the expected Multihash from a hasher. To explain what I mean, take this examples:

let mut hasher = Sha3_256::default();
hasher.update(b"Hello world!");
let hash = Code::Sha2_256.wrap(&hasher.finalize());

This won't be a compile-time error, not even a run-time error. You accidentally end up with the wrong Multihash (SHA2 vs. SHA3). With the current code, you'll always get the correct SHA3 Multihash.

[Stebalien]

Yes, but I expect users will usually use Code::SomeHash.digest. Basically, I'm separating the "multihash" part of this library and the "hasher" part.

I'm also working on a patch to fix #141 which should allow streaming as well. I.e., you'd call Code::SomeHash.digest_reader(...).

[Stebalien]

See #159 for hashing from a stream.

[dignifiedquire]

I like simplifications, but it removes a level of typesafety that I find very useful. In addition the Digest type has been used in all the rustcrypto hashers and that works very nicely (and is what a large part of the ecosystem expects).

[Stebalien]

From my perspective, it:

1. Mixes concerns. The "hasher" layer and "multihash" are independent.
2. Prevents hashers from being object safe and makes them harder to work with.

I like simplifications, but it removes a level of typesafety that I find very useful.

Can you give an example? I can't think of a case where I'd rather manually use a hasher then wrap it in a multihash instead of just calling Code::HashCode.digest.

[dignifiedquire]

when I have an api and I want to enforce a specific hash digest as input, how can I do this with the new construction?

[Stebalien]

when I have an api and I want to enforce a specific hash digest as input, how can I do this with the new construction?

Is that really in scope? Multihash is all about abstracting over hash functions.

I guess my question is, do you have a concrete case where you needed this? In practice, I'd expect to want to specify something like "I accept a multihash (not just a raw digest) with some set of constraints" (which isn't something we're going to be able to support without dependent typing).

[dignifiedquire]

To be fair, I don’t use multihash atm, so I don’t have a particular use case. Really I would allmost never use the hashing part of multihash and hardcode hash digests/be generic over them until I have to interact with the outside world. In my experience being entirely generic over hashfunctions is really not great.

[vmx]

Really I would allmost never use the hashing part of multihash

I'm not sure I fully understand. Do you mean that you would use some hashing within your system and then at the end wrap it into a Multihash? So you don't need the digest() or digest_reader() calls?

If that's the case, I think then this PR makes sense. I agree with @Stebalien that with the introduction of streaming hashers (#159), I think you would hardly ever use any hashers directly. So making the code simpler makes sense to me. The central entry point for this library is the generated Code enum.

[mriise]

Ive been doing some work recently to try and remove the proc macro altogether and use an enum with a trait impl to do the work of the macro and it will still require having extra traits to define the size of the array without using unstable rust.

note: this is more of an aside in case it was relevant than saying anything about this PR

[vmx]

I've merged it into one commit and rebased it on top of master. @Stebalien you can't review this PR as it is your's, a thumbs up would be good anyway.

@dignifiedquire Do you still have objections that would prevent merging this PR? It seems like @mriise @Stebalien and myself are in favour of making this change.

[Stebalien]

no objections to merging