clean up Merkle Root types; update bitcoin_hashes to 1.1 - #373
Open
apoelstra wants to merge 7 commits into
Open
Conversation
apoelstra
force-pushed
the
2026-07/hashes-1.1
branch
from
July 27, 2026 14:37
7a74ffa to
7884f97
Compare
… master) No version bump of simplicity-sys; there are no API-visible changes.
apoelstra
force-pushed
the
2026-07/hashes-1.1
branch
from
July 27, 2026 15:06
7884f97 to
d4aa7ac
Compare
There is a type confusion in all the Merkle root types: we have these initialization vector constants which conceptually are "midstates" (or "initial states" or whatever). But for e.g. the Amr IVs we have typed them as Amrs. Then in the `impl_midstate_wrapper` macro we add all these methods to the `Amr` type to do "update" operations which conceptually are only applicable to midstates. If we instead just type the IVs (which are private constants anyway) as `Midstate`s, then we can add a private extension trait `MidstateExt` to get the updating functionality, and eliminate (most of) the macro and also the type confusion. I will do the other mr types in the next commit; doing this one separately because it was the simplest change so it worked as a demo.
Drop all the midstate-manipulation functions, which don't belong on the various MR types, and make the macro invocation more evocative of the code that it generates.
In bitcoin_hashes 1.0 the Midstate type is expanded to include the number of hashed bits as well as the output of the most recent compression function. It no longer makes sense to use it as a sort of 32-byte array wrapper.
And eliminate some use of hashes::hex, which is not an available module in hashes 1.0.
apoelstra
force-pushed
the
2026-07/hashes-1.1
branch
2 times, most recently
from
July 27, 2026 15:40
e597400 to
b29f61d
Compare
This update gives us a richer Midstate type, which will be important for working with Ctx8 Simplicity values. Unfortunately rust-elements and bitcoin-miniscript are still using hashes 0.14.x, so there is a type compatibility. Fortunately, we rarely use "bare hashes" and instead use newtypes everywhere, meaning that it's easy to work around the type incompatibility by converting to/from byte arrays. In fact, we don't even need to do that. Instead we just directly import the old version of `hashes` from `elements` or `miniscript` in the places where it's needed. As we update those deps, the imports will update along with it.
apoelstra
force-pushed
the
2026-07/hashes-1.1
branch
from
July 27, 2026 15:56
b29f61d to
2d1f560
Compare
delta1
approved these changes
Jul 28, 2026
| } | ||
| impl CTxEnv { | ||
| pub fn sighash_all(&self) -> sha256::Hash { | ||
| // This is a little bit sketchy, directly interpreting a midstate as a sha256 hash. Bit |
Contributor
There was a problem hiding this comment.
typo nit
Suggested change
| // This is a little bit sketchy, directly interpreting a midstate as a sha256 hash. Bit | |
| // This is a little bit sketchy, directly interpreting a midstate as a sha256 hash. But |
Collaborator
Author
There was a problem hiding this comment.
I'm gonna leave this one be since it's an internal comment and the typo doesn't cause confusion. May fix in a followup.
Contributor
|
ACK 2d1f560 |
Collaborator
Author
|
ACK 2d1f560; successfully ran local tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We had a bunch of type confusion in
Cmr,Amr, etc., where we'd represent our merkle roots assha256::Midstates (which is okay, but will stop working with bitcoin_hashes 1.0 which adds a length counter to the midstate types) and then represent our IVs as the mr type (not okay, and required a bunch of macro hackery to make work).We want the new bitcoin_hashes 1.0 midstate type, which aside from being more compatible with the upcoming rust-elements release, will let us convert midstates and engines directly into
Ctx8Simplicity values. So move to that, and clean up the existing types along the way.