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

Bolt11 feature bits #656

Open
wants to merge 2 commits into
base: master
from

Conversation

@rustyrussell
Copy link
Collaborator

commented Aug 6, 2019

We will want these for the payment-nonce proposal, since we'd eventually like to make that compulsory. So I'm pushing this now so we can all deploy it ASAP.

(Note: this PR is simply extract from #643 )

rustyrussell added some commits Jul 18, 2019

BOLT 11: Add feature bits.
Most obviously, we want this for BASE AMP, but it's useful in future.

Even though even bits won't cause existing implementations to know
they can't pay the invoice, it will allow it in future once everyone
has upgraded.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
BOLT 11: Add test vectors for feature bitfield.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@t-bast

t-bast approved these changes Aug 6, 2019

Copy link
Collaborator

left a comment

LGTM

@@ -140,6 +141,9 @@ Currently defined tagged fields are:
* `fee_base_msat` (32 bits, big-endian)
* `fee_proportional_millionths` (32 bits, big-endian)
* `cltv_expiry_delta` (16 bits, big-endian)
* `9` (5): `data_length` variable. One or more bytes containing features

This comment has been minimized.

Copy link
@Roasbeef

Roasbeef Aug 16, 2019

Member

Any particular reason why 9 is chosen here when the existing defined tags are letters?

- if the `9` field contains unknown _odd_ bits that are non-zero:
- MUST ignore the bit.
- if the `9` field contains unknown _even_ bits that are non-zero:
- MUST fail.

This comment has been minimized.

Copy link
@Roasbeef

Roasbeef Aug 16, 2019

Member

What does fail in this context mean? Stop parsing, fail the payment, or both?

In terms of delivering a better UX (say we want to pay, but the invoice only supports AMP and we don't), we should recommend the parser deliver a well specified error back to the caller, so this can eventually be propagated back to the user.

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