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

Add pico amount example to Bolt11 test vectors #692

Open
Sword-Smith opened this issue Oct 31, 2019 · 8 comments

Comments

@Sword-Smith
Copy link

@Sword-Smith Sword-Smith commented Oct 31, 2019

Several libraries (at least two libraries and one web service) have problems with the p numbers representing 10^(-12) BTC. So an example with this encoding should be included in the test vectors. You can use this one if you like:

lnbc9678785340p1pwmna7lpp5gc3xfm08u9qy06djf8dfflhugl6p7lgza6dsjxq454gxhj9t7a0sd8dgfkx7cmtwd68yetpd5s9xar0wfjn5gpc8qhrsdfq24f5ggrxdaezqsnvda3kkum5wfjkzmfqf3jkgem9wgsyuctwdus9xgrcyqcjcgpzgfskx6eqf9hzqnteypzxz7fzypfhg6trddjhygrcyqezcgpzfysywmm5ypxxjemgw3hxjmn8yptk7untd9hxwg3q2d6xjcmtv4ezq7pqxgsxzmnyyqcjqmt0wfjjq6t5v4khxxqyjw5qcqp2rzjq0gxwkzc8w6323m55m4jyxcjwmy7stt9hwkwe2qxmy8zpsgg7jcuwz87fcqqeuqqqyqqqqlgqqqqn3qq9qs4x9qlmd57lq7wwr23n3a6pkayy3jpfucyptlncs2maswe3dnnjy3ce2cgrvykmxlfpvn6ptqfqz4df5uaulvd4hjkckuqxrqqkz8jgphputwh
@Sword-Smith

This comment has been minimized.

Copy link
Author

@Sword-Smith Sword-Smith commented Oct 31, 2019

Libraries and web services unable to decode this invoice

0: btcpayserver/BTCPayServer.Lightning#25 (interprets p as 10^(-11) BTC)
1: bitcoinjs/bolt11#27
2: https://lightningdecoder.com/ (reported Amount is outside of valid range on 2019.10.31)

Sword-Smith added a commit to Sword-Smith/lightning-rfc that referenced this issue Nov 1, 2019
This addresses lightningnetwork#692
Sword-Smith added a commit to Sword-Smith/lightning-rfc that referenced this issue Nov 1, 2019
This addresses lightningnetwork#692
@NicolasDorier

This comment has been minimized.

Copy link

@NicolasDorier NicolasDorier commented Nov 1, 2019

m (milli): multiply by 0.001
u (micro): multiply by 0.000001
n (nano): multiply by 0.000000001
p (pico): multiply by 0.000000000001

In BTCPay our conversion in Millisatoshi is the following:

    public enum LightMoneyUnit : ulong
    {
        BTC = 100_000_000_000,
        MilliBTC = 100_000_000,
        Bit = 100_000,
        Micro = 100_000,
        Satoshi = 1000,
        Nano = 100,
        MilliSatoshi = 1,
        Pico = 1
    }

So 1000 millisatoshi = 1000 pico = 1 satoshi. I can't see the issue.

@NicolasDorier

This comment has been minimized.

Copy link

@NicolasDorier NicolasDorier commented Nov 1, 2019

ah sorry 10 milli satoshi = 1 pico ?

@NicolasDorier

This comment has been minimized.

Copy link

@NicolasDorier NicolasDorier commented Nov 1, 2019

Ah got it... a pico is less than a milli satoshi, but all API deals with Milli satoshi...

@NicolasDorier

This comment has been minimized.

Copy link

@NicolasDorier NicolasDorier commented Nov 1, 2019

@Sword-Smith I am wondering if I fix it as nobody is using less than MilliSatoshi.

Internally our class is doing math on a long millisatoshi whose max value is
9,223,372,036,854,770,000.

As you can see, we can represent 21 millions bitcoin in term of MilliSatoshi on a long:

02,100,000,000,000,000,000
09,223,372,036,854,770,000

But if we do our arithmetic on pico, then we can't represent the maximum amount of bitcoin on a long.

@NicolasDorier

This comment has been minimized.

Copy link

@NicolasDorier NicolasDorier commented Nov 1, 2019

Sorry for the spam. I decided to not support pico for now:

btcpayserver/BTCPayServer.Lightning#25 (comment)

@NicolasDorier

This comment has been minimized.

Copy link

@NicolasDorier NicolasDorier commented Nov 2, 2019

I decided to support pico, just not values that are not multiple of 10. Changing the minimum denomination below 1 millisatoshi is impossible at this point in my library without breaking lot's of things.

I think that a similar restriction should be placed in BOLT11, as sub milli satoshi is not possible at the protocol level.

@Sword-Smith

This comment has been minimized.

Copy link
Author

@Sword-Smith Sword-Smith commented Nov 2, 2019

I decided to support pico, just not values that are not multiple of 10. Changing the minimum denomination below 1 millisatoshi is impossible at this point in my library without breaking lot's of things.

I think that a similar restriction should be placed in BOLT11, as sub milli satoshi is not possible at the protocol level.

I honestly think this is the correct solution in that this is what every one else is doing. I couldn't find that requirement in the specification. If it isn't there, I fully agree that it should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.