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

Fix compilation of pk()-only policies into tr() descriptors #677

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

shesek
Copy link
Contributor

@shesek shesek commented Apr 30, 2024

Before this fix, the added test failed with: Unexpected("Empty Miniscript compilation")

@tcharding
Copy link
Member

Thanks for the contribution!

Can you put the unit test as a separate patch after the fix please so that during review I can swap them around and verify the failure.

shesek added 2 commits May 1, 2024 00:01
Prior to this fix, the test failed with: `Unexpected("Empty Miniscript compilation")`
@shesek
Copy link
Contributor Author

shesek commented Apr 30, 2024

@tcharding Yes for sure, I updated my branch.

@tcharding
Copy link
Member

Bother, the CI fail is unrelated to this PR. I hit the same thing elsewhere yesterday, I'll push a PR to fix it (it requires dependency pinning).

@tcharding
Copy link
Member

I verified the unit test does indeed fail and the fix does fix the test. I have no view on the sanity of the change, requires @apoelstra or @sanket1729 who actually know this crate. I'm just the code monkey :)

Thanks for the contribution.

@sanket1729
Copy link
Member

It's been a while since I looked at MSRV things. What is the current rust-bitcoin MSRV?

The CI is failing at

error: package cc v1.0.96 cannot be built because it requires rustc 1.63 or newer, while the currently active rustc version is 1.56.1

@tcharding
Copy link
Member

Will need rebasing once #678 goes in.

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.

None yet

3 participants