Split out tiny hashes crate

[tcharding]

I have an idea/plan about hashes.

I've pulled out the cryptographic hash functions from bitcoin_hashes (i,e., none of the Bitcoin specific stuff) and released a new crate rust-chf.

If this idea gets traction I'd like to move that crate to the rust-bitcoin org.

In rust-chf I've removed the Hash trait and hacked the HashEngine trait, posted below.

What this buys us is:

• Separates all the Bitcoin stuff from the plain old hashing stuff, this allows for cleaner abstractions and a cleaner API
• Puts the Bitcoin stuff in rust-bitcoin where it should be (e.g., display backwards logic)
• Allows us to provide a clear simple hash crate that can be stabalised now (TM)

We can then:

• Depend on rust-chf in rust-secp256k1 (no more dependency hole, ya!)
• Depend on rust-chf in bitcoin_hashes

I've proofed it all out locally and it seems to work.

[tcharding]

/// A hashing engine which bytes can be serialized into.
pub trait HashEngine: Clone + Default {
    /// The digest returned by this hash engine.
    ///
    /// This is expected to be an array.
    type Digest: Copy
        + Clone
        + PartialEq
        + Eq
        + PartialOrd
        + Ord
        + hash::Hash
        + borrow::Borrow<[u8]>;

    /// Byte array representing the internal state of the hash engine.
    type Midstate;

    /// Length of the hash's internal block size, in bytes.
    const BLOCK_SIZE: usize;

    /// Creates a new hash engine.
    fn new() -> Self { Default::default() }

    /// Add data to the hash engine.
    fn input(&mut self, data: &[u8]);

    /// Return the number of bytes already n_bytes_hashed(inputted).
    fn n_bytes_hashed(&self) -> usize;

    /// Returns the final digest from the current state of the hash engine.
    fn finalize(self) -> Self::Digest;

    /// Creates a default hash engine, adds `bytes` to it, then finalizes the engine.
    ///
    /// # Returns
    ///
    /// The digest created by hashing `bytes` with engine's hashing algorithm.
    fn hash(bytes: &[u8]) -> Self::Digest {
        let mut engine = Self::new();
        engine.input(bytes);
        engine.finalize()
    }

    /// Hashes all the byte slices retrieved from the iterator together.
    fn hash_byte_chunks<B, I>(byte_slices: I) -> Self::Digest
    where
        B: AsRef<[u8]>,
        I: IntoIterator<Item = B>,
    {
        let mut engine = Self::new();
        for slice in byte_slices {
            engine.input(slice.as_ref());
        }
        engine.finalize()
    }

    /// Outputs the midstate of the hash engine. This function should not be
    /// used directly unless you really know what you're doing.
    fn midstate(&self) -> Self::Midstate;

    /// Create a new [`HashEngine`] from a [`Self::Midstate`].
    ///
    /// Only use this function if you know what you are doing.
    fn from_midstate(midstate: Self::Midstate, length: usize) -> Self;
}

[apoelstra]

• Depend on rust-chf in rust-secp256k1 (no more dependency hole, ya!)

Well, the dependency hole just goes away because you've moved the crate out of this repo :).

• Puts the Bitcoin stuff in rust-bitcoin where it should be (e.g., display backwards logic)

I'd have to see this. I'm unconvinced that hash display logic actually belongs in rust-bitcoin and not in the hashing crate, for example. I also don't see anything about tagged hashes in chf, and those are definitely not bitcoin-specific. Once you return those, I'm not sure how different the result would be from bitcoin-hashes?

Allows us to provide a clear simple hash crate that can be stabalised now (TM)

Well, the HashEngine trait is in chf, and that's the main thing we need to sort out in order to stabilize. So I'm not sure what this separation gains us vs just sorting that out directly in bitcoin-hashes.

[apoelstra]

I get that the idea of tagging is not Bitcoin specific (I forget the name you told me, domain specific something?) But is the exact construct not a Bitcoin specific thing? It is explicitly defined in BIP-341 to be SHA256(SHA256(tag) || SHA256(tag) || x). Its this construct that sha256t.sh implements, right?

