diff --git a/common/features.c b/common/features.c index 29f42ae45..558b5dade 100644 --- a/common/features.c +++ b/common/features.c @@ -15,6 +15,72 @@ static const u32 our_features[] = { OPTIONAL_FEATURE(OPT_STATIC_REMOTEKEY), }; +enum feature_copy_style { + /* Feature is not exposed (importantly, being 0, this is the default!). */ + FEATURE_DONT_REPRESENT, + /* Feature is exposed. */ + FEATURE_REPRESENT, + /* Feature is exposed, but always optional. */ + FEATURE_REPRESENT_AS_OPTIONAL, +}; + +enum feature_place { + INIT_FEATURE, + GLOBAL_INIT_FEATURE, + NODE_ANNOUNCE_FEATURE, + BOLT11_FEATURE, +}; +#define NUM_FEATURE_PLACE (BOLT11_FEATURE+1) + +struct feature_style { + u32 bit; + enum feature_copy_style copy_style[NUM_FEATURE_PLACE]; +}; + +static const struct feature_style feature_styles[] = { + { OPT_DATA_LOSS_PROTECT, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, + [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT } }, + { OPT_INITIAL_ROUTING_SYNC, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT_AS_OPTIONAL, + [NODE_ANNOUNCE_FEATURE] = FEATURE_DONT_REPRESENT } }, + { OPT_UPFRONT_SHUTDOWN_SCRIPT, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, + [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT } }, + { OPT_GOSSIP_QUERIES, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, + [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT } }, + { OPT_GOSSIP_QUERIES_EX, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, + [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT } }, + { OPT_VAR_ONION, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, + [GLOBAL_INIT_FEATURE] = FEATURE_REPRESENT, + [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT, + [BOLT11_FEATURE] = FEATURE_REPRESENT } }, + { OPT_STATIC_REMOTEKEY, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, + [GLOBAL_INIT_FEATURE] = FEATURE_REPRESENT, + [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT } }, + { OPT_PAYMENT_SECRET, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, + [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT, + [BOLT11_FEATURE] = FEATURE_REPRESENT } }, + { OPT_BASIC_MPP, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, + [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT, + [BOLT11_FEATURE] = FEATURE_REPRESENT } }, +}; + +static enum feature_copy_style feature_copy_style(u32 f, enum feature_place p) +{ + for (size_t i = 0; i < ARRAY_SIZE(feature_styles); i++) { + if (feature_styles[i].bit == COMPULSORY_FEATURE(f)) + return feature_styles[i].copy_style[p]; + } + abort(); +} + /* BOLT #1: * * All data fields are unsigned big-endian unless otherwise specified. @@ -39,31 +105,44 @@ static bool test_bit(const u8 *features, size_t byte, unsigned int bit) return features[tal_count(features) - 1 - byte] & (1 << (bit % 8)); } -static u8 *mkfeatures(const tal_t *ctx, const u32 *arr, size_t n) +static u8 *mkfeatures(const tal_t *ctx, enum feature_place place) { u8 *f = tal_arr(ctx, u8, 0); - for (size_t i = 0; i < n; i++) - set_feature_bit(&f, arr[i]); + for (size_t i = 0; i < ARRAY_SIZE(our_features); i++) { + switch (feature_copy_style(our_features[i], place)) { + case FEATURE_DONT_REPRESENT: + continue; + case FEATURE_REPRESENT: + set_feature_bit(&f, our_features[i]); + continue; + case FEATURE_REPRESENT_AS_OPTIONAL: + set_feature_bit(&f, OPTIONAL_FEATURE(our_features[i])); + continue; + } + abort(); + } return f; } -/* 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). - * - * FIXME: Add bolt ref when finalized! - */ u8 *get_offered_nodefeatures(const tal_t *ctx) { - return mkfeatures(ctx, - our_features, ARRAY_SIZE(our_features)); + return mkfeatures(ctx, NODE_ANNOUNCE_FEATURE); +} + +u8 *get_offered_initfeatures(const tal_t *ctx) +{ + return mkfeatures(ctx, INIT_FEATURE); +} + +u8 *get_offered_globalinitfeatures(const tal_t *ctx) +{ + return mkfeatures(ctx, GLOBAL_INIT_FEATURE); } -u8 *get_offered_features(const tal_t *ctx) +u8 *get_offered_bolt11features(const tal_t *ctx) { - return mkfeatures(ctx, - our_features, ARRAY_SIZE(our_features)); + return mkfeatures(ctx, BOLT11_FEATURE); } bool feature_is_set(const u8 *features, size_t bit) @@ -155,7 +234,10 @@ static const char *feature_name(const tal_t *ctx, size_t f) "option_gossip_queries", "option_var_onion_optin", "option_gossip_queries_ex", - "option_static_remotekey" }; + "option_static_remotekey", + "option_payment_secret", + "option_basic_mpp", + }; assert(f / 2 < ARRAY_SIZE(fnames)); return tal_fmt(ctx, "%s/%s", diff --git a/common/features.h b/common/features.h index bcb8ccf4c..cb8150350 100644 --- a/common/features.h +++ b/common/features.h @@ -9,8 +9,10 @@ int features_unsupported(const u8 *features); /* For sending our features: tal_count() returns length. */ -u8 *get_offered_features(const tal_t *ctx); +u8 *get_offered_initfeatures(const tal_t *ctx); +u8 *get_offered_globalinitfeatures(const tal_t *ctx); u8 *get_offered_nodefeatures(const tal_t *ctx); +u8 *get_offered_bolt11features(const tal_t *ctx); /* Is this feature bit requested? (Either compulsory or optional) */ bool feature_offered(const u8 *features, size_t f); @@ -55,6 +57,14 @@ void set_feature_bit(u8 **ptr, u32 bit); #define OPT_GOSSIP_QUERIES_EX 10 #define OPT_STATIC_REMOTEKEY 12 +/* BOLT-3a09bc54f8443c4757b47541a5310aff6377ee21 #9: + * + * | 14/15 | `payment_secret` |... IN9 ... + * | 16/17 | `basic_mpp` |... IN9 ... + */ +#define OPT_PAYMENT_SECRET 14 +#define OPT_BASIC_MPP 16 + /* BOLT #9: * * ## Assigned `globalfeatures` flags diff --git a/common/test/run-features.c b/common/test/run-features.c index fccbef750..bb1a1644c 100644 --- a/common/test/run-features.c +++ b/common/test/run-features.c @@ -87,7 +87,7 @@ int main(void) assert(features_unsupported(bits) == -1); /* We must support our own features. */ - lf = get_offered_features(tmpctx); + lf = get_offered_initfeatures(tmpctx); assert(features_unsupported(lf) == -1); /* We can add random odd features, no problem. */ diff --git a/connectd/peer_exchange_initmsg.c b/connectd/peer_exchange_initmsg.c index 014c6b064..514613ebe 100644 --- a/connectd/peer_exchange_initmsg.c +++ b/connectd/peer_exchange_initmsg.c @@ -35,6 +35,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn, { u8 *msg = cryptomsg_decrypt_body(tmpctx, &peer->cs, peer->msg); u8 *globalfeatures, *features; + int unsup; if (!msg) return io_close(conn); @@ -73,12 +74,10 @@ 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_unsupported(features) != -1) { - 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)); + unsup = features_unsupported(features); + if (unsup != -1) { + msg = towire_errorfmt(NULL, NULL, "Unsupported feature %u", + unsup); msg = cryptomsg_encrypt_msg(NULL, &peer->cs, take(msg)); return io_write(conn, msg, tal_count(msg), io_close_cb, NULL); } @@ -157,17 +156,18 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn, * 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. + * but debate still raged 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. */ + * bit here. + * + * Finally, we agreed that bits below 13 could be put in both, but + * from now on they'll all go in initfeatures. */ peer->msg = towire_init(NULL, - /* Features so nice, we send it twice! */ - get_offered_features(tmpctx), - get_offered_features(tmpctx)); + get_offered_globalinitfeatures(tmpctx), + get_offered_initfeatures(tmpctx)); status_peer_io(LOG_IO_OUT, &peer->id, peer->msg); peer->msg = cryptomsg_encrypt_msg(peer, &peer->cs, take(peer->msg)); diff --git a/tests/test_connection.py b/tests/test_connection.py index 9abc6e492..c5af38e5a 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1835,10 +1835,7 @@ 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 - + format(len(lf) // 2, '04x') - + lf + l1.daemon.wait_for_log(r"\[OUT\] 0010.*" # lflen + format(len(lf) // 2, '04x') + lf)