From 722dd05e9d06569776321072142e3535f09a5550 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 24 Jul 2018 15:48:59 +0930 Subject: [PATCH] lightningd: keep features arrays for connected peers. As a side-effect, we only print them for connected peers (which avoids an O(n^2) traversal). Signed-off-by: Rusty Russell --- lightningd/opening_control.c | 15 ++++++++----- lightningd/peer_control.c | 43 ++++++++++++++++++------------------ lightningd/peer_control.h | 6 ++++- wallet/test/run-wallet.c | 2 +- wallet/wallet.c | 2 +- 5 files changed, 38 insertions(+), 30 deletions(-) diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 1189b0b4b..1f839a0f6 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -596,7 +596,8 @@ static struct uncommitted_channel * new_uncommitted_channel(struct lightningd *ld, struct funding_channel *fc, const struct pubkey *peer_id, - const struct wireaddr_internal *addr) + const struct wireaddr_internal *addr, + const u8 *gfeatures, const u8 *lfeatures) { struct uncommitted_channel *uc = tal(ld, struct uncommitted_channel); char *idname; @@ -604,7 +605,7 @@ new_uncommitted_channel(struct lightningd *ld, /* We make a new peer if necessary. */ uc->peer = peer_by_id(ld, peer_id); if (!uc->peer) - uc->peer = new_peer(ld, 0, peer_id, addr); + uc->peer = new_peer(ld, 0, peer_id, addr, gfeatures, lfeatures); if (uc->peer->uncommitted_channel) return tal_free(uc); @@ -686,7 +687,7 @@ u8 *peer_accept_channel(const tal_t *ctx, const struct pubkey *peer_id, const struct wireaddr_internal *addr, const struct crypto_state *cs, - const u8 *gfeatures UNUSED, const u8 *lfeatures UNUSED, + const u8 *gfeatures, const u8 *lfeatures, int peer_fd, int gossip_fd, const struct channel_id *channel_id, const u8 *open_msg) @@ -700,7 +701,8 @@ u8 *peer_accept_channel(const tal_t *ctx, assert(fromwire_peektype(open_msg) == WIRE_OPEN_CHANNEL); /* Fails if there's already one */ - uc = new_uncommitted_channel(ld, NULL, peer_id, addr); + uc = new_uncommitted_channel(ld, NULL, peer_id, addr, + gfeatures, lfeatures); if (!uc) return towire_errorfmt(ctx, channel_id, "Multiple channels unsupported"); @@ -766,7 +768,7 @@ static void peer_offer_channel(struct lightningd *ld, struct funding_channel *fc, const struct wireaddr_internal *addr, const struct crypto_state *cs, - const u8 *gfeatures UNUSED, const u8 *lfeatures UNUSED, + const u8 *gfeatures, const u8 *lfeatures, int peer_fd, int gossip_fd) { u8 *msg; @@ -778,7 +780,8 @@ static void peer_offer_channel(struct lightningd *ld, list_del_from(&ld->fundchannels, &fc->list); tal_del_destructor(fc, remove_funding_channel_from_list); - fc->uc = new_uncommitted_channel(ld, fc, &fc->peerid, addr); + fc->uc = new_uncommitted_channel(ld, fc, &fc->peerid, addr, + gfeatures, lfeatures); /* We asked to release this peer, but another raced in? Corner case, * close this is easiest. */ diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index e41e1fdea..8e79ba346 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -80,7 +80,8 @@ static void copy_to_parent_log(const char *prefix, struct peer *new_peer(struct lightningd *ld, u64 dbid, const struct pubkey *id, - const struct wireaddr_internal *addr) + const struct wireaddr_internal *addr, + const u8 *gfeatures, const u8 *lfeatures) { /* We are owned by our channels, and freed manually by destroy_channel */ struct peer *peer = tal(NULL, struct peer); @@ -96,6 +97,10 @@ struct peer *new_peer(struct lightningd *ld, u64 dbid, peer->addr.itype = ADDR_INTERNAL_WIREADDR; peer->addr.u.wireaddr.type = ADDR_TYPE_PADDING; } + peer->global_features = tal_dup_arr(peer, u8, + gfeatures, tal_count(gfeatures), 0); + peer->local_features = tal_dup_arr(peer, u8, + lfeatures, tal_count(lfeatures), 0); list_head_init(&peer->channels); peer->direction = get_channel_direction(&peer->ld->id, &peer->id); @@ -704,18 +709,6 @@ struct getpeers_args { struct pubkey *specific_id; }; -static void json_add_features(struct json_result *response, - const struct peer_features *pf) -{ - json_add_hex(response, "global_features", - pf->global_features, - tal_len(pf->global_features)); - - json_add_hex(response, "local_features", - pf->local_features, - tal_len(pf->local_features)); -} - static void connectd_getpeers_complete(struct subd *connectd, const u8 *msg, const int *fds UNUSED, struct getpeers_args *gpa) @@ -756,6 +749,9 @@ static void connectd_getpeers_complete(struct subd *connectd, const u8 *msg, } json_add_bool(response, "connected", connected); + /* If it's not connected, features are unreliable: we don't + * store them in the database, and they would only reflect + * their features *last* time they connected. */ if (connected) { json_array_start(response, "netaddr"); if (p->addr.itype != ADDR_INTERNAL_WIREADDR @@ -765,14 +761,13 @@ static void connectd_getpeers_complete(struct subd *connectd, const u8 *msg, struct wireaddr_internal, &p->addr)); json_array_end(response); - } + json_add_hex(response, "global_features", + p->global_features, + tal_len(p->global_features)); - /* Search connectd reply for this ID, to add extra info. */ - for (size_t i = 0; i < tal_count(ids); i++) { - if (pubkey_eq(&ids[i], &p->id)) { - json_add_features(response, pf[i]); - break; - } + json_add_hex(response, "local_features", + p->local_features, + tal_len(p->local_features)); } json_array_start(response, "channels"); @@ -905,7 +900,13 @@ static void connectd_getpeers_complete(struct subd *connectd, const u8 *msg, /* Fake state. */ json_add_string(response, "state", "GOSSIPING"); json_add_pubkey(response, "id", ids+i); - json_add_features(response, pf[i]); + json_add_hex(response, "global_features", + pf[i]->global_features, + tal_len(pf[i]->global_features)); + + json_add_hex(response, "local_features", + pf[i]->local_features, + tal_len(pf[i]->local_features)); json_array_start(response, "netaddr"); if (addrs[i].itype != ADDR_INTERNAL_WIREADDR || addrs[i].u.wireaddr.type != ADDR_TYPE_PADDING) diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index a84009b09..7c5f2530c 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -41,6 +41,9 @@ struct peer { /* Where we connected to, or it connected from. */ struct wireaddr_internal addr; + /* We keep a copy of their feature bits */ + const u8 *local_features, *global_features; + /* If we open a channel our direction will be this */ u8 direction; @@ -54,7 +57,8 @@ struct peer *find_peer_by_dbid(struct lightningd *ld, u64 dbid); struct peer *new_peer(struct lightningd *ld, u64 dbid, const struct pubkey *id, - const struct wireaddr_internal *addr); + const struct wireaddr_internal *addr, + const u8 *gfeatures, const u8 *lfeatures); /* Also removes from db. */ void delete_peer(struct peer *peer); diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 64f91ab64..83dfee31a 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -762,7 +762,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) mempat(scriptpubkey, tal_len(scriptpubkey)); c1.first_blocknum = 1; c1.final_key_idx = 1337; - p = new_peer(ld, 0, &pk, NULL); + p = new_peer(ld, 0, &pk, NULL, NULL, NULL); c1.peer = p; c1.dbid = wallet_get_channel_dbid(w); c1.state = CHANNELD_NORMAL; diff --git a/wallet/wallet.c b/wallet/wallet.c index c9b97dca3..fab2868f9 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -522,7 +522,7 @@ static struct peer *wallet_peer_load(struct wallet *w, const u64 dbid) addrp = NULL; peer = new_peer(w->ld, sqlite3_column_int64(stmt, 0), - &id, addrp); + &id, addrp, NULL, NULL); db_stmt_done(stmt); return peer;