The term you're looking for is "domain separation", and yes, this particular scheme is defined in BIP 340, but that doesn't make it bitcoin-specific. I use BIP-340 tagged hashes pretty-much everywhere because they are a particularly natural and efficient way to domain-separate sha2 hashes.

My claim is that chf can be a "regular" hashing crate, any Rust dev should be able to look at it and work it out within a very short amount of time. Where as the bitcoin_hashes crate requires a dev to have knowledge of Bitcoin to work the API design out.

Currently bitcoin_hashes is a "regular" hashing crate with the one extra caveat that it supports byte-reversed serialization. And you can use the API perfectly fine without ever knowing or caring that that feature exists.

The current API sucks not because it requires a "knowledge of Bitcoin" (which won't help you) but just because it sucks.

If chf is separate we can hope to more easily do more SIMD stuff and any other optimisations / improvements that some random hashing nerd wants to have a go at. While its buried inside unrelated Bitcoin stuff we only have access to devs that want to dig through Bitcoin stuff. This might be pie-in-the-sky dreaming ...

If anyone is avoiding working on this crate because of its bitcoin association, changing the name won't help with that. We'd also have to replace all of the maintainers and move it out of the org.

[apoelstra]

We would also want to drop support for ripemd160 which is basically only used in Bitcoin, and is far more obscure than tagged hashes are.

[tcharding]

I use BIP-340 tagged hashes pretty-much everywhere because they are a particularly natural and efficient way to domain-separate sha2 hashes.

This is a solid point, which pretty much mutes me as far as sha256t goes. Consider me schooled.

The current API sucks not because it requires a "knowledge of Bitcoin" (which won't help you) but just because it sucks.

At some stage I'd like to hear what you think sucks about it if you disagree with my conclusion on the display backward thing being the root cause.

I have been in a bit of a state this last month, for either personal reasons (which I don't know) or because of the release cycle. But the little work I have done, outside of pushing the release, has been on hashes and I have thus far been unable to make any material improvements because it is, IMNSHO, a total tangled up mess. At first I thought it was because of all the macros but, although they are do make the code particularly hard to hack on, I think the problems are deeper. I am obviously coming at this as a non-cryptographer so what is not obvious to me might be obvious to others. I'd hazard that most users are not cryptographers either though so I rekon we should aim for an API that is exactly correct for cryptographers and obvious / easy to use for non-cryptographers.

[apoelstra]

The backward display thing is a single flag and if you don't set it then you get normal forward display. I really don't think it has anything to do with the complexity of this API. But it is the only "bitcoin-specific" thing in this crate, other than the choice of which hash functions to implement.

It is not a cause, root or otherwise, of the API being complex.

At some stage I'd like to hear what you think sucks about it if you disagree with my conclusion on the display backward thing being the root cause.

We have a Hash trait that you need to import to do basically anything, and all of the hash types themselves are called Hash, and the intended use is that you write sha256d::Hash e.g. but this is completely undiscoverable. There is no obvious way to just compute the hash of a slice/string/file/whatever. It is not at all obvious what the entry point of the crate is.

What we need are a bunch of concrete hash types which are visible at the crate root and which have obvious constructors.

[tcharding]

What we need are a bunch of concrete hash types which are visible at the crate root and which have obvious constructors.

I'll have another go and limit my scope to just this, and see what drops out.

It is not a cause, root or otherwise, of the API being complex.

Lets agree to disagree for now, if it stays with me after the work above I'll have another go at convincing you :)

[elichai]

Just one small note :)
I think you want AsRef<[u8]> and not Borrow,
As Borrow is usually for abstracting over owned/ref and different kinds of refs that way you can use ToOwned if you need an owned variant which might just return itself in the owned scenario (this trait is the counterpart to the CoW enum)
while AsRef is more for "this type is something that can be referenced as T" (ie any buffer type no matter it's Owned counterpart)

[tcharding]

Cool thanks! I tried reading the docs on Borrow and AsRef a bunch of times and did not work this out. Will use as suggested.

[tcharding]

Done in: tcharding/rust-chf#2

I'll try and keep this in mind and work it into bitcoin_hashes, the crate is moving slowly right now and I have a bunch of stuff queue up, if I forget temporarily please take it as absent mindedness not intentional omission. In the long run I wont' forget.