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

feat: add support for signet lntbs prefixes #58

Closed
wants to merge 1 commit into from

Conversation

sangaman
Copy link

This adds support for signet invoice prefixes, which are currently supported by lnd and other implementations. I spent some time trying to add some fixtures to the tests, but I was struggling with properly generating them so I just left it out. Manually decoding signet invoices worked properly for me in my own tests, however.

@junderw
Copy link
Member

junderw commented Apr 19, 2022

Please link something that confirms these network details. a BIP, a BOLT, some source code, anything.

@sangaman
Copy link
Author

Please link something that confirms these network details. a BIP, a BOLT, some source code, anything.

This is what I referenced: https://github.com/btcsuite/btcd/pull/1692/files#diff-9f06541084892d1eb141f46d7a578be16bd96312c9d935448a83b6f0f0eb35f5R697

I'll add it to the code in a comment.

The pubKeyHash value is also referenced in the bitcoin wiki: https://en.bitcoin.it/wiki/Signet

A different value of ADDRESSVERSION field ensures no signet Bitcoin addresses will work on the production network. (0x6F rather than 0x00)

@junderw
Copy link
Member

junderw commented Apr 22, 2022

  1. Please allow me to make edits.
  2. Post a link showing justification for "tbs"
  3. Add a test + switch from Travis to Github Actions (I made commits for this, can't push)

@sangaman
Copy link
Author

Closing this as evidently the discrepancy between the tbs prefix used for signet invoices mismatches the tb prefix used (at least by bitcoin core) for signet on-chain addresses. That has negative implications for the fallback address. Adding signet support would require refactoring the fallback address logic to use a different prefix from the invoice itself. In the meantime signet invoices can be decoded using a custom network.

@sangaman sangaman closed this Apr 22, 2022
@junderw
Copy link
Member

junderw commented Apr 22, 2022

OK. Just found a big problem.

I can't merge this.

LND decided that signet bolt11 invoices should use tbs... Bitcoin Core uses tb for bech32 addresses. The convention was lnXX1 where the XX would coincide with the network bech32 hrp ie. bc1 for mainnet would produce lnbc1 for mainnet lightning.

Now it's tb1 for bech32 and tbs1 for lightning for signet only.

Problem:

The same network object and same HRP string is used for both the lnXX1 XX part AND the bech32 HRP part for any fallback addresses inside the invoice.

Solution:

  1. Use a custom Network during decode ( add the network object with tbs into the second arg for decode)
  2. If a fallback address is present and it starts with tbs1 convert it to tb1 so it's a valid address manually...... or ignore fallback addresses for signet only.

@junderw
Copy link
Member

junderw commented Apr 22, 2022

ref: ElementsProject/lightning#4929

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

Successfully merging this pull request may close these issues.

2 participants