common/features: use bitmaps internally, have explicit init function.

This is to prepare for dynamic features, including making plugins first
class citizens at setting them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/features.c

@@ -1,19 +1,39 @@
 #include "features.h"
 #include <assert.h>
 #include <ccan/array_size/array_size.h>
+#include <common/memleak.h>
 #include <common/utils.h>
 #include <wire/peer_wire.h>
 
-static const u32 our_features[] = {
+/* We keep a map of our features for each context, with the assumption that
-	OPTIONAL_FEATURE(OPT_DATA_LOSS_PROTECT),
+ * the init features is a superset of the others. */
-	OPTIONAL_FEATURE(OPT_UPFRONT_SHUTDOWN_SCRIPT),
+static struct feature_set *our_features;
-	OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES),
+
-	OPTIONAL_FEATURE(OPT_VAR_ONION),
+/* FIXME: Remove once all subdaemons call features_init() */
-	OPTIONAL_FEATURE(OPT_PAYMENT_SECRET),
+static const u8 *our_feature_bits(enum feature_place place)
-	OPTIONAL_FEATURE(OPT_BASIC_MPP),
+{
-	OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES_EX),
+	if (!our_features) {
-	OPTIONAL_FEATURE(OPT_STATIC_REMOTEKEY),
+		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_VAR_ONION),
+			OPTIONAL_FEATURE(OPT_PAYMENT_SECRET),
+			OPTIONAL_FEATURE(OPT_BASIC_MPP),
+			OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES_EX),
+			OPTIONAL_FEATURE(OPT_STATIC_REMOTEKEY),
+		};
+		u8 *f = tal_arr(NULL, u8, 0);
+
+		for (size_t i = 0; i < ARRAY_SIZE(our_features); i++)
+			set_feature_bit(&f, our_features[i]);
+
+		features_core_init(take(f));
+	}
+
+	/* This is currently always a superset of other bits */
+	return our_features->bits[place];
+}
 
 enum feature_copy_style {
 	/* Feature is not exposed (importantly, being 0, this is the default!). */
@@ -24,15 +44,6 @@ enum feature_copy_style {
 	FEATURE_REPRESENT_AS_OPTIONAL,
 };
 
-enum feature_place {
-	INIT_FEATURE,
-	GLOBAL_INIT_FEATURE,
-	NODE_ANNOUNCE_FEATURE,
-	CHANNEL_FEATURE,
-	BOLT11_FEATURE,
-};
-#define NUM_FEATURE_PLACE (BOLT11_FEATURE+1)
-
 struct feature_style {
 	u32 bit;
 	enum feature_copy_style copy_style[NUM_FEATURE_PLACE];
@@ -82,6 +93,51 @@ static enum feature_copy_style feature_copy_style(u32 f, enum feature_place p)
 	abort();
 }
 
+static u8 *mkfeatures(const tal_t *ctx, enum feature_place place)
+{
+	u8 *f = tal_arr(ctx, u8, 0);
+	const u8 *base = our_features->bits[INIT_FEATURE];
+
+	assert(place != INIT_FEATURE);
+	for (size_t i = 0; i < tal_bytelen(base)*8; i++) {
+		if (!feature_is_set(base, i))
+			continue;
+
+		switch (feature_copy_style(i, place)) {
+		case FEATURE_DONT_REPRESENT:
+			continue;
+		case FEATURE_REPRESENT:
+			set_feature_bit(&f, i);
+			continue;
+		case FEATURE_REPRESENT_AS_OPTIONAL:
+			set_feature_bit(&f, OPTIONAL_FEATURE(i));
+			continue;
+		}
+		abort();
+	}
+	return f;
+}
+
+struct feature_set *features_core_init(const u8 *feature_bits)
+{
+	assert(!our_features);
+	our_features = notleak(tal(NULL, struct feature_set));
+
+	our_features->bits[INIT_FEATURE]
+		= tal_dup_talarr(our_features, u8, feature_bits);
+
+	/* Make other masks too */
+	for (enum feature_place f = INIT_FEATURE+1; f < NUM_FEATURE_PLACE; f++)
+		our_features->bits[f] = mkfeatures(our_features, f);
+
+	return our_features;
+}
+
+void features_cleanup(void)
+{
+	our_features = tal_free(our_features);
+}
+
 /* BOLT #1:
  *
  * All data fields are unsigned big-endian unless otherwise specified.
@@ -106,39 +162,19 @@ 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, enum feature_place place)
-{
-	u8 *f = tal_arr(ctx, u8, 0);
-
-	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;
-}
-
 u8 *get_offered_nodefeatures(const tal_t *ctx)
 {
-	return mkfeatures(ctx, NODE_ANNOUNCE_FEATURE);
+	return tal_dup_talarr(ctx, u8, our_feature_bits(NODE_ANNOUNCE_FEATURE));
 }
 
 u8 *get_offered_initfeatures(const tal_t *ctx)
 {
-	return mkfeatures(ctx, INIT_FEATURE);
+	return tal_dup_talarr(ctx, u8, our_feature_bits(INIT_FEATURE));
 }
 
 u8 *get_offered_globalinitfeatures(const tal_t *ctx)
 {
-	return mkfeatures(ctx, GLOBAL_INIT_FEATURE);
+	return tal_dup_talarr(ctx, u8, our_feature_bits(GLOBAL_INIT_FEATURE));
 }
 
 static void clear_feature_bit(u8 *features, u32 bit)
@@ -159,7 +195,7 @@ static void clear_feature_bit(u8 *features, u32 bit)
  */
 u8 *get_agreed_channelfeatures(const tal_t *ctx, const u8 *theirfeatures)
 {
-	u8 *f = mkfeatures(ctx, CHANNEL_FEATURE);
+	u8 *f = tal_dup_talarr(ctx, u8, our_feature_bits(CHANNEL_FEATURE));
 	size_t max_len = 0;
 
 	/* Clear any features which they didn't offer too */
@@ -181,7 +217,7 @@ u8 *get_agreed_channelfeatures(const tal_t *ctx, const u8 *theirfeatures)
 
 u8 *get_offered_bolt11features(const tal_t *ctx)
 {
-	return mkfeatures(ctx, BOLT11_FEATURE);
+	return tal_dup_talarr(ctx, u8, our_feature_bits(BOLT11_FEATURE));
 }
 
 bool feature_is_set(const u8 *features, size_t bit)
@@ -200,23 +236,10 @@ bool feature_offered(const u8 *features, size_t f)
 		|| feature_is_set(features, OPTIONAL_FEATURE(f));
 }
 
-static bool feature_supported(int feature_bit,
-			      const u32 *supported,
-			      size_t num_supported)
-{
-	for (size_t i = 0; i < num_supported; i++) {
-		if (OPTIONAL_FEATURE(supported[i])
-		    == OPTIONAL_FEATURE(feature_bit))
-			return true;
-	}
-	return false;
-}
-
 bool feature_negotiated(const u8 *lfeatures, size_t f)
 {
-	if (!feature_offered(lfeatures, f))
+	return feature_offered(lfeatures, f)
-		return false;
+		&& feature_offered(our_feature_bits(INIT_FEATURE), f);
-	return feature_supported(f, our_features, ARRAY_SIZE(our_features));
 }
 
 /**
@@ -227,14 +250,10 @@ bool feature_negotiated(const u8 *lfeatures, size_t f)
  * the required features.
  *
  * @bitmap: the features bitmap the peer is asking for
- * @supported: array of features we support
- * @num_supported: how many elements in supported
  *
  * Returns -1 on success, or first unsupported feature.
  */
-static int all_supported_features(const u8 *bitmap,
+static int all_supported_features(const u8 *bitmap)
-				   const u32 *supported,
-				   size_t num_supported)
 {
 	size_t len = tal_count(bitmap) * 8;
 
@@ -243,7 +262,7 @@ static int all_supported_features(const u8 *bitmap,
 		if (!test_bit(bitmap, bitnum/8, bitnum%8))
 			continue;
 
-		if (feature_supported(bitnum, supported, num_supported))
+		if (feature_offered(our_feature_bits(INIT_FEATURE), bitnum))
 			continue;
 
 		return bitnum;
@@ -259,9 +278,7 @@ int features_unsupported(const u8 *features)
 			   COMPULSORY_FEATURE(OPT_INITIAL_ROUTING_SYNC)))
 		return COMPULSORY_FEATURE(OPT_INITIAL_ROUTING_SYNC);
 
-	return all_supported_features(features,
+	return all_supported_features(features);
-				      our_features,
-				      ARRAY_SIZE(our_features));
 }
 
 static const char *feature_name(const tal_t *ctx, size_t f)
