Add bolt11 test vector with amount in `p` units #699

Sword-Smith · 2019-11-13T22:09:49Z

This addresses #692. But as part of #692 some have also requested that the RFC is changed to specify that any amount with the pico-multiplier MUST be divisible by 10. This request is addressed in PR #700 (now merged).
+
+Breakdown:
+
+* lnbc: prefix, Lightning on bitcoin mainnet
+* 9678785340p: amount (9678785340 pico-bitcoin = 967878534 milli satoshi)
+* 1: Bech32 separator
+* pwmna7l: timestamp (1572468703)
+* p: payment hash...
+* d: short description

- 8d: data_length (8 = 7, d = 13; 7 * 32 + 13 == 237)
- gfkx7cmtwd68yetpd5s9xar0wfjn5gpc8qhrsdfq24f5ggrxdaezqsnvda3kkum5wfjkzmfqf3jkgem9wgsyuctwdus9xgrcyqcjcgpzgfskx6eqf9hzqnteypzxz7fzypfhg6trddjhygrcyqezcgpzfysywmm5ypxxjemgw3hxjmn8yptk7untd9hxwg3q2d6xjcmtv4ezq7pqxgsxzmnyyqcjqmt0wfjjq6t5v4khx: 'Blockstream Store: 88.85 USD for Blockstream Ledger Nano S x 1, "Back In My Day" Sticker x 2, "I Got Lightning Working" Sticker x 2 and 1 more items'
  +* x: expiry time
- qy: data_length (q = 0, y = 2; 0 * 32 + 4 == 4)
- jw5q: 604800 seconds (j = 18, w = 14, 5 = 20, q = 0; 18 * 32^3 + 14 * 32^2 + 20 * 32 + 0 == 604800)
  +* r: tagged field: route information
- zj: data_length (z = 2, j = 18; 2 * 32 + 18 == 82)
  +* s4x9qlmd57lq7wwr23n3a6pkayy3jpfucyptlncs2maswe3dnnjy3ce2cgrvykmxlfpvn6ptqfqz4df5uaulvd4hjkckuqxrqqkz8jgp: signature
  +* hputwh: Bech32 checksum

Please send $30 for coffee beans to the same peer, which supports features 1 and 9

lnbc25m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdees9qzsze992adudgku8p05pstl6zh7av6rx2f297pv89gu5q93a0hf3g7lynl3xq56t23dpvah6u7y9qey9lccrdml3gaqwc6nxsl5ktzm464sq73t7cl

Sword-Smith · 2019-11-13T22:28:53Z

The error reported is some sort of alleged spelling error. But both words are correctly spelled.

"Misspelled words in 11-payment-encoding.md: Blockstream USD Blockstream"

t-bast · 2019-11-14T08:36:21Z

The error reported is some sort of alleged spelling error.

You need to update the spellchecker dictionary (.aspell.en.pws) to add new unknown words (in your case USB / Blockstream).

Sword-Smith · 2019-11-14T11:15:12Z

The error reported is some sort of alleged spelling error.

You need to update the spellchecker dictionary (.aspell.en.pws) to add new unknown words (in your case USB / Blockstream).

Commit has been updated with these additional changes.

t-bast · 2019-11-14T12:04:41Z

Thanks, I'll test this vector as soon as I have a bit of time ;)

Sword-Smith · 2019-12-02T11:05:35Z

How's it looking with your time, @t-bast? :)

t-bast · 2019-12-02T12:07:45Z

I'm sorry, I'm really swamped right now with Trampoline work...
But I'll find a bit of time this week to test this (so that it's tested before next monday's meeting).

t-bast · 2019-12-03T08:26:26Z

Sorry for the wait, thanks for your patience!
It looks like the test vector wasn't signed with the test private key (see https://github.com/lightningnetwork/lightning-rfc/blob/master/11-payment-encoding.md#examples).
We make all test vectors with a reference private key (e126f68f7eafcc8b74f54d269fe206be715000f94dac067d1c04a8ca3b2db734) so that test suites can sign and verify that they generate the exact same invoice when using the same parameters.

rustyrussell · 2020-01-31T02:28:58Z

OK, I've pushed a fixup which completes the decode description (this is our only example with an embedded route!), and also reencoded it using the example privkey.

t-bast · 2020-01-31T08:42:51Z

Thanks @rustyrussell, I verified the test vector with eclair and it works fine.
cdecker mentioned adding a test vector with an invalid pico-amount (not ending with 0), why not also add that?

See lightning/bolts#699

rustyrussell · 2020-02-03T00:15:55Z

Thanks @rustyrussell, I verified the test vector with eclair and it works fine.
cdecker mentioned adding a test vector with an invalid pico-amount (not ending with 0), why not also add that?

Good point, see #736

See-also: lightning/bolts#699 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

t-bast

ACK 1fbccd3

See-also: lightning/bolts#699 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

See lightning/bolts#699 and lightning/bolts#736

* Ignore fields with invalid length As per the spec: > A reader: > * MUST skip over unknown fields, OR an f field with unknown version, OR p, h, s or n fields that do NOT have data_lengths of 52, 52, 52 or 53, respectively. * Add more Bolt 11 tests See lightning/bolts#699 and lightning/bolts#736 Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>

This addresses lightning#692.

…iption. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Sword-Smith · 2020-02-24T18:17:13Z

Fixed merge conflict by rebasing and force pushing. This test vector is now presented as the last in the list of test vectors.

t-bast · 2020-03-03T08:23:26Z

@cfromknecht or @Roasbeef, do we have an ACK to merge this?

cfromknecht

LGTM, I added this vector to lnd and we're also passing

cfromknecht · 2020-03-02T23:51:43Z

