search

BlockstreamResearch / rust-simplicity

Creative Commons Zero v1.0 Universal
58 stars 12 forks source link

Value refactor #237

Closed uncomputable closed 1 month ago

uncomputable commented 1 month ago

Rename the variants of Value and construct {256, 512}-bit values safely.

uncomputable commented 1 month ago

Fixed the jets-bench subcrate.

apoelstra commented 1 month ago

utACK 1a818692c63e5b0a447e389511e559ec94d4708b. Let me know if you want to address the above comments, and I can ack/merge.

uncomputable commented 1 month ago

Rebased on master after #239 got merged

uncomputable commented 1 month ago

Removed the unnecessary vector.

apoelstra commented 1 month ago

Ok, awesome. ACK 85fd5b37122d8fff5feae29a6651d5a4aa41805b except that I'd prefer that these new functions took arrays by value, not by reference. It is almost certainly slower, and more annoying for the programmer, to be passing 32-byte objects around by reference.

apoelstra commented 1 month ago

I did not actually test this or use the github "approve" button. I did not intend my comment to be read as an ACK.

uncomputable commented 1 month ago

The github-merge script from the bitcoin maintainer tools interpreted the comment as an ACK. Sorry for that. I will open a fixup PR to pass the 32 / 64 bytes by value.

apoelstra commented 1 month ago

No worries -- I shouldn't have written ACK in all caps so that the script wouldn't pick it up. And because this is such a minor thing I should've been clearer that I wanted you to respond to it before merging.