@@ -278,7 +295,10 @@ static const char *feature_name(const tal_t *ctx, size_t f)
 		"option_basic_mpp",
 	};
 
-	assert(f / 2 < ARRAY_SIZE(fnames));
+	if (f / 2 >= ARRAY_SIZE(fnames))
+		return tal_fmt(ctx, "option_unknown_%zu/%s",
+			       COMPULSORY_FEATURE(f), (f & 1) ? "odd" : "even");
+
 	return tal_fmt(ctx, "%s/%s",
 		       fnames[f / 2], (f & 1) ? "odd" : "even");
 }
@@ -287,8 +307,10 @@ const char **list_supported_features(const tal_t *ctx)
 {
 	const char **list = tal_arr(ctx, const char *, 0);
 
-	for (size_t i = 0; i < ARRAY_SIZE(our_features); i++)
+	for (size_t i = 0; i < tal_bytelen(our_feature_bits(INIT_FEATURE)) * 8; i++) {
-		tal_arr_expand(&list, feature_name(list, our_features[i]));
+		if (test_bit(our_feature_bits(INIT_FEATURE), i / 8, i % 8))
+			tal_arr_expand(&list, feature_name(list, i));
+	}
 
 	return list;
 }

common/features.h

@@ -4,6 +4,26 @@
 #include <ccan/short_types/short_types.h>
 #include <ccan/tal/tal.h>
 
