Skip to content

Flat features: far simpler solution for feature bits. #666

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

Merged
rustyrussell merged 3 commits into from
Nov 25, 2019
Merged

Flat features: far simpler solution for feature bits. #666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1634704
BOLT 9: flatten feature fields.
rustyrussell Sep 17, 2019
2e225b2
BOLT 7: always propagate announcements with unknown features.
rustyrussell Sep 17, 2019
0b7bbeb
BOLT 11: use the same features for BOLT11 invoices as for others.
rustyrussell Oct 30, 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
1 change: 1 addition & 0 deletions .aspell.en.pws
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,3 +371,4 @@ tlvs
snprintf
GitHub
IRC
bitmasks
20 changes: 10 additions & 10 deletions 01-messaging.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,29 +226,30 @@ The following convenience types are also defined:

Once authentication is complete, the first message reveals the features supported or required by this node, even if this is a reconnection.

[BOLT #9](09-features.md) specifies lists of global and local features. Each feature is generally represented in `globalfeatures` or `localfeatures` by 2 bits. The least-significant bit is numbered 0, which is _even_, and the next most significant bit is numbered 1, which is _odd_.
[BOLT #9](09-features.md) specifies lists of features. Each feature is generally represented by 2 bits. The least-significant bit is numbered 0, which is _even_, and the next most significant bit is numbered 1, which is _odd_. For historical reasons, features are divided into global and local feature bitmasks.

Both fields `globalfeatures` and `localfeatures` MUST be padded to bytes with 0s.
The `features` field MUST be padded to bytes with 0s.

1. type: 16 (`init`)
2. data:
* [`u16`:`gflen`]
* [`gflen*byte`:`globalfeatures`]
* [`u16`:`lflen`]
* [`lflen*byte`:`localfeatures`]
* [`u16`:`flen`]
* [`flen*byte`:`features`]

The 2-byte `gflen` and `lflen` fields indicate the number of bytes in the immediately following field.

#### Requirements

The sending node:
- MUST send `init` as the first Lightning message for any connection.
- MUST set feature bits as defined in [BOLT #9](09-features.md).
- MUST set any undefined feature bits to 0.
- SHOULD use the minimum lengths required to represent the feature fields.
- SHOULD NOT set features greater than 13 in `globalfeatures`.
- SHOULD use the minimum length required to represent the `features` field.

The receiving node:
- MUST wait to receive `init` before sending any other messages.
- MUST combine (logical OR) the two feature bitmaps into one logical `features` map.
- MUST respond to known feature bits as specified in [BOLT #9](09-features.md).
- upon receiving unknown _odd_ feature bits that are non-zero:
- MUST ignore the bit.
Expand All @@ -257,15 +258,14 @@ The receiving node:

#### Rationale

There used to be two feature bitfields here, but for backwards compatibility they're now
combined into one.

This semantic allows both future incompatible changes and future backward compatible changes. Bits should generally be assigned in pairs, in order that optional features may later become compulsory.

Nodes wait for receipt of the other's features to simplify error
diagnosis when features are incompatible.

The feature masks are split into local features (which only affect the
protocol between these two nodes) and global features (which can affect
HTLCs and are thus also advertised to other nodes).

### The `error` Message

For simplicity of diagnosis, it's often useful to tell a peer that something is incorrect.
Expand Down
8 changes: 0 additions & 8 deletions 02-peer-protocol.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,6 @@ The `shutdown_scriptpubkey` allows the sending node to commit to where
funds will go on mutual close, which the remote node should enforce
even if a node is compromised later.

[ FIXME: Describe dangerous feature bit for larger channel amounts. ]

#### Requirements

The sending node:
Expand Down Expand Up @@ -264,12 +262,6 @@ are above both `dust_limit_satoshis`.

Details for how to handle a channel failure can be found in [BOLT 5:Failing a Channel](05-onchain.md#failing-a-channel).

#### Future

It would be easy to have a local feature bit which indicated that a
receiving node was prepared to fund a channel, which would reverse this
protocol.

### The `accept_channel` Message

This message contains information about a node and indicates its
Expand Down
19 changes: 7 additions & 12 deletions 07-routing-gossip.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,16 +180,15 @@ The origin node:
- MUST set `bitcoin_signature_1` and `bitcoin_signature_2` to valid
signatures of the hash `h` (using `bitcoin_key_1` and `bitcoin_key_2`'s
respective secrets).
- SHOULD set `len` to the minimum length required to hold the `features` bits
- MUST set `features` based on what features were negotiated for this channel, according to [BOLT #9](09-features.md#assigned-features-flags)
- MUST set `len` to the minimum length required to hold the `features` bits
it sets.

The receiving node:
- MUST verify the integrity AND authenticity of the message by verifying the
signatures.
- if there is an unknown even bit in the `features` field:
- MUST NOT parse the remainder of the message.
- MUST NOT add the channel to its local network view.
- SHOULD NOT forward the announcement.
- MUST NOT attempt to route messages through the channel.
Comment thread
t-bast marked this conversation as resolved.
- if the `short_channel_id`'s output does NOT correspond to a P2WSH (using
`bitcoin_key_1` and `bitcoin_key_2`, as specified in
[BOLT #3](03-transactions.md#funding-transaction-output)) OR the output is
Expand Down Expand Up @@ -243,8 +242,6 @@ New channel features are possible in the future: backwards compatible (or
optional) features will have _odd_ feature bits, while incompatible features
will have _even_ feature bits
(["It's OK to be odd!"](00-introduction.md#glossary-and-terminology-guide)).
Incompatible features will result in the announcement not being forwarded by
nodes that do not understand them.

## The `node_announcement` Message

Expand Down Expand Up @@ -311,6 +308,7 @@ The origin node:
to 0.
- SHOULD ensure `ipv4_addr` AND `ipv6_addr` are routable addresses.
- MUST NOT include more than one `address descriptor` of the same type.
- MUST set `features` according to [BOLT #9](09-features.md#assigned-features-flags)
- SHOULD set `flen` to the minimum length required to hold the `features`
bits it sets.

Expand All @@ -324,11 +322,7 @@ any future fields appended to the end):
- SHOULD fail the connection.
- MUST NOT process the message further.
- if `features` field contains _unknown even bits_:
- MUST NOT parse the remainder of the message.
- MAY discard the message altogether.
- SHOULD NOT connect to the node.
- MAY forward `node_announcement`s that contain an _unknown_ `features` _bit_,
regardless of if it has parsed the announcement or not.
- SHOULD ignore the first `address descriptor` that does NOT match the types
defined above.
- if `addrlen` is insufficient to hold the address descriptors of the
Expand All @@ -352,8 +346,9 @@ any future fields appended to the end):

New node features are possible in the future: backwards compatible (or
optional) ones will have _odd_ `feature` _bits_, incompatible ones will have
_even_ `feature` _bits_. These may be propagated by nodes even if they
cannot process the announcements themselves.
_even_ `feature` _bits_. These will be propagated normally; incompatible
feature bits here refer to the nodes, not the `node_announcement` message
itself.

New address types may be added in the future; as address descriptors have
to be ordered in ascending order, unknown ones can be safely ignored.
Expand Down
63 changes: 36 additions & 27 deletions 09-features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,53 @@
# BOLT #9: Assigned Feature Flags

This document tracks the assignment of `localfeatures` and `globalfeatures`
flags in the `init` message ([BOLT #1](01-messaging.md)) along with the
`features` flag fields in the `channel_announcement` and `node_announcement`
messages ([BOLT #7](07-routing-gossip.md)).
The flags are tracked separately, since new flags will likely be added over time.

The `features` flags in the routing messages are a subset of the
`globalfeatures` flags, as `localfeatures`, by definition, are only of interest
to direct peers.
This document tracks the assignment of `features` flags in the `init`
message ([BOLT #1](01-messaging.md)), as well as `features` fields in
the `channel_announcement` and `node_announcement` messages ([BOLT
#7](07-routing-gossip.md)). The flags are tracked separately, since
new flags will likely be added over time.

Flags are numbered from the least-significant bit, at bit 0 (i.e. 0x1,
an _even_ bit). They are generally assigned in pairs so that features
can be introduced as optional (_odd_ bits) and later upgraded to be compulsory
(_even_ bits), which will be refused by outdated nodes:
see [BOLT #1: The `init` Message](01-messaging.md#the-init-message).

## Assigned `localfeatures` flags

These flags may only be used in the `init` message:

| Bits | Name | Description | Link |
|-------|----------------------------------|---------------------------------------------------------------------------|------------------------------|
| 0/1 | `option_data_loss_protect` | Requires or supports extra `channel_reestablish` fields | [BOLT #2][bolt02-retransmit] |
| 3 | `initial_routing_sync` | Indicates that the sending node needs a complete routing information dump | [BOLT #7][bolt07-sync] |
| 4/5 | `option_upfront_shutdown_script` | Commits to a shutdown scriptpubkey when opening channel | [BOLT #2][bolt02-open] |
| 6/7 | `gossip_queries` | More sophisticated gossip control | [BOLT #7][bolt07-query] |
| 10/11 | `gossip_queries_ex` | Gossip queries can include additional information | [BOLT #7][bolt07-query] |
| 12/13| `option_static_remotekey` | Static key for remote output | [BOLT #3](03-transactions.md) |
Some features don't make sense on a per-channels or per-node basis, so
each feature defines how it is presented in those contexts. Some
features may be required for opening a channel, but not a requirement
for use of the channel, so the presentation of those features depends
on the feature itself.

## Assigned `globalfeatures` flags
The Context column decodes as follows:
* `I`: presented in the `init` message.
Copy link
Copy Markdown
Collaborator

@cfromknecht cfromknecht Oct 9, 2019

Choose a reason for hiding this comment

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

If this is I, what will our letter for invoice feature bits be? Maybe P for payment request?

Should we carve out the space for invoice feature bits as well in this PR?

Copy link
Copy Markdown
Collaborator

@cfromknecht cfromknecht Oct 9, 2019

Choose a reason for hiding this comment

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

from the candidate features, we could start to specify that var_onion_option should be in invoices

Copy link
Copy Markdown
Collaborator

@t-bast t-bast Oct 9, 2019

Choose a reason for hiding this comment

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

I think your first comment raises an interesting point to discuss: do we want to merge invoice feature bits with node feature bits? If we do #643 can't use bits 0-3 (which it currently does) since it clashes with node features.

I think it may be useful to merge invoice feature bits with node feature bits as it sounds more future-proof. The only downside I can see is that it uses more space in the invoice, so QR codes might end up being big. But we can probably compress the features bitfield quite efficiently if it becomes large but we only want to set a few bits in it. WDYT?

Copy link
Copy Markdown
Collaborator

@cfromknecht cfromknecht Oct 9, 2019

Choose a reason for hiding this comment

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

at this point I think we should forbid colliding feature bits altogether, so all feature bit spaces should share the same namespace. however there are some node features that don’t make sense as invoice features, e.g. support for gossip sync protocols, and those don’t need to be present.

the space for routing hints will probably dwarf the feature bits in the invoice for some time, but agree that a more space efficient representation for feature bits is worth exploring. I’ve been looking into a combination of RLE and/or sparse encodings, tho the more difficult task will be signaling support for the newer format without parsing any feature bits, but we have options :P

Copy link
Copy Markdown
Collaborator

@t-bast t-bast Oct 9, 2019

Choose a reason for hiding this comment

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

tho the more difficult task will be signaling support for the newer format without parsing any feature bits, but we have options :P

Feature-bit-ception 💥

* `N`: presented in the `node_announcement` messages
* `C`: presented in the `channel_announcement` message.
Copy link
Copy Markdown
Collaborator

@Roasbeef Roasbeef Sep 17, 2019

Choose a reason for hiding this comment

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

What about the channel update message? We also need possibly extend the existing hop hints to also encompass this information. One could say the current set of feature bits handles that, however it's possible to provide multiple hop hints, and each of those channels could themselves advertise/require distinct sets of hop hints.

Copy link
Copy Markdown
Collaborator Author

@rustyrussell rustyrussell Sep 18, 2019

Choose a reason for hiding this comment

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

We don't have a general feature field in channel_update (yet?). But we can add a field if we need to.

Copy link
Copy Markdown
Collaborator

@Roasbeef Roasbeef Sep 18, 2019

Choose a reason for hiding this comment

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

I think the main thing here is the inability to express distinct feature sets for each channel with the current BOLT 11 feature bit vector extension.

* `C-`: presented in the `channel_announcement` message, but always odd (optional).
* `C+`: presented in the `channel_announcement` message, but always even (required).
* `9`: presented in [BOLT 11](11-payment-encoding.md) invoices.
Copy link
Copy Markdown
Collaborator

@cfromknecht cfromknecht Nov 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

does this mean that invoice features will share the same namespace as everything else? i think that's the right call, should var_onion_optin be changed to IN9?

Copy link
Copy Markdown
Collaborator Author

@rustyrussell rustyrussell Nov 12, 2019

Choose a reason for hiding this comment

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

Indeed, #643 does this (or will, once rebased on this)


The following `globalfeatures` bits are currently assigned by this specification:

| Bits | Name | Description | Link |
|------|-------------------|--------------------------------------------------------------------|---------------------------------------|
| 8/9 | `var_onion_optin` | This node requires/supports variable-length routing onion payloads | [Routing Onion Specification][bolt04] |
| Bits | Name | Description | Context | Link |
|-------|----------------------------------|-----------------------------------------------------------|----------|---------------------------------------|
| 0/1 | `option_data_loss_protect` | Requires or supports extra `channel_reestablish` fields | IN | [BOLT #2][bolt02-retransmit] |
| 3 | `initial_routing_sync` | Sending node needs a complete routing information dump | I | [BOLT #7][bolt07-sync] |
| 4/5 | `option_upfront_shutdown_script` | Commits to a shutdown scriptpubkey when opening channel | IN | [BOLT #2][bolt02-open] |
| 6/7 | `gossip_queries` | More sophisticated gossip control | IN | [BOLT #7][bolt07-query] |
| 8/9 | `var_onion_optin` | Requires/supports variable-length routing onion payloads | IN | [Routing Onion Specification][bolt04] |
Copy link
Copy Markdown
Collaborator

@Roasbeef Roasbeef Sep 17, 2019

Choose a reason for hiding this comment

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

Why should a node level feature gate if two peers can/will-continue-to communicate on the connection level? As is, if I went to open a channel with a peer that doesn't support the new onion payload, but they required this bit, then the connection would fail upon feature bit comparison.

Copy link
Copy Markdown
Collaborator Author

@rustyrussell rustyrussell Sep 18, 2019

Choose a reason for hiding this comment

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

Yes, it's overbroad: this is the tradeoff. Previously you could in theory gossip with a node which had an even global feature you didn't like, too.

Putting it in the channel_announce would also be overbroad (since both nodes might not need it). We could add a feature bitfield (TLV-style) to channel_update, but it's marginally more efficient to use node_announce, since it's really per-node not per channel in any sane implementation.

Did we want to go there?

Copy link
Copy Markdown
Collaborator

@cfromknecht cfromknecht Nov 6, 2019

Choose a reason for hiding this comment

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

var_onion_optin does not affect the p2p layer in lnd at all, does it in other implementations? is the rationale is it mainly to decide whether the node supports TLV onions during channel creation?

Copy link
Copy Markdown
Collaborator

@t-bast t-bast Nov 8, 2019

Choose a reason for hiding this comment

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

We're not using it either for p2p, it will currently only be useful in the Bolt 11 invoice or inferred by other feature bits (MPP, Trampoline, etc).

| 10/11 | `gossip_queries_ex` | Gossip queries can include additional information | IN | [BOLT #7][bolt07-query] |
Copy link
Copy Markdown
Collaborator

@cfromknecht cfromknecht Oct 3, 2019

Choose a reason for hiding this comment

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

missing 12/13 for option_static_remote_key? maybe just needs rebase?

| 12/13| `option_static_remotekey` | Static key for remote output | IN | [BOLT #3](03-transactions.md) |

## Requirements

The origin node:
* If it supports a feature above, SHOULD set the corresponding odd
bit in all feature fields indicated by the Context column unless
indicated that it must set the even feature bit instead.
* If it requires a feature above, MUST set the corresponding even
feature bit in all feature fields indicated by the Context column,
unless indicated that it must set the odd feature bit instead.
* MUST NOT set feature bits it does not support.
* MUST NOT set feature bits in fields not specified by the table above.

The requirements for receiving specific bits are defined in the linked sections in the table above.
The requirements for feature bits that are not defined
above can be found in [BOLT #1: The `init` Message](01-messaging.md#the-init-message).
Expand Down
3 changes: 2 additions & 1 deletion 11-payment-encoding.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,8 @@ 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.
_it's ok to be odd_ rule. Features appropriate for use in the `9` field
are marked in [BOLT 9](09-features.md).

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