[backport] Several small fixes for 12.x#735
Merged
apoelstra merged 4 commits intorust-bitcoin:release-12.xfrom Nov 27, 2024
Merged
[backport] Several small fixes for 12.x#735apoelstra merged 4 commits intorust-bitcoin:release-12.xfrom
apoelstra merged 4 commits intorust-bitcoin:release-12.xfrom
Conversation
409374a to
8f54b5e
Compare
Member
Author
|
Actually I will hold off on this. Will add a patch for #736 and anything else I find over the next few days. |
We are incorrectly serializing and_n as "and_b". In master this is fixed by rust-bitcoin#722 which rewrites the Display impl completely.
98ea9df to
d3adcf3
Compare
d3adcf3 to
da7932f
Compare
apoelstra
added a commit
to apoelstra/rust-miniscript
that referenced
this pull request
Sep 1, 2024
When fuzzing my new non-recursive tree parsing logic, I noticed that we were deviating from the released 12.0 in the way that we display l:0. This is an ambiguous fragment which can be displayed either as l:0 or u:0. In our released code we use u:0 so stick with that. This was unintentially changed as part of rust-bitcoin#722. Change it back. While we are at it add a regression test for rust-bitcoin#735
This was referenced Sep 1, 2024
da7932f to
9cedc49
Compare
Member
Author
|
Ready for review. I have fuzzed this against my rewritten code for 40 hours now without any new incompatibilities cropping up. |
apoelstra
added a commit
that referenced
this pull request
Sep 3, 2024
1259375 miniscript: make display prefer 'u' over 'l' in the fragment l:0 (Andrew Poelstra) 67fdc50 descriptor: reject strings of the form "tr(<key>,)" (Andrew Poelstra) 00cac40 descriptor: add unit test demonstrating sanity-checking behavior in <= 12.x (Andrew Poelstra) Pull request description: This PR has three changes which are mostly unrelated except that they were all found when fuzzing my "rewrite expression parser to be nonrecursive" branch against 12.x. * First is a unit test demonstrating #734. It doesn't fix anything, just tests the current behavior. * Second is a fix for #736 (backported in #735). * Third tweaks the new `Display` code from #722 to change how the ambiguous `l:0`/`u:0` is serialized, to match 12.x. ACKs for top commit: sanket1729: ACK 1259375 Tree-SHA512: 921d65a1efd49bda0f9db488a2854b004e14518f584d832497a9dbc13a845ceec99544375385570c6ac42d4985277e8dcbb3aa8654de93235cf9067ba601f91d
Member
Author
|
cc @sanket1729 oops, I totally forgot about this one. We should merge it too. |
sanket1729
approved these changes
Nov 27, 2024
Member
sanket1729
left a comment
There was a problem hiding this comment.
ACK 9cedc49
Oops. I just missed this in swarm of notifications :(
Member
Author
|
Tagged and published. Will update my other PR to include a fuzztest against this. |
heap-coder
added a commit
to heap-coder/rust-miniscript
that referenced
this pull request
Sep 27, 2025
When fuzzing my new non-recursive tree parsing logic, I noticed that we were deviating from the released 12.0 in the way that we display l:0. This is an ambiguous fragment which can be displayed either as l:0 or u:0. In our released code we use u:0 so stick with that. This was unintentially changed as part of #722. Change it back. While we are at it add a regression test for rust-bitcoin/rust-miniscript#735
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We are incorrectly serializing and_n as "and_b", and incorrectly parsing
tr(<key>,).In master the
and_bthing is fixed by #722 which rewrites the Display impl completely and thetr(<key>,)thing will be fixed as part of a coming series of PRs to clean up expression parsing.Also includes #740 since it showed up in time.