Skip to content

Feature bit unification and renaming #571

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

Closed
rustyrussell wants to merge 5 commits into from
Closed

Feature bit unification and renaming #571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bb479a8
BOLT 1, 7, 9: rename feature bits, combine in gossip.
rustyrussell May 6, 2019
733428e
BOLT 9: Note that features share the same number space.
rustyrussell May 6, 2019
0f1f7c6
BOLT 1: don't need to disconnect on unknown `channelfeatures`.
rustyrussell May 6, 2019
783790a
BOLT 7: Explicitly mirror channelfeatures in channel_announcement.
rustyrussell May 6, 2019
14ced3b
BOLT 7: advertize both feature sets in node_announcement.
rustyrussell May 6, 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: 14 additions & 12 deletions 01-messaging.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,18 @@ a buffer with 6-bytes of pre-padding.

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 channel and node features. Each feature is generally represented in `channelfeatures` or `nodefeatures` 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_.

Both fields `globalfeatures` and `localfeatures` MUST be padded to bytes with 0s.
Both fields `channelfeatures` and `nodefeatures` MUST be padded to bytes with 0s.

1. type: 16 (`init`)
2. data:
* [`2`:`gflen`]
* [`gflen`:`globalfeatures`]
* [`2`:`lflen`]
* [`lflen`:`localfeatures`]
* [`2`:`cflen`]
* [`cflen`:`channelfeatures`]
* [`2`:`nflen`]
* [`nflen`:`nodefeatures`]

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

#### Requirements

Expand All @@ -112,9 +112,9 @@ The sending node:
The receiving node:
- MUST wait to receive `init` before sending any other messages.
- MUST respond to known feature bits as specified in [BOLT #9](09-features.md).
- upon receiving unknown _odd_ feature bits that are non-zero:
- upon receiving unknown _odd_ `nodefeatures` bits that are non-zero:
- MUST ignore the bit.
- upon receiving unknown _even_ feature bits that are non-zero:
- upon receiving unknown _even_ `nodefeatures` bits that are non-zero:
- MUST fail the connection.

#### Rationale
Expand All @@ -124,9 +124,11 @@ This semantic allows both future incompatible changes and future backward compat
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 feature masks are split into node features (which only affect the
protocol between these two nodes) and channel features (which can affect
routing and are thus also advertised to other nodes). A node may
connect to a node with unknown channel features, even though it won't be
able to establish a channel.

### The `error` Message

Expand Down
2 changes: 1 addition & 1 deletion 02-peer-protocol.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ ids.

#### Future

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

Expand Down
30 changes: 10 additions & 20 deletions 07-routing-gossip.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ announcement message: this is accomplished by having a signature from each
* [`64`:`bitcoin_signature_1`]
* [`64`:`bitcoin_signature_2`]
* [`2`:`len`]
* [`len`:`features`]
* [`len`:`channelfeatures`]
* [`32`:`chain_hash`]
* [`8`:`short_channel_id`]
* [`33`:`node_id_1`]
Expand Down Expand Up @@ -173,16 +173,16 @@ 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
- SHOULD set `len` to the minimum length required to hold the `channelfeatures` bits
it sets.
- MUST set a `channelfeatures` bit for each `channelfeature` negotiated for
the channel, according to [BOLT #9](09-features.md).

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.
- if there is an unknown even bit in the `channelfeatures` field:
Comment thread
cfromknecht marked this conversation as resolved.
- MUST NOT add the channel to its local network view.
- SHOULD NOT forward the announcement.
- 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 @@ -248,8 +248,8 @@ nodes not associated with an already known channel are ignored.
1. type: 257 (`node_announcement`)
2. data:
* [`64`:`signature`]
* [`2`:`flen`]
* [`flen`:`features`]
* [`2`:`cflen`]
* [`cflen`:`combinedfeatures`]
* [`4`:`timestamp`]
* [`33`:`node_id`]
* [`3`:`rgb_color`]
Expand Down Expand Up @@ -304,8 +304,9 @@ 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.
- SHOULD set `flen` to the minimum length required to hold the `features`
- SHOULD set `cflen` to the minimum length required to hold the `combinedfeatures`
bits it sets.
- MUST set `combinedfeatures` to the logical OR of `nodefeatures` and `channelfeatures`.

The receiving node:
- if `node_id` is NOT a valid compressed public key:
Expand All @@ -316,12 +317,6 @@ The receiving node:
any future fields appended to the end):
- SHOULD fail the connection.
- MUST NOT process the message further.
- if `features` field contains _unknown even bits_:
Copy link
Copy Markdown
Collaborator

@cfromknecht cfromknecht Jul 15, 2019

Choose a reason for hiding this comment

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

if i understand correctly, this will removes the ability to partition the gossip network as a result of nodes not understanding an even feature bit? do we still want to retain the ability to do so via unknown TLV types?

Copy link
Copy Markdown
Collaborator Author

@rustyrussell rustyrussell Aug 6, 2019

Choose a reason for hiding this comment

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

Hmm, that's an excellent question! I... don't know. Allowing "invalid" TLVs to propagate seems like a burden on implementations, but maybe it's a feature somehow?

Copy link
Copy Markdown
Collaborator

@cfromknecht cfromknecht Aug 8, 2019

Choose a reason for hiding this comment

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

In due time we may discover, I see arguments for both sides 🤷‍♂️

- 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_,
Copy link
Copy Markdown
Collaborator

@cfromknecht cfromknecht Jul 15, 2019

Choose a reason for hiding this comment

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

this seems like a good change, we probably shouldn't be forwarding messages that we were unable to parse

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 @@ -343,11 +338,6 @@ any future fields appended to the end):

