Skip to content

Wrong BOLT11 prefix in regtest#1150

Merged
Roasbeef merged 1 commit intobtcsuite:masterfrom
vapopov:hrprefix-btcsuite
Apr 18, 2018
Merged

Wrong BOLT11 prefix in regtest#1150
Roasbeef merged 1 commit intobtcsuite:masterfrom
vapopov:hrprefix-btcsuite

Conversation

@vapopov
Copy link
Contributor

@vapopov vapopov commented Mar 21, 2018

Initially was observed in lnd project for this issue lightningnetwork/lnd#882

if to try to generate invoice for regtest net, we get encoded invoice with wrong prefix in human readable part

stevenroose added a commit to stevenroose/bips that referenced this pull request Mar 22, 2018
A mismatch between implementation occurred on the bech32 prefix for
regtest mode.  Bitcoin Core decided to use the `bcrt` prefix in order
for it to be distinguishable from the testnet prefix.  btcd on the other
hand uses the same prefix as testnet because of lack of a separate
prefix specified in this BIP.

Core: bitcoin/bitcoin#12314
btcd: btcsuite/btcd#1150
@stevenroose
Copy link
Contributor

stevenroose commented Mar 22, 2018

I can't find any mention of this prefix in either BOLT11 or BIP 173 (bech32).

So btcd doesn't use a wrong prefix, just a different one from Core.

@vapopov
Copy link
Contributor Author

vapopov commented Mar 22, 2018

сс @DanielWeigl

@DanielWeigl
Copy link

@stevenroose yes, thats true - but that also makes it impossible to test payments to invoices across different implementations in regtest

I also already proposed to add a clarification to bolt11 lightning/bolts#397

I think it would make sense to have a common format in regtest, or?

@DanielWeigl
Copy link

DanielWeigl commented Mar 24, 2018

FYI, current state:

(all nodes run in regtest)

So, lnbcrt is currently in minority, but i still think it would make more sense to use it - but whatever the decision is, i guess it would be really needed to have the same.

@vapopov vapopov force-pushed the hrprefix-btcsuite branch from b39bae2 to 04fd662 Compare April 2, 2018 11:55
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💈

@Roasbeef Roasbeef merged commit 675abc5 into btcsuite:master Apr 18, 2018
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.

4 participants