Conversation
As we do for other hash types put an inherent function on the `Hmac` type. Allows removal of a few `GeneralHash` trait imports.
As we do for other hash types (by way of the `GeneralHash` trait) add two helper functions to the `Hmac` type. Note we need a key and it has the same type as the data being hashed - possibly confusing.
| } | ||
|
|
||
| /// Hashes some bytes. | ||
| pub fn hash(key: &[u8], data: &[u8]) -> Self |
There was a problem hiding this comment.
Damn, this sucks, but I don't see any sensible way of fixing it. These are the times when I wish for Swift-like named arguments.
| } | ||
|
|
||
| /// Hashes all the byte slices retrieved from the iterator together. | ||
| pub fn hash_byte_chunks<B, I>(key: &[u8], byte_slices: I) -> Self |
There was a problem hiding this comment.
In #3130 (comment) @apoelstra noted that this may be dangerous if people are using it incorrectly. Which makes me think if we even should have it. But if we do, I think these should have a big warning about it.
There was a problem hiding this comment.
I think we should just drop it (well, deprecate it). It's a pretty obscure API (I always forget it exists except when it comes up in PRs like this).
The "manual" implementation isn't much longer -- let eng = HashEngine::new(key); byte_slices.map(|sl| eng.input(s)).count(); eng.finalize() and composes more naturally with other iterator adaptors.
There was a problem hiding this comment.
I agree. I was also thinking of having something like this:
#[derive(Default, Clone)]
pub struct HashBuilder<T: HashEngine>(T);
impl<T: HashEngine> HashBuilder<T> {
fn new() -> Self where T: Default() { /* ... */ }
fn input_fixed<const N: usize>(self, data: &[u8; N]) -> Self { self.0.input(data); self }
fn input_len_prefixed<const INTEGER_SIZE: usize>(self, data: &[u8]) -> Result<Self, InputTooLongErrror> { /* tries to serialize data.len() as a INTEGER_SIZE bytes long integer and write it */ }
fn input_assume_non_semantic_chunk(self, data: &[u8]) -> Self { self.0.input(data), self }
}This would both make hash building a bit more convenient (builder API with Iterator::fold compatibility) and also help enforce good practices.
There was a problem hiding this comment.
This is a neat API! But I'd prefer not to prioritize it because there are lots of open questions around it. For example, how do we do the length prefixes? If we use bitcoin-style varints is this any better than just being able to consensus-encoded a byteslice into a hash engine?
Also if users want to be putting structured data into hashes in general they probably want to be using something like merlin anyway.
There was a problem hiding this comment.
not to prioritize
Yes, obviously. :) This can be done after 1.0. I'd certainly not waste time on this right now.
I was thinking just some basics that several applications might already do. Similar to LenPrefixed encoder in tokio (at the time it was introduced I was coincidentally just working on a protocol that had that exact thing).
|
Needs rebase because of #4085 in any case. |
Done as part of #4196. These are the easy ones, all the other missing things require knowing the length of the inner array I believe. Patch 2 is already tricky enough to warrant a PR.