BIP 77: Delimit fragment params with - instead of +
#1890
Merged
+25
−9
Conversation
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
murchandamus
reviewed
Jul 7, 2025
nothingmuch
force-pushed
the
fragment-fixes
branch
3 times, most recently
from
555dff7 to
cbf69f3
Compare
murchandamus
reviewed
Jul 8, 2025
DanGould
added a commit
to payjoin/rust-payjoin
that referenced
this pull request
Jul 9, 2025
This PR contains two changes, using `-` instead of `+` as the fragment parameter delimiter, and lexicographically ordering the fragment parameters. This implements bitcoin/bips#1890
Since only Bull Bitcoin Mobile and Cake wallet are currently deployed in production, both using PDK, and the `+` character is causing some friction, this change seems justified to avoid similar issues with future implementations.
nothingmuch
force-pushed
the
fragment-fixes
branch
from
cbf69f3 to
3c36a5f
Compare
There was a problem hiding this comment.
LGTM, should we wait for a review from @DanGould or is this ready to go?
Nit: We use ISO-8601 formatted dates (YYYY-MM-DD) in the preamble, so I would prefer also using the same format in the Changelog, but either is fine and this is not a blocker.
nothingmuch
force-pushed
the
fragment-fixes
branch
from
3c36a5f to
1474f0d
Compare
I'd be more comfortable with an explicit ACK even though the corresponding code changes were merged into PDK
ISO-8601 is better, done |
|
ACK 1474f0d |
nothingmuch
force-pushed
the
fragment-fixes
branch
from
1474f0d to
5d99fd6
Compare
The rationale for this change is that since `-` instead of `+` breaks compatibility anyway, the marginal cost of removing this unusual/surprising requirement for reverse lexicographical ordering is zero.
nothingmuch
force-pushed
the
fragment-fixes
branch
from
5d99fd6 to
8b83c34
Compare
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.
This PR addresses the concerns raised in #1885 with regards using
+as a delimiter, by switching to-. Some URI libraries implement RFC 1866's (HTML 2.0) "keyword" delimitation in query parameters by decoding+asAdditionally fragment parameters must now ordered lexicographically (instead of reverse), the motivation for a canonical ordering was to reduce implementation fingerprinting concerns, but the reverse lexicographical ordering was specified simply because that was the behavior that was already implemented in PDK. Since that is somewhat surprising and the delimiter change breaks backwards compatibility anyway, the marginal cost in terms of compatibility of reducing the surprisingness of the reversed ordering is arguably zero, so this is also addressed here.
cc @kumulynja