+enum feature_place {
+	INIT_FEATURE,
+	GLOBAL_INIT_FEATURE,
+	NODE_ANNOUNCE_FEATURE,
+	CHANNEL_FEATURE,
+	BOLT11_FEATURE,
+};
+#define NUM_FEATURE_PLACE (BOLT11_FEATURE+1)
+
+/* The complete set of features for all contexts */
+struct feature_set {
+	u8 *bits[NUM_FEATURE_PLACE];
+};
+
+/* Initialize core features (for lightningd). */
+struct feature_set *features_core_init(const u8 *features TAKES);
+
+/* Free feature allocations */
+void features_cleanup(void);
+
 /* Returns -1 if we're OK with all these offered features, otherwise first
  * unsupported (even) feature. */
 int features_unsupported(const u8 *features);

common/test/run-bolt11.c

@@ -5,6 +5,7 @@
 #include "../features.c"
 #include "../node_id.c"
 #include "../hash_u5.c"
+#include "../memleak.c"
 #include "../wire/fromwire.c"
 #include "../wire/towire.c"
 #include <ccan/err/err.h>
@@ -148,6 +149,7 @@ int main(void)
 	wally_init(0);
 	secp256k1_ctx = wally_get_secp_context();
 	setup_tmpctx();
+	features_core_init(NULL);
 
 	/* BOLT #11:
 	 *
@@ -572,5 +574,6 @@ int main(void)
 	/* FIXME: Test the others! */
 	wally_cleanup(0);
 	tal_free(tmpctx);
