New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Android] Problem with BitPay invoice #812

Closed
jonas-lundqvist opened this Issue Jul 27, 2018 · 10 comments

Comments

Projects
None yet
5 participants
@jonas-lundqvist

jonas-lundqvist commented Jul 27, 2018

Yesterday I had some issues paying a BitPay invoice using the Android version of Electron Cash (3.3 "Classic", latest on Google Play Store). This is the first time I used anything else then the desktop (Linux) version for BitPay invoices.

At checkout at the merchant the QR code on the invoice was show and I scanned it with my phone successfully. I then entered my PIN code and sent the transaction but I got an error message (in the web browser) after 5-ish seconds that the invoice had expired from BitPay.

I think less approximately 2 minute had passed between the time the invoice was shown and the error message appeared, which is way less then the 15 minute timeout that is the default.

I had a mail discussion with the BitPay support and received a refund. I could the successfully pay the same item with the desktop Linux version (3.3.1) of Electron Cash.

BitPay could not find any issues on their side and wanted me to contact Electron Cash (you guys). I can send the mail conversation containing txid's and stuff if you provide a mail address .

Phone: Nokia 8 (TA-1008)
OS: Android 8.1.0, July 1 security patch level

@fyookball

This comment has been minimized.

Collaborator

fyookball commented Jul 27, 2018

I don't know if Android supports Bip70.

@jonas-lundqvist

This comment has been minimized.

jonas-lundqvist commented Jul 27, 2018

I would call that a quite serious issue since Electron Cash is listed as supported on the BitPay website. The UX of a failed payment that needs to be refunded by BitPay is pretty catastrophic to users.

If I had time over I would like to look into this issue, but sadly spare time is not something I have much of. Is there any news/updates concerning the native Android app that I think I heard about at Reddit? If I would find time it would be a shame if it were a waste since that code would be obsolete.

@fyookball

This comment has been minimized.

Collaborator

fyookball commented Jul 27, 2018

native android wont be ready for a bit although its coming along. we can look into fixing this for classic android

@rt121212121

This comment has been minimized.

Collaborator

rt121212121 commented Jul 28, 2018

What we can do in the short term is get Bitpay to make sure only desktop and maybe if it supports it, ios, are noted as supporting payment requests.

@jonas-lundqvist

This comment has been minimized.

jonas-lundqvist commented Jul 29, 2018

@deeb33

This comment has been minimized.

deeb33 commented Aug 28, 2018

