lightningd: only use non-numeric JSON ids if plugin says we can.

We also remember whether the id is a string or not, for replacement in JSON passthrough. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
ElementsProject · Nov 21, 2022 · b7500c7 · b7500c7
1 parent a4c3927
commit b7500c7
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 47 deletions.
diff --git a/lightningd/bitcoind.c b/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);

diff --git a/lightningd/invoice.c b/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 = {

diff --git a/lightningd/jsonrpc.c b/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");
 	}

diff --git a/lightningd/jsonrpc.h b/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,

diff --git a/lightningd/plugin.c b/lightningd/plugin.c
@@ -1143,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);
@@ -1152,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;
@@ -1531,7 +1532,7 @@ static const char *plugin_parse_getmanifest_response(const char *buffer,
 
 	tok = json_get_member(buffer, resulttok, "nonnumericids");
 	if (tok) {
-		if (!json_to_bool(&plugin->non_numeric_ids, buffer, tok))
+		if (!json_to_bool(buffer, tok, &plugin->non_numeric_ids))
 			return tal_fmt(plugin,
 				       "Invalid nonnumericids: %.*s",
 				       json_tok_full_len(tok),
@@ -1745,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);
@@ -1926,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);

diff --git a/lightningd/plugin_hook.c b/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);
 

diff --git a/lightningd/signmessage.c b/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);
 		}

diff --git a/lightningd/test/run-invoice-select-inchan.c b/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,

diff --git a/tests/test_plugin.py b/tests/test_plugin.py
@@ -1501,8 +1501,9 @@ def test_libplugin(node_factory):
 
     myname = os.path.splitext(os.path.basename(sys.argv[0]))[0]
 
-    # Side note: getmanifest will trace back to plugin_start
-    l1.daemon.wait_for_log(r": {}:plugin#[0-9]*/cln:getmanifest#[0-9]*\[OUT\]".format(myname))
+    # Note: getmanifest always uses numeric ids, since it doesn't know
+    # yet whether strings are allowed:
+    l1.daemon.wait_for_log(r"test_libplugin: [0-9]*\[OUT\]")
 
     # Test commands
     assert l1.rpc.call("helloworld") == {"hello": "world"}
@@ -3237,3 +3238,18 @@ def test_block_added_notifications(node_factory, bitcoind):
     sync_blockheight(bitcoind, [l2])
     ret = l2.rpc.call("blockscatched")
     assert len(ret) == 3 and ret[1] == next_l2_base + 1 and ret[2] == next_l2_base + 2
+
+
+def test_numeric_json_ids(node_factory):
+    """Test that we use numeric json IDs when in deprecated mode (unless
+plugin says otherwise!)"""
+    l1 = node_factory.get_node(options={'allow-deprecated-apis': True,
+                                        'log-level': 'io'})
+
+    # getmanifest and init
+    l1.daemon.logsearch_start = 0
+    l1.daemon.wait_for_logs([r"plugin-commando: [0-9]*\[OUT\]"] * 2)
+
+    # This is in a plugin.
+    l1.rpc.commando_rune()
+    l1.daemon.wait_for_log(r"plugin-commando: [0-9]*\[OUT\]")