Skip to content

Fix ForEachKey impls for policies #546

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

Merged
merged 4 commits into from
May 8, 2023

Conversation

apoelstra
Copy link
Member

Our ForEachKey impls for the two policy types did not work. Both had an infinite type recursion which meant that if anybody had tried to use them they would not have compiled. (Though the fact that this has been happening for at least 2 years with zero bug reports suggests that nobody has tried to use them..)

Recent Rust compiler nightlies have started failing compilation even when the offending impls are not used, which means we need to prioritize fixing this. Possibly we should even backport it.

Fixes #541

apoelstra added 4 commits May 8, 2023 16:03
The compiler seems to need a `where Pk:'a` clause on the ForEachKey
trait definition, but not on any of the impls. Since this clause costs
us two LOC per impl and doesn't have much value for the reader of the
code, just drop it.
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 f2f95fa

@sanket1729 sanket1729 merged commit bf43cd3 into rust-bitcoin:master May 8, 2023
@apoelstra apoelstra deleted the 2023-05--fix-recursion branch May 8, 2023 20:58
apoelstra added a commit that referenced this pull request Jun 12, 2023
dd6996c bump 7.0.1 to 7.0.2 (Andrew Poelstra)
7a8e0ce concrete: fix infinite recursion in Policy (Andrew Poelstra)
31f2064 semantic: fix todo and infinite recursion in Policy (Andrew Poelstra)

Pull request description:

  Backport of #546.

ACKs for top commit:
  sanket1729:
    utACK dd6996c

Tree-SHA512: 83c3d2a56569361a58a1b41d8c133bf6527ab020281686e99b9b63c7c7b03eba66aa95862c65788dd11f0663df9e11c76024e310be0f531272e8d474464d052c
phlip9 added a commit to lexe-app/rust-miniscript that referenced this pull request Jul 13, 2023
* This fix is backported from latest `miniscript` v0.10.0 release.
* See <rust-bitcoin#546> for
  original PR with fix.
gruve-p pushed a commit to Groestlcoin/rust-miniscript that referenced this pull request Aug 28, 2023
f2f95fa cargo fmt (Andrew Poelstra)
a6340fe remove a bunch of unnecessary where clauses (Andrew Poelstra)
663bc00 concrete: fix infinite recursion in Policy (Andrew Poelstra)
520b9db semantic: fix todo and infinite recursion in Policy (Andrew Poelstra)

Pull request description:

  Our `ForEachKey` impls for the two policy types did not work. Both had an infinite type recursion which meant that if anybody had tried to use them they would not have compiled. (Though the fact that this has been happening for at least 2 years with zero bug reports suggests that nobody *has* tried to use them..)

  Recent Rust compiler nightlies have started failing compilation even when the offending impls are *not* used, which means we need to prioritize fixing this. Possibly we should even backport it.

  Fixes rust-bitcoin#541

ACKs for top commit:
  sanket1729:
    ACK f2f95fa

Tree-SHA512: a5d673929cd43187b157cd09941e9f922c605668085870d849fff84a404cb19dbc64694d1c173d4dcb5ff0d85dff450de1c6c6d4ceae038971dafca88ba40447
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.

Infinite type-level recursion in ForEachKey impl in Policy
2 participants