getAlby / bitcoin-connect

Connecting lightning wallets to your webapp has never been easier. Enable WebLN in all browsers with a single button
https://bitcoin-connect.com
MIT License
79 stars 26 forks source link

Remove external payment options from payment request modal #212

Closed positiveblue closed 2 months ago

positiveblue commented 2 months ago

Problem

The payment request modal currently offers users two options:

Screenshot 2024-04-24 at 6 15 19 PM

I am integrating bitcoin-connect into a service based on the L402 protocol. For the protocol to function correctly, it is essential that the client has access to the invoice preimage. Allowing the user to pay the invoice with an external wallet that is not connected introduces complications and makes it difficult to properly implement the L402 protocol.

Proposed solution

I propose making the "or + QR + copy invoice" HTML components optional in the payment request modal UI.

By providing a way to disable or hide these elements when needed, we make it really hard for the user to "accidentally" pay the invoice in a way that the client does not have access to the preimage.

Alternative Solution/Workarounds

I am more than happy to create a PR adding the prop + logic to hide this section of the html but I want to hear professional advice first :)

rolznz commented 2 months ago

Hi @positiveblue

Optimally Polling the server to check if the payment has been received would be the best option, but I can see it might not always be possible.

Maybe we can have a allowExternalPayment boolean that is default to true. If false, it hides the external payment options.

CC @reneaaron @bumi

positiveblue commented 2 months ago

Polling the server to check if the payment has been received would be the best option

Agree. We take the preimage as proof of payment so it's possible that the server/service that takes the credentials does not have access to an invoice status.

Maybe we can have a allowExternalPayment boolean that is default to true.

I agree that by default most of the people want it set to true. So whatever route we follow (showEncodedInvocie or hideEncodedInvoice the default should to have the QR visible.

Also, should we consider yet another section with a text field (from) for introducing the preimage manually (I would go with no, but in case you see it necessary)

rolznz commented 2 months ago

should we consider yet another section with a text field (from) for introducing the preimage manually

No, I think users should never have to manually paste a preimage. Most people have no idea what it is, and I'm not sure what they gain from doing this or even where they get that preimage from

rolznz commented 2 months ago

It also makes sense to only show the external options in some cases. I wonder if instead we add a property paymentMethods which can be "all" or "internal" or "external"