11-payment-encoding.md

+* `p`: payment hash.
+  * `p5`: `data_length` (`p` = 1, `5` = 20; 1 * 32 + 20 == 52)
+  * `gc3xfm08u9qy06djf8dfflhugl6p7lgza6dsjxq454gxhj9t7a0s`: payment hash 462264ede7e14047e9b249da94fefc47f41f7d02ee9b091815a5506bc8abf75f
+* `d`: short description


might be too late at this point, just some thoughts:

the rest of these fields seem tangential to testing pico amount? imo more minimal, targeted test cases are preferable. i see also that it was updated to test route hints as well, perhaps that should have been its own test case?

there are also a lot of constants that are defined and only used for this test, e.g. payment hash, expiry, description, etc. that could be shared with other tests since they don't seem to be exercising explicit edge cases.

True, this test case may be covering too much. But that doesn't mean an implementation has to do all the checks if they're redundant with other tests, right? I don't mind updating this PR, but I'd rather not spend too much time on in since it's quite a trivial addition.

I think it depends on your testing framework, lnd asserts encoding/decoding and the in-memory representation. I wouldn't explain what I just went through as trivial, but it is done now :P

Allright then now that the painful code is written, let's merge this PR and let this test case live a long and happy life :D

Sword-Smith mentioned this pull request Nov 13, 2019

Add bolt11 example with amount in pico-BTC #693

Closed

Sword-Smith force-pushed the master branch 2 times, most recently from 1fb0897 to a7811b5 Compare November 13, 2019 22:14

Sword-Smith force-pushed the master branch from a7811b5 to 8b5ac44 Compare November 14, 2019 11:11

rustyrussell added the Meeting Discussion Raise at next meeting label Dec 5, 2019

t-bast added a commit to ACINQ/eclair that referenced this pull request Jan 31, 2020

Add pico-amount Bolt11 test vector

3b580de

See lightning/bolts#699

rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Feb 3, 2020

common: add pico-BTC parsing test from proposed BOLT.

9b1ab8a

See-also: lightning/bolts#699 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

t-bast approved these changes Feb 3, 2020

View reviewed changes

t-bast mentioned this pull request Feb 4, 2020

Better Bolt 11 parsing spec compliance ACINQ/eclair#1302

Closed

cdecker pushed a commit to ElementsProject/lightning that referenced this pull request Feb 5, 2020

common: add pico-BTC parsing test from proposed BOLT.

d8e933a

See-also: lightning/bolts#699 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

t-bast added a commit to ACINQ/eclair that referenced this pull request Feb 10, 2020

Add more Bolt 11 tests

08fbad9

See lightning/bolts#699 and lightning/bolts#736

Sword-Smith and others added 2 commits February 24, 2020 19:02

Add bolt11 test vector with amount in p units

ba0ba03

This addresses lightning#692.

BOLT 11: update example to use example privkey, complete decode descr…

aa41992

…iption. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Sword-Smith force-pushed the master branch from 1fbccd3 to aa41992 Compare February 24, 2020 18:09

cdecker mentioned this pull request Mar 3, 2020

Lightning Specification Meeting 2020/03/02 #744

Closed

15 tasks

cfromknecht approved these changes Mar 3, 2020

View reviewed changes

t-bast merged commit 85068c5 into lightning:master Mar 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bolt11 test vector with amount in `p` units #699

Add bolt11 test vector with amount in `p` units #699

Sword-Smith commented Nov 13, 2019 •

edited

Loading

Sword-Smith commented Nov 13, 2019 •

edited

Loading

t-bast commented Nov 14, 2019

Sword-Smith commented Nov 14, 2019

t-bast commented Nov 14, 2019

Sword-Smith commented Dec 2, 2019

t-bast commented Dec 2, 2019

t-bast commented Dec 3, 2019

rustyrussell commented Jan 31, 2020

t-bast commented Jan 31, 2020

rustyrussell commented Feb 3, 2020

t-bast left a comment

Sword-Smith commented Feb 24, 2020

t-bast commented Mar 3, 2020

cfromknecht left a comment

cfromknecht Mar 2, 2020

t-bast Mar 3, 2020

cfromknecht Mar 3, 2020

t-bast Mar 3, 2020

Add bolt11 test vector with amount in p units #699

Add bolt11 test vector with amount in p units #699

Conversation

Sword-Smith commented Nov 13, 2019 • edited Loading

Please send $30 for coffee beans to the same peer, which supports features 1 and 9

Sword-Smith commented Nov 13, 2019 • edited Loading

t-bast commented Nov 14, 2019

Sword-Smith commented Nov 14, 2019

t-bast commented Nov 14, 2019

Sword-Smith commented Dec 2, 2019

t-bast commented Dec 2, 2019

t-bast commented Dec 3, 2019

rustyrussell commented Jan 31, 2020

t-bast commented Jan 31, 2020

rustyrussell commented Feb 3, 2020

t-bast left a comment

Choose a reason for hiding this comment

Sword-Smith commented Feb 24, 2020

t-bast commented Mar 3, 2020

cfromknecht left a comment

Choose a reason for hiding this comment

cfromknecht Mar 2, 2020

Choose a reason for hiding this comment

t-bast Mar 3, 2020

Choose a reason for hiding this comment

cfromknecht Mar 3, 2020

Choose a reason for hiding this comment

t-bast Mar 3, 2020

Choose a reason for hiding this comment

Add bolt11 test vector with amount in `p` units #699

Add bolt11 test vector with amount in `p` units #699

Sword-Smith commented Nov 13, 2019 •

edited

Loading

Sword-Smith commented Nov 13, 2019 •

edited

Loading