The folks at BTCPayServer did research on this earlier this year and it seems we have reached enough wallets supporting completely uppercase URIs for the QR code
Uppercase ‘bitcoin:’ in QR code URI #410
pull benthecarman wants to merge 1 commits into bitcoin-core:master from benthecarman:uppercase-uri changing 1 files +5 −1-
benthecarman commented at 9:01 pm on August 23, 2021: contributor
-
hebasto renamed this:
gui: Uppercase 'bitcoin:' in QR code URI
Uppercase 'bitcoin:' in QR code URI
on Aug 23, 2021 -
hebasto added the label Feature on Aug 23, 2021
-
ben-kaufman approved
-
ben-kaufman commented at 9:30 pm on August 23, 2021: contributorACK ae7bc4dc4014ffca0d68f53cdc9b27ed0aae52f0
-
kristapsk commented at 10:02 am on August 24, 2021: contributorIt only makes sense to do that if final
paramCount
is0
, as BIP21 parameters are case sensitive. And, apart for QR code encoding, BIP173 says that lower case is preferred. See https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/972, how I propose to do it for JoinMarket GUI. -
benthecarman force-pushed on Aug 24, 2021
-
gui: Uppercase 'bitcoin:' in QR code URI b6704cf7e1
-
benthecarman force-pushed on Aug 24, 2021
-
benthecarman commented at 2:47 pm on August 24, 2021: contributorUsed @kristapsk suggestion
-
kristapsk approved
-
kristapsk commented at 3:39 pm on August 24, 2021: contributorACK b6704cf7e1b005d4122c032485c9b5bba1359da8
-
hebasto requested review from meshcollider on Aug 24, 2021
-
hebasto requested review from achow101 on Aug 26, 2021
-
meshcollider commented at 11:34 pm on August 26, 2021: contributor
I don’t know much about BIP21 but this LGTM.
utACK b6704cf7e1b005d4122c032485c9b5bba1359da8
-
shaavan commented at 3:27 pm on August 30, 2021: contributor
Tested on Ubuntu 20.04.
This PR allows the capitalization of the schema of a “bitcoin:” based URI. This is done only when all the parameters of URI are not prespecified. The motivation of this PR is that QR codes for all uppercase URIs are much simpler and hence easier to scan, as discussed here.
Tested this PR by creating a payment receiving request and copying the URI of that request. The URI with no specified info works as expected. The schema is non-capitalized on master while it is capitalized on PR.
Copying a URI with no prespecified info:
Master PR bitcoin:TB1Q7A0G8SFXRFKCGY77PPDTS964ZHAHP7KMRFF9VH BITCOIN:TB1Q7A0G8SFXRFKCGY77PPDTS964ZHAHP7KMRFF9VH While tested okay with URI of no-predefined info. I observed some weird behavior while copying a URI with a specified label.
Copying a URI with a “test_label”:
Master PR bitcoin:TB1QFT0PFF87246F607S895CUSYEHJNGWUZLN69A3F?label=test_label bitcoin:tb1qft0pff87246f607s895cusyehjngwuzln69a3f?label=test_label When I specified the request label, the PR lowercased the address part of the URI. I don’t know if this is a deliberate behavior by the PR or a bug. If this is indeed an intentional behavior, then the PR description should address and explain this. Otherwise, OP should look into this issue.
-
benthecarman commented at 3:32 pm on August 30, 2021: contributor
When I specified the request label, the PR lowercased the address part of the URI. I don’t know if this is a deliberate behavior by the PR or a bug. If this is indeed an intentional behavior, then the PR description should address and explain this. Otherwise, OP should look into this issue.
This is intentional and shouldn’t have an effect on users. Since the address is case insensitive it doesn’t change anything.
-
shaavan commented at 4:46 pm on August 30, 2021: contributor
This is intentional and shouldn’t have an effect on users. Since the address is case insensitive it doesn’t change anything.
Makes sense. But I would personally prefer to avoid adding any unnecessary changes, that are changing the behavior with no actual benefits.
-
benthecarman commented at 8:12 pm on August 30, 2021: contributorThere are benefits, the code is much cleaner this way and the URI makes more sense to have a lowercase bech32 addr when we aren’t going full caps since this is the default defined by the BIP
-
shaavan commented at 8:30 pm on August 30, 2021: contributor
Now you do mention it, I think these benefits are worth the change.
tACK b6704cf7e1b005d4122c032485c9b5bba1359da8
-
hebasto added this to the milestone 23.0 on Sep 3, 2021
-
hebasto added the label Needs release notes on Sep 3, 2021
-
hebasto added the label Wallet on Sep 3, 2021
-
hebasto commented at 11:22 am on September 3, 2021: member
Tested b6704cf7e1b005d4122c032485c9b5bba1359da8.
tb1q4dcd7mdjdlfgdhjhz8rg5gw2834udp2u0n9jjs
:
master PR I see no benefits for encoding: both QR codes have the same 29x29 size.
tb1qnle64twzzjae9mrryy02z30hqaca5747rsanxt
+amount=0.42000000
:
master PR Both QR codes have the same 33x33 size. Is this behavior change justified (at least, it is not mentioned in the PR description)?
There are benefits, the code is much cleaner this way…
I guess, this statement is wrong.
-
benthecarman commented at 3:54 pm on September 3, 2021: contributor
I see no benefits for encoding: both QR codes have the same 29x29 size.
Huh, does qrencode not support mixed modes? I don’t see anything about it on the repo https://github.com/fukuchi/libqrencode
-
benthecarman commented at 3:56 pm on September 3, 2021: contributor
There are benefits, the code is much cleaner this way… I guess, this statement is wrong.
This was inrespect to @ShaMan239’s comment about keeping the address uppercase even when everything else is lowercase, but still doing all uppercase when possible
-
hebasto commented at 4:01 pm on September 3, 2021: member
There are benefits, the code is much cleaner this way… I guess, this statement is wrong.
This was inrespect to @ShaMan239’s comment about keeping the address uppercase even when everything else is lowercase, but still doing all uppercase when possible
I beg pardon for misunderstanding on my side. I was thinking about QR code, not the C++ code. Sorry again.
-
hebasto commented at 4:49 pm on September 3, 2021: member
I see no benefits for encoding: both QR codes have the same 29x29 size.
Huh, does qrencode not support mixed modes? I don’t see anything about it on the repo https://github.com/fukuchi/libqrencode
It does. If an URI were 47 or less symbols length, this PR makes the GUI generate a QR code version 2 (25x25).
But the shortest URI with bech32 address is 50 characters long.
-
benthecarman commented at 10:00 pm on September 4, 2021: contributor
It does. If an URI were 47 or less symbols length, this PR makes the GUI generate a QR code version 2 (25x25). But the shortest URI with bech32 address is 50 characters long.
hmm so is it even worth it to do caps at all?
-
hebasto commented at 7:46 pm on September 7, 2021: member
It does. If an URI were 47 or less symbols length, this PR makes the GUI generate a QR code version 2 (25x25). But the shortest URI with bech32 address is 50 characters long.
hmm so is it even worth it to do caps at all?
For the one address type—P2WPKH—no, not worth it. Other address types need some research work.
-
hebasto removed this from the milestone 23.0 on Nov 21, 2021
-
RandyMcMillan commented at 8:45 pm on November 22, 2021: contributor
tested b6704cf7e1b005d4122c032485c9b5bba1359da8
on MacOS 10.15.7
-
achow101 commented at 5:07 am on December 7, 2021: memberI don’t see the benefit. With regtest taproot addresses (so the longest addresses we may make), the QR codes have identical 29x29 dimensions:
-
hebasto commented at 8:17 am on April 3, 2022: memberClosing this for now. Feel free to re-open if it seems required.
-
hebasto closed this on Apr 3, 2022
-
bitcoin-core locked this on Apr 3, 2023
-
hebasto removed the label Needs release notes on May 16, 2023
This is a metadata mirror of the GitHub repository bitcoin-core/gui. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-22 02:20 UTC
More mirrored repositories can be found on mirror.b10c.me