ACINQ / phoenix

Phoenix is a self-custodial Bitcoin wallet using Lightning to send/receive payments.
https://phoenix.acinq.co
Apache License 2.0
644 stars 97 forks source link

Allow phoenix: uri prefix in in-app qr scanner #368

Closed davidknezic closed 1 year ago

davidknezic commented 1 year ago

It's great to see the existence of the phoenix: uri prefix for deep-linking into Phoenix specifically:

I noticed that when using the in-app scanner, the phoenix: uri prefix breaks reading an invoice or LNURL:

phoenix:lightning:LNURL... [lightning:]LNURL...
phoenix phoenix

Would you accept a contribution to add support for the phoenix: and phoenix:// prefixes to the in-app scanner, too?

This would make the behaviour consistent no matter if scanned with a system qr scanner or inside of Phoenix itself.

dpad85 commented 1 year ago

Hello David, thanks for reporting this issue. I was actually testing that this morning, and noticed that the scanner would not trim the phoenix scheme from the scanned data. I already made a fix which should be available in the next version (though that version will not be released soon, as it contains many significant changes, see #356).

In any case I don't think the QR code should contain the phoenix:lightning: scheme, or any scheme in fact. It should only contain the relevant payload, in your case the LNURL data. The scheme makes the QR code heavier (making in marginally harder to read) and is ignored anyway.

That being said, Phoenix should be smart and be able to read that kind of QR codes, so the fix is still needed.

dpad85 commented 1 year ago

Just to be sure, I tried scanning a QR-code containing the phoenix:lightning: scheme directly from the OS camera app, wondering if it would redirect to Phoenix (like it would with a phoenix:lightning: hyperlink):

So having the scheme in the QR code could still be useful in some cases.

dpad85 commented 1 year ago

Closing this ticket as the issue is fixed, re-open it if not, thanks!