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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Sep 3, 2019
Merged

Bolt11 feature bits #656

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44490c3
BOLT 11: Add feature bits.
rustyrussell Jul 18, 2019
a76d61d
BOLT 11: Add test vectors for feature bitfield.
rustyrussell Jul 18, 2019
9becc96
fixup! BOLT 11: Add test vectors for feature bitfield.
rustyrussell Aug 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion 11-payment-encoding.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ over Lightning.
* [Human-Readable Part](#human-readable-part)
* [Data Part](#data-part)
* [Tagged Fields](#tagged-fields)
* [Feature Bits](#feature-bits)
* [Payer / Payee Interactions](#payer--payee-interactions)
* [Payer / Payee Requirements](#payer--payee-requirements)
* [Implementation](#implementation)
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think 9 was chosen to refer to Bolt 9 (which is where feature bits are usually defined - even though this might create confusion between node feature bits and invoice feature bits).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it seemed the least desirable "real estate", character-wise, and since it doesn't have a specific meaning, I thought I'd use it. f was already taken, of course.

supported or required for receiving this payment.
See [Feature Bits](#feature-bits).

### Requirements

Expand All @@ -156,10 +160,13 @@ A writer:
through some unspecified means.
- SHOULD use a complete description of the purpose of the payment.
- MAY include one `x` field.
- if `x` is included:
- SHOULD use the minimum `data_length` possible.
- MAY include one `c` field.
- MUST set `c` to the minimum `cltv_expiry` it will accept for the last
HTLC in the route.
- SHOULD use the minimum `data_length` possible for `x` and `c` fields.
- if `c` is included:
- SHOULD use the minimum `data_length` possible.
- MAY include one `n` field. (Otherwise performing signature recovery is required)
- MUST set `n` to the public key used to create the `signature`.
- MAY include one or more `f` fields.
Expand All @@ -175,13 +182,21 @@ A writer:
`fee_base_msat`, `fee_proportional_millionths`, and `cltv_expiry_delta` are as
specified in [BOLT #7](07-routing-gossip.md#the-channel_update-message).
- MAY include more than one `r` field to provide multiple routing options.
- if `9` contains non-zero bits:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be on the same format as the other fields? MAY include one 9 field.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it either MUST or MUST NOT.

- SHOULD use the minimum `data_length` possible.
Copy link
Collaborator

Choose a reason for hiding this comment

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

might help to clarify that this encoded directly to base32

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

... this same language is used elsewhere for minimal encoding requirements.

But I did change:

-* `9` (5): `data_length` variable. One or more bytes containing features
+* `9` (5): `data_length` variable. One or more 5-bit values containing features

Copy link
Collaborator

Choose a reason for hiding this comment

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

on the first pass i took minimally encoded to mean wire format, minimally encoded, but i think this resolves that ambiguity 👍

- otherwise:
- MUST omit the `9` field altogether.
- MUST pad field data to a multiple of 5 bits, using 0s.
- if a writer offers more than one of any field type, it:
- MUST specify the most-preferred field first, followed by less-preferred fields, in order.

A reader:
- MUST skip over unknown fields, OR an `f` field with unknown `version`, OR `p`, `h`, or
`n` fields that do NOT have `data_length`s of 52, 52, or 53, respectively.
- 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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fail parsing, yes, and by implication not try to make the payment.

Copy link
Collaborator Author

@rustyrussell rustyrussell Aug 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We use the phrase MUST fail the payment elsewhere, so I've reused that:

   - MUST fail the payment
   - SHOULD indicate the unknown bit index to the user.

- MUST check that the SHA2 256-bit hash in the `h` field exactly matches the hashed
description.
- if a valid `n` field is provided:
Expand Down Expand Up @@ -253,6 +268,15 @@ engines that support SQL or other dynamically interpreted querying languages.

Don't be like the school of [Little Bobby Tables](https://xkcd.com/327/).

## Feature Bits

Feature bits allow forward and backward compatibility, and follow the
_it's ok to be odd_ rule.

The field is big-endian. The least-significant bit is numbered 0,
which is _even_, and the next most significant bit is numbered 1,
which is _odd_.

# Payer / Payee Interactions

These are generally defined by the rest of the Lightning BOLT series,
Expand Down