Make modern string-based JSON ids opt in, deprecate numeric ones

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

@@ -917,6 +917,7 @@ def _getmanifest(self, **kwargs) -> JSONType:
             'subscriptions': list(self.subscriptions.keys()),
             'hooks': hooks,
             'dynamic': self.dynamic,
+            'nonnumericids': True,
             'notifications': [
                 {"method": name} for name in self.notification_topics
             ],

doc/PLUGINS.md

@@ -136,6 +136,7 @@ example:
 	  "method": "mycustomnotification"
 	}
   ],
+  "nonnumericids": true,
   "dynamic": true
 }
 ```
@@ -158,6 +159,13 @@ 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 `nonnumericids` indicates that the plugin can handle
+string JSON request `id` fields: prior to v22.11 lightningd used numbers 
+for these, and the change to strings broke some plugins.  If not set,
+then strings will be used once this feature is removed after v23.05.
+See [the lightning-rpc documentation][lightning-rpc.7.md] for how to handle
+JSON `id` fields!
+
 The `dynamic` indicates if the plugin can be managed after `lightningd`
 has been started using the [plugin][lightning-plugin] JSON-RPC command. Critical plugins that should not be stopped should set it
 to false. Plugin `options` can be passed to dynamic plugins as argument to the `plugin` command .
@@ -1781,3 +1789,4 @@ The plugin must broadcast it and respond with the following fields:
 [bolt9]: https://github.com/lightning/bolts/blob/master/09-features.md
 [lightning-plugin]: lightning-plugin.7.md
 [pyln-client]: ../contrib/pyln-client
+[lightning-rpc.7.md]: lightning-rpc.7.md

doc/lightningd-rpc.7.md

@@ -42,6 +42,38 @@ The lightning-cli(1) tool uses ordered parameters by default, but
 named parameters if explicitly specified or the first parameter
 contains an '='.
 
+JSON IDS
+--------
+
+JSON `id` fields in requests are used to match requests and responses.
+These used to be simple numbers, but with modern plugins that is deprecated:
+we use a specific format, which makes them very useful for debugging
+and tracking the cause of commands:
+
+```EBNF
+JSONID := IDPART ['/' IDPART]*
+IDPART := PREFIX ':' METHOD '#' NUMBER
+```
+
+`PREFIX` is cln for the main daemon, cli for lightning-cli, and should
+be the plugin name for plugins.  `METHOD` is an internal identifier,
+indicating what caused the request: for `cli` it's simply the method
+it's invoking, but for plugins it may be the routine which created the
+request.  And `NUMBER` ensures uniqueness (it's usually a simple
+increment).
+
+Importantly for plugins, incoming requests often trigger outgoing
+requests, and for these, the outgoing request id is created by
+appending a `/` and another id part into the incoming.  This makes the
+chain of responsibility much clearer.  e.g, this shows the JSON `id`
+of a `sendrawtransaction` RPC call, and we can tell that lightning-cli
+has invoked the `withdraw` command, which lightningd passes through
+to the `txprepare` plugin, which called `sendrawtransaction`.
+
+```
+cli:withdraw#123/cln:withdraw#7/txprepare:sendpsbt#1/cln:sendrawtransaction#9
+```
+
 JSON REPLIES
 ------------
 

lightningd/bitcoind.c

@@ -49,7 +49,8 @@ static void config_plugin(struct plugin *plugin)
 	struct jsonrpc_request *req;
 	void *ret;
 
-	req = jsonrpc_request_start(plugin, "init", NULL, plugin->log,
+	req = jsonrpc_request_start(plugin, "init", NULL,
+				    plugin->non_numeric_ids, plugin->log,
 	                            NULL, plugin_config_cb, plugin);
 	plugin_populate_init_request(plugin, req);
 	jsonrpc_request_end(req);
@@ -237,7 +238,7 @@ void bitcoind_estimate_fees_(struct bitcoind *bitcoind,
 	call->cb = cb;
 	call->arg = arg;
 
-	req = jsonrpc_request_start(bitcoind, "estimatefees", NULL,
+	req = jsonrpc_request_start(bitcoind, "estimatefees", NULL, true,
 				    bitcoind->log,
 				    NULL, estimatefees_callback, call);
 	jsonrpc_request_end(req);
@@ -314,7 +315,8 @@ void bitcoind_sendrawtx_(struct bitcoind *bitcoind,
 	call->cb_arg = cb_arg;
 	log_debug(bitcoind->log, "sendrawtransaction: %s", hextx);
 
-	req = jsonrpc_request_start(bitcoind, "sendrawtransaction", id_prefix,
+	req = jsonrpc_request_start(bitcoind, "sendrawtransaction",
+				    id_prefix, true,
 				    bitcoind->log,
 				    NULL, sendrawtx_callback,
 				    call);
@@ -401,7 +403,7 @@ void bitcoind_getrawblockbyheight_(struct bitcoind *bitcoind,
 	call->cb = cb;
 	call->cb_arg = cb_arg;
 
-	req = jsonrpc_request_start(bitcoind, "getrawblockbyheight", NULL,
+	req = jsonrpc_request_start(bitcoind, "getrawblockbyheight", NULL, true,
 				    bitcoind->log,
 				    NULL,  getrawblockbyheight_callback,
 				    call);
@@ -482,7 +484,7 @@ void bitcoind_getchaininfo_(struct bitcoind *bitcoind,
 	call->cb_arg = cb_arg;
 	call->first_call = first_call;
 
-	req = jsonrpc_request_start(bitcoind, "getchaininfo", NULL,
+	req = jsonrpc_request_start(bitcoind, "getchaininfo", NULL, true,
 				    bitcoind->log,
 				    NULL, getchaininfo_callback, call);
 	jsonrpc_request_end(req);
@@ -555,7 +557,8 @@ void bitcoind_getutxout_(struct bitcoind *bitcoind,
 	call->cb = cb;
 	call->cb_arg = cb_arg;
 
-	req = jsonrpc_request_start(bitcoind, "getutxout", NULL, bitcoind->log,
+	req = jsonrpc_request_start(bitcoind, "getutxout", NULL, true,
+				    bitcoind->log,
 				    NULL, getutxout_callback, call);
 	json_add_txid(req->stream, "txid", &outpoint->txid);
 	json_add_num(req->stream, "vout", outpoint->n);

lightningd/invoice.c

@@ -1241,22 +1241,21 @@ static struct command_result *json_invoice(struct command *cmd,
 	if (fallback_scripts)
 		info->b11->fallbacks = tal_steal(info->b11, fallback_scripts);
 
+	/* We can't generate routehints without listincoming. */
+	plugin = find_plugin_for_command(cmd->ld, "listincoming");
+	if (!plugin) {
+		return invoice_complete(info, true,
+					false, false, false, false, false);
+	}
+
 	req = jsonrpc_request_start(info, "listincoming",
-				    cmd->id,
+				    cmd->id, plugin->non_numeric_ids,
 				    command_log(cmd),
 				    NULL, listincoming_done,
 				    info);
 	jsonrpc_request_end(req);
-
-	plugin = find_plugin_for_command(cmd->ld, "listincoming");
-	if (plugin) {
-		plugin_request_send(plugin, req);
-		return command_still_pending(cmd);
-	}
-
-	/* We can't generate routehints without listincoming. */
-	return invoice_complete(info, true,
-				false, false, false, false, false);
+	plugin_request_send(plugin, req);
+	return command_still_pending(cmd);
 }
 
 static const struct json_command invoice_command = {

lightningd/jsonrpc.c

@@ -1376,7 +1376,7 @@ void jsonrpc_notification_end(struct jsonrpc_notification *n)
 
 struct jsonrpc_request *jsonrpc_request_start_(
     const tal_t *ctx, const char *method,
-    const char *id_prefix, struct log *log,
+    const char *id_prefix, bool id_as_string, struct log *log,
     bool add_header,
     void (*notify_cb)(const char *buffer,
 		      const jsmntok_t *methodtok,
@@ -1389,11 +1389,17 @@ struct jsonrpc_request *jsonrpc_request_start_(
 {
 	struct jsonrpc_request *r = tal(ctx, struct jsonrpc_request);
 	static u64 next_request_id = 0;
-	if (id_prefix)
-		r->id = tal_fmt(r, "%s/cln:%s#%"PRIu64,
-				id_prefix, method, next_request_id);
-	else
-		r->id = tal_fmt(r, "cln:%s#%"PRIu64, method, next_request_id);
+
+	r->id_is_string = id_as_string;
+	if (r->id_is_string) {
+		if (id_prefix)
+			r->id = tal_fmt(r, "%s/cln:%s#%"PRIu64,
+					id_prefix, method, next_request_id);
+		else
+			r->id = tal_fmt(r, "cln:%s#%"PRIu64, method, next_request_id);
+	} else {
+		r->id = tal_fmt(r, "%"PRIu64, next_request_id);
+	}
 	if (taken(id_prefix))
 		tal_free(id_prefix);
 	next_request_id++;
@@ -1409,7 +1415,10 @@ struct jsonrpc_request *jsonrpc_request_start_(
 	if (add_header) {
 		json_object_start(r->stream, NULL);
 		json_add_string(r->stream, "jsonrpc", "2.0");
-		json_add_string(r->stream, "id", r->id);
+		if (r->id_is_string)
+			json_add_string(r->stream, "id", r->id);
+		else
+			json_add_primitive(r->stream, "id", r->id);
 		json_add_string(r->stream, "method", method);
 		json_object_start(r->stream, "params");
 	}

lightningd/jsonrpc.h

@@ -73,6 +73,7 @@ struct jsonrpc_notification {
 
 struct jsonrpc_request {
 	const char *id;
+	bool id_is_string;
 	const char *method;
 	struct json_stream *stream;
 	void (*notify_cb)(const char *buffer,
@@ -226,9 +227,9 @@ void jsonrpc_notification_end(struct jsonrpc_notification *n);
  * start a JSONRPC request; id_prefix is non-NULL if this was triggered by
  * another JSONRPC request.
  */
-#define jsonrpc_request_start(ctx, method, id_prefix, log, notify_cb, response_cb, response_cb_arg) \
+#define jsonrpc_request_start(ctx, method, id_prefix, id_as_string, log, notify_cb, response_cb, response_cb_arg) \
 	jsonrpc_request_start_(					\
-	    (ctx), (method), (id_prefix), (log), true,		\
+	    (ctx), (method), (id_prefix), (id_as_string), (log), true, \
 	    typesafe_cb_preargs(void, void *, (notify_cb), (response_cb_arg),	\
 				const char *buffer,		\
 				const jsmntok_t *idtok,		\
@@ -240,9 +241,9 @@ void jsonrpc_notification_end(struct jsonrpc_notification *n);
 				const jsmntok_t *idtok),	\
 	    (response_cb_arg))
 
-#define jsonrpc_request_start_raw(ctx, method, id_prefix, log, notify_cb, response_cb, response_cb_arg) \
+#define jsonrpc_request_start_raw(ctx, method, id_prefix, id_as_string,log, notify_cb, response_cb, response_cb_arg) \
 	jsonrpc_request_start_(						\
-		(ctx), (method), (id_prefix), (log), false,		\
+		(ctx), (method), (id_prefix), (id_as_string), (log), false, \
 	    typesafe_cb_preargs(void, void *, (notify_cb), (response_cb_arg), \
 				const char *buffer,			\
 				const jsmntok_t *idtok,			\
@@ -256,7 +257,9 @@ void jsonrpc_notification_end(struct jsonrpc_notification *n);
 
 struct jsonrpc_request *jsonrpc_request_start_(
     const tal_t *ctx, const char *method,
-    const char *id_prefix TAKES, struct log *log, bool add_header,
+    const char *id_prefix TAKES,
+    bool id_as_string,
+    struct log *log, bool add_header,
     void (*notify_cb)(const char *buffer,
 		      const jsmntok_t *idtok,
 		      const jsmntok_t *methodtok,

lightningd/plugin.c

@@ -291,6 +291,7 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES,
 	p->notification_topics = tal_arr(p, const char *, 0);
 	p->subscriptions = NULL;
 	p->dynamic = false;
+	p->non_numeric_ids = false;
 	p->index = plugins->plugin_idx++;
 
 	p->log = new_log(p, plugins->log_book, NULL, "plugin-%s", p->shortname);
@@ -1142,7 +1143,8 @@ static struct command_result *plugin_rpcmethod_dispatch(struct command *cmd,
 	call = tal(plugin, struct plugin_rpccall);
 	call->cmd = cmd;
 
-	req = jsonrpc_request_start_raw(plugin, cmd->json_cmd->name, cmd->id,
+	req = jsonrpc_request_start_raw(plugin, cmd->json_cmd->name,
+					cmd->id, plugin->non_numeric_ids,
 					plugin->log,
 					plugin_notify_cb,
 					plugin_rpcmethod_cb, call);
@@ -1151,7 +1153,7 @@ static struct command_result *plugin_rpcmethod_dispatch(struct command *cmd,
 	list_add_tail(&plugin->pending_rpccalls, &call->list);
 
 	json_stream_forward_change_id(req->stream, buffer, toks, idtok, req->id,
-				      true);
+				      req->id_is_string);
 	json_stream_double_cr(req->stream);
 	plugin_request_send(plugin, req);
 	req->stream = NULL;
@@ -1528,6 +1530,20 @@ static const char *plugin_parse_getmanifest_response(const char *buffer,
 		}
 	}
 
+	tok = json_get_member(buffer, resulttok, "nonnumericids");
+	if (tok) {
+		if (!json_to_bool(buffer, tok, &plugin->non_numeric_ids))
+			return tal_fmt(plugin,
+				       "Invalid nonnumericids: %.*s",
+				       json_tok_full_len(tok),
+				       json_tok_full(buffer, tok));
+		if (!deprecated_apis && !plugin->non_numeric_ids)
+			return tal_fmt(plugin,
+				       "Plugin does not allow nonnumericids");
+	} else
+		/* Default is false in deprecated mode */
+		plugin->non_numeric_ids = !deprecated_apis;
+
 	err = plugin_notifications_add(buffer, resulttok, plugin);
 	if (!err)
 		err = plugin_opts_add(plugin, buffer, resulttok);
@@ -1730,8 +1746,8 @@ const char *plugin_send_getmanifest(struct plugin *p, const char *cmd_id)
 	 * write-only on p->stdin */
 	p->stdout_conn = io_new_conn(p, stdoutfd, plugin_stdout_conn_init, p);
 	p->stdin_conn = io_new_conn(p, stdinfd, plugin_stdin_conn_init, p);
-	req = jsonrpc_request_start(p, "getmanifest", cmd_id, p->log,
-				    NULL, plugin_manifest_cb, p);
+	req = jsonrpc_request_start(p, "getmanifest", cmd_id, p->non_numeric_ids,
+				    p->log, NULL, plugin_manifest_cb, p);
 	json_add_bool(req->stream, "allow-deprecated-apis", deprecated_apis);
 	jsonrpc_request_end(req);
 	plugin_request_send(p, req);
@@ -1911,8 +1927,8 @@ plugin_config(struct plugin *plugin)
 	struct jsonrpc_request *req;
 
 	plugin_set_timeout(plugin);
-	req = jsonrpc_request_start(plugin, "init", NULL, plugin->log,
-	                            NULL, plugin_config_cb, plugin);
+	req = jsonrpc_request_start(plugin, "init", NULL, plugin->non_numeric_ids,
+				    plugin->log, NULL, plugin_config_cb, plugin);
 	plugin_populate_init_request(plugin, req);
 	jsonrpc_request_end(req);
 	plugin_request_send(plugin, req);

lightningd/plugin.h

@@ -81,6 +81,9 @@ struct plugin {
 	 * C-lightning should terminate as well.  */
 	bool important;
 
+	/* Can this handle non-numeric JSON ids? */
+	bool non_numeric_ids;
+
 	/* Parameters for dynamically-started plugins. */
 	const char *parambuf;
 	const jsmntok_t *params;

lightningd/plugin_hook.c

@@ -235,6 +235,7 @@ static void plugin_hook_call_next(struct plugin_hook_request *ph_req)
 	log_debug(ph_req->ld->log, "Calling %s hook of plugin %s",
 		  ph_req->hook->name, ph_req->plugin->shortname);
 	req = jsonrpc_request_start(NULL, hook->name, ph_req->cmd_id,
+				    ph_req->plugin->non_numeric_ids,
 				    plugin_get_log(ph_req->plugin),
 				    NULL,
 				    plugin_hook_callback, ph_req);
@@ -380,7 +381,9 @@ void plugin_hook_db_sync(struct db *db)
 
 		/* FIXME: id_prefix from caller? */
 		/* FIXME: do IO logging for this! */
-		req = jsonrpc_request_start(NULL, hook->name, NULL, NULL, NULL,
+		req = jsonrpc_request_start(NULL, hook->name, NULL,
+					    dwh_req->plugin->non_numeric_ids,
+					    NULL, NULL,
 					    db_hook_response,
 					    dwh_req);
 

lightningd/signmessage.c

@@ -211,17 +211,18 @@ static struct command_result *json_checkmessage(struct command *cmd,
 
 		node_id_from_pubkey(&can->id, &reckey);
 		can->cmd = cmd;
-		req = jsonrpc_request_start(cmd, "listnodes",
-					    cmd->id,
-					    command_log(cmd),
-					    NULL, listnodes_done,
-					    can);
-		json_add_node_id(req->stream, "id", &can->id);
-		jsonrpc_request_end(req);
 
 		/* Only works if we have listnodes! */
 		plugin = find_plugin_for_command(cmd->ld, "listnodes");
 		if (plugin) {
+			req = jsonrpc_request_start(cmd, "listnodes",
+						    cmd->id,
+						    plugin->non_numeric_ids,
+						    command_log(cmd),
+						    NULL, listnodes_done,
+						    can);
+			json_add_node_id(req->stream, "id", &can->id);
+			jsonrpc_request_end(req);
 			plugin_request_send(plugin, req);
 			return command_still_pending(cmd);
 		}

lightningd/test/run-invoice-select-inchan.c

@@ -517,7 +517,9 @@ void jsonrpc_request_end(struct jsonrpc_request *request UNNEEDED)
 /* Generated stub for jsonrpc_request_start_ */
 struct jsonrpc_request *jsonrpc_request_start_(
     const tal_t *ctx UNNEEDED, const char *method UNNEEDED,
-    const char *id_prefix TAKES UNNEEDED, struct log *log UNNEEDED, bool add_header UNNEEDED,
+    const char *id_prefix TAKES UNNEEDED,
+    bool id_as_string UNNEEDED,
+    struct log *log UNNEEDED, bool add_header UNNEEDED,
     void (*notify_cb)(const char *buffer UNNEEDED,
 		      const jsmntok_t *idtok UNNEEDED,
 		      const jsmntok_t *methodtok UNNEEDED,

plugins/libplugin.c

@@ -888,6 +888,7 @@ handle_getmanifest(struct command *getmanifest_cmd,
 	}
 
 	json_add_bool(params, "dynamic", p->restartability == PLUGIN_RESTARTABLE);
+	json_add_bool(params, "nonnumericids", true);
 
 	json_array_start(params, "notifications");
 	for (size_t i = 0; p->notif_topics && i < p->num_notif_topics; i++) {