Skip to content

some cleanups and new functionality for Tmr and Value - #374

Open
apoelstra wants to merge 4 commits into
BlockstreamResearch:masterfrom
apoelstra:2026-08/cleanups
Open

some cleanups and new functionality for Tmr and Value#374
apoelstra wants to merge 4 commits into
BlockstreamResearch:masterfrom
apoelstra:2026-08/cleanups

Conversation

@apoelstra

Copy link
Copy Markdown
Collaborator

Ahead of adding "quoting" functionality where we produce Simplicity fragments that compute the CMRs of other Simplicity fragments, constify a few functions, cleanup some others, and add Value constructors from SHA256 contexts.

The next PR will use the new bitcoin-hashes release from rust-bitcoin/rust-bitcoin#6646 which will let us do some more cleanups and constification, and then we can start doing "real" changes.

We were storing Word::n as a u32 even though it fits into a u8. u8 can
be converted to every unsigned integer type without casts, while u32
cannot. So by using u8, we can eliminate casts everywhere.

@stringhandler stringhandler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ConceptACK.

Seems fine. Added some comments but they are not blocking

Comment thread src/jet/type_name.rs

for c in self.0.iter().rev() {
match c {
b'1' => stack.push(Tmr::unit()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These repeated mappings might be a source of errors in future. Not how to clean it up though.

Comment thread src/jet/type_name.rs

#[test]
fn all_jet_tmrs() {
for jet in &Core::ALL {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core is probably sufficient, but there may be some types in Elements that are not in Core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants