shinghim / rust-bitcoin / 10223219320 / 1

83%

master: 84%

push

github

Merge rust-bitcoin/rust-bitcoin#3113: Push up the Default bound on HashEngine in order to better support keyed hash functions

<a class=hub.com/shinghim/rust-bitcoin/commit/<a class="double-link" href="https://git"><a class=hub.com/shinghim/rust-bitcoin/commit/2969b032f9dcc2b6436ff4d29e6b260efc2474fe">2969b032f Push up the Default bound on HashEngine (Nick Johnson)

Pull request description:

  Backstory for this patch is in #3084, and this might be enough to close that issue, but I think some follow up work might be required.

  I took Kixunil's advice and pushed up the `Default` bound on `HashEngine` all the way to the default methods themselves on the `GeneralHash` trait. I initially placed the bound just on the assocated type `Engine` of `GeneralHash`, which is a tad cleaner and keeps the bound from "leaking" downstream into things like `Hmac` and `Hkdf` implementations. However, that restricts `GeneralHash` implementations to just unkeyed hash functions and I believe there will be value in having `Poly1305` and `SipHash24` still leverage the interface. I struggled a bit on the best spots to put these bounds (even found the syntax a little jarring at times, I found [RFC2289](https://rust-lang.github.io/rfcs/2289-associated-type-bounds.html) helpful if others do too).

  Refactored the existing keyed hash function, `SipHash24`, to no longer have `Default` functions with zero-value keys. I kept the test coverage though by just hardcoding the zero-value keys over in the tests.

  Refactoring the `hash_type` macro for keyed hashes was getting a little hairy, so backed off and just wrote the bare minimum for `SipHash24` inline. Once `Poly1305` lands there will be two keyed hash functions and I think it will make more sense to then generalize over them.

ACKs for top commit:
  Kixunil:
    ACK 2969b032f9dcc2b6436ff4d29e6b260efc2474fe
  apoelstra:
    ACK 2969b032f9dcc2b6436ff4d29e6b260efc2474fe

Tree-SHA512: 9ca8f8baaed5e299658bfa2a48869328f5a

19683 of 23653 relevant lines covered (83.22%)

15606.55 hits per line