Feature bit unification and renaming #571

rustyrussell · 2019-02-12T04:05:41Z

This is 5 commits, each standalone:

Rename "local" to "node" features, "global" to "channel" features. The original idea was that
"local" features only affect direct peers. That still stands, but the new names are clearer.
Note: There are still no "channel" features defined.
Note that features share the same number space, needed for minor: updated open channel diagram #5.
Remove init message requirement to disconnect on unknown channelfeatures. This just
means you can't open a channel. Unknown node features means you can't even talk.
Explicitly mirror channelfeatures in channel_announcement. These are things you might need to
know to use that specific channel, or might be required, depending on the feature itself. Removes
the "don't relay these if they have an unknown feature" rule.
Explicitly mirror what channelfeatures we support in node_announcement (usually as optional
features, of course). This lets you find nodes with specific features.

Concrete Examples

Consider a node which supports a d-log-HTLC channel, and doesn't support legacy HTLCs on that channel:

This is clearly a channelfeature.
It may have negotiated this with the peer as an optional or required feature.
It must advertize this in channel_announcement as a required feature, since you need to understand it to use the channel at all.
It gets advertized in node_announcement, as required or optional, depending on 2 above.

Now, consider a node which can create wumbo (> 2^24 satoshi) channels:

This is a nodefeature.
It's (presumably) optional.
It doesn't appear in channel_announcement.
It gets advertized in node_announcement, as (presumably) optional.

Finally, consider a node which can handle wumbo HTLCs:

This is a channelfeature.
It's (presumably) optional.
It gets advertized in channel_announcement as optional, since you don't need to understand it.
It gets advertized in node_announcmenet, as (presumably) optional.

Rename `globalfeatures` to `channelfeatures` and `localfeatures` to `nodefeatures`.

Signed-off-by: Rusty Russell <[email protected]>

You can still gossip. Signed-off-by: Rusty Russell <[email protected]>

If we really want a new gossip message which old nodes will ignore, we'll use a new type, so having it discard unknown features is overzealous. Each feature can itself specify how it's advertized here: an key-exchange-instead-of-hash-preimage feature would need to advertize as even (you need to understand it to use it), for example, but a wumbo feature would advertize as odd. Signed-off-by: Rusty Russell <[email protected]>

This lets you find out what nodes support what node features, rather than connecting and probing. Like channel_announcement, we won't use feature bits for incompatible changes; we'll use a separate type. So don't discard messages with unknown ones. Similarly, you can try to connect to a node with unknown bits; you might fail, but that's OK. Either it was an unknown node feature, and you'll find out from their init msg, or it's a channel feature and you won't be able to open a channel. Signed-off-by: Rusty Russell <[email protected]>

t-bast · 2019-05-28T07:06:46Z

I think this is a good clarification.
ACK from me for next meeting.

t-bast · 2019-06-28T15:17:12Z

As discussed in the IRC meeting, people still find the names confusing so here are my suggestions, I'll let the consensus judge if they're better or worse ;)

