Skip to content
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

BIP353 seems not to be implemented correctly #588

Closed
tohrxyz opened this issue Jul 11, 2024 · 5 comments
Closed

BIP353 seems not to be implemented correctly #588

tohrxyz opened this issue Jul 11, 2024 · 5 comments

Comments

@tohrxyz
Copy link

tohrxyz commented Jul 11, 2024

I was reading about BIP353 today and found written here, where it says the new standard of encoding payment instructions according to BIP353 should use In order to somewhat reduce the incidence of such confusion, a ₿ prefix is used. .

From this I understand that BIP353 encoded addresses should use as prefix, because this can lead to incompatibility with other wallets.

Phoenix currently generates only [email protected] address, not [email protected] , which when copied and pasted for example to Wallet of Satoshi, that app recognizes it as familiar thing - lightning BOLT11 address, tries to parse it but fails.

In my opinion, if it had used ₿ prefix, devs of other wallets could see this as something different and it wouldn't cause confusion, because now when someone looks at it, they don't know if it's BIP353 DNS-based instruction or regular BOLT11 lightning address.

@Sjors
Copy link

Sjors commented Jul 11, 2024

@tohrxyz I think the BIP means it should be displayed with a prefix ₿. It's not part of the address itself.


Oh and for copy-pasting it should indeed add the ₿ prefix. But the BIP also says you should copy the URI, not the address - as that avoids another DNS lookup.

@Sjors
Copy link

Sjors commented Jul 11, 2024

I just opened a PR to the BIP repo to try and clarify the intention.

IIUC Phoenix should:

  1. Be able to parse [email protected] (it currently can't) and [email protected] (works, but see Prefer BIP353 over LNURL if found #589)
  2. When it has its own address, it should display that as [email protected] address (haven't tried, it's not on iOs yet)
  3. When its bolt12 is "paired" via a bip353 DNS record, ditto
  4. When putting such address on the clipboard, it should use the underlying bitcoin:?lno=lno1... URI, unless the user insists to copy the human readable version, in which case it should put [email protected] on the clipboard. (I haven't tried)

@Sjors
Copy link

Sjors commented Jul 11, 2024

@tohrxyz wrote:

that app recognizes it as familiar thing - lightning BOLT11 address, tries to parse it but fails

I think you mean an LNURL address?

because now when someone looks at it, they don't know if it's BIP353 DNS-based instruction or regular BOLT11 lightning address

I think it's fine that it could be either, apps just have to pick the "best" option. My own opinion: #589.

If the address is given with a ₿ prefix then the app knows for sure it needs to interpret is as bip353, as you say. But bip353 request apps to also work without the ₿ prefix, in which case it has to choose.

@tohrxyz
Copy link
Author

tohrxyz commented Jul 12, 2024

@Sjors

I think you mean an LNURL address?

Yeah, I mixed a few things in a rush to post this.

When putting such address on the clipboard, it should use the underlying bitcoin:?lno=lno1... URI, unless the user insists to copy the human readable version, in which case it should put ₿[email protected] on the clipboard. (I haven't tried)

That's interesting approach, might make sense to work with underlying code by default and let human-readable address be for when people are saying that to each other by speech or some different low-bandwidth method.

pm47 added a commit to ACINQ/phoenixd that referenced this issue Jul 12, 2024
@dpad85 dpad85 closed this as completed in 58d67c8 Jul 12, 2024
@dpad85
Copy link
Member

dpad85 commented Jul 12, 2024

Thank you for reporting this issue. We did miss a few things when implementing BIP353. In addition to the problems mentioned above, Phoenix could not handle BIP21 URI with a lno parameter. Most of these issues should be fixed with 58d67c8.

Regarding what should be copied, when hitting "Copy" on the offer screen, the app will let you pick what you want to copy. On Android:
image

The fix should be available in version 2.3.4, released soon. Do not hesitate to reopen this issue if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants