Plugins can deprecate too!

contrib/plugins/fail/failtimeout.py

@@ -8,7 +8,7 @@
 import time
 
 
-def json_getmanifest(request):
+def json_getmanifest(request, **kwargs):
     # Timeout is 60 seconds, so wait more
     time.sleep(61)
     return {

contrib/pyln-client/pyln/client/plugin.py

@@ -33,14 +33,15 @@ class Method(object):
      - HOOK registered to be called synchronously by lightningd
     """
     def __init__(self, name, func, mtype=MethodType.RPCMETHOD, category=None,
-                 desc=None, long_desc=None):
+                 desc=None, long_desc=None, deprecated=False):
         self.name = name
         self.func = func
         self.mtype = mtype
         self.category = category
         self.background = False
         self.desc = desc
         self.long_desc = long_desc
+        self.deprecated = deprecated
 
 
 class Request(dict):
@@ -161,7 +162,7 @@ def convert_featurebits(bits):
         self.write_lock = RLock()
 
     def add_method(self, name, func, background=False, category=None, desc=None,
-                   long_desc=None):
+                   long_desc=None, deprecated=False):
         """Add a plugin method to the dispatch table.
 
         The function will be expected at call time (see `_dispatch`)
@@ -191,14 +192,16 @@ def add_method(self, name, func, background=False, category=None, desc=None,
         The `category` argument can be used to specify the category of the
         newly created rpc command.
 
+        `deprecated` means that it won't appear unless `allow-deprecated-apis`
+        is true (the default).
         """
         if name in self.methods:
             raise ValueError(
                 "Name {} is already bound to a method.".format(name)
             )
 
         # Register the function with the name
-        method = Method(name, func, MethodType.RPCMETHOD, category, desc, long_desc)
+        method = Method(name, func, MethodType.RPCMETHOD, category, desc, long_desc, deprecated)
         method.background = background
         self.methods[name] = method
 
@@ -242,7 +245,8 @@ def decorator(f):
             return f
         return decorator
 
-    def add_option(self, name, default, description, opt_type="string"):
+    def add_option(self, name, default, description, opt_type="string",
+                   deprecated=False):
         """Add an option that we'd like to register with lightningd.
 
         Needs to be called before `Plugin.run`, otherwise we might not
@@ -263,16 +267,18 @@ def add_option(self, name, default, description, opt_type="string"):
             'description': description,
             'type': opt_type,
             'value': None,
+            'deprecated': deprecated,
         }
 
-    def add_flag_option(self, name, description):
+    def add_flag_option(self, name, description, deprecated=False):
         """Add a flag option that we'd like to register with lightningd.
 
         Needs to be called before `Plugin.run`, otherwise we might not
         end up getting it set.
 
         """
-        self.add_option(name, None, description, opt_type="flag")
+        self.add_option(name, None, description, opt_type="flag",
+                        deprecated=deprecated)
 
     def get_option(self, name):
         if name not in self.options:
@@ -283,25 +289,27 @@ def get_option(self, name):
         else:
             return self.options[name]['default']
 
-    def async_method(self, method_name, category=None, desc=None, long_desc=None):
+    def async_method(self, method_name, category=None, desc=None, long_desc=None, deprecated=False):
         """Decorator to add an async plugin method to the dispatch table.
 
         Internally uses add_method.
         """
         def decorator(f):
             self.add_method(method_name, f, background=True, category=category,
-                            desc=desc, long_desc=long_desc)
+                            desc=desc, long_desc=long_desc,
+                            deprecated=deprecated)
             return f
         return decorator
 
-    def method(self, method_name, category=None, desc=None, long_desc=None):
+    def method(self, method_name, category=None, desc=None, long_desc=None, deprecated=False):
         """Decorator to add a plugin method to the dispatch table.
 
         Internally uses add_method.
         """
         def decorator(f):
             self.add_method(method_name, f, background=False, category=category,
-                            desc=desc, long_desc=long_desc)
+                            desc=desc, long_desc=long_desc,
+                            deprecated=deprecated)
             return f
         return decorator
 
@@ -523,6 +531,12 @@ def run(self):
             partial = self._multi_dispatch(msgs)
 
     def _getmanifest(self, **kwargs):
+        if 'allow-deprecated-apis' in kwargs:
+            self.deprecated_apis = kwargs['allow-deprecated-apis']
+        else:
+            # 0.9.0 and before didn't offer this, so assume "yes".
+            self.deprecated_apis = True
+
         methods = []
         hooks = []
         for method in self.methods.values():

doc/PLUGINS.md

@@ -57,9 +57,11 @@ interface.
 
 ### The `getmanifest` method
 
-The `getmanifest` method is required for all plugins and will be called on
-startup without any params. It MUST return a JSON object similar to
-this example:
+The `getmanifest` method is required for all plugins and will be
+called on startup with optional parameters (in particular, it may have
+`allow-deprecated-apis: false`, but you should accept, and ignore,
+other parameters).  It MUST return a JSON object similar to this
+example:
 
 ```json
 {
@@ -68,7 +70,8 @@ this example:
       "name": "greeting",
       "type": "string",
       "default": "World",
-      "description": "What name should I call you?"
+      "description": "What name should I call you?",
+      "deprecated": false
     }
   ],
   "rpcmethods": [
@@ -81,7 +84,8 @@ this example:
       "name": "gettime",
       "usage": "",
       "description": "Returns the current time in {timezone}",
-      "long_description": "Returns the current time in the timezone that is given as the only parameter.\nThis description may be quite long and is allowed to span multiple lines."
+      "long_description": "Returns the current time in the timezone that is given as the only parameter.\nThis description may be quite long and is allowed to span multiple lines.",
+      "deprecated": false
     }
   ],
   "subscriptions": [
@@ -115,6 +119,11 @@ are mandatory, while the `long_description` can be omitted (it'll be
 set to `description` if it was not provided). `usage` should surround optional
 parameter names in `[]`.
 
+`options` and `rpcmethods` can mark themselves `deprecated: true` if
+you plan on removing them: this will disable them if the user sets
+`allow-deprecated-apis` to false (which every developer should do,
+right?).
+
 The `dynamic` indicates if the plugin can be managed after `lightningd`
 has been started. Critical plugins that should not be stopped should set it
 to false.

doc/STYLE.md

@@ -202,7 +202,7 @@ You can always add a new output JSON field (Changelog-Added), but you
 cannot remove one without going through a 6-month deprecation cycle
 (Changelog-Deprecated)
 
-So, only output it if `deprecated-apis` is true, so users can test
+So, only output it if `allow-deprecated-apis` is true, so users can test
 their code is futureproof.  In 6 months remove it (Changelog-Removed).
 
 Changing existing input parameters is harder, and should generally be

lightningd/jsonrpc.c

@@ -321,8 +321,13 @@ static void json_add_help_command(struct command *cmd,
 {
 	char *usage;
 
-	usage = tal_fmt(cmd, "%s %s",
+	/* If they disallow deprecated APIs, don't even list them */
+	if (!deprecated_apis && json_command->deprecated)
+		return;
+
+	usage = tal_fmt(cmd, "%s%s %s",
 			json_command->name,
+			json_command->deprecated ? " (DEPRECATED!)" : "",
 			strmap_get(&cmd->ld->jsonrpc->usagemap,
 				   json_command->name));
 	json_object_start(response, NULL);
@@ -387,6 +392,11 @@ static struct command_result *json_help(struct command *cmd,
 					    "Unknown command '%.*s'",
 					    cmdtok->end - cmdtok->start,
 					    buffer + cmdtok->start);
+		if (!deprecated_apis && one_cmd->deprecated)
+			return command_fail(cmd, JSONRPC2_METHOD_NOT_FOUND,
+					    "Deprecated command '%.*s'",
+					    json_tok_full_len(cmdtok),
+					    json_tok_full(buffer, cmdtok));
 	} else
 		one_cmd = NULL;
 
@@ -839,9 +849,9 @@ parse_request(struct json_connection *jcon, const jsmntok_t tok[])
 	}
 	if (c->json_cmd->deprecated && !deprecated_apis) {
 		return command_fail(c, JSONRPC2_METHOD_NOT_FOUND,
-				    "Command '%.*s' is deprecated",
-				    method->end - method->start,
-				    jcon->buffer + method->start);
+				    "Command %.*s is deprecated",
+				    json_tok_full_len(method),
+				    json_tok_full(jcon->buffer, method));
 	}
 
 	rpc_hook = tal(c, struct rpc_command_hook_payload);

lightningd/pay.c

@@ -97,7 +97,7 @@ void json_add_payment_fields(struct json_stream *response,
 	if (amount_msat_greater(t->msatoshi, AMOUNT_MSAT(0)))
 		json_add_amount_msat_compat(response, t->msatoshi, "msatoshi",
 					    "amount_msat");
-	else
+	else if (deprecated_apis)
 		json_add_null(response, "amount_msat");
 
 

lightningd/plugin.c

@@ -556,6 +556,10 @@ char *plugin_opt_set(const char *arg, struct plugin_opt *popt)
 	char *endp;
 	long long l;
 
+	/* Warn them that this is deprecated */
+	if (popt->deprecated && !deprecated_apis)
+		return tal_fmt(tmpctx, "deprecated option (will be removed!)");
+
 	tal_free(popt->value->as_str);
 
 	popt->value->as_str = tal_strdup(popt, arg);
@@ -599,12 +603,13 @@ static void destroy_plugin_opt(struct plugin_opt *opt)
 static const char *plugin_opt_add(struct plugin *plugin, const char *buffer,
 				  const jsmntok_t *opt)
 {
-	const jsmntok_t *nametok, *typetok, *defaulttok, *desctok;
+	const jsmntok_t *nametok, *typetok, *defaulttok, *desctok, *deptok;
 	struct plugin_opt *popt;
 	nametok = json_get_member(buffer, opt, "name");
 	typetok = json_get_member(buffer, opt, "type");
 	desctok = json_get_member(buffer, opt, "description");
 	defaulttok = json_get_member(buffer, opt, "default");
+	deptok = json_get_member(buffer, opt, "deprecated");
 
 	if (!typetok || !nametok || !desctok) {
 		return tal_fmt(plugin,
@@ -617,6 +622,15 @@ static const char *plugin_opt_add(struct plugin *plugin, const char *buffer,
 	popt->name = tal_fmt(popt, "--%.*s", nametok->end - nametok->start,
 			     buffer + nametok->start);
 	popt->description = NULL;
+	if (deptok) {
+		if (!json_to_bool(buffer, deptok, &popt->deprecated))
+			return tal_fmt(plugin,
+				       "%s: invalid \"deprecated\" field %.*s",
+				       popt->name,
+				       deptok->end - deptok->start,
+				       buffer + deptok->start);
+	} else
+		popt->deprecated = false;
 
 	if (json_tok_streq(buffer, typetok, "string")) {
 		popt->type = "string";
@@ -806,7 +820,8 @@ static const char *plugin_rpcmethod_add(struct plugin *plugin,
 					const char *buffer,
 					const jsmntok_t *meth)
 {
-	const jsmntok_t *nametok, *categorytok, *desctok, *longdesctok, *usagetok;
+	const jsmntok_t *nametok, *categorytok, *desctok, *longdesctok,
+		*usagetok, *deptok;
 	struct json_command *cmd;
 	const char *usage;
 
@@ -815,6 +830,7 @@ static const char *plugin_rpcmethod_add(struct plugin *plugin,
 	desctok = json_get_member(buffer, meth, "description");
 	longdesctok = json_get_member(buffer, meth, "long_description");
 	usagetok = json_get_member(buffer, meth, "usage");
+	deptok = json_get_member(buffer, meth, "deprecated");
 
 	if (!nametok || nametok->type != JSMN_STRING) {
 		return tal_fmt(plugin,
@@ -860,7 +876,16 @@ static const char *plugin_rpcmethod_add(struct plugin *plugin,
 	} else
 		usage = "[params]";
 
-	cmd->deprecated = false;
+	if (deptok) {
+		if (!json_to_bool(buffer, deptok, &cmd->deprecated))
+			return tal_fmt(plugin,
+				       "%s: invalid \"deprecated\" field %.*s",
+				       cmd->name,
+			               deptok->end - deptok->start,
+				       buffer + deptok->start);
+	} else
+		cmd->deprecated = false;
+
 	cmd->dispatch = plugin_rpcmethod_dispatch;
 	if (!jsonrpc_command_add(plugin->plugins->ld->jsonrpc, cmd, usage)) {
 		return tal_fmt(plugin,
@@ -1254,6 +1279,9 @@ const char *plugin_send_getmanifest(struct plugin *p)
 	p->stdin_conn = io_new_conn(p, stdin, plugin_stdin_conn_init, p);
 	req = jsonrpc_request_start(p, "getmanifest", p->log,
 				    plugin_manifest_cb, p);
+	/* Adding allow-deprecated-apis is part of the deprecation cycle! */
+	if (!deprecated_apis)
+		json_add_bool(req->stream, "allow-deprecated-apis", deprecated_apis);
 	jsonrpc_request_end(req);
 	plugin_request_send(p, req);
 	p->plugin_state = AWAITING_GETMANIFEST_RESPONSE;
@@ -1425,6 +1453,9 @@ void json_add_opt_plugins(struct json_stream *response,
 		if (!list_empty(&p->plugin_opts)) {
 			json_object_start(response, "options");
 			list_for_each(&p->plugin_opts, opt, list) {
+				if (!deprecated_apis && opt->deprecated)
+					continue;
+
 				/* Trim the `--` that we added before */
 				opt_name = opt->name + 2;
 				if (opt->value->as_bool) {

lightningd/plugin.h

@@ -125,6 +125,7 @@ struct plugin_opt {
 	const char *type;
 	const char *description;
 	struct plugin_opt_value *value;
+	bool deprecated;
 };
 
 /**