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

listpeers: add features array using BOLT9 names. #3963

Conversation

rustyrussell
Copy link
Contributor

It's actually not possible to currently tell if you're using anchor_outputs
with a peer (since it depends on whether you both supported it at channel open).

Signed-off-by: Rusty Russell rusty@rustcorp.com.au
Changelog-added: JSON-RPC: listpeers shows features list for each channel.

It's actually not possible to currently tell if you're using anchor_outputs
with a peer (since it depends on whether you both supported it at *channel open*).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-added: JSON-RPC: `listpeers` shows `features` list for each channel.
@rustyrussell rustyrussell force-pushed the indiciate-channel-features-in-listpeers branch from be979eb to 07b328d Compare August 21, 2020 02:04
@@ -155,6 +155,18 @@ void json_add_uncommitted_channel(struct json_stream *response,
json_add_amount_msat_compat(response, total,
"msatoshi_total", "total_msat");
}

json_array_start(response, "features");
if (feature_negotiated(uc->peer->ld->our_features,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it not be better (and more easy to extend with new features in the future) to have an array of structs like so?

static const struct { const char *name; size_t feature; } known_features[] = {
        { "option_static_remotekey", OPT_STATIC_REMOTEKEY},
        { "option_anchor_outputs", OPT_ANCHOR_OUTPUTS}
};

Then just iterate:

for (size_t i = 0; i < ARRAY_SIZE(known_features); ++i)
        if (feature_negotiated(our_features, their_features, known_features[i].feature))
                json_add_string(response, NULL, known_features[i].name);

Also, is not their_features set on init message, and the peer could be initially an old software, create channel with us pre-anchor-commitments, then upgrade so they now both support anchor-commitments but the actual channel is not using anchor commitments yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably include the feature names in the per-peer part of the JSON output, yes, but that information is technically redundant (since you can find out our features and their features already, by bitmap).

But these two features are sticky based on what was negotiated at opening time, so you can't intuit them by looking at currently negotiated features. Hence these test the flag directly, (from the db).

There's a proposal to upgrade on-the-fly, which would also Just Work with this API.

Copy link
Collaborator

@niftynei niftynei left a comment

Choose a reason for hiding this comment

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

ACK 07b328d

@niftynei niftynei merged commit dd8cd81 into ElementsProject:master Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants