[backport] Several small fixes for 12.x #735
Open
Conversation
This file contains 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
apoelstra
force-pushed
the
release-12.x
branch
from
409374a
to
8f54b5e
Compare
|
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.
apoelstra
force-pushed
the
2024-08--fix-12x
branch
from
98ea9df
to
d3adcf3
Compare
apoelstra
force-pushed
the
2024-08--fix-12x
branch
from
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
apoelstra
force-pushed
the
2024-08--fix-12x
branch
from
da7932f
to
9cedc49
Compare
apoelstra
changed the title
|
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
|
cc @sanket1729 oops, I totally forgot about this one. We should merge it too. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.