Remove Hash impl from new hash types created with hash_newtype

[tcharding]

If #2708 gets any traction this PR is not needed.

The first 4 patches are preparation, the last patch is:

New hash types created with `hash_newtype` are not meant to be general
purpose hashes but currently we implement `Hash` for the newtype, this
makes the API for the newtype misleading.

Instead of implementing `Hash` we can make the `Hash` trait methods
inherent on the new type, this allows the module that creates the new
type to hash stuff ergonomically but does not pollute the public API for
the type.

Please note the changes to the docs on is_sighash_single_bug - shows that now the new hash types don't enable getting an engine that can be used as a writer to pass to the encode to writer functions in sighash (eg, legacy_encode_signing_data_to). Is this acceptable?

[tcharding]

This change is the result of quite a few sessions trying to re-work the hashes API. The purpose of this PR is to make sure I'm going in the right direction and understand the problems with the current API.

[coveralls]

Pull Request Test Coverage Report for Build 8745032843

Details

• 62 of 65 (95.38%) changed or added relevant lines in 9 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.007%) to 83.138%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
hashes/src/util.rs	36	39	92.31%

Totals
Change from base Build 8743779815:	0.007%
Covered Lines:	19190
Relevant Lines:	23082

---

💛 - Coveralls

[tcharding]

I"ll fix CI if I get a concept ack.

[apoelstra]

Yep, concept ACK.

[tcharding]

Please see new text in PR description.

[apoelstra]

Is this acceptable?

Interesting question. I'm gonna say no, it's not.

In general, the hash newtypes shouldn't have "engines" that users create and can write arbitrary data to (I believe Kix made this point in one of the "overhaul Hash" issues). But in specific cases I think users should be able to, and I think that sighashes are one of these cases.

Maybe we should have a second macro that creates an engine?

I guess this also then brings up the question of how to properly tie engine types to hash types..

[tcharding]

I guess this also then brings up the question of how to properly tie engine types to hash types..

This is the crux of the whole API re-write in my opinion. When I re-wrote the HashEngine trait in that other PR it made it way nicer but totally broke the relationship between a hash and its engine, thereby creating massive footguns. The current way of finalizing the hash by way of from_engine is actually quite an elegant solution (even though it makes the code odd to read, to me at least, until one works this relationship thing out). I'm actually struggling to work out in my head what the Hash abstraction is, so far I have:

1. It provides for a type that can be Copy (needed in maps)
2. It provides a way to describe parsing to/from hex (including the backwards/forwards thing)
3. It ties an engine to the thing in (1) and (2)
4. It provides a way to find the length of the digest output from the engine

[apoelstra]

1, 2 and 4 are all straightforward attributes of a wrapper around [u8; N] and I think we've agreed on how to do that.

As for 3, in #2496 (comment) I suggested that we use HashEngine (possibly under a new name) as our primary "generic hash" type.

I think if the hash engines implemented Write, then we could continue calling the sighash methods with specific hashes. But we'd call them with specific engines rather than specific hashes. It would be nice to go so far as to put an io::Write bound on HashEngine but I think that would play hell with nostd so instead we should just be disciplined about implementing io::Write on everything that we implement HashEngine for.

I believe this would require us to have a macro alongside hash_newtype, or an option to hash_newtype, or whatever, to also produce a hashengine type.