In 4287bf2ff35ca21cdd1f1e27341239fbcc499965:

The new as_byte_array method should be const.

Other than that d3f36fe5a76dde18bb9238cd54f9612d6d5e5943 looks great! I wonder if later we will want to extend this macro to also add the as_byte_array etc inherents.

Only change is adding the const.

#2893 should fix CI.

Note to self, pick this up after #3010 merges.

Re-basing this after all the recent work on hashes was quite serendipitous. Turns out we missed a few things from siphash::Hash. This is now a nice little clean up.

There are bugs in siphash still. Will try again another time.

Some considerations:

• The "reverse" stuff is bitcoin specific, invalidating it being put in hex, right?
• internals would then have to depend on hex, so far I had it in mind that internals was a leaf crate but I guess that because hex is not bitcoin specific then it will never be allowed to depend on bitcoin-internals so this is ok.
• If we want hashes to stabalize we need to be careful not to change the output of this macro (and others) eventually. Having them over in internals makes me nervous that we will want the change somewhere else and forget hashes, I'm inclined to think that as we stabalize we will want to share macros between crates less or somehow mark them as 'used for stable code generation, don't modify the output'

But if none of that matters, I can move it.

• The "reverse" stuff is bitcoin specific, invalidating it being put in hex, right?

I prefer keeping it out until there's a good reason (e.g. perf problems reversing the hashes). But I'm not strongly opposed to having it in hex - it should be mostly harmless.

• internals would then have to depend on hex, so far I had it in mind that internals was a leaf crate but I guess that because hex is not bitcoin specific then it will never be allowed to depend on bitcoin-internals so this is ok.

That's not a problem, internals is allowed to depend on whatever, though it needs to be gated. The idea of enforcing a crate to stay a leaf indefinitely is not useful.

• or somehow mark them as 'used for stable code generation, don't modify the output'

This was my intention for a while and I still think it's a good approach. But note that we want to also forbid adding items to the generated types because Rust considers them to belong to the caller crate which means orphan rules don't apply and adding any trait impl or associated item later is a breaking change. Versioning them separately might be needed/useful.

Can we do it as a follow up (#3320) or do you guys want it done now?

I'd prefer as a followup. It's more risky than I'd considered.

Sweet ,lets go!

Can't serde simply call as_ref instead of indexing? Should be a trivial change.

This PR does not add SliceIndex, it maintains the current behavour and removal of SliceIndex is unrelated to this PR. FTR I've tried it before and I tried it again this morning. Unless I am being brain dead it is not trivial to remove the SliceIndex impl.

This PR has been stalled for almost three months, can we just get it in and move on?

Holy f***, there is already a draft PR to do the SliceIndex stuff: #3296 (comment)

Currently is broken. Note the first patch was added after I struggled with the compiler only to find the same problem in patch 3.

First one gets this error:

error: expressions must be enclosed in braces to be used as const generic arguments
   --> hashes/src/util.rs:192:49
    |
192 |           $crate::impl_bytelike_traits!($newtype, <$newtype as $crate::Hash>::LEN, <$newtype as $crat...

Then if adding {} as suggested on gets this error:

error[E0308]: mismatched types
   --> hashes/src/util.rs:192:51
    |
192 |           $crate::impl_bytelike_traits!($newtype, { <$newtype as $crate::Hash>::LEN }, <$newtype as $crate::Hash>::DISPLAY_BACKWARD);

Before I rebased I did not see this problem, and I hard reset to double check nothing had changed - well something has changed but I can't work out what.

Hey @apoelstra if you get a second could you take a look at this please, non-urgent though. If you check out the first commit and try and build you'll get

error: expressions must be enclosed in braces to be used as const generic arguments

Looks simple, but I cannot workout how to fix it. The compiler suggestion just leads to another error.

Thank in advance.

@tcharding super weird. But this "fixes" it

diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs
index c231b237b..625b8d8d0 100644
--- a/hashes/src/internal_macros.rs
+++ b/hashes/src/internal_macros.rs
@@ -20,7 +20,7 @@
 /// `from_engine` obviously implements the finalization algorithm.
 macro_rules! hash_trait_impls {
     ($bits:expr, $reverse:expr $(, $gen:ident: $gent:ident)*) => {
-        $crate::impl_bytelike_traits!(Hash, $bits / 8, $reverse $(, $gen: $gent)*);
+        $crate::impl_bytelike_traits!(Hash, { $bits / 8 }, $reverse $(, $gen: $gent)*);

         impl<$($gen: $gent),*> $crate::GeneralHash for Hash<$($gen),*> {
             type Engine = HashEngine;
diff --git a/hashes/src/macros.rs b/hashes/src/macros.rs
index fe658b17b..7a0cfdbb5 100644
--- a/hashes/src/macros.rs
+++ b/hashes/src/macros.rs
@@ -41,7 +41,7 @@ fn from_str(s: &str) -> $crate::_export::_core::result::Result<Self, Self::Err>
         $crate::hex::impl_fmt_traits! {
             #[display_backward($reverse)]
             impl<$($gen: $gent),*> fmt_traits for $ty<$($gen),*> {
-                const LENGTH: usize = $len;
+                const LENGTH: usize = ($len); // parens required due to rustc parser weirdness
             }
         }

IMO there is some bug in the rustc parser somewhere.

const LENGTH: usize = ($len); // parens required due to rustc parser weirdness

Nice find, out of interest what led you to try this? I tried putting {} on all the other places that use $len but I did not try to paranthesise this one.

The compiler suggested adding {}s as I did. At first I tried putting braces around all the uses of $len but it didn't like that, so I put it on the call site as it suggested. Then it complained that I needed to add ()s at the call site. I tried that and it went back to wanting braces. So instead I tried adding them at the usage sites, and that worked.

So instead I tried adding them at the usage sites, and that worked.

Nice! I did everything else you did but did not make this leap.