### Rationale

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.

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.
Additional fields beyond `addresses` may also be added in the future—with
Expand Down Expand Up @@ -727,7 +717,7 @@ In the case where the `channel_announcement` is nonetheless missed,
## Initial Sync

If a node requires an initial sync of gossip messages, it will be flagged
in the `init` message, via a feature flag ([BOLT #9](09-features.md#assigned-localfeatures-flags)).
in the `init` message, via a feature flag ([BOLT #9](09-features.md#assigned-nodefeatures-flags)).

Note that the `initial_routing_sync` feature is overridden (and should
be considered equal to 0) by the `gossip_queries` feature if the
Expand Down
14 changes: 8 additions & 6 deletions 09-features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
# BOLT #9: Assigned Feature Flags

This document tracks the assignment of `localfeatures` and `globalfeatures`
This document tracks the assignment of `nodefeatures` and `channelfeatures`
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 flags use the same number space, but are semantically separate; directly
connected peers are affected by `nodefeatures`, and anyone trying to route
a payment is affected by `channelfeatures`.

The `features` flags in the routing messages are a subset of the
`globalfeatures` flags, as `localfeatures`, by definition, are only of interest
`channelfeatures` flags, as `nodefeatures`, by definition, are only of interest
to direct peers.

Flags are numbered from the least-significant bit, at bit 0 (i.e. 0x1,
Expand All @@ -16,7 +18,7 @@ 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
## Assigned `nodefeatures` flags

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

Expand All @@ -27,9 +29,9 @@ These flags may only be used in the `init` message:
| 4/5 | `option_upfront_shutdown_script` | Commits to a shutdown scriptpubkey when opening channel | [BOLT #2](02-peer-protocol.md#the-open_channel-message) |
| 6/7 | `gossip_queries` | More sophisticated gossip control | [BOLT #7](07-routing-gossip.md#query-messages) |

## Assigned `globalfeatures` flags
## Assigned `channelfeatures` flags

There are currently no `globalfeatures` flags.
There are currently no `channelfeatures` flags.

## Requirements

Expand Down