plugins: actually change global features when plugins tell us to.

This cleans up the boutique handling of features, and importantly, it
means that if a plugin says to offer a feature in init, we will now
*accept* that feature.

Changelog-Fixed: Plugins: setting an 'init' feature bit allows us to accept it from peers.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/features.c

@@ -126,6 +126,28 @@ void features_cleanup(void)
 	our_features = tal_free(our_features);
 }
 
+bool features_additional(const struct feature_set *newfset)
+{
+	/* Check first, before we change anything! */
+	for (size_t i = 0; i < ARRAY_SIZE(newfset->bits); i++) {
+		/* FIXME: We could allow a plugin to upgrade an optional feature
+		 * to a compulsory one? */
+		for (size_t b = 0; b < tal_bytelen(newfset->bits[i])*8; b++) {
+			if (feature_is_set(newfset->bits[i], b)
+			    && feature_is_set(our_features->bits[i], b))
+				return false;
+		}
+	}
+
+	for (size_t i = 0; i < ARRAY_SIZE(newfset->bits); i++) {
+		for (size_t b = 0; b < tal_bytelen(newfset->bits[i])*8; b++) {
+			if (feature_is_set(newfset->bits[i], b))
+				set_feature_bit(&our_features->bits[i], b);
+		}
+	}
+	return true;
+}
+
 /* BOLT #1:
  *
  * All data fields are unsigned big-endian unless otherwise specified.

common/features.h

@@ -31,6 +31,9 @@ struct feature_set *fromwire_feature_set(const tal_t *ctx,
 					 const u8 **ptr, size_t *max);
 void towire_feature_set(u8 **pptr, const struct feature_set *fset);
 
+/* Add features supplied by a plugin: returns false if we already have them */
+bool features_additional(const struct feature_set *feature_set);
+
 /* Returns -1 if we're OK with all these offered features, otherwise first
  * unsupported (even) feature. */
 int features_unsupported(const u8 *features);

doc/PLUGINS.md

@@ -94,6 +94,7 @@ this example:
   ],
   "features": {
     "node": "D0000000",
+    "channel": "D0000000",
     "init": "0E000000",
     "invoice": "00AD0000"
   },
@@ -126,7 +127,7 @@ invoices. Custom protocol extensions can be implemented for example using the
 the `htlc_accepted` hook. The keys in the `features` object are `node` for
 features that should be announced via the `node_announcement` to all nodes in
 the network, `init` for features that should be announced to direct peers
-during the connection setup, and `invoice` for features that should be
+during the connection setup, `channel` for features which should apply to `channel_announcement`, and `invoice` for features that should be
 announced to a potential sender of a payment in the invoice. The low range of
 featurebits is reserved for standardize features, so please pick random, high
 position bits for experiments. If you'd like to standardize your extension

lightningd/plugin.c

@@ -869,7 +869,11 @@ static void plugin_manifest_timeout(struct plugin *plugin)
 }
 
 /* List of JSON keys matching `plugin_features_type`. */
-static const char *plugin_features_type_names[] = {"node", "init", "invoice"};
+static const char *plugin_features_type_names[] = {"node", "init", "invoice", "channel"};
+static const size_t plugin_features_fset[] = {NODE_ANNOUNCE_FEATURE,
+					      INIT_FEATURE,
+					      BOLT11_FEATURE,
+					      CHANNEL_FEATURE};
 
 bool plugin_parse_getmanifest_response(const char *buffer,
                                        const jsmntok_t *toks,
@@ -877,7 +881,6 @@ bool plugin_parse_getmanifest_response(const char *buffer,
                                        struct plugin *plugin)
 {
 	const jsmntok_t *resulttok, *dynamictok, *featurestok, *tok;
-	bool have_featurebits = false;
 	u8 *featurebits;
 
 	resulttok = json_get_member(buffer, toks, "result");
@@ -893,6 +896,9 @@ bool plugin_parse_getmanifest_response(const char *buffer,
 	featurestok = json_get_member(buffer, resulttok, "featurebits");
 
 	if (featurestok) {
+		bool have_featurebits = false;
+		struct feature_set *fset = talz(tmpctx, struct feature_set);
+
 		for (int i = 0; i < NUM_PLUGIN_FEATURES_TYPE; i++) {
 			tok = json_get_member(buffer, featurestok,
 					      plugin_features_type_names[i]);
@@ -915,17 +921,24 @@ bool plugin_parse_getmanifest_response(const char *buffer,
 				    tok->end - tok->start, buffer + tok->start);
 				return true;
 			}
+			fset->bits[plugin_features_fset[i]] = featurebits;
 		}
-	}
 
-	if (plugin->dynamic && have_featurebits) {
+		if (plugin->dynamic && have_featurebits) {
-		plugin_kill(plugin,
+			plugin_kill(plugin,
-			    "Custom featurebits only allows for non-dynamic "
+				    "Custom featurebits only allows for non-dynamic "
-			    "plugins: dynamic=%d, featurebits=%.*s",
+				    "plugins: dynamic=%d, featurebits=%.*s",
-			    plugin->dynamic,
+				    plugin->dynamic,
-			    featurestok->end - featurestok->start,
+				    featurestok->end - featurestok->start,
-			    buffer + featurestok->start);
+				    buffer + featurestok->start);
-		return true;
+			return true;
+		}
+
+		if (!features_additional(fset)) {
+			plugin_kill(plugin,
+				    "Custom featurebits already present");
+			return true;
+		}
 	}
 
 	if (!plugin_opts_add(plugin, buffer, resulttok) ||

lightningd/plugin.h

@@ -39,6 +39,7 @@ enum plugin_features_type {
 	PLUGIN_FEATURES_NODE,
 	PLUGIN_FEATURES_INIT,
 	PLUGIN_FEATURES_INVOICE,
+	PLUGIN_FEATURES_CHANNEL,
 };
 #define NUM_PLUGIN_FEATURES_TYPE (PLUGIN_FEATURES_INVOICE+1)