Skip to content

flat features#3685

Merged
joostjager merged 5 commits intolightningnetwork:masterfrom
cfromknecht:flat-features
Nov 9, 2019
Merged

flat features#3685
joostjager merged 5 commits intolightningnetwork:masterfrom
cfromknecht:flat-features

Conversation

@cfromknecht
Copy link
Contributor

An implementation of lightning/bolts#666

Copy link
Contributor

@joostjager joostjager left a comment

Choose a reason for hiding this comment

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

First pass looking good. Main comment is about order of commits

@cfromknecht cfromknecht force-pushed the flat-features branch 4 times, most recently from 77cea27 to 9f80240 Compare November 8, 2019 10:36
Copy link
Contributor

@joostjager joostjager left a comment

Choose a reason for hiding this comment

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

Nice restructuring

@cfromknecht cfromknecht force-pushed the flat-features branch 4 times, most recently from b13cb6d to 79a0753 Compare November 8, 2019 11:45
This commit introduces a feature.Manager, which derives feature vectors
for various contexts within the daemon. The sets can be described via a
staticly compiled format, which makes any runtime adjustments to the
feature sets when the manager is initialized.
Copy link
Contributor

@halseth halseth left a comment

Choose a reason for hiding this comment

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

LGTM 💯

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

Clean set of changes! We should wait on merge until we confirm proper interop tests w/ the other main implementations, and also for the final version of the spec to be merged.

LGTM 💥

@joostjager joostjager merged commit b222b6e into lightningnetwork:master Nov 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants