Qt: advise users not to switch wallets when opening a BIP70 URI.

jameshilliard commented at 11:12 am on September 12, 2019: contributor

It would probably be a good idea to have something like this before #15584 is merged.

Qt: advise users not to switch wallets when opening a BIP70 URI. 1153caf78e

fanquake added the label GUI on Sep 12, 2019

laanwj commented at 11:21 am on September 12, 2019: member

Thanks! (hadn’t noticed that this was a PR, not an issue)

laanwj added this to the milestone 0.19.0 on Sep 12, 2019

jonasschnelli commented at 12:41 pm on September 12, 2019: contributor

Thanks for adding this. utACK 1153caf78e21f6a9406430b704c7c74eb90975ca Without further objectsions, this gets merged on the 15th of Sept.

TheBlueMatt commented at 7:35 pm on September 12, 2019: member

String-only ACK.

in src/qt/paymentserver.cpp:332 in 1153caf78e

327@@ -328,7 +328,9 @@ void PaymentServer::handleURIOrFile(const QString& s)
328 #ifndef ENABLE_BIP70
329                     if (uri.hasQueryItem("r")) {  // payment request
330                         Q_EMIT message(tr("URI handling"),
331-                            tr("Cannot process payment request because BIP70 support was not compiled in."),
332+                            tr("Cannot process payment request because BIP70 support was not compiled in.")+
333+                            tr("Due to widespread security flaws in BIP70 it's strongly recommended that any merchant instructions to switch wallets be ignored.")+

achow101 commented at 8:00 pm on September 12, 2019:

nit: Spaces at beginning of each sentence, or maybe a <br>. Currently the notification puts these as one line and it kind of runs on. It’s a giant block of text.

jameshilliard commented at 11:14 am on September 14, 2019:

I’ll probably do a follow up commit to clean up formatting once I’m no longer traveling.

achow101 commented at 8:02 pm on September 12, 2019: member

Screenshot:

Screenshot_20190912_160205

fanquake approved

fanquake commented at 7:24 am on September 13, 2019: member

ACK 1153caf78e21f6a9406430b704c7c74eb90975ca

Whilst I agree that some more spacing would be nice, if this commit doesn’t get updated it can just be merged as is. Did bare-minimum testing on macOS.

message

laanwj commented at 12:52 pm on September 13, 2019: member

With so much text, it might make sense to turn this into an a popup instead of a notification. No one reads so much text in a notification so quickly.

TheBlueMatt commented at 11:11 am on September 14, 2019: member

Maybe drop the last sentence or just rephrase to “Cannot process payment request - BIP70 has been disabled by default due to widespread security flaws. Ignore any recommendations to change wallets to use BIP 70”.

jameshilliard commented at 11:17 am on September 14, 2019: contributor

@TheBlueMatt I think it’s important to have an explicit recommendation on what’s needed to fix the error. The instructions for users to contact merchants to provide a BIP21 compatible URI should help discourage merchants from forcing BIP70 by increasing support requests if they refuse to provide BIP21 compatible URI’s(reduced support costs are the main reason BIP70 is still being forced by processors).

TheBlueMatt commented at 11:48 am on September 14, 2019: member

Hmm, but its ultimately not up to the user - asking for BIP21 from the merchant isn’t so different from asking for “not BIP70”.

jameshilliard commented at 12:03 pm on September 14, 2019: contributor

Well it’s up to the merchant ultimately to support it, but if a bunch of users submit support requests to the merchant asking for BIP21 compatible URI’s then it’s much more likely the merchant will add support for BIP21 due to customer demand as opposed to potentially some other protocol.

TheBlueMatt commented at 12:42 pm on September 14, 2019: member

I mean BIP21 is really just an address and bitcoin: in front, I don’t know that the response would be anything but that, no?

On Sep 14, 2019, at 13:04, James Hilliard notifications@github.com wrote:

Well it’s up to the merchant ultimately to support it, but if a bunch of users submit support requests to the merchant asking for BIP21 compatible URI’s then it’s much more likely the merchant will add support for BIP21 due to customer demand as opposed to potentially some other protocol.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jameshilliard commented at 1:06 pm on September 14, 2019: contributor

Well you would want to specify BIP21 so that the merchant doesn’t just suggest a different incompatible BIP70-like payment method.

TheBlueMatt commented at 1:25 pm on September 14, 2019: member

Does literally anyone implement that? I really don’t think it’s unclear and no users are gonna read that long text.

On Sep 14, 2019, at 14:06, James Hilliard notifications@github.com wrote:

Well you would want to specify BIP21 so that the merchant doesn’t just suggest a different incompatible BIP70-like payment method.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jameshilliard commented at 5:18 pm on September 14, 2019: contributor

Does literally anyone implement that?

Bitpay does apparently as an alternative to BIP70.

TheBlueMatt commented at 5:34 pm on September 14, 2019: member

Lol I meant on the client side.

On Sep 14, 2019, at 18:18, James Hilliard notifications@github.com wrote:

Does literally anyone implement that?

Bitpay does apparently as an alternative to BIP70.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jameshilliard commented at 6:41 pm on September 14, 2019: contributor

Well I think the point is to ensure that customers are not just requesting a BIP70 alternative but BIP21 specifically as Bitpay is likely to suggest using jsonPaymentProtocol if the request is not explicitly for BIP21 support. I wonder if it would make sense to link to a tool like https://github.com/achow101/payment-proto-interface in the error message for decoding bitpay requests.

TheBlueMatt commented at 8:58 pm on September 14, 2019: member

I mean I’m not sure how you capture that in a way that (a) users understand and (b) is actually short enough that any user will read it (cause the current text no one is gonna read).

jonasschnelli commented at 6:07 pm on September 15, 2019: contributor

Going to merge this. Turing into a popup, if that makes sense, can be done later.

jonasschnelli referenced this in commit a40ccbb195 on Sep 15, 2019

jonasschnelli merged this on Sep 15, 2019

jonasschnelli closed this on Sep 15, 2019

sidhujag referenced this in commit b2edf4eee8 on Sep 16, 2019

luke-jr referenced this in commit b7087a66b0 on Sep 21, 2019

DrahtBot locked this on Dec 16, 2021

Qt: advise users not to switch wallets when opening a BIP70 URI. #16858