+	features_cleanup();
 	return 0;
 }

common/test/run-features.c

@@ -1,4 +1,5 @@
 #include "../features.c"
+#include "../memleak.c"
 #include <ccan/err/err.h>
 #include <ccan/mem/mem.h>
 #include <ccan/str/hex/hex.h>
@@ -51,14 +52,32 @@ static void test_featurebits_or(void)
 	    memeq(result, tal_bytelen(result), control, tal_bytelen(control)));
 }
 
+static void setup_features(void)
+{
+	static const u32 default_features[] = {
+		OPTIONAL_FEATURE(OPT_DATA_LOSS_PROTECT),
+		OPTIONAL_FEATURE(OPT_UPFRONT_SHUTDOWN_SCRIPT),
+		OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES),
+		OPTIONAL_FEATURE(OPT_VAR_ONION),
+		OPTIONAL_FEATURE(OPT_PAYMENT_SECRET),
+		OPTIONAL_FEATURE(OPT_BASIC_MPP),
+		OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES_EX),
+		OPTIONAL_FEATURE(OPT_STATIC_REMOTEKEY),
+	};
+	u8 *f = tal_arr(NULL, u8, 0);
+	for (size_t i = 0; i < ARRAY_SIZE(default_features); i++)
+		set_feature_bit(&f, default_features[i]);
+	features_core_init(take(f));
+}
+
 int main(void)
 {
 	u8 *bits, *lf;
-
 	setup_locale();
 	wally_init(0);
 	secp256k1_ctx = wally_get_secp_context();
 	setup_tmpctx();
+	setup_features();
 
 	bits = tal_arr(tmpctx, u8, 0);
 	for (size_t i = 0; i < 100; i += 3)
@@ -127,8 +146,7 @@ int main(void)
 			assert(features_unsupported(bits) == i);
 		} else {
 			assert((features_unsupported(bits) == -1)
-			       == feature_supported(i, our_features,
+			       == feature_offered(our_features->bits[INIT_FEATURE], i));
-						    ARRAY_SIZE(our_features)));
 		}
 	}
 
@@ -137,5 +155,6 @@ int main(void)
 	wally_cleanup(0);
 	tal_free(tmpctx);
 	take_cleanup();
+	features_cleanup();
 	return 0;
 }

connectd/test/run-initiator-success.c

@@ -6,6 +6,7 @@
 #include <unistd.h>
 #include <ccan/err/err.h>
 #include <ccan/io/io.h>
+#include <common/memleak.h>
 #include <common/status.h>
 #include <wire/wire.h>
 
