common/features: only support a single feature bitset.

CHANGELOG.md

@@ -39,6 +39,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).

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,

common/features.c

@@ -4,15 +4,12 @@
 #include <common/utils.h>
 #include <wire/peer_wire.h>
 
-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;
 }

common/features.h

@@ -5,19 +5,17 @@
 #include <ccan/tal/tal.h>
 
 /* 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 */

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);

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