Clean up hashes and related types ahead of `bitcoin_hashes` 1.0 by apoelstra · Pull Request #278 · ElementsProject/rust-elements

apoelstra · 2026-06-12T14:57:14Z

I am going to PR soon to update to bitcoin-hashes 1.0. There are several big changes versus the old version of bitcoin-hashes that we were using:

Hash wrapper types (like Txid, Blockhash, etc) no longer have general hashing methods like hash that let you hash up arbitrary data; the intention of these types is that they only be constructable from byte slices or by hashing the right kind of data
We no longer directly support construction from byte slices; instead you must use arrays. stdlib has a TryFrom<&[u8; N]> for &[u8] impl which is just a pointer cast, so you can get the old behavior by using this TryFrom
The sha256::Midstate type now carries a length with it, so it behaves something like a deconstructed hash engine rather than being a "hash" which is computed from the sha256 compression function; you can no longer effectively wrap a midstate the way you would wrap a sha256 hash, so instead we directly wrap [u8; 32]s for the types that did this
Many changes to make the macros more ergonomic, standard trait impls more consistent and complete, etc

This PR makes a bunch of prepatory changes to make this transition smaller. In particular, it

eliminate all the bare sha256::Midstate uses by introducing newtypes for each midstate (asset entropy, dynafed elided params, etc)
replace slices with arrays in many places throughout the codebase, eliminating a ton of unreachable panic paths
makes Address::p2wpkh and Address::p2shwpkh panic if you give them uncompressed keys rather than producing unspendable addresses; later we will clean this up further by type-separating compressed keys, but for now ISTM that a panic is better than silently creating black-hole addresses
does a whole pile of code cleanups

Stop using `actual-serde` as a dependency; use `dep:` syntax. Also stop using `#[macro_use]` on the import; instead import the macros like a normal symbol. Also, rather than `use serde::{Serialize, Deserialize}`, fully-qualify the trait names every time they're used. This helps avoid compiler confusion between the serde methods and any other methods named serialize() that happen to be in scope.

I'm not sure why we were using generic sha256::Hashes here, but it's the wrong type, and it will complicate our transition to hashes 1.0 if we keep using it.

The leaf_hash method is better-implemented as a passthrough to TapLeafHash::from_script (which saves a manual hash computation which is difficult to port to hashes 1.0). To do this we need to fix the type of leaf_version.

In hashes 1.0 we no longer have Hash::from_slice; we have Hash::from_byte_array and if the user has a slice, they should use one of the TryFrom impls in the stdlib to get an array from it. This replaces a hashes error with a stdlib error which is likely easier to deal with.

Now that we are using arrays everywhere for conversion to/from hashes, we no longer have runtime length errors.

While I was looking at this error enum I realized a bunch of these are out of date.

For several types we are going to rewrite them to no longer wrap sha256 midstates (or to be a newtype rather than an unwrapped sha256 midstate). To ensure this does not break serde, test round-tripping here.

…ropy In hashes 1.0, the sha256::Midstate type is no longer a raw 32-byte blob. It also contains a counter. This is necessary to safely move between a hash engine and a midstate, e.g. when using midstates as a "preinitialized hash state". Most users of the existing type can just stick some .0s onto the new type, swear at us a bit, and move on. Probably we can do that too. But in our case the correct thing to do is to introduce a fresh type for asset entropy, so we might as well take this opportunity to do it.

In general, I hate macros, but I'm going to use identical boilerplate for like half a dozen types through the next several commits, so I think that it's worth it. Putting this in its own commit so you can easily review it with `git show --color-moved-ws=ignore-all-space` -- though I changed some doc comments and also added LowerHex and UpperHex impls which I forgot to do in the previous commit.

In `hashes 1.0` the Midstate type is pretty annoying to work with. Better to just directly use an array backing. This also makes the string-encoding reversal logic more explicit. This also replaces the serde impls for AssetId, but because we added a regression test (a few commits ago) we can be sure that this didn't break anything. This also removes the AssetId::from_inner and to_inner methods, which are unused in the library and seem inappropriate for the public API. This also removes the Encodable/Decodable impls for sha256::Midstate, which won't work properly with hashes 1.0 and don't seem to be used anyway.

