From bd55f6d9408d7928852637e9581a3d9d5107111f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 11 Oct 2019 02:52:04 +0000 Subject: [PATCH] common/features: only support a single feature bitset. This is mainly an internal-only change, especially since we don't offer any globalfeatures. However, LND (as of next release) will offer global features, and also expect option_static_remotekey to be a *global* feature. So we send our (merged) feature bitset as both global and local in init, and fold those bitsets together when we get an init msg. Signed-off-by: Rusty Russell --- CHANGELOG.md | 3 + channeld/channeld.c | 8 +-- common/features.c | 73 +++++++-------------- common/features.h | 26 +++----- common/test/run-features.c | 29 +++----- connectd/connect_wire.csv | 6 +- connectd/connectd.c | 47 +++++-------- connectd/connectd.h | 3 +- connectd/peer_exchange_initmsg.c | 50 +++++++++----- doc/PLUGINS.md | 3 +- doc/lightning-listpeers.7 | 6 +- doc/lightning-listpeers.7.md | 3 +- gossipd/gossipd.c | 2 +- gossipd/routing.c | 4 +- lightningd/channel_control.c | 2 +- lightningd/gossip_control.c | 6 +- lightningd/gossip_msg.c | 33 ++-------- lightningd/gossip_msg.h | 11 +--- lightningd/opening_control.c | 9 ++- lightningd/peer_control.c | 40 ++++++----- lightningd/peer_control.h | 2 +- lightningd/test/run-invoice-select-inchan.c | 2 +- openingd/openingd.c | 12 ++-- tests/test_connection.py | 21 +++--- tests/test_gossip.py | 2 +- wallet/test/run-wallet.c | 2 +- 26 files changed, 167 insertions(+), 238 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f0aabaaa..f95768bfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,9 @@ changes. - JSON-API: `fundchannel` now uses `amount` as the parameter name to replace `satoshi` - JSON-API: `fundchannel_start` now uses `amount` as the parameter name to replace `satoshi` +- JSON API: `listpeers` and `listnodes` fields `localfeatures` and `globalfeatures` (now just `features`). +- Plugin: `peer_connected` hook fields `localfeatures` and `globalfeatures` (now just `features`). + ### Removed - JSON API: `short_channel_id` parameters in JSON commands with `:` separators (deprecated since 0.7.0). diff --git a/channeld/channeld.c b/channeld/channeld.c index 525d01d45..e76b3bd4d 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -70,7 +70,7 @@ struct peer { u64 next_index[NUM_SIDES]; /* Features peer supports. */ - u8 *localfeatures; + u8 *features; /* Tolerable amounts for feerate (only relevant for fundee). */ u32 feerate_min, feerate_max; @@ -2227,8 +2227,8 @@ static void peer_reconnect(struct peer *peer, bool dataloss_protect, check_extra_fields; const u8 **premature_msgs = tal_arr(peer, const u8 *, 0); - dataloss_protect = local_feature_negotiated(peer->localfeatures, - LOCAL_DATA_LOSS_PROTECT); + dataloss_protect = feature_negotiated(peer->features, + OPT_DATA_LOSS_PROTECT); /* Both these options give us extra fields to check. */ check_extra_fields @@ -2998,7 +2998,7 @@ static void init_channel(struct peer *peer) &funding_signed, &peer->announce_depth_reached, &last_remote_per_commit_secret, - &peer->localfeatures, + &peer->features, &peer->remote_upfront_shutdown_script, &remote_ann_node_sig, &remote_ann_bitcoin_sig, diff --git a/common/features.c b/common/features.c index 2d2b023ad..e423138ee 100644 --- a/common/features.c +++ b/common/features.c @@ -4,15 +4,12 @@ #include #include -static const u32 our_localfeatures[] = { - OPTIONAL_FEATURE(LOCAL_DATA_LOSS_PROTECT), - OPTIONAL_FEATURE(LOCAL_UPFRONT_SHUTDOWN_SCRIPT), - OPTIONAL_FEATURE(LOCAL_GOSSIP_QUERIES), - OPTIONAL_FEATURE(LOCAL_GOSSIP_QUERIES_EX), - OPTIONAL_FEATURE(LOCAL_STATIC_REMOTEKEY), -}; - -static const u32 our_globalfeatures[] = { +static const u32 our_features[] = { + OPTIONAL_FEATURE(OPT_DATA_LOSS_PROTECT), + OPTIONAL_FEATURE(OPT_UPFRONT_SHUTDOWN_SCRIPT), + OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES), + OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES_EX), + OPTIONAL_FEATURE(OPT_STATIC_REMOTEKEY), }; /* BOLT #1: @@ -48,12 +45,6 @@ static u8 *mkfeatures(const tal_t *ctx, const u32 *arr, size_t n) return f; } -u8 *get_offered_globalfeatures(const tal_t *ctx) -{ - return mkfeatures(ctx, - our_globalfeatures, ARRAY_SIZE(our_globalfeatures)); -} - /* We currently advertize everything in node_announcement, except * initial_routing_sync which the spec says not to (and we don't set * any more anyway). @@ -63,13 +54,13 @@ u8 *get_offered_globalfeatures(const tal_t *ctx) u8 *get_offered_nodefeatures(const tal_t *ctx) { return mkfeatures(ctx, - our_localfeatures, ARRAY_SIZE(our_localfeatures)); + our_features, ARRAY_SIZE(our_features)); } -u8 *get_offered_localfeatures(const tal_t *ctx) +u8 *get_offered_features(const tal_t *ctx) { return mkfeatures(ctx, - our_localfeatures, ARRAY_SIZE(our_localfeatures)); + our_features, ARRAY_SIZE(our_features)); } bool feature_is_set(const u8 *features, size_t bit) @@ -100,6 +91,13 @@ static bool feature_supported(int feature_bit, return false; } +bool feature_negotiated(const u8 *lfeatures, size_t f) +{ + if (!feature_offered(lfeatures, f)) + return false; + return feature_supported(f, our_features, ARRAY_SIZE(our_features)); +} + /** * all_supported_features - Check if we support what's being asked * @@ -130,36 +128,17 @@ static bool all_supported_features(const u8 *bitmap, return true; } -bool features_supported(const u8 *globalfeatures, const u8 *localfeatures) +bool features_supported(const u8 *features) { /* BIT 2 would logically be "compulsory initial_routing_sync", but * that does not exist, so we special case it. */ - if (feature_is_set(localfeatures, - COMPULSORY_FEATURE(LOCAL_INITIAL_ROUTING_SYNC))) + if (feature_is_set(features, + COMPULSORY_FEATURE(OPT_INITIAL_ROUTING_SYNC))) return false; - return all_supported_features(globalfeatures, - our_globalfeatures, - ARRAY_SIZE(our_globalfeatures)) - && all_supported_features(localfeatures, - our_localfeatures, - ARRAY_SIZE(our_localfeatures)); -} - -bool local_feature_negotiated(const u8 *lfeatures, size_t f) -{ - if (!feature_offered(lfeatures, f)) - return false; - return feature_supported(f, our_localfeatures, - ARRAY_SIZE(our_localfeatures)); -} - -bool global_feature_negotiated(const u8 *gfeatures, size_t f) -{ - if (!feature_offered(gfeatures, f)) - return false; - return feature_supported(f, our_globalfeatures, - ARRAY_SIZE(our_globalfeatures)); + return all_supported_features(features, + our_features, + ARRAY_SIZE(our_features)); } static const char *feature_name(const tal_t *ctx, size_t f) @@ -182,10 +161,8 @@ const char **list_supported_features(const tal_t *ctx) { const char **list = tal_arr(ctx, const char *, 0); - /* The local/global number spaces are to be distinct, so this works */ - for (size_t i = 0; i < ARRAY_SIZE(our_localfeatures); i++) - tal_arr_expand(&list, feature_name(list, our_localfeatures[i])); - for (size_t i = 0; i < ARRAY_SIZE(our_globalfeatures); i++) - tal_arr_expand(&list, feature_name(list, our_globalfeatures[i])); + for (size_t i = 0; i < ARRAY_SIZE(our_features); i++) + tal_arr_expand(&list, feature_name(list, our_features[i])); + return list; } diff --git a/common/features.h b/common/features.h index f6d23853b..8c7a1bb5f 100644 --- a/common/features.h +++ b/common/features.h @@ -5,19 +5,17 @@ #include /* Returns true if we're OK with all these offered features. */ -bool features_supported(const u8 *globalfeatures, const u8 *localfeatures); +bool features_supported(const u8 *features); /* For sending our features: tal_count() returns length. */ -u8 *get_offered_globalfeatures(const tal_t *ctx); -u8 *get_offered_localfeatures(const tal_t *ctx); +u8 *get_offered_features(const tal_t *ctx); u8 *get_offered_nodefeatures(const tal_t *ctx); /* Is this feature bit requested? (Either compulsory or optional) */ bool feature_offered(const u8 *features, size_t f); /* Was this feature bit offered by them and us? */ -bool local_feature_negotiated(const u8 *lfeatures, size_t f); -bool global_feature_negotiated(const u8 *gfeatures, size_t f); +bool feature_negotiated(const u8 *lfeatures, size_t f); /* Return a list of what features we advertize. */ const char **list_supported_features(const tal_t *ctx); @@ -47,18 +45,14 @@ void set_feature_bit(u8 **ptr, u32 bit); * | 4/5 | `option_upfront_shutdown_script` |... * | 6/7 | `gossip_queries` |... * | 10/11 | `gossip_queries_ex` |... - */ -#define LOCAL_DATA_LOSS_PROTECT 0 -#define LOCAL_INITIAL_ROUTING_SYNC 2 -#define LOCAL_UPFRONT_SHUTDOWN_SCRIPT 4 -#define LOCAL_GOSSIP_QUERIES 6 -#define LOCAL_GOSSIP_QUERIES_EX 10 - -/* BOLT #9: - * | Bits | Name |... * | 12/13| `option_static_remotekey` |... */ -#define LOCAL_STATIC_REMOTEKEY 12 +#define OPT_DATA_LOSS_PROTECT 0 +#define OPT_INITIAL_ROUTING_SYNC 2 +#define OPT_UPFRONT_SHUTDOWN_SCRIPT 4 +#define OPT_GOSSIP_QUERIES 6 +#define OPT_GOSSIP_QUERIES_EX 10 +#define OPT_STATIC_REMOTEKEY 12 /* BOLT #9: * @@ -67,6 +61,6 @@ void set_feature_bit(u8 **ptr, u32 bit); * | Bits | Name | ... * | 8/9 | `var_onion_optin` | ... */ -#define GLOBAL_VAR_ONION 8 +#define OPT_VAR_ONION 8 #endif /* LIGHTNING_COMMON_FEATURES_H */ diff --git a/common/test/run-features.c b/common/test/run-features.c index 5a994ac0f..81ced7d78 100644 --- a/common/test/run-features.c +++ b/common/test/run-features.c @@ -33,7 +33,7 @@ const void *fromwire_fail(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) int main(void) { - u8 *bits, *lf, *gf; + u8 *bits, *lf; setup_locale(); wally_init(0); @@ -84,22 +84,17 @@ int main(void) /* We always support no features. */ memset(bits, 0, tal_count(bits)); - assert(features_supported(bits, bits)); + assert(features_supported(bits)); /* We must support our own features. */ - lf = get_offered_globalfeatures(tmpctx); - gf = get_offered_globalfeatures(tmpctx); - assert(features_supported(gf, lf)); + lf = get_offered_features(tmpctx); + assert(features_supported(lf)); /* We can add random odd features, no problem. */ for (size_t i = 1; i < 16; i += 2) { bits = tal_dup_arr(tmpctx, u8, lf, tal_count(lf), 0); set_feature_bit(&bits, i); - assert(features_supported(gf, bits)); - - bits = tal_dup_arr(tmpctx, u8, gf, tal_count(gf), 0); - set_feature_bit(&bits, i); - assert(features_supported(bits, lf)); + assert(features_supported(bits)); } /* We can't add random even features. */ @@ -109,18 +104,12 @@ int main(void) /* Special case for missing compulsory feature */ if (i == 2) { - assert(!features_supported(gf, bits)); + assert(!features_supported(bits)); } else { - assert(features_supported(gf, bits) - == feature_supported(i, our_localfeatures, - ARRAY_SIZE(our_localfeatures))); + assert(features_supported(bits) + == feature_supported(i, our_features, + ARRAY_SIZE(our_features))); } - - bits = tal_dup_arr(tmpctx, u8, gf, tal_count(gf), 0); - set_feature_bit(&bits, i); - assert(features_supported(bits, lf) - == feature_supported(i, our_globalfeatures, - ARRAY_SIZE(our_globalfeatures))); } wally_cleanup(0); diff --git a/connectd/connect_wire.csv b/connectd/connect_wire.csv index 89c7a6144..9407889a1 100644 --- a/connectd/connect_wire.csv +++ b/connectd/connect_wire.csv @@ -52,10 +52,8 @@ msgtype,connect_peer_connected,2002 msgdata,connect_peer_connected,id,node_id, msgdata,connect_peer_connected,addr,wireaddr_internal, msgdata,connect_peer_connected,pps,per_peer_state, -msgdata,connect_peer_connected,gflen,u16, -msgdata,connect_peer_connected,globalfeatures,u8,gflen -msgdata,connect_peer_connected,lflen,u16, -msgdata,connect_peer_connected,localfeatures,u8,lflen +msgdata,connect_peer_connected,flen,u16, +msgdata,connect_peer_connected,features,u8,flen # master -> connectd: peer has disconnected. msgtype,connectctl_peer_disconnected,2015 diff --git a/connectd/connectd.c b/connectd/connectd.c index 535f1370f..a54c16b01 100644 --- a/connectd/connectd.c +++ b/connectd/connectd.c @@ -283,13 +283,12 @@ static void connected_to_peer(struct daemon *daemon, * Every peer also has read-only access to the gossip_store, which is handed * out by gossipd too, and also a "gossip_state" indicating where we're up to. * - * The 'localfeatures' is a field in the `init` message, indicating properties - * when you're connected to it like we are: there are also 'globalfeatures' - * which specify requirements to route a payment through a node. + * 'features' is a field in the `init` message, indicating properties of the + * node. */ static bool get_gossipfds(struct daemon *daemon, const struct node_id *id, - const u8 *localfeatures, + const u8 *features, struct per_peer_state *pps) { bool gossip_queries_feature, initial_routing_sync, success; @@ -298,13 +297,13 @@ static bool get_gossipfds(struct daemon *daemon, /*~ The way features generally work is that both sides need to offer it; * we always offer `gossip_queries`, but this check is explicit. */ gossip_queries_feature - = local_feature_negotiated(localfeatures, LOCAL_GOSSIP_QUERIES); + = feature_negotiated(features, OPT_GOSSIP_QUERIES); /*~ `initial_routing_sync` is supported by every node, since it was in * the initial lightning specification: it means the peer wants the * backlog of existing gossip. */ initial_routing_sync - = feature_offered(localfeatures, LOCAL_INITIAL_ROUTING_SYNC); + = feature_offered(features, OPT_INITIAL_ROUTING_SYNC); /*~ We do this communication sync, since gossipd is our friend and * it's easier. If gossipd fails, we fail. */ @@ -343,8 +342,7 @@ struct peer_reconnected { struct node_id id; struct wireaddr_internal addr; struct crypto_state cs; - const u8 *globalfeatures; - const u8 *localfeatures; + const u8 *features; }; /*~ For simplicity, lightningd only ever deals with a single connection per @@ -362,8 +360,7 @@ static struct io_plan *retry_peer_connected(struct io_conn *conn, /*~ Usually the pattern is to return this directly, but we have to free * our temporary structure. */ plan = peer_connected(conn, pr->daemon, &pr->id, &pr->addr, &pr->cs, - take(pr->globalfeatures), - take(pr->localfeatures)); + take(pr->features)); tal_free(pr); return plan; } @@ -375,8 +372,7 @@ static struct io_plan *peer_reconnected(struct io_conn *conn, const struct node_id *id, const struct wireaddr_internal *addr, const struct crypto_state *cs, - const u8 *globalfeatures TAKES, - const u8 *localfeatures TAKES) + const u8 *features TAKES) { u8 *msg; struct peer_reconnected *pr; @@ -395,13 +391,9 @@ static struct io_plan *peer_reconnected(struct io_conn *conn, pr->cs = *cs; pr->addr = *addr; - /*~ Note that tal_dup_arr() will do handle the take() of - * globalfeatures and localfeatures (turning it into a simply - * tal_steal() in those cases). */ - pr->globalfeatures - = tal_dup_arr(pr, u8, globalfeatures, tal_count(globalfeatures), 0); - pr->localfeatures - = tal_dup_arr(pr, u8, localfeatures, tal_count(localfeatures), 0); + /*~ Note that tal_dup_arr() will do handle the take() of features + * (turning it into a simply tal_steal() in those cases). */ + pr->features = tal_dup_arr(pr, u8, features, tal_count(features), 0); /*~ ccan/io supports waiting on an address: in this case, the key in * the peer set. When someone calls `io_wake()` on that address, it @@ -421,35 +413,30 @@ struct io_plan *peer_connected(struct io_conn *conn, const struct node_id *id, const struct wireaddr_internal *addr, const struct crypto_state *cs, - const u8 *globalfeatures TAKES, - const u8 *localfeatures TAKES) + const u8 *features TAKES) { u8 *msg; struct per_peer_state *pps; if (node_set_get(&daemon->peers, id)) - return peer_reconnected(conn, daemon, id, addr, cs, - globalfeatures, localfeatures); + return peer_reconnected(conn, daemon, id, addr, cs, features); /* We've successfully connected. */ connected_to_peer(daemon, conn, id); /* We promised we'd take it by marking it TAKEN above; prepare to free it. */ - if (taken(globalfeatures)) - tal_steal(tmpctx, globalfeatures); - if (taken(localfeatures)) - tal_steal(tmpctx, localfeatures); + if (taken(features)) + tal_steal(tmpctx, features); /* This contains the per-peer state info; gossipd fills in pps->gs */ pps = new_per_peer_state(tmpctx, cs); /* If gossipd can't give us a file descriptor, we give up connecting. */ - if (!get_gossipfds(daemon, id, localfeatures, pps)) + if (!get_gossipfds(daemon, id, features, pps)) return io_close(conn); /* Create message to tell master peer has connected. */ - msg = towire_connect_peer_connected(NULL, id, addr, pps, - globalfeatures, localfeatures); + msg = towire_connect_peer_connected(NULL, id, addr, pps, features); /*~ daemon_conn is a message queue for inter-daemon communication: we * queue up the `connect_peer_connected` message to tell lightningd diff --git a/connectd/connectd.h b/connectd/connectd.h index 67e5cd52f..8e16e3ab7 100644 --- a/connectd/connectd.h +++ b/connectd/connectd.h @@ -19,7 +19,6 @@ struct io_plan *peer_connected(struct io_conn *conn, const struct node_id *id, const struct wireaddr_internal *addr, const struct crypto_state *cs, - const u8 *globalfeatures TAKES, - const u8 *localfeatures TAKES); + const u8 *features TAKES); #endif /* LIGHTNING_CONNECTD_CONNECTD_H */ diff --git a/connectd/peer_exchange_initmsg.c b/connectd/peer_exchange_initmsg.c index 9214d1cb1..fee78f537 100644 --- a/connectd/peer_exchange_initmsg.c +++ b/connectd/peer_exchange_initmsg.c @@ -34,7 +34,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn, struct peer *peer) { u8 *msg = cryptomsg_decrypt_body(tmpctx, &peer->cs, peer->msg); - u8 *globalfeatures, *localfeatures; + u8 *globalfeatures, *features; if (!msg) return io_close(conn); @@ -50,13 +50,20 @@ static struct io_plan *peer_init_received(struct io_conn *conn, if (unlikely(is_unknown_msg_discardable(msg))) return read_init(conn, peer); - if (!fromwire_init(peer, msg, &globalfeatures, &localfeatures)) { + if (!fromwire_init(tmpctx, msg, &globalfeatures, &features)) { status_debug("peer %s bad fromwire_init '%s', closing", type_to_string(tmpctx, struct node_id, &peer->id), tal_hex(tmpctx, msg)); return io_close(conn); } + /* The globalfeatures field is now unused, but there was a + * window where it was: combine the two. */ + for (size_t i = 0; i < tal_bytelen(globalfeatures) * 8; i++) { + if (feature_is_set(globalfeatures, i)) + set_feature_bit(&features, i); + } + /* BOLT #1: * * The receiving node: @@ -66,16 +73,12 @@ static struct io_plan *peer_init_received(struct io_conn *conn, * - upon receiving unknown _even_ feature bits that are non-zero: * - MUST fail the connection. */ - if (!features_supported(globalfeatures, localfeatures)) { - const u8 *our_globalfeatures = get_offered_globalfeatures(msg); - const u8 *our_localfeatures = get_offered_localfeatures(msg); - msg = towire_errorfmt(NULL, NULL, "Unsupported features %s/%s:" - " we only offer globalfeatures %s" - " and localfeatures %s", - tal_hex(msg, globalfeatures), - tal_hex(msg, localfeatures), - tal_hex(msg, our_globalfeatures), - tal_hex(msg, our_localfeatures)); + if (!features_supported(features)) { + const u8 *our_features = get_offered_features(msg); + msg = towire_errorfmt(NULL, NULL, "Unsupported features %s:" + " we only offer features %s", + tal_hex(msg, features), + tal_hex(msg, our_features)); msg = cryptomsg_encrypt_msg(NULL, &peer->cs, take(msg)); return io_write(conn, msg, tal_count(msg), io_close_cb, NULL); } @@ -84,8 +87,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn, * be disconnected if it's a reconnect. */ return peer_connected(conn, peer->daemon, &peer->id, &peer->addr, &peer->cs, - take(globalfeatures), - take(localfeatures)); + take(features)); } static struct io_plan *peer_init_hdr_received(struct io_conn *conn, @@ -147,9 +149,25 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn, * - MUST send `init` as the first Lightning message for any * connection. */ + /* Initially, there were two sets of feature bits: global and local. + * Local affected peer nodes only, global affected everyone. Both were + * sent in the `init` message, but node_announcement only advertized + * globals. + * + * But we didn't have any globals for a long time, and it turned out + * that people wanted us to broadcast local features so they could do + * peer selection. We agreed that the number spaces should be distinct, + * but debate still rages on how to handle them. + * + * Meanwhile, we finally added a global bit to the spec, so now it + * matters. And LND v0.8 decided to make option_static_remotekey a + * GLOBAL bit, not a local bit, so we need to send that as a global + * bit here. Thus, we send our full, combo, bitset as both global + * and local bits. */ peer->msg = towire_init(NULL, - get_offered_globalfeatures(tmpctx), - get_offered_localfeatures(tmpctx)); + /* Features so nice, we send it twice! */ + get_offered_features(tmpctx), + get_offered_features(tmpctx)); status_peer_io(LOG_IO_OUT, peer->msg); peer->msg = cryptomsg_encrypt_msg(peer, &peer->cs, take(peer->msg)); diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index e76a51cd0..7e3c92454 100644 --- a/doc/PLUGINS.md +++ b/doc/PLUGINS.md @@ -485,8 +485,7 @@ the cryptographic handshake. The parameters have the following structure if ther "peer": { "id": "03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f", "addr": "34.239.230.56:9735", - "globalfeatures": "", - "localfeatures": "" + "features": "" } } ``` diff --git a/doc/lightning-listpeers.7 b/doc/lightning-listpeers.7 index 553c122ba..1e8732b67 100644 --- a/doc/lightning-listpeers.7 +++ b/doc/lightning-listpeers.7 @@ -47,6 +47,7 @@ of 0 or more objects\. Each object in the list contains the following data: +.RS .IP \[bu] \fIid\fR : The unique id of the peer .IP \[bu] @@ -54,15 +55,14 @@ Each object in the list contains the following data: .IP \[bu] \fInetaddr\fR : A list of network addresses the node is listening on .IP \[bu] -\fIglobalfeatures\fR : Bit flags showing supported global features (BOLT #9) -.IP \[bu] -\fIlocalfeatures\fR : Bit flags showing supported local features (BOLT #9) +\fIfeatures\fR : Bit flags showing supported features (BOLT #9) .IP \[bu] \fIchannels\fR : An list of channel id’s open on the peer .IP \[bu] \fIlog\fR : Only present if \fIlevel\fR is set\. List logs related to the peer at the specified \fIlevel\fR +.RE If \fIid\fR is supplied and no matching nodes are found, a "peers" object with an empty list is returned\. diff --git a/doc/lightning-listpeers.7.md b/doc/lightning-listpeers.7.md index da6f9fcfd..bce9343f2 100644 --- a/doc/lightning-listpeers.7.md +++ b/doc/lightning-listpeers.7.md @@ -45,8 +45,7 @@ Each object in the list contains the following data: - *id* : The unique id of the peer - *connected* : A boolean value showing the connection status - *netaddr* : A list of network addresses the node is listening on -- *globalfeatures* : Bit flags showing supported global features (BOLT \#9) -- *localfeatures* : Bit flags showing supported local features (BOLT \#9) +- *features* : Bit flags showing supported features (BOLT \#9) - *channels* : An list of channel id’s open on the peer - *log* : Only present if *level* is set. List logs related to the peer at the specified *level* diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index bd241a479..d3f0a33ea 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -1029,7 +1029,7 @@ static void add_node_entry(const tal_t *ctx, e->nodeid = n->id; if (get_node_announcement(ctx, daemon, n, e->color, e->alias, - &e->globalfeatures, + &e->features, &e->addresses)) { e->last_timestamp = n->bcast.timestamp; } else { diff --git a/gossipd/routing.c b/gossipd/routing.c index bc1121540..3e26a5d9f 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1769,7 +1769,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate, * `features` _bit_, regardless of if it has parsed the announcement * or not. */ - if (!features_supported(features, NULL)) { + if (!features_supported(features)) { status_debug("Ignoring channel announcement, unsupported features %s.", tal_hex(pending, features)); goto ignored; @@ -2573,7 +2573,7 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann, * - MAY discard the message altogether. * - SHOULD NOT connect to the node. */ - if (!features_supported(features, NULL)) { + if (!features_supported(features)) { status_debug("Ignoring node announcement for node %s, unsupported features %s.", type_to_string(tmpctx, struct node_id, &node_id), tal_hex(tmpctx, features)); diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 149ac2f9a..ec9850c4e 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -470,7 +470,7 @@ void peer_start_channeld(struct channel *channel, funding_signed, reached_announce_depth, &last_remote_per_commit_secret, - channel->peer->localfeatures, + channel->peer->features, channel->remote_upfront_shutdown_script, remote_ann_node_sig, remote_ann_bitcoin_sig, diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index b17649aa0..927549d0b 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -262,8 +262,10 @@ static void json_getnodes_reply(struct subd *gossip UNUSED, const u8 *reply, nodes[i]->color, ARRAY_SIZE(nodes[i]->color)); json_add_u64(response, "last_timestamp", nodes[i]->last_timestamp); - json_add_hex_talarr(response, "globalfeatures", - nodes[i]->globalfeatures); + if (deprecated_apis) + json_add_hex_talarr(response, "globalfeatures", + nodes[i]->features); + json_add_hex_talarr(response, "features", nodes[i]->features); json_array_start(response, "addresses"); for (j=0; jaddresses); j++) { json_add_address(response, NULL, &nodes[i]->addresses[j]); diff --git a/lightningd/gossip_msg.c b/lightningd/gossip_msg.c index 363358dbb..fd57099e6 100644 --- a/lightningd/gossip_msg.c +++ b/lightningd/gossip_msg.c @@ -21,8 +21,8 @@ struct gossip_getnodes_entry *fromwire_gossip_getnodes_entry(const tal_t *ctx, } flen = fromwire_u16(pptr, max); - entry->globalfeatures = tal_arr(entry, u8, flen); - fromwire_u8_array(pptr, max, entry->globalfeatures, flen); + entry->features = tal_arr(entry, u8, flen); + fromwire_u8_array(pptr, max, entry->features, flen); numaddresses = fromwire_u8(pptr, max); @@ -49,9 +49,8 @@ void towire_gossip_getnodes_entry(u8 **pptr, if (entry->last_timestamp < 0) return; - towire_u16(pptr, tal_count(entry->globalfeatures)); - towire_u8_array(pptr, entry->globalfeatures, - tal_count(entry->globalfeatures)); + towire_u16(pptr, tal_count(entry->features)); + towire_u8_array(pptr, entry->features, tal_count(entry->features)); towire_u8(pptr, tal_count(entry->addresses)); for (size_t i = 0; i < tal_count(entry->addresses); i++) { towire_wireaddr(pptr, &entry->addresses[i]); @@ -172,30 +171,6 @@ void towire_gossip_getchannels_entry(u8 **pptr, towire_bool(pptr, false); } -struct peer_features * -fromwire_peer_features(const tal_t *ctx, const u8 **pptr, size_t *max) -{ - struct peer_features *pf = tal(ctx, struct peer_features); - size_t len; - - len = fromwire_u16(pptr, max); - pf->localfeatures = tal_arr(pf, u8, len); - fromwire_u8_array(pptr, max, pf->localfeatures, len); - - len = fromwire_u16(pptr, max); - pf->globalfeatures = tal_arr(pf, u8, len); - fromwire_u8_array(pptr, max, pf->globalfeatures, len); - return pf; -} - -void towire_peer_features(u8 **pptr, const struct peer_features *pf) -{ - towire_u16(pptr, tal_count(pf->localfeatures)); - towire_u8_array(pptr, pf->localfeatures, tal_count(pf->localfeatures)); - towire_u16(pptr, tal_count(pf->globalfeatures)); - towire_u8_array(pptr, pf->globalfeatures, tal_count(pf->globalfeatures)); -} - struct exclude_entry *fromwire_exclude_entry(const tal_t *ctx, const u8 **pptr, size_t *max) { diff --git a/lightningd/gossip_msg.h b/lightningd/gossip_msg.h index a7f226766..c66d1fabf 100644 --- a/lightningd/gossip_msg.h +++ b/lightningd/gossip_msg.h @@ -6,15 +6,10 @@ struct route_info; -struct peer_features { - u8 *localfeatures; - u8 *globalfeatures; -}; - struct gossip_getnodes_entry { struct node_id nodeid; s64 last_timestamp; /* -1 means never: following fields ignored */ - u8 *globalfeatures; + u8 *features; struct wireaddr *addresses; u8 alias[32]; u8 color[3]; @@ -45,10 +40,6 @@ fromwire_gossip_getnodes_entry(const tal_t *ctx, const u8 **pptr, size_t *max); void towire_gossip_getnodes_entry(u8 **pptr, const struct gossip_getnodes_entry *entry); -struct peer_features * -fromwire_peer_features(const tal_t *ctx, const u8 **pptr, size_t *max); -void towire_peer_features(u8 **pptr, const struct peer_features *features); - void fromwire_route_hop(const u8 **pprt, size_t *max, struct route_hop *entry); void towire_route_hop(u8 **pprt, const struct route_hop *entry); diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 98a9eb60b..2081dd1f6 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -217,8 +217,7 @@ wallet_commit_channel(struct lightningd *ld, */ /* i.e. We set it now for the channel permanently. */ option_static_remotekey - = local_feature_negotiated(uc->peer->localfeatures, - LOCAL_STATIC_REMOTEKEY); + = feature_negotiated(uc->peer->features, OPT_STATIC_REMOTEKEY); channel = new_channel(uc->peer, uc->dbid, NULL, /* No shachain yet */ @@ -948,9 +947,9 @@ void peer_start_openingd(struct peer *peer, uc->minimum_depth, feerate_min(peer->ld, NULL), feerate_max(peer->ld, NULL), - peer->localfeatures, - local_feature_negotiated(peer->localfeatures, - LOCAL_STATIC_REMOTEKEY), + peer->features, + feature_negotiated(peer->features, + OPT_STATIC_REMOTEKEY), send_msg, IFDEV(peer->ld->dev_fast_gossip, false)); subd_send_msg(uc->openingd, take(msg)); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index afc602d5e..9224e8100 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -87,18 +87,11 @@ static void copy_to_parent_log(const char *prefix, log_(parent_log, level, false, "%s %s", prefix, str); } -static void peer_update_features(struct peer *peer, - const u8 *globalfeatures TAKES, - const u8 *localfeatures TAKES) +static void peer_update_features(struct peer *peer, const u8 *features TAKES) { - tal_free(peer->globalfeatures); - tal_free(peer->localfeatures); - peer->globalfeatures = tal_dup_arr(peer, u8, - globalfeatures, - tal_count(globalfeatures), 0); - peer->localfeatures = tal_dup_arr(peer, u8, - localfeatures, - tal_count(localfeatures), 0); + tal_free(peer->features); + peer->features = tal_dup_arr(peer, u8, + features, tal_count(features), 0); } struct peer *new_peer(struct lightningd *ld, u64 dbid, @@ -113,7 +106,7 @@ struct peer *new_peer(struct lightningd *ld, u64 dbid, peer->id = *id; peer->uncommitted_channel = NULL; peer->addr = *addr; - peer->globalfeatures = peer->localfeatures = NULL; + peer->features = NULL; list_head_init(&peer->channels); peer->direction = node_id_idx(&peer->ld->id, &peer->id); #if DEVELOPER @@ -797,8 +790,11 @@ peer_connected_serialize(struct peer_connected_hook_payload *payload, json_add_string( stream, "addr", type_to_string(stream, struct wireaddr_internal, &payload->addr)); - json_add_hex_talarr(stream, "globalfeatures", p->globalfeatures); - json_add_hex_talarr(stream, "localfeatures", p->localfeatures); + if (deprecated_apis) { + json_add_hex_talarr(stream, "globalfeatures", NULL); + json_add_hex_talarr(stream, "localfeatures", p->features); + } + json_add_hex_talarr(stream, "features", p->features); json_object_end(stream); /* .peer */ } @@ -926,7 +922,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, int peer_fd, int gossip_fd, int gossip_store_fd) { struct node_id id; - u8 *globalfeatures, *localfeatures; + u8 *features; struct peer *peer; struct peer_connected_hook_payload *hook_payload; @@ -935,7 +931,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, if (!fromwire_connect_peer_connected(hook_payload, msg, &id, &hook_payload->addr, &hook_payload->pps, - &globalfeatures, &localfeatures)) + &features)) fatal("Connectd gave bad CONNECT_PEER_CONNECTED message %s", tal_hex(msg, msg)); @@ -954,7 +950,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, tal_steal(peer, hook_payload); hook_payload->peer = peer; - peer_update_features(peer, globalfeatures, localfeatures); + peer_update_features(peer, features); /* Can't be opening, since we wouldn't have sent peer_disconnected. */ assert(!peer->uncommitted_channel); @@ -1119,10 +1115,12 @@ static void json_add_peer(struct lightningd *ld, struct wireaddr_internal, &p->addr)); json_array_end(response); - json_add_hex_talarr(response, "globalfeatures", - p->globalfeatures); - json_add_hex_talarr(response, "localfeatures", - p->localfeatures); + if (deprecated_apis) { + json_add_hex_talarr(response, "globalfeatures", NULL); + json_add_hex_talarr(response, "localfeatures", + p->features); + } + json_add_hex_talarr(response, "features", p->features); } json_array_start(response, "channels"); diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index a89e738de..ee678a802 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -43,7 +43,7 @@ struct peer { struct wireaddr_internal addr; /* We keep a copy of their feature bits */ - const u8 *localfeatures, *globalfeatures; + const u8 *features; /* If we open a channel our direction will be this */ u8 direction; diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 1cc794a4b..65fc2210e 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -89,7 +89,7 @@ void fatal(const char *fmt UNNEEDED, ...) bool fromwire_channel_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED) { fprintf(stderr, "fromwire_channel_dev_memleak_reply called!\n"); abort(); } /* Generated stub for fromwire_connect_peer_connected */ -bool fromwire_connect_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct node_id *id UNNEEDED, struct wireaddr_internal *addr UNNEEDED, struct per_peer_state **pps UNNEEDED, u8 **globalfeatures UNNEEDED, u8 **localfeatures UNNEEDED) +bool fromwire_connect_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct node_id *id UNNEEDED, struct wireaddr_internal *addr UNNEEDED, struct per_peer_state **pps UNNEEDED, u8 **features UNNEEDED) { fprintf(stderr, "fromwire_connect_peer_connected called!\n"); abort(); } /* Generated stub for fromwire_gossip_get_incoming_channels_reply */ bool fromwire_gossip_get_incoming_channels_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct route_info **route_info UNNEEDED) diff --git a/openingd/openingd.c b/openingd/openingd.c index efab07dd1..bca0c8f45 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -59,7 +59,7 @@ struct state { struct per_peer_state *pps; /* Features they offered */ - u8 *localfeatures; + u8 *features; /* Constraints on a channel they open. */ u32 minimum_depth; @@ -549,8 +549,8 @@ static u8 *funder_channel_start(struct state *state, * `payment_basepoint`, or `delayed_payment_basepoint` are not * valid DER-encoded compressed secp256k1 pubkeys. */ - if (local_feature_negotiated(state->localfeatures, - LOCAL_UPFRONT_SHUTDOWN_SCRIPT)) { + if (feature_negotiated(state->features, + OPT_UPFRONT_SHUTDOWN_SCRIPT)) { if (!fromwire_accept_channel_option_upfront_shutdown_script(state, msg, &id_in, &state->remoteconf.dust_limit, @@ -880,8 +880,8 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) * `payment_basepoint`, or `delayed_payment_basepoint` are not valid * DER-encoded compressed secp256k1 pubkeys. */ - if (local_feature_negotiated(state->localfeatures, - LOCAL_UPFRONT_SHUTDOWN_SCRIPT)) { + if (feature_negotiated(state->features, + OPT_UPFRONT_SHUTDOWN_SCRIPT)) { if (!fromwire_open_channel_option_upfront_shutdown_script(state, open_channel_msg, &chain_hash, &state->channel_id, @@ -1430,7 +1430,7 @@ int main(int argc, char *argv[]) &state->our_funding_pubkey, &state->minimum_depth, &state->min_feerate, &state->max_feerate, - &state->localfeatures, + &state->features, &state->option_static_remotekey, &inner, &dev_fast_gossip)) diff --git a/tests/test_connection.py b/tests/test_connection.py index d38c7efab..a3f0a879a 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1478,7 +1478,7 @@ def test_peerinfo(node_factory, bitcoind): # Gossiping but no node announcement yet assert l1.rpc.getpeer(l2.info['id'])['connected'] assert len(l1.rpc.getpeer(l2.info['id'])['channels']) == 0 - assert l1.rpc.getpeer(l2.info['id'])['localfeatures'] == lfeatures + assert l1.rpc.getpeer(l2.info['id'])['features'] == lfeatures # Fund a channel to force a node announcement chan = l1.fund_channel(l2, 10**6) @@ -1487,23 +1487,23 @@ def test_peerinfo(node_factory, bitcoind): l1.daemon.wait_for_logs(['Received node_announcement for node ' + l2.info['id']]) l2.daemon.wait_for_logs(['Received node_announcement for node ' + l1.info['id']]) - # Should have announced the same global features as told to peer. + # Should have announced the same features as told to peer. nodes1 = l1.rpc.listnodes(l2.info['id'])['nodes'] nodes2 = l2.rpc.listnodes(l2.info['id'])['nodes'] peer1 = l1.rpc.getpeer(l2.info['id']) peer2 = l2.rpc.getpeer(l1.info['id']) - assert only_one(nodes1)['globalfeatures'] == peer1['localfeatures'] - assert only_one(nodes2)['globalfeatures'] == peer2['localfeatures'] + assert only_one(nodes1)['features'] == peer1['features'] + assert only_one(nodes2)['features'] == peer2['features'] - assert l1.rpc.getpeer(l2.info['id'])['localfeatures'] == lfeatures - assert l2.rpc.getpeer(l1.info['id'])['localfeatures'] == lfeatures + assert l1.rpc.getpeer(l2.info['id'])['features'] == lfeatures + assert l2.rpc.getpeer(l1.info['id'])['features'] == lfeatures # If it reconnects after db load, it should know features. l1.restart() wait_for(lambda: l1.rpc.getpeer(l2.info['id'])['connected']) wait_for(lambda: l2.rpc.getpeer(l1.info['id'])['connected']) - assert l1.rpc.getpeer(l2.info['id'])['localfeatures'] == lfeatures - assert l2.rpc.getpeer(l1.info['id'])['localfeatures'] == lfeatures + assert l1.rpc.getpeer(l2.info['id'])['features'] == lfeatures + assert l2.rpc.getpeer(l1.info['id'])['features'] == lfeatures # Close the channel to forget the peer l1.rpc.close(chan) @@ -1731,8 +1731,9 @@ def test_dataloss_protection(node_factory, bitcoind): l1.rpc.connect(l2.info['id'], 'localhost', l2.port) # l1 should send out WIRE_INIT (0010) l1.daemon.wait_for_log(r"\[OUT\] 0010" - # gflen == 0 - "0000" + # gflen + + format(len(lf) // 2, '04x') + + lf # lflen + format(len(lf) // 2, '04x') + lf) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 1a761d8cf..68b241fd0 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -1034,7 +1034,7 @@ def test_node_reannounce(node_factory, bitcoind): lfeatures = '28a2' # Make sure it gets features correct. - assert only_one(l2.rpc.listnodes(l1.info['id'])['nodes'])['globalfeatures'] == lfeatures + assert only_one(l2.rpc.listnodes(l1.info['id'])['nodes'])['features'] == lfeatures l1.stop() l1.daemon.opts['alias'] = 'SENIORBEAM' diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 40ee8edec..17978d43b 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -106,7 +106,7 @@ bool fromwire_channel_offer_htlc_reply(const tal_t *ctx UNNEEDED, const void *p bool fromwire_channel_sending_commitsig(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u64 *commitnum UNNEEDED, u32 *feerate UNNEEDED, struct changed_htlc **changed UNNEEDED, struct bitcoin_signature *commit_sig UNNEEDED, secp256k1_ecdsa_signature **htlc_sigs UNNEEDED) { fprintf(stderr, "fromwire_channel_sending_commitsig called!\n"); abort(); } /* Generated stub for fromwire_connect_peer_connected */ -bool fromwire_connect_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct node_id *id UNNEEDED, struct wireaddr_internal *addr UNNEEDED, struct per_peer_state **pps UNNEEDED, u8 **globalfeatures UNNEEDED, u8 **localfeatures UNNEEDED) +bool fromwire_connect_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct node_id *id UNNEEDED, struct wireaddr_internal *addr UNNEEDED, struct per_peer_state **pps UNNEEDED, u8 **features UNNEEDED) { fprintf(stderr, "fromwire_connect_peer_connected called!\n"); abort(); } /* Generated stub for fromwire_gossip_get_channel_peer_reply */ bool fromwire_gossip_get_channel_peer_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct node_id **peer_id UNNEEDED)