Uppercase 'bitcoin:' in QR code URI

benthecarman commented at 9:01 pm on August 23, 2021: contributor

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

https://github.com/btcpayserver/btcpayserver/issues/2110

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: contributor

ACK ae7bc4dc4014ffca0d68f53cdc9b27ed0aae52f0

kristapsk commented at 10:02 am on August 24, 2021: contributor

It only makes sense to do that if final paramCount is 0, 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: contributor

Used @kristapsk suggestion

kristapsk approved

kristapsk commented at 3:39 pm on August 24, 2021: contributor

ACK 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: contributor

There 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)?

@benthecarman

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

Screen Shot 2021-11-22 at 3 39 49 PM

achow101 commented at 5:07 am on December 7, 2021: member

I don’t see the benefit. With regtest taproot addresses (so the longest addresses we may make), the QR codes have identical 29x29 dimensions: Screenshot_20211206_235940

hebasto commented at 8:17 am on April 3, 2022: member

Closing 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

Uppercase ‘bitcoin:’ in QR code URI #410