There are actually 3 separate Merkle roots at play here, which are all currently typed as sha256::Midstate. Had we designed this a few years later we likely would have domain-separated all of them to prevent them from ever being collided with each other, though given the nature of the data they hash, there should be no risk of that anyway. This change was surprisingly easy. This prevents these roots from being confused with each other or with other midstates. It changes the debug output of one of the merkle roots which for some reason we were unit testing.

apoelstra · 2026-06-14T02:15:59Z

On f6d247e successfully ran local tests

stringhandler

utACK dbfbaf8

stringhandler · 2026-06-15T14:29:45Z

 #[cfg(feature = "serde")]
-#[macro_use]
-pub extern crate actual_serde as serde;
+pub extern crate serde;


nit: probably doesn't need to be pub. If the users want serde, they will probably reference it anyway

For serde this is probably true because there is only one major version of serde that anybody will ever use.

But as a matter of policy we like to do pub extern crate so that users can easily get "the version of the dependency used by rust-elements".

stringhandler · 2026-06-15T14:31:13Z

-actual-serde = { package = "serde", version = "1.0.103", features = [
-    "derive",
-], optional = true }
+serde = { version = "1.0.103", features = [ "derive" ], optional = true }


nit: This can probably just be 1 as it's a library and we want to specify lowest compatible version

That would mean 1.0.0, which doesn't even compile on modern Rust compilers. 1.0.103 is the minimum version that I've tested to be compatible. (Though it actually looks suspiciously low ... on rust-bitcoin we have been using 1.0.158 or something for a while.)

But I think cleaning up the serde dep is out of scope for this PR.

stringhandler

utACK dc701a5

stringhandler

utACK 0c006d7

stringhandler

utACK da967f8

One thing to note is that the error returned has changed, but I couldn't see any checks for the Pset error in particular

stringhandler

utACK 7ec80fe

stringhandler

utACK 90183f4

stringhandler

utACK 5f54fef

stringhandler

utACK da8942a

stringhandler

utACK b985342

stringhandler

utACK 8153c4a

stringhandler

utACK 5c22659

stringhandler

utACK fe59e1a

The panic comment is potentially wrong though

