API design

[CasualX]

I am looking through the code and I am curious about some of the API choices made:

1. Why is Sha1 not an affine type

   Sha1::digest could take self by value and produce a Digest. This let's the type system prevent mistakes where one reuses the Sha1 without also resetting.

   I'm not sure if it's useful anywhere to produce a digest and not reset/be done with the Sha1 state.

   Sha1::reset is not necessary, to reset a variable simply overwrite it with Sha1::new(). The compiler will optimize this as efficient as using a reset method but with benefit of type safety.

2. Why doesn't Digest just store the bytes in the correct endianness?

   This would clean up the as_bytes() API to just return a &[u8] making multiple calls more efficient, both API wise and performance wise.

3. Implement Debug which dumps internal state and reserve Display on Digest for actually displaying the bytes as expected.

I understand that changing API is a big commitment and there may be no desire to do so as the code has existed for a long time already. That said I am willing to make the changes and pull request them.

Any thoughts?

[newpavlov]

I would like to endorse my version of sha1 which was derived from rust-crypto. I believe it addresses all issues your mentioned, except Debug. (which I don't think is necessary for the working digest states)

I hope one day it will be published under sha1 name, as readme states:

This might go away in the future if rust-crypto or some libraries like that split into smaller parts.

But unfortunately mitsuhiko didn't give a clear answer to my request.

[mitsuhiko]

@newpavlov i replied to an email you sent that I will not replace this crate with a widely different one. You are free to publish rust-crypto-sha1 etc.

[mitsuhiko]

@CasualX to answer the questions:

I'm not sure if it's useful anywhere to produce a digest and not reset/be done with the Sha1 state.

I don't see a reason why one would want to restrict this functionality. I have written code that requires this in the past. Likewise I am not sure what the point is in removing reset now that it already exists. I'm not in for breaking APIs again just to annoy people.

Why doesn't Digest just store the bytes in the correct endianness?

I don't think there is a reason to this other than that some of the changes that made it work without stdlib changed the implementation to be based this way.

Implement Debug which dumps internal state and reserve Display on Digest for actually displaying the bytes as expected.

Works for me, i would accept patches for this.

[newpavlov]

@mitsuhiko
In the only reply you've sent me you only said that it will depend on compatibility and that I should send pull request, after that zero reaction. If you are not planning to cooperate in any way, then at least change readme so it will not confuse others.

[mitsuhiko]

@newpavlov I am not sure what exactly you want from me other than complete surrender of the crate here. I already mentioned before that I do not like the idea that rust-crypto crates are not prefixed with rust-crypto or something because it means you will for all eternity to achieve consistency need to get crates from people that are named after hash algorithms etc.

There is already a lot of code that depends on this crate and replacing this crate with a completely different one is not something I think is a reasonable step forward.

Not sure what you want in the README. If people are looking for rust crypto they will find it.

[newpavlov]

@mitsuhiko
I am not insisting on surrounding crate by any means. I've created PR with code I had because you asked me to do so. Actually you implementation even seems to be superior to rust-crypto's in terms of performance. I was asking you if you are willing to change your crate so it will be in sync with other hash crates, so essentially it means using Digest trait, no_std capability and optional use of crypto-tests for testing and benchmarking. Yes, doing so will introduce several dependencies, but all of them will be quite small and written in pure Rust, so thanks to Cargo end user will face only a minor API change.

Argument about dependent code looks like fallacy to me. We have semver exactly for such kind of situations. All users who don't want or unable to update their code to a new API will continue to use sha1 v0.2 as nothing happened.

Also I am not insisting on transferring crate ownership (though I think it would be reasonable if in case of cooperation you'll give a write access to RustCrypto admins, so it will be possible to publish updates in your absence) and on closing this repository. I can mirror code to RustCrypto and your repository can stay as the main one. (though it will be certainly inconvenient and confusing for potential contributors)

Regarding readme, I was talking about removal of this part:

This might go away in the future if rust-crypto or some libraries like that split into smaller parts.

[mitsuhiko]

I'm going to close this issue as I'm unable to do something with it.

[pickfire]

@mitsuhiko Is this still the same until now? This crate seemed unmaintained as it have no commit since 2018.