Suggestions for renaming local features
- connection or conn features (since it only impacts people I'm connected to)
- peer features (it only impacts my direct peers)
- neighbour or neighbor features (meh)
- keep local (I honestly think local is a good name, it's more the global part that I found a bit misleading)
Suggestions for renaming global features
- network features (they are interesting to the whole network)
- routing features (if we assume that routing is the only thing we're using remote nodes for - not sure if that will always be true)
- remote features (meh)

I'll edit this comment if I have other ideas.

rustyrussell · 2019-07-08T21:12:35Z

I like local and routing features, TBH.

Roasbeef · 2019-07-12T02:03:57Z

Thanks for those examples! I understand the underlying motivation now after reading through them. As for names, my vote goes for node features (former global) and link features.

Roasbeef · 2019-07-12T02:04:56Z

It seems the only "pure" link feature we have is initial-routing-sync, the rest can lie in either domain.

t-bast · 2019-07-12T07:52:40Z

As for names, my vote goes for node features (former global) and link features.

I think node is confusing. Rusty proposed it to replace the current local and you're now proposing it to replace the current global. That clearly indicates that it's misleading, doesn't it? :)

I also think that link is confusing, I can see arguments for using it to replace either local or global...
Because it could mean that it defines properties of your local links (channels?) so it's a local feature, or it's a global feature because it defines properties that remote non-neighbor nodes should know about your links (channel?).

cfromknecht · 2019-07-15T22:59:34Z

As discussed on IRC, the naming that makes the most sense to me personally is:

global -> node: functionally equivalent to service bits in bitcoin, apart from the fact that they are gossiped. Emphasize that the features are applicable to the node as a whole (depending on the feature, that it applies to all of a node's channels), e.g. can this node: support variable frame onions, extended gossip queries, DLP, link-level payment sharding, etc.
local -> connection: emphasizes that these feature bits are only applied for the duration of the connection, and may change upon a subsequent reconnect, e.g. will this connection: allow funding wumbo channels, extended gossip queries, DLP, etc.

To me routing is too specific, as there are already several features that could be advertised that have nothing to do with routing (and we are likely to add more in the future). local is okay, but it's a little ambiguous what that means and could be clearer.

cfromknecht

unification changes look pretty good, maybe still some bikeshedding to be done on naming tho :) i have a few other questions i've left inline

07-routing-gossip.md

cfromknecht · 2019-07-15T23:18:44Z

07-routing-gossip.md

-    - 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_,


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

cfromknecht · 2019-07-15T23:19:33Z

07-routing-gossip.md

@@ -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_:


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?

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?

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

t-bast · 2019-08-05T20:46:39Z

I quite like renaming local to connection (seems like we all have a rough agreement on that one).
I'm not completely convinced by node to replace global but my best proposal (network) isn't perfect either, so I could live with it.

Roasbeef · 2019-08-08T22:07:32Z

node+connection SGTM!

rustyrussell added this to the v1.1 milestone Feb 12, 2019

rustyrussell force-pushed the guilt/feature-bit-unification branch from 4131dfa to b2de709 Compare February 12, 2019 06:01

rustyrussell added the 2019-02-18 label Feb 18, 2019

sstone mentioned this pull request Feb 28, 2019

[WIP] BOLT 7: Inventory-based gossip #584

Closed

rustyrussell added the 2019-03-04 label Mar 4, 2019

pm47 mentioned this pull request Mar 11, 2019

BOLT7: extend channel range queries with optional fields #557

Merged

rustyrussell removed 2019-02-18 labels Mar 18, 2019

araspitzu mentioned this pull request Mar 27, 2019

BOLT 9: add features wumbo and wumborama #590

Closed

cdecker added the Meeting Discussion Raise at next meeting label Apr 1, 2019

rustyrussell added 5 commits May 6, 2019 16:23

BOLT 1, 7, 9: rename feature bits, combine in gossip.

bb479a8

Rename `globalfeatures` to `channelfeatures` and `localfeatures` to `nodefeatures`.

BOLT 9: Note that features share the same number space.

733428e

Signed-off-by: Rusty Russell <[email protected]>

BOLT 1: don't need to disconnect on unknown channelfeatures.

0f1f7c6

You can still gossip. Signed-off-by: Rusty Russell <[email protected]>

rustyrussell changed the title ~~Feature bit unification and assignment~~ Feature bit unification and renaming May 6, 2019

rustyrussell force-pushed the guilt/feature-bit-unification branch from b2de709 to 14ced3b Compare May 6, 2019 07:05

araspitzu mentioned this pull request May 31, 2019

Single-option large channel proposal #596

Merged

t-bast mentioned this pull request Jul 2, 2019

bolt04: Variable hop_payload for the sphinx onion #619

Merged

Roasbeef assigned Roasbeef and unassigned Roasbeef Jul 12, 2019

cfromknecht reviewed Jul 15, 2019

View reviewed changes

niftynei removed the Meeting Discussion Raise at next meeting label Jul 22, 2019

t-bast added the Meeting Discussion Raise at next meeting label Aug 5, 2019

This was referenced Aug 28, 2019

option_static_remotekey #642

Merged

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

Merged

rustyrussell closed this Oct 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature bit unification and renaming #571

Feature bit unification and renaming #571

rustyrussell commented Feb 12, 2019 •

edited

Loading

t-bast commented May 28, 2019

t-bast commented Jun 28, 2019 •

edited

Loading

rustyrussell commented Jul 8, 2019

Roasbeef commented Jul 12, 2019

Roasbeef commented Jul 12, 2019

t-bast commented Jul 12, 2019

cfromknecht commented Jul 15, 2019

cfromknecht left a comment

cfromknecht Jul 15, 2019

cfromknecht Jul 15, 2019

rustyrussell Aug 6, 2019

cfromknecht Aug 8, 2019

t-bast commented Aug 5, 2019

Roasbeef commented Aug 8, 2019

Feature bit unification and renaming #571

Feature bit unification and renaming #571

Conversation

rustyrussell commented Feb 12, 2019 • edited Loading

Concrete Examples

t-bast commented May 28, 2019

t-bast commented Jun 28, 2019 • edited Loading

rustyrussell commented Jul 8, 2019

Roasbeef commented Jul 12, 2019

Roasbeef commented Jul 12, 2019

t-bast commented Jul 12, 2019

cfromknecht commented Jul 15, 2019

cfromknecht left a comment

Choose a reason for hiding this comment

cfromknecht Jul 15, 2019

Choose a reason for hiding this comment

cfromknecht Jul 15, 2019

Choose a reason for hiding this comment

rustyrussell Aug 6, 2019

Choose a reason for hiding this comment

cfromknecht Aug 8, 2019

Choose a reason for hiding this comment

t-bast commented Aug 5, 2019

Roasbeef commented Aug 8, 2019

rustyrussell commented Feb 12, 2019 •

edited

Loading

t-bast commented Jun 28, 2019 •

edited

Loading