I just stumbled across this as well. (Wish I'd have seen the issue before.) Anyways, the error from the Bitpay web page was that I didn't pay using a payment protocol supporting wallet. I've sent off to Bitpay to get a refund and will report any additional information here.

As an aside, I tried to install the Windows version on a Windows 7 machine. The install fails due to missing DLLs.

Edit: Ok, got the reply from Bitpay. They reiterate that the payment was from an incompatible wallet (or was not sent using the payment protocol). They also say they don't accept transactions using unconfirmed inputs. I specifically waited for a confirmation on the tx to the Electron Cash wallet before proceeding with the Bitpay payment.

However, after the tx to the Electron Cash wallet confirmed, the wallet indicated that the tx was "Not Verified" and remains in that state.

I agree with a previous suggestion to disable handing of payment protocol requests until the issue is resolved.

Electron Cash: 3.3.1 android from the play store
Android: 4.2
Phone: Blu Advance 4.0

@pieterpoorthuis

This comment has been minimized.

pieterpoorthuis commented Sep 3, 2018

Pieter from BitPay here. Most common issues with payment protocol are:

  1. Transaction is broadcasted to the P2P network before the server (BitPay in this case) has responded with HTTP 200 to the client.
  2. Transaction contains unconfirmed inputs
  3. Transaction is sent with RBF

(1) is rejected by BitPay in the way that OP describes. So this is likely happening with Android Electron Cash. It can be fixed by following payment protocol to the end and thus sending the transaction to the server before broadcasting the transaction to the P2P network.
(2) and (3) will display an error on the BitPay invoice.

BitPay has written a technical write up about a JSON implementation of PayPro here: https://github.com/bitpay/jsonPaymentProtocol/blob/master/specification.md

We are working on improving the messaging in the invoice on our end, so issues can be found easier.

@jonas-lundqvist

This comment has been minimized.

jonas-lundqvist commented Sep 10, 2018

@pieterpoorthuis
Just out of curiosity... is the HTTP 200 you speak about an "invisible" arrow from 'Merchant Server' back to the 'Wallet App' after the payment is submitted in the BIP-70 specification (https://raw.githubusercontent.com/bitcoin/bips/master/bip-0070/Protocol_Sequence.png) but before the wallet app broadcasts the transaction?

@pieterpoorthuis

This comment has been minimized.

pieterpoorthuis commented Sep 11, 2018

I'm talking about the PaymentACK, after the wallet sends the payment to the server.
The BIP70 spec is not clear on what should happen if the server rejects the payment. We have clarified this here: https://github.com/bitpay/jsonPaymentProtocol/blob/master/specification.md

Most important clarification is that the wallet should not broadcast the payment to the P2P network until the server has accepted the payment.

jonas-lundqvist added a commit to jonas-lundqvist/Electron-Cash that referenced this issue Sep 24, 2018

Do not broadcast transaction for a payment request
A transaction from a BIP70 invoice should not be broadcasted by the
wallet.
If a valid PaymentACK is received it is up to the merchant to broadcast
the transaction.
This also fixes a nasty race condition where a transaction was
broadcasted before the merchant had ACK'd it which would likley cause
the user to loose money.

Also updated the memo field from "Electrum" to "Electron Cash".

Fixes Electron-Cash#812

jonas-lundqvist added a commit to jonas-lundqvist/Electron-Cash that referenced this issue Sep 26, 2018

Do not broadcast transaction for a payment request
A transaction from a BIP70 invoice should not be broadcasted by the
wallet.
If a valid PaymentACK is received it is up to the merchant to broadcast
the transaction.
This also fixes a nasty race condition where a transaction was
broadcasted before the merchant had ACK'd it which would likley cause
the user to loose money.

Also updated the memo field from "Electrum" to "Electron Cash".

Fixes Electron-Cash#812
@jonas-lundqvist

This comment has been minimized.

jonas-lundqvist commented Sep 30, 2018

I have done some investigations on this issue and noticed that it also affects the desktop version of Electron Cash where a transaction might reach BitPay via the P2P network before it is posted as a BIP-70 payment. I was able to trigger that behavior on the master branch using test.bitpay.com where the payment failed but the wallet had already broadcasted the transaction.

However I twist and turn it the requirement of the user to not broadcast the transaction before the merchant has acked it seems to be non compliant with Payment Protocol (BIP-70). That being said, I do agree that BitPays requirement makes sense and provides a much better UX. Does this break any other service using BIP-70? Perhaps some merchants are using a BIP-70 service that actually waits for the transaction to be received via the P2P network before considering the invoice paid. That would be "less" of a problem because the user has not lost any money. Still bad because the payment failed and it was not the users fault.
@pieterpoorthuis Have you considered increasing the "payment_details_version" so wallets can distinguish between the standard BIP-70 behavior and BitPay behavior? Obviously that would need a new BIP and community consensus.

The PR that is linked to this issue should fix BitPay invoices for the desktop version. I have not done the work needed in the Android code but I feel like this is something that should be taken seriously and fixed!

@cculianu cculianu closed this in #868 Oct 3, 2018

cculianu added a commit that referenced this issue Oct 3, 2018

Do not broadcast transaction for a payment request (#868)
* Do not broadcast transaction for a payment request

A transaction from a BIP70 invoice should not be broadcasted by the
wallet.
If a valid PaymentACK is received it is up to the merchant to broadcast
the transaction.
This also fixes a nasty race condition where a transaction was
broadcasted before the merchant had ACK'd it which would likley cause
the user to loose money.

Also updated the memo field from "Electrum" to "Electron Cash".

Fixes #812

* Explicitly check for HTTP 200 OK for PaymentACK

A payment processor using BIP70 shall respond with HTTP 200 OK status
response.

* Propagate error message from paymentrequest

* Change send_ack to send_payment in paymentrequest

The function is sending a Payment message and receiving a PaymentACK
message and thus it was incorrectly named.

* Dont save and mark payment req as paid on failure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment