Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backport] cleanup: refactor a bunch of concatenation-related code in satisfy #803

Merged

Conversation

apoelstra
Copy link
Member

This backports the main commit in #798 so that the code in (released) 12.x is substantially the same as the code in (unreleased) 13.x. This makes it easier to write and maintain regression tests.

There is a bunch of essentially-repeated "combine two satisfactions"
code throughout satisfy.rs. It's hard to tell at a glance what order the
concatenantions are in, even though this is essential, or whether the
combination is actually done correctly. (In particular, we routinely use
the opposite order for Witness::combine and for ||'ing the has_sig, and
we routinely assume that timelock conditions are None for
dissatisfactions, which is true, but it's still a lot to keep in your
head.)
@apoelstra apoelstra force-pushed the 2025-03--798-backport branch from 6204b2a to 9d81dcf Compare March 31, 2025 13:12
@apoelstra
Copy link
Member Author

apoelstra commented Mar 31, 2025

I backported one lint fix that I couldn't even compile without (a change to deny(missing_docs) behavior) but we still expect the Docs/Lint jobs to fail on this branch.

I may later backport #799 with an old version of rust nightly to fix this as well.

Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK 9d81dcf

@apoelstra apoelstra merged commit 9d658a9 into rust-bitcoin:release-12.x Apr 1, 2025
10 of 12 checks passed
@apoelstra
Copy link
Member Author

Tagged and published.

@apoelstra apoelstra deleted the 2025-03--798-backport branch April 1, 2025 01:33
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