search

bitcoinjs / bolt11

A library for encoding and decoding lightning network payment requests as defined in BOLT #11.
MIT License
92 stars 64 forks source link

feat: add support for signet lntbs prefixes #58

Closed sangaman closed 2 years ago

sangaman commented 2 years ago

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 commented 2 years ago

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

sangaman commented 2 years ago

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 commented 2 years ago
  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)
junderw commented 2 years ago

Found tbs:

https://github.com/lightningnetwork/lnd/blob/440f4d982c67aec6105657cf62abbc966ae72d3c/zpay32/decode.go#L48-L58

sangaman commented 2 years ago

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.

junderw commented 2 years ago

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 commented 2 years ago

ref: https://github.com/ElementsProject/lightning/pull/4929