stringhandler · 2026-06-15T16:24:11Z

            payload: Payload::WitnessProgram {
                version: Fe32::Q,
-                program: WPubkeyHash::from_engine(hash_engine)[..].to_vec(),
+                program: pk.wpubkey_hash().expect("public key must be uncompressed").as_byte_array().to_vec(),


In fe59e1a

Did you mean "must be compressed"? Otherwise the comment above it wrong.

oops, yes, I mean "must be compressed"

stringhandler

In f6d247e

Unfortunately I'm not familiar enough with this code to know if it's correct or not. The one comment does seem off though

stringhandler · 2026-06-15T16:28:31Z

-        pk.write_into(&mut hash_engine)
-            .expect("engines don't error");
-
+        let pkh = pk.wpubkey_hash().expect("public key must be uncompressed");


In f6d247e

This should either be "compressed" or the doc comment be "compressed"

This one still remains @apoelstra

We already re-export and use rust-bitcoin's PublicKey type. The two hashes of keys that are used in Script are identical in Bitcoin and Elements, and have convenience methods that come with the bitcoin PublicKey type. To use these convenience methods, drop the PubkeyHash and WPubkeyHash types and just re-export the rust-bitcoin ones. We keep the scripthash types separate because Elements Script is meaningfully different from Bitcoin Script. **Importantly**, because upstream our WPubkeyHash returns an error if you try to give it an uncompressed public key, we panic in this case. The 'correct' way to handle this would be to have distinct compressed/uncompressed public key types, but those are not yet available from rust-bitcoin. Our current behavior is to compute a "wpkh" output which is unspendable, which seems like a very serious loss-of-funds problem, so we panic instead.

stringhandler

utACK b553acb

apoelstra · 2026-06-15T18:13:04Z

On 577e503 successfully ran local tests

This commit was originally just supposed to add constructors for WScriptHash and ScriptHash to allow them to be created from scripts without calling the generic `hash()` method (which will go away in 1.0). But in doing so, I noticed that the Address::p2shwpkh constructor is wrong. It computes the ScriptHash of a public key rather than a WPubkeyHash. Now, these two hashes are both just hash160s so there's no difference and no user-visible bug here. But jeez.

stringhandler

utACK b553acb

stringhandler

utACK fa3ea85

apoelstra · 2026-06-15T18:36:23Z

On fa3ea85 successfully ran local tests

5e0d577 pset: fix slicing in KeySource::deserialize (Andrew Poelstra) 4384012 address: do a better job slicing bech32 data (Andrew Poelstra) cae359d transaction: use better error typing for pegin destructuring (Andrew Poelstra) 6809a7f blind: use array splitting in TxOut::unblind (fix potential DoS?) (Andrew Poelstra) ee50a5f rewrite Address::from_base58 to eliminate all the unwraps (Andrew Poelstra) ed024b2 add bitcoin-internals dependency (Andrew Poelstra) Pull request description: Uses the `bitcoin-internals` library which provides us a bunch of array/slice methods that aren't available in std in our MSRV. Eventually we will be able to drop the dep and get these features directly from std. This replaces a bunch of slice accesses with arrays, cleaning up error paths and eliminating panics (though most are inaccessible assuming you use known-valid data from the blockchain). Builds on #278 (which I will merge in the next hour). Will do a followup that cleans up the error types; I deliberately avoided breaking the API to make this one easy to backport. ACKs for top commit: apoelstra: ACK 5e0d577; successfully ran local tests philipr-za: ACK 5e0d577 Tree-SHA512: b9238a0a2c642061c47671b955eb19771485448f8b455be268f2a5ee223f29f0ef09f67f792fcf9400481826c5a9c5ec2d0ee6eceb2a3da458819178f5f54d37

apoelstra added 11 commits June 12, 2026 14:49

taproot: use TapNodeHash for internal hashes in Taptrees

dc701a5

I'm not sure why we were using generic sha256::Hashes here, but it's the wrong type, and it will complicate our transition to hashes 1.0 if we keep using it.

sighash: strongly type leaf_version field in SighashCache

0c006d7

The leaf_hash method is better-implemented as a passthrough to TapLeafHash::from_script (which saves a manual hash computation which is difficult to port to hashes 1.0). To do this we need to fix the type of leaf_version.

pset: remove unused FromSliceError

7ec80fe

Now that we are using arrays everywhere for conversion to/from hashes, we no longer have runtime length errors.

pset: remove a bunch of other ununused error variants

90183f4

While I was looking at this error enum I realized a bunch of these are out of date.

hash_types: add serde regression tests for all 32-byte hashes

5f54fef

For several types we are going to rewrite them to no longer wrap sha256 midstates (or to be a newtype rather than an unwrapped sha256 midstate). To ensure this does not break serde, test round-tripping here.

apoelstra force-pushed the 2026-06/hash-types branch from b779dd3 to f6d247e Compare June 12, 2026 15:06

stringhandler reviewed Jun 15, 2026

View reviewed changes

apoelstra force-pushed the 2026-06/hash-types branch from f6d247e to 577e503 Compare June 15, 2026 16:44

stringhandler approved these changes Jun 15, 2026

View reviewed changes

apoelstra mentioned this pull request Jun 15, 2026

Improve slicing and error handling around slices #279

Merged

apoelstra force-pushed the 2026-06/hash-types branch 3 times, most recently from 577e503 to b553acb Compare June 15, 2026 18:28

apoelstra force-pushed the 2026-06/hash-types branch from 577e503 to fa3ea85 Compare June 15, 2026 18:29

stringhandler approved these changes Jun 15, 2026

View reviewed changes

apoelstra merged commit bab8a59 into ElementsProject:master Jun 15, 2026
10 checks passed

apoelstra deleted the 2026-06/hash-types branch June 15, 2026 18:40

philipr-za mentioned this pull request Jun 15, 2026

Backport slicing improvements to 0.25.x #280

Merged

Conversation

apoelstra commented Jun 12, 2026

Uh oh!

apoelstra commented Jun 14, 2026

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra commented Jun 15, 2026

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

stringhandler left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra commented Jun 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees