some cleanups and new functionality for Tmr and Value - #374
Conversation
1dbc471 to
b0ea014
Compare
stringhandler
left a comment
stringhandler
left a comment
There was a problem hiding this comment.
ConceptACK.
Seems fine. Added some comments but they are not blocking
|
|
||
| for c in self.0.iter().rev() { | ||
| match c { | ||
| b'1' => stack.push(Tmr::unit()), |
There was a problem hiding this comment.
These repeated mappings might be a source of errors in future. Not how to clean it up though.
There was a problem hiding this comment.
Yeah, Rust is really bad at expressing abstract algorithms. Not much we can do.
|
|
||
| #[test] | ||
| fn all_jet_tmrs() { | ||
| for jet in &Core::ALL { |
There was a problem hiding this comment.
Core is probably sufficient, but there may be some types in Elements that are not in Core.
There was a problem hiding this comment.
I considered that, but then I'd have to feature-gate the test and it seemed like the effort was not worth the additional coverage.
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.
b0ea014 to
7143d87
Compare
|
On 7143d87 successfully ran local tests |
|
cc @canndrew can you review this? |
|
ping |
|
ACK 7143d87 ran cargo test |
2 participants
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
Valueconstructors 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.