[Lightning-dev] Comments on BOLT#11

[Rusty Russell]

Jonathan Underwood <junderwood at bitcoinbank.co.jp> writes:
> Hello all,
>
> Recently I have implemented BOLT11 in node JS. You can find it on github at
> bitcoinjs/bolt11 (check the wip branch, I am still working on tests and
> looking at maybe splitting encode up into two functions, encode and sign,
> if anyone wants to help)

Hi, this is great!

> Here are some comments:
>
> 1. There should be a tags_length integer before the first tag. This will
> get rid of the awkward dance of parsing from the front and end at the same
> time and make it easier to detect poorly encoded data quickly.

I'm not sure what you're referring to?  I've written two decoders, and
both pull off the header and fixed fields, then iterate
while (data_len > 520 / 5).

> 2. Why is a description or description commit hash a MUST include tag... it
> should be SHOULD.

I disagree.  Obviously we can't force people to be descriptive (that's a
SHOULD), but not including a description of the purpose of the payment
is a mistake.

The invoice and the preimage provides a cryptographic proof of
purchase.  But that's undermined if the description is missing.

> 3. Descriptions say they can encode ASCII only. Sorry, but this is
> nonsense. Full unicode support via UTF8 should be supported.

>From the ML welcome: "Be excellent to each other please!"

I understand this mistake upset you, but "Sorry, but this is nonsense"
makes me want to find spurious justifications for it.  I prefer "This
seems unfairly limiting", "This is a mistake" or "Woah, time to go 8
bits, granddad!".

The problem is the spec describes the 'd' field like so:

        short description of purpose of payment (ASCII),  e.g. '1 cup of coffee'

There's nothing in the *requirements* section, at all.

So, I think we should:

1.  s/ASCII/UTF-8/
    - It seems everyone handles this fine anyway.
2.  Add another example, here, which is in UTF-8, say "or 'ナンセンスのカップ'" :)
3.  Add a requirement that the writer MUST use valid UTF-8.
4.  Modify one of the examples to use a UTF-8 description.

> 4. There should be a maximum length (in bytes) for description. (Otherwise
> why have a commit hash tag at all?)

There is; no field can be greater than 640 bytes (1023 5-bit values).
Also, the transport might have practical limits, so you might need to
consider that in total.  (Maybe not though, if you're going for QR
codes, as they go pretty large).

> 5. There should be a recommendation for ways for the parsing client to
> fetch the commited purpose description data. (Maybe encode a URL after the
> 32 byte commit hash?)

Yes there should, but we left it open for the moment because there's no
good solution here, and someone needs to come up with one.  This makes
description hash fairly useless for now, IMHO.

A URL is the obvious solution, but has the terrible property that it
de-anonymizes the wallet :( (even before the person has decided whether
they want to pay).

FWIW, there are a range of possible solutions, depending on the scenario:
1. If you have some container format, eg. HTML, include the description as a
   separate tag.
2. Roasbeef wants to implement HORNET over our network, and that could
   be used to retrieve the full description.

Meanwhile, 640 bytes should be enough for anyone!

> 6. Lightning payment requests can get pretty long, but perhaps there should
> be a limit. Apparently bech32 with the given parameters can only handle one
> error correction once it is longer than 1023 words.

Actually I think bech32 loses all guarantees at 1023 words, but it's no
worse than a 30-bit checksum.  We mainly use it because it's simple and
available, and of course any fixed-length code will have some limit.

What actually happens if we decode badly is that we derive the wrong
public key for the node (50% chance, other 50% we get a signature
failure) and we can't route to it or pay to it.  So no funds are
actually lost.

> Now for some things on the BOLT document itself:
>
> 1. I made a pull request for adding signature preimage hex data etc. to the
> examples to help people writing decoders. Some of the language was unclear
> to me and lead to some goof ups.

Yes, this was *awesome*; I double-checked the values by hacking my
python encoder, and adjusted for the recent r-encoding change.  It's
merged, thanks!

        https://github.com/lightningnetwork/lightning-rfc/pull/312

> 2. Would be nice to have a fuller set of test cases on the RFC. I will work
> on some cases to increase coverage on the bitcoinjs repo. Would be happy
> for any help.

That would be fantastic!  We've talked about changing the test vectors
(for all the BOLTs) into a more machine-friendly JSON encoding, so if
you're feeling ambitious you could start with BOLT 11 :)

Thanks!
Rusty.