From c993691badd670725f096a349e0b990ba85a224f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 25 Feb 2025 21:54:58 +1030 Subject: [PATCH] plugins: all plugins must now support non-numeric JSON RPC id fields. Signed-off-by: Rusty Russell Changelog-Removed: plugins which didn't accept string JSON RPC fields (deprecated v23.08, disabled by default in v24.11). --- doc/developers-guide/deprecations.md | 1 - .../a-day-in-the-life-of-a-plugin.md | 2 +- lightningd/bitcoind.c | 12 +++--- lightningd/invoice.c | 2 +- lightningd/jsonrpc.c | 39 +++++++++---------- lightningd/jsonrpc.h | 9 ++--- lightningd/plugin.c | 24 ++++-------- lightningd/plugin.h | 3 -- lightningd/plugin_hook.c | 2 - lightningd/signmessage.c | 1 - lightningd/test/run-invoice-select-inchan.c | 1 - 11 files changed, 37 insertions(+), 59 deletions(-) diff --git a/doc/developers-guide/deprecations.md b/doc/developers-guide/deprecations.md index b761e99de6f4..b9c10ed753e1 100644 --- a/doc/developers-guide/deprecations.md +++ b/doc/developers-guide/deprecations.md @@ -11,7 +11,6 @@ hidden: false | rest-protocol.clnrest-prefix | Config | v23.11 | v24.11 | Autodetect where we need to rename `rest-protocol` to `clnrest-protocol` (added in v23.11) | | rest-host.clnrest-prefix | Config | v23.11 | v24.11 | Autodetect where we need to rename `rest-host` to `clnrest-host` (added in v23.11) | | rest-certs.clnrest-prefix | Config | v23.11 | v24.11 | Autodetect where we need to rename `rest-certs` to `clnrest-certs` (added in v23.11) | -| plugin.nonumericids | Getmanifest Reply | v23.08 | v24.08 | Plugins must specify that they can accept non-numeric command ids (numeric ids are deprecated) | | listchannels.include_private | Field(s) | v24.02 | v24.08 | `listchannels` including private channels (now use listpeerchannels which gives far more detail) | | max-locktime-blocks | Config | v24.05 | v24.11 | --max-locktime-blocks is now set to 2016 in the BOLT 4 spec | | commando-rune | Command | v23.08 | v25.02 | replaced with `lightning-createrune` | diff --git a/doc/developers-guide/plugin-development/a-day-in-the-life-of-a-plugin.md b/doc/developers-guide/plugin-development/a-day-in-the-life-of-a-plugin.md index 039ed1de19f1..e6e971a19ca6 100644 --- a/doc/developers-guide/plugin-development/a-day-in-the-life-of-a-plugin.md +++ b/doc/developers-guide/plugin-development/a-day-in-the-life-of-a-plugin.md @@ -104,7 +104,7 @@ The `rpcmethods` are methods that will be exposed via `lightningd`'s JSON-RPC ov The `subscriptions` array indicates what [Event Notifications](doc:event-notifications) your plugin wants to receive. You should subscribe to `deprecated_oneshot` if you have any deprecated commands or output, so users can use the `deprecations` API to control it on a per-connection basis. You can specify `*` here to subscribe to all other subscriptions (since *v23.08*). -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 [lightningd-rpc](ref:lightningd-rpc) documentation for how to handle JSON `id` fields! +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 present, this must be `true` (since v23.05). See the [lightningd-rpc](ref:lightningd-rpc) documentation for how to handle JSON `id` fields! The `dynamic` indicates if the plugin can be managed after `lightningd` has been started using the [lightning-plugin](ref: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 . diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 8bae8d722171..15b7689f622b 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -52,7 +52,7 @@ static void config_plugin(struct plugin *plugin) void *ret; req = jsonrpc_request_start(plugin, "init", NULL, - plugin->non_numeric_ids, plugin->log, + plugin->log, NULL, plugin_config_cb, plugin); plugin_populate_init_request(plugin, req); jsonrpc_request_end(req); @@ -361,7 +361,7 @@ void bitcoind_estimate_fees_(const tal_t *ctx, call->cb = cb; call->cb_arg = cb_arg; - req = jsonrpc_request_start(call, "estimatefees", NULL, true, + req = jsonrpc_request_start(call, "estimatefees", NULL, bitcoind->log, NULL, estimatefees_callback, call); jsonrpc_request_end(req); @@ -438,7 +438,7 @@ void bitcoind_sendrawtx_(const tal_t *ctx, log_debug(bitcoind->log, "sendrawtransaction: %s", hextx); req = jsonrpc_request_start(call, "sendrawtransaction", - id_prefix, true, + id_prefix, bitcoind->log, NULL, sendrawtx_callback, call); @@ -531,7 +531,7 @@ void bitcoind_getrawblockbyheight_(const tal_t *ctx, trace_span_start("plugin/bitcoind", call); trace_span_tag(call, "method", "getrawblockbyheight"); trace_span_suspend(call); - req = jsonrpc_request_start(call, "getrawblockbyheight", NULL, true, + req = jsonrpc_request_start(call, "getrawblockbyheight", NULL, bitcoind->log, NULL, getrawblockbyheight_callback, call); @@ -606,7 +606,7 @@ void bitcoind_getchaininfo_(const tal_t *ctx, call->cb = cb; call->cb_arg = cb_arg; - req = jsonrpc_request_start(call, "getchaininfo", NULL, true, + req = jsonrpc_request_start(call, "getchaininfo", NULL, bitcoind->log, NULL, getchaininfo_callback, call); json_add_u32(req->stream, "last_height", height); @@ -677,7 +677,7 @@ void bitcoind_getutxout_(const tal_t *ctx, call->cb = cb; call->cb_arg = cb_arg; - req = jsonrpc_request_start(call, "getutxout", NULL, true, + req = jsonrpc_request_start(call, "getutxout", NULL, bitcoind->log, NULL, getutxout_callback, call); json_add_txid(req->stream, "txid", &outpoint->txid); diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 18ef0b5ee669..edf15f0a0463 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -1229,7 +1229,7 @@ static struct command_result *json_invoice(struct command *cmd, } req = jsonrpc_request_start(info, "listincoming", - cmd->id, plugin->non_numeric_ids, + cmd->id, command_logger(cmd), NULL, listincoming_done, info); diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 1ddf80b67d09..4b240808c83c 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -1586,7 +1586,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, bool id_as_string, struct logger *log, + const char *id_prefix, struct logger *log, bool add_header, void (*notify_cb)(const char *buffer, const jsmntok_t *methodtok, @@ -1600,27 +1600,24 @@ struct jsonrpc_request *jsonrpc_request_start_( struct jsonrpc_request *r = tal(ctx, struct jsonrpc_request); static u64 next_request_id = 0; - r->id_is_string = id_as_string; - if (r->id_is_string) { - if (id_prefix) { - /* Strip "" and otherwise sanity-check */ - if (strstarts(id_prefix, "\"") - && strlen(id_prefix) > 1 - && strends(id_prefix, "\"")) { - id_prefix = tal_strndup(tmpctx, id_prefix + 1, - strlen(id_prefix) - 2); - } - /* We could try escaping, but TBH they're - * messing with us at this point! */ - if (json_escape_needed(id_prefix, strlen(id_prefix))) - id_prefix = "weird-id"; - - 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 = true; + if (id_prefix) { + /* Strip "" and otherwise sanity-check */ + if (strstarts(id_prefix, "\"") + && strlen(id_prefix) > 1 + && strends(id_prefix, "\"")) { + id_prefix = tal_strndup(tmpctx, id_prefix + 1, + strlen(id_prefix) - 2); + } + /* We could try escaping, but TBH they're + * messing with us at this point! */ + if (json_escape_needed(id_prefix, strlen(id_prefix))) + id_prefix = "weird-id"; + + r->id = tal_fmt(r, "\"%s/cln:%s#%"PRIu64"\"", + id_prefix, method, next_request_id); } else { - r->id = tal_fmt(r, "%"PRIu64, next_request_id); + r->id = tal_fmt(r, "\"cln:%s#%"PRIu64"\"", method, next_request_id); } if (taken(id_prefix)) tal_free(id_prefix); diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h index e547a5863209..c1e8172b690a 100644 --- a/lightningd/jsonrpc.h +++ b/lightningd/jsonrpc.h @@ -235,9 +235,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, id_as_string, log, notify_cb, response_cb, response_cb_arg) \ +#define jsonrpc_request_start(ctx, method, id_prefix, log, notify_cb, response_cb, response_cb_arg) \ jsonrpc_request_start_( \ - (ctx), (method), (id_prefix), (id_as_string), (log), true, \ + (ctx), (method), (id_prefix), (log), true, \ typesafe_cb_preargs(void, void *, (notify_cb), (response_cb_arg), \ const char *buffer, \ const jsmntok_t *idtok, \ @@ -249,9 +249,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, id_as_string,log, notify_cb, response_cb, response_cb_arg) \ +#define jsonrpc_request_start_raw(ctx, method, id_prefix, log, notify_cb, response_cb, response_cb_arg) \ jsonrpc_request_start_( \ - (ctx), (method), (id_prefix), (id_as_string), (log), false, \ + (ctx), (method), (id_prefix), (log), false, \ typesafe_cb_preargs(void, void *, (notify_cb), (response_cb_arg), \ const char *buffer, \ const jsmntok_t *idtok, \ @@ -266,7 +266,6 @@ 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, - bool id_as_string, struct logger *log, bool add_header, void (*notify_cb)(const char *buffer, const jsmntok_t *idtok, diff --git a/lightningd/plugin.c b/lightningd/plugin.c index f81a6e04cd59..fb67dce2d0b8 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -363,7 +363,6 @@ 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->stdout_conn = NULL; @@ -1324,7 +1323,7 @@ static struct command_result *plugin_rpcmethod_check(struct command *cmd, /* Send check command through, it says it can handle it! */ req = jsonrpc_request_start_raw(plugin, "check", - cmd->id, plugin->non_numeric_ids, + cmd->id, plugin->log, plugin_notify_cb, plugin_rpcmethod_cb, cmd); @@ -1368,7 +1367,7 @@ static struct command_result *plugin_rpcmethod_dispatch(struct command *cmd, } } req = jsonrpc_request_start_raw(plugin, cmd->json_cmd->name, - cmd->id, plugin->non_numeric_ids, + cmd->id, plugin->log, plugin_notify_cb, plugin_rpcmethod_cb, cmd); @@ -1796,23 +1795,16 @@ 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)) + bool non_numeric_ids; + if (!json_to_bool(buffer, tok, &non_numeric_ids)) return tal_fmt(plugin, "Invalid nonnumericids: %.*s", json_tok_full_len(tok), json_tok_full(buffer, tok)); - if (!plugin->non_numeric_ids - && !lightningd_deprecated_in_ok(ld, ld->log, ld->deprecated_ok, - "plugin", "nonnumericids", - "v23.08", "v24.08", NULL)) { + if (!non_numeric_ids) { return tal_fmt(plugin, "Plugin does not allow nonnumericids"); } - } else { - /* Default is false in deprecated mode */ - plugin->non_numeric_ids = !lightningd_deprecated_out_ok(ld, ld->deprecated_ok, - "plugin", "nonnumericids", - "v23.08", "v24.08"); } tok = json_get_member(buffer, resulttok, "cancheck"); @@ -2026,7 +2018,7 @@ 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->non_numeric_ids, + req = jsonrpc_request_start(p, "getmanifest", cmd_id, p->log, NULL, plugin_manifest_cb, p); json_add_bool(req->stream, "allow-deprecated-apis", p->plugins->ld->deprecated_ok); @@ -2236,7 +2228,7 @@ plugin_config(struct plugin *plugin) struct jsonrpc_request *req; plugin_set_timeout(plugin); - req = jsonrpc_request_start(plugin, "init", NULL, plugin->non_numeric_ids, + req = jsonrpc_request_start(plugin, "init", NULL, plugin->log, NULL, plugin_config_cb, plugin); plugin_populate_init_request(plugin, req); jsonrpc_request_end(req); @@ -2360,7 +2352,6 @@ struct command_result *plugin_set_dynamic_opt(struct command *cmd, return command_check_done(cmd); req = jsonrpc_request_start(cmd, "check", cmd->id, - plugin->non_numeric_ids, command_logger(cmd), NULL, plugin_setconfig_done, psr); @@ -2368,7 +2359,6 @@ struct command_result *plugin_set_dynamic_opt(struct command *cmd, } else { req = jsonrpc_request_start(cmd, "setconfig", cmd->id, - plugin->non_numeric_ids, command_logger(cmd), NULL, plugin_setconfig_done, psr); diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 9c10a2a2a3b5..2450db84d443 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -80,9 +80,6 @@ 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; diff --git a/lightningd/plugin_hook.c b/lightningd/plugin_hook.c index 0fc25278f4e4..b22d15463c10 100644 --- a/lightningd/plugin_hook.c +++ b/lightningd/plugin_hook.c @@ -203,7 +203,6 @@ static void plugin_hook_call_next(struct plugin_hook_request *ph_req) log_trace(ph_req->ld->log, "Calling %s hook of plugin %s", ph_req->hook->name, plugin->shortname); req = jsonrpc_request_start(NULL, hook->name, ph_req->cmd_id, - plugin->non_numeric_ids, plugin_get_logger(plugin), NULL, plugin_hook_callback, ph_req); @@ -346,7 +345,6 @@ 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, - dwh_req->plugin->non_numeric_ids, NULL, NULL, db_hook_response, dwh_req); diff --git a/lightningd/signmessage.c b/lightningd/signmessage.c index 07f5bb97780d..7ecc8423edde 100644 --- a/lightningd/signmessage.c +++ b/lightningd/signmessage.c @@ -220,7 +220,6 @@ static struct command_result *json_checkmessage(struct command *cmd, if (plugin) { req = jsonrpc_request_start(cmd, "listnodes", cmd->id, - plugin->non_numeric_ids, command_logger(cmd), NULL, listnodes_done, can); diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index a4d639625839..195db8a2cfc7 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -695,7 +695,6 @@ void jsonrpc_request_end(struct jsonrpc_request *request UNNEEDED) struct jsonrpc_request *jsonrpc_request_start_( const tal_t *ctx UNNEEDED, const char *method UNNEEDED, const char *id_prefix TAKES UNNEEDED, - bool id_as_string UNNEEDED, struct logger *log UNNEEDED, bool add_header UNNEEDED, void (*notify_cb)(const char *buffer UNNEEDED, const jsmntok_t *idtok UNNEEDED,