Qt: advise users not to switch wallets when opening a BIP70 URI. #16858
Conversation
|
Thanks! (hadn't noticed that this was a PR, not an issue) |
|
Thanks for adding this. |
|
String-only ACK. |
| Q_EMIT message(tr("URI handling"), | ||
| tr("Cannot process payment request because BIP70 support was not compiled in."), | ||
| tr("Cannot process payment request because BIP70 support was not compiled in.")+ | ||
| tr("Due to widespread security flaws in BIP70 it's strongly recommended that any merchant instructions to switch wallets be ignored.")+ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I'll probably do a follow up commit to clean up formatting once I'm no longer traveling.
|
Screenshot:
|
|
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. |
|
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". |
|
@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). |
|
Hmm, but its ultimately not up to the user - asking for BIP21 from the merchant isn't so different from asking for "not BIP70". |
|
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. |
|
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 ***@***.***> 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.
|
|
Well you would want to specify BIP21 so that the merchant doesn't just suggest a different incompatible BIP70-like payment method. |
|
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 ***@***.***> 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.
|
Bitpay does apparently as an alternative to BIP70. |
|
Lol I meant on the client side.
… On Sep 14, 2019, at 18:18, James Hilliard ***@***.***> 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.
|
|
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. |
|
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). |
|
Going to merge this. |
…IP70 URI. 1153caf Qt: advise users not to switch wallets when opening a BIP70 URI. (James Hilliard) Pull request description: It would probably be a good idea to have something like this before #15584 is merged. ACKs for top commit: jonasschnelli: utACK 1153caf fanquake: ACK 1153caf Tree-SHA512: 6e682dd280c44eaafb1206c32439df42a20173c33297bf93dd607f0a7a2faec8e2d17fff83c85027083ebd11a71795b443e707992251574370dd1d46b7bff060
…ing a BIP70 URI. 1153caf Qt: advise users not to switch wallets when opening a BIP70 URI. (James Hilliard) Pull request description: It would probably be a good idea to have something like this before bitcoin#15584 is merged. ACKs for top commit: jonasschnelli: utACK 1153caf fanquake: ACK 1153caf Tree-SHA512: 6e682dd280c44eaafb1206c32439df42a20173c33297bf93dd607f0a7a2faec8e2d17fff83c85027083ebd11a71795b443e707992251574370dd1d46b7bff060
Github-Pull: bitcoin#16858 Rebased-From: 1153caf
It would probably be a good idea to have something like this before #15584 is merged.