@@ -32,6 +33,9 @@ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED)
 /* Generated stub for fromwire_fail */
 const void *fromwire_fail(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
 { fprintf(stderr, "fromwire_fail called!\n"); abort(); }
+/* Generated stub for notleak_ */
+void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
+{ fprintf(stderr, "notleak_ called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
 /* No randomness please, we want to replicate test vectors. */

connectd/test/run-responder-success.c

@@ -6,6 +6,7 @@
 #include <unistd.h>
 #include <ccan/err/err.h>
 #include <ccan/io/io.h>
+#include <common/memleak.h>
 #include <common/status.h>
 #include <wire/wire.h>
 
@@ -32,6 +33,9 @@ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED)
 /* Generated stub for fromwire_fail */
 const void *fromwire_fail(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
 { fprintf(stderr, "fromwire_fail called!\n"); abort(); }
+/* Generated stub for notleak_ */
+void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
+{ fprintf(stderr, "notleak_ called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
 /* No randomness please, we want to replicate test vectors. */

gossipd/test/run-extended-info.c

@@ -7,6 +7,7 @@
 #include <common/json.h>
 #include <common/json_helpers.h>
 #include <common/json_stream.h>
+#include <common/memleak.h>
 #include <stdio.h>
 
 #ifdef NDEBUG
@@ -77,6 +78,9 @@ void json_object_start(struct json_stream *ks UNNEEDED, const char *fieldname UN
 /* Generated stub for master_badmsg */
 void master_badmsg(u32 type_expected UNNEEDED, const u8 *msg)
 { fprintf(stderr, "master_badmsg called!\n"); abort(); }
+/* Generated stub for notleak_ */
+void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
+{ fprintf(stderr, "notleak_ called!\n"); abort(); }
 /* Generated stub for peer_supplied_good_gossip */
 void peer_supplied_good_gossip(struct peer *peer UNNEEDED, size_t amount UNNEEDED)
 { fprintf(stderr, "peer_supplied_good_gossip called!\n"); abort(); }

gossipd/test/run-next_block_range.c

@@ -1,6 +1,7 @@
 #include "../seeker.c"
 #include <ccan/err/err.h>
 #include <common/json_stream.h>
+#include <common/memleak.h>
 #include <common/wireaddr.h>
 #include <stdarg.h>
 #include <stdio.h>
@@ -37,6 +38,9 @@ struct oneshot *new_reltimer_(struct timers *timers UNNEEDED,
 			      struct timerel expire UNNEEDED,
 			      void (*cb)(void *) UNNEEDED, void *arg UNNEEDED)
 { fprintf(stderr, "new_reltimer_ called!\n"); abort(); }
+/* Generated stub for notleak_ */
+void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
+{ fprintf(stderr, "notleak_ called!\n"); abort(); }
 /* Generated stub for query_channel_range */
 bool query_channel_range(struct daemon *daemon UNNEEDED,
 			 struct peer *peer UNNEEDED,

lightningd/lightningd.c

@@ -57,6 +57,7 @@
 /*~ This is common code: routines shared by one or more executables
  *  (separate daemons, or the lightning-cli program). */
 #include <common/daemon.h>
+#include <common/features.h>
 #include <common/memleak.h>
 #include <common/timeout.h>
 #include <common/utils.h>

lightningd/options.c

@@ -1004,11 +1004,34 @@ void setup_color_and_alias(struct lightningd *ld)
 	}
 }
 
+static void setup_default_features(void)
+{
+	static const u32 default_features[] = {
+		OPTIONAL_FEATURE(OPT_DATA_LOSS_PROTECT),
+		OPTIONAL_FEATURE(OPT_UPFRONT_SHUTDOWN_SCRIPT),
+		OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES),
+		OPTIONAL_FEATURE(OPT_VAR_ONION),
+		OPTIONAL_FEATURE(OPT_PAYMENT_SECRET),
+		OPTIONAL_FEATURE(OPT_BASIC_MPP),
+		OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES_EX),
+		OPTIONAL_FEATURE(OPT_STATIC_REMOTEKEY),
+	};
+	u8 *f = tal_arr(NULL, u8, 0);
+
+	for (size_t i = 0; i < ARRAY_SIZE(default_features); i++)
+		set_feature_bit(&f, default_features[i]);
+
+	features_core_init(take(f));
+}
+
 void handle_early_opts(struct lightningd *ld, int argc, char *argv[])
 {
 	/* Make ccan/opt use tal for allocations */
 	setup_option_allocators();
 
+	/* Make sure options are populated. */
+	setup_default_features();
+
 	/*~ List features immediately, before doing anything interesting */
 	opt_register_early_noarg("--list-features-only",
 				 list_features_and_exit,

tests/test_misc.py

@@ -1770,10 +1770,10 @@ def test_list_features_only(node_factory):
                 'option_upfront_shutdown_script/odd',
                 'option_gossip_queries/odd',
                 'option_var_onion_optin/odd',
-                'option_payment_secret/odd',
-                'option_basic_mpp/odd',
                 'option_gossip_queries_ex/odd',
                 'option_static_remotekey/odd',
+                'option_payment_secret/odd',
+                'option_basic_mpp/odd',
                 ]
     assert features == expected