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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 28, 2020
Merged

listpeers: add features array using BOLT9 names. #3963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/lightning-listpeers.7
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions doc/lightning-listpeers.7.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ The objects in the *channels* array will have at least these fields:
a number followed by a string unit.
* *total\_msat*: A string describing the total capacity of the channel;
a number followed by a string unit.
* *features*: An array of feature names supported by this channel.

These fields may exist if the channel has gotten beyond the `"OPENINGD"`
state, or in various circumstances:
Expand Down
12 changes: 12 additions & 0 deletions lightningd/opening_control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Contributor

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.

uc->peer->their_features,
OPT_STATIC_REMOTEKEY))
json_add_string(response, NULL, "option_static_remotekey");

if (feature_negotiated(uc->peer->ld->our_features,
uc->peer->their_features,
OPT_ANCHOR_OUTPUTS))
json_add_string(response, NULL, "option_anchor_outputs");
json_array_end(response);
json_object_end(response);
}

Expand Down
7 changes: 7 additions & 0 deletions lightningd/peer_control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,13 @@ static void json_add_channel(struct lightningd *ld,
response, "private",
!(channel->channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL));

json_array_start(response, "features");
if (channel->option_static_remotekey)
json_add_string(response, NULL, "option_static_remotekey");
if (channel->option_anchor_outputs)
json_add_string(response, NULL, "option_anchor_outputs");
json_array_end(response);

// FIXME @conscott : Modify this when dual-funded channels
// are implemented
json_object_start(response, "funding_allocation_msat");
Expand Down
32 changes: 32 additions & 0 deletions tests/test_connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2330,3 +2330,35 @@ def test_wumbo_channels(node_factory, bitcoind):
# Exact amount depends on fees, but it will be wumbo!
amount = [c['funding_msat'][l1.info['id']] for c in only_one(l1.rpc.listpeers(l2.info['id'])['peers'])['channels'] if c['state'] == 'CHANNELD_NORMAL'][0]
assert Millisatoshi(amount) > Millisatoshi(str((1 << 24) - 1) + "sat")


def test_channel_features(node_factory, bitcoind):
l1, l2 = node_factory.line_graph(2, fundchannel=False)

bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['bech32'], 0.1)
bitcoind.generate_block(1)
wait_for(lambda: l1.rpc.listfunds()['outputs'] != [])

l1.rpc.fundchannel(l2.info['id'], 'all')

# We should see features in unconfirmed channels.
chan = only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])
assert 'option_static_remotekey' in chan['features']
if EXPERIMENTAL_FEATURES:
assert 'option_anchor_outputs' in chan['features']

# l2 should agree.
assert only_one(only_one(l2.rpc.listpeers()['peers'])['channels'])['features'] == chan['features']

# Confirm it.
bitcoind.generate_block(1)
wait_for(lambda: only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])['state'] == 'CHANNELD_NORMAL')
wait_for(lambda: only_one(only_one(l2.rpc.listpeers()['peers'])['channels'])['state'] == 'CHANNELD_NORMAL')

chan = only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])
assert 'option_static_remotekey' in chan['features']
if EXPERIMENTAL_FEATURES:
assert 'option_anchor_outputs' in chan['features']

# l2 should agree.
assert only_one(only_one(l2.rpc.listpeers()['peers'])['channels'])['features'] == chan['features']