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.

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

Features rework #3609

Merged
merged 7 commits into from
Mar 31, 2020
Merged

Features rework #3609

merged 7 commits into from
Mar 31, 2020

Conversation

rustyrussell
Copy link
Contributor

This is all in preparation for a --wumbo option.

First we update the spec, which means we gain the ability to have multiple addresses of the same type.

Then we make our feature infrastructure more flexible. We keep each feature bitmap, and make sure every subdaemon which needs it has one.

This already helps the plugin-adds-features case, which wasn't complete before (we wouldn't accept features we advertized, since we were checking against our compiled-in feature set).

This restriction was removed from the spec as of
86c2ebcc5973a4133d3ce4d80ae1c203061a1646.

We also fix up some strange formatting in that part of the documentation.

Changelog-changed: We now announce multiple addresses of the same type, if given.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Copy link
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

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

Nice consolidation of how features are handled 👍

ACK b97b4f3

Comment on lines +133 to +134
/* FIXME: We could allow a plugin to upgrade an optional feature
* to a compulsory one? */
Copy link
Member

Choose a reason for hiding this comment

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

Probably not a good idea at runtime, and the features registered by the plugin are limited to optional featurebits only at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, are they? I didn't see that in the code or documentation?

Copy link
Member

Choose a reason for hiding this comment

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

My bad, I misremembered. I had considered making them odd-only initially in the hope of reducing the fallout from a plugin crashing, but must've decided against it.

@cdecker
Copy link
Member

cdecker commented Mar 30, 2020

Seems CI is just getting upset at some whitespace, should be easy to fix.

This is all typo/clarity fixes, no substantive changes.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This will be needed for wumbo, for example.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is to prepare for dynamic features, including making plugins first
class citizens at setting them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This cleans up the boutique handling of features, and importantly, it
means that if a plugin says to offer a feature in init, we will now
*accept* that feature.

Changelog-Fixed: Plugins: setting an 'init' feature bit allows us to accept it from peers.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We now manipulate the global features directly.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@cdecker
Copy link
Member

cdecker commented Mar 31, 2020

ACK e3b5ea4

@cdecker cdecker merged commit b7db588 into ElementsProject:master Mar 31, 2020
rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Nov 5, 2021
This surprised me, since the CHANGELOG for [0.8.2] said:

	We now announce multiple addresses of the same type, if given. ([3609](ElementsProject#3609))

But it lied!

Changelog-Fixed: We really do allow providing multiple addresses of the same type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Nov 6, 2021
This surprised me, since the CHANGELOG for [0.8.2] said:

	We now announce multiple addresses of the same type, if given. ([3609](ElementsProject#3609))

But it lied!

Changelog-Fixed: We really do allow providing multiple addresses of the same type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Nov 10, 2021
This surprised me, since the CHANGELOG for [0.8.2] said:

	We now announce multiple addresses of the same type, if given. ([3609](ElementsProject#3609))

But it lied!

Changelog-Fixed: We really do allow providing multiple addresses of the same type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
cdecker pushed a commit that referenced this pull request Nov 14, 2021
This surprised me, since the CHANGELOG for [0.8.2] said:

	We now announce multiple addresses of the same type, if given. ([3609](#3609))

But it lied!

Changelog-Fixed: We really do allow providing multiple addresses of the same type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants