Skip to content

common/features: only support a single feature bitset.#3145

Merged
rustyrussell merged 2 commits into
ElementsProject:masterfrom
rustyrussell:guilt/flatten-features
Oct 11, 2019
Merged

common/features: only support a single feature bitset.#3145
rustyrussell merged 2 commits into
ElementsProject:masterfrom
rustyrussell:guilt/flatten-features

Conversation

@rustyrussell
Copy link
Copy Markdown
Contributor

This is mainly an internal-only change, especially since we don't
offer any globalfeatures.

However, LND (as of next release) will offer global features, and also
expect option_static_remotekey to be a global feature. So we send
our (merged) feature bitset as both global and local in init, and fold
those bitsets together when we get an init msg.

This works with every proposal:

lightning/bolts#680
lightning/bolts#666

@rustyrussell rustyrussell added this to the 0.7.3 milestone Oct 9, 2019
@rustyrussell rustyrussell requested a review from cdecker as a code owner October 9, 2019 02:19
@rustyrussell rustyrussell added gossip protocol These issues are protocol level issues that should be discussed on the protocol spec repo labels Oct 9, 2019
@rustyrussell rustyrussell force-pushed the guilt/flatten-features branch from be5886d to 52207ac Compare October 9, 2019 06:26
darosior
darosior approved these changes Oct 9, 2019
Comment thread CHANGELOG.md
Note: You should always set `allow-deprecated-apis=false` to test for
changes.

- JSON API: `listpeers` and `listnodes` fields `localfeatures` and `globalfeatures` (now just `features`).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also change the plugin hooks/notifications API then ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(peer_connected for example)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

peer_connected was updated in 52207ac, the documentation is lagging however.

Comment thread connectd/connectd.h Outdated
const struct wireaddr_internal *addr,
const struct crypto_state *cs,
const u8 *globalfeatures TAKES,
const u8 *localfeatures TAKES);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

localfeatures -> features ?

Comment thread CHANGELOG.md
Note: You should always set `allow-deprecated-apis=false` to test for
changes.

- JSON API: `listpeers` and `listnodes` fields `localfeatures` and `globalfeatures` (now just `features`).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

peer_connected was updated in 52207ac, the documentation is lagging however.

@niftynei
Copy link
Copy Markdown
Collaborator

went ahead and fixed up my two nits; updated commit is here niftynei@ae6a3db

This is mainly an internal-only change, especially since we don't
offer any globalfeatures.

However, LND (as of next release) will offer global features, and also
expect option_static_remotekey to be a *global* feature.  So we send
our (merged) feature bitset as both global and local in init, and fold
those bitsets together when we get an init msg.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the guilt/flatten-features branch from 52207ac to 40bfdb3 Compare October 10, 2019 02:44
@rustyrussell
Copy link
Copy Markdown
Contributor Author

OK, that was weird. Rebased on master, pushed a fixup for review (squash on merge please!)

@niftynei niftynei self-requested a review October 10, 2019 03:01
@rustyrussell rustyrussell merged commit bd55f6d into ElementsProject:master Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gossip protocol These issues are protocol level issues that should be discussed on the protocol spec repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants