From 0539ebc28b17e42ed625bc8d22e35dd8a8c0c9db Mon Sep 17 00:00:00 2001 From: daywalker90 Date: Thu, 6 Aug 2026 15:36:13 +0200 Subject: [PATCH] lightningd: accept options array in plugin start The plugin start command only accepted plugin options flattened onto the RPC call (e.g. via -k/--keyword), but the plugin RPC schema documents an explicit 'options' array. This mismatch meant that callers using named parameters against generated RPC bindings (cln-rpc, grpc, protobuf), which cannot flatten arbitrary options, failed with 'unknown parameter options'. Options without a value are treated as boolean flags, matching the flattened form. Changelog-Fixed: JSON-RPC: `plugin start` now accepts plugin options as an `options` array of `keyword=value` strings, as documented in the `plugin` schema. --- contrib/msggen/msggen/schema.json | 2 +- doc/schemas/plugin.json | 2 +- lightningd/plugin_control.c | 93 ++++++++++++++++++++++++- tests/plugins/dynamic_option.py | 36 ++++++++-- tests/test_plugin.py | 108 +++++++++++++++++++++++++++++- 5 files changed, 228 insertions(+), 13 deletions(-) diff --git a/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json index c3dabf28e48f..cc1b9ee5df71 100644 --- a/contrib/msggen/msggen/schema.json +++ b/contrib/msggen/msggen/schema.json @@ -32987,7 +32987,7 @@ ], "description": [ "Determines what action is taken:", - " - *subcommand* **start** takes a *path* to an executable as argument and starts it as plugin. *path* may be an absolute path or a path relative to the plugins directory (default *~/.lightning/plugins*). If the plugin is already running and the executable (checksum) has changed, the plugin is killed and restarted except if its an important (or builtin) plugin. If the plugin doesn't complete the 'getmanifest' and 'init' handshakes within 60 seconds, the command will timeout and kill the plugin. Additional *options* may be passed to the plugin, but requires all parameters to be passed as keyword=value pairs using the `-k|--keyword` option which is recommended. For example the following command starts the plugin helloworld.py (present in the plugin directory) with the option greeting set to 'A crazy':", + " - *subcommand* **start** takes a *path* to an executable as argument and starts it as plugin. *path* may be an absolute path or a path relative to the plugins directory (default *~/.lightning/plugins*). If the plugin is already running and the executable (checksum) has changed, the plugin is killed and restarted except if its an important (or builtin) plugin. If the plugin doesn't complete the 'getmanifest' and 'init' handshakes within 60 seconds, the command will timeout and kill the plugin. Additional *options* may be passed to the plugin, either as an *options* array of *keyword=value* strings, or as flattened keyword=value parameters using the `-k|--keyword` option which is recommended. For example the following command starts the plugin helloworld.py (present in the plugin directory) with the option greeting set to 'A crazy':", " ```shell", " lightning-cli -k plugin subcommand=start plugin=helloworld.py greeting='A crazy'", " ```", diff --git a/doc/schemas/plugin.json b/doc/schemas/plugin.json index 11e643a24674..a635f4673912 100644 --- a/doc/schemas/plugin.json +++ b/doc/schemas/plugin.json @@ -30,7 +30,7 @@ ], "description": [ "Determines what action is taken:", - " - *subcommand* **start** takes a *path* to an executable as argument and starts it as plugin. *path* may be an absolute path or a path relative to the plugins directory (default *~/.lightning/plugins*). If the plugin is already running and the executable (checksum) has changed, the plugin is killed and restarted except if its an important (or builtin) plugin. If the plugin doesn't complete the 'getmanifest' and 'init' handshakes within 60 seconds, the command will timeout and kill the plugin. Additional *options* may be passed to the plugin, but requires all parameters to be passed as keyword=value pairs using the `-k|--keyword` option which is recommended. For example the following command starts the plugin helloworld.py (present in the plugin directory) with the option greeting set to 'A crazy':", + " - *subcommand* **start** takes a *path* to an executable as argument and starts it as plugin. *path* may be an absolute path or a path relative to the plugins directory (default *~/.lightning/plugins*). If the plugin is already running and the executable (checksum) has changed, the plugin is killed and restarted except if its an important (or builtin) plugin. If the plugin doesn't complete the 'getmanifest' and 'init' handshakes within 60 seconds, the command will timeout and kill the plugin. Additional *options* may be passed to the plugin, either as an *options* array of *keyword=value* strings, or as flattened keyword=value parameters using the `-k|--keyword` option which is recommended. For example the following command starts the plugin helloworld.py (present in the plugin directory) with the option greeting set to 'A crazy':", " ```shell", " lightning-cli -k plugin subcommand=start plugin=helloworld.py greeting='A crazy'", " ```", diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 89073f9904fa..30051083fa7d 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -1,4 +1,5 @@ #include "config.h" +#include #include #include #include @@ -86,6 +87,59 @@ plugin_dynamic_start(struct plugin_command *pcmd, const char *plugin_path, return command_still_pending(pcmd->cmd); } +/* Returns true if params is an object with keys other than the fixed + * subcommand/plugin/options. */ +static bool plugin_has_extra_params(const char *buffer, + const jsmntok_t *params) +{ + size_t i; + const jsmntok_t *t; + + json_for_each_obj(i, t, params) + if (!json_tok_streq(buffer, t, "subcommand") + && !json_tok_streq(buffer, t, "plugin") + && !json_tok_streq(buffer, t, "options")) + return true; + return false; +} + +/* Build an object of name/value pairs from an "options" array of + * "keyword=value" strings, suitable for plugin_add_params(). An element + * without an '=' is treated as a boolean flag. */ +static jsmntok_t *plugin_start_params(const tal_t *ctx, const char *buffer, + const jsmntok_t *options, char **parambuf) +{ + size_t i; + const jsmntok_t *t; + char *newbuf = tal_strdup(ctx, "{"); + bool first = true; + + json_for_each_arr(i, t, options) { + const char *opt = json_strdup(tmpctx, buffer, t); + const char *eq = strchr(opt, '='); + struct json_escape *esc; + + if (!first) + tal_append_fmt(&newbuf, ","); + first = false; + if (eq) { + esc = json_escape(tmpctx, + take(tal_strndup(tmpctx, opt, eq - opt))); + tal_append_fmt(&newbuf, "\"%s\":", esc->s); + esc = json_escape(tmpctx, eq + 1); + tal_append_fmt(&newbuf, "\"%s\"", esc->s); + } else { + /* Boolean flags are given without a value. */ + esc = json_escape(tmpctx, opt); + tal_append_fmt(&newbuf, "\"%s\":true", esc->s); + } + } + + tal_append_fmt(&newbuf, "}"); + *parambuf = newbuf; + return json_parse_simple(ctx, newbuf, strlen(newbuf)); +} + /** * Called when trying to start a plugin directory through RPC, it registers * all contained plugins recursively and then starts them. @@ -235,23 +289,54 @@ static struct command_result *json_plugin_control(struct command *cmd, return plugin_dynamic_stop(cmd, plugin_name); } else if (streq(subcmd, "start")) { const char *plugin_path; + const jsmntok_t *options = NULL; + char *mod_buffer; jsmntok_t *mod_params; if (!param_check(cmd, buffer, params, p_req("subcommand", param_ignore, cmd), p_req("plugin", param_string, &plugin_path), + p_opt("options", param_array, &options), p_opt_any(), NULL)) return command_param_failed(); + /* The "options" array is documented as containing keyword=value + * strings: reject anything else before we mangle it. */ + if (options) { + size_t i; + const jsmntok_t *t; + + json_for_each_arr(i, t, options) + if (t->type != JSMN_STRING) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "options array entries must be strings"); + } + /* Manually parse any remaining options (only for objects, * since plugin options must be explicitly named!). */ if (params->type == JSMN_ARRAY) { - if (params->size != 2) + if (params->size > (options ? 3 : 2)) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Extra parameters must be in object"); - mod_params = NULL; + if (options) { + mod_params = plugin_start_params(cmd, buffer, + options, &mod_buffer); + } else { + mod_buffer = NULL; + mod_params = NULL; + } + } else if (options) { + /* Either flattened keyword options or an explicit + * "options" array, never both. */ + if (plugin_has_extra_params(buffer, params)) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Cannot mix 'options' array " + "with keyword options"); + mod_params = plugin_start_params(cmd, buffer, + options, &mod_buffer); } else { + mod_buffer = NULL; mod_params = json_tok_copy(cmd, params); json_tok_remove(&mod_params, mod_params, @@ -271,7 +356,9 @@ static struct command_result *json_plugin_control(struct command *cmd, if (command_check_only(cmd)) return command_check_done(cmd); - return plugin_dynamic_start(pcmd, plugin_path, buffer, mod_params); + return plugin_dynamic_start(pcmd, plugin_path, + mod_buffer ? mod_buffer : buffer, + mod_params); } else if (streq(subcmd, "startdir")) { const char *dir_path; diff --git a/tests/plugins/dynamic_option.py b/tests/plugins/dynamic_option.py index 7ac3717ffe89..686381fa249a 100755 --- a/tests/plugins/dynamic_option.py +++ b/tests/plugins/dynamic_option.py @@ -5,16 +5,20 @@ plugin = Plugin() -@plugin.method('dynamic-option-report') +@plugin.method("dynamic-option-report") def record_lookup(plugin): - return {'test-dynamic-config': plugin.get_option('test-dynamic-config')} + return { + "test-dynamic-config": plugin.get_option("test-dynamic-config"), + "test-dynamic-int": plugin.get_option("test-dynamic-int"), + "test-dynamic-bool": plugin.get_option("test-dynamic-bool"), + "test-dynamic-flag": plugin.get_option("test-dynamic-flag"), + } def on_config_change(plugin, config: str, value: Optional[Any]) -> None: - """Callback method called when a config value is changed. - """ + """Callback method called when a config value is changed.""" plugin.log(f"Setting config {config} to {value}") - if value == 'bad value': + if value == "bad value": raise RpcException("I don't like bad values!") @@ -26,5 +30,27 @@ def on_config_change(plugin, config: str, value: Optional[Any]) -> None: on_change=on_config_change, ) +plugin.add_option( + name="test-dynamic-int", + description="An int option which can be changed at run-time", + default=0, + opt_type="int", + dynamic=True, +) + +plugin.add_option( + name="test-dynamic-bool", + description="A bool option which can be changed at run-time", + default=False, + opt_type="bool", + dynamic=True, +) + +plugin.add_flag_option( + name="test-dynamic-flag", + description="A flag option which can be changed at run-time", + dynamic=True, +) + plugin.run() diff --git a/tests/test_plugin.py b/tests/test_plugin.py index bb4604aaf33c..3ce7f85567d4 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -3060,6 +3060,108 @@ def test_dynamic_args(node_factory): assert 'greeting' not in l1.rpc.listconfigs()['configs'] +def test_dynamic_args_options_array(node_factory): + """plugin start accepts options as an explicit array, matching the schema.""" + plugin_path = os.path.join(os.getcwd(), "tests/plugins/dynamic_option.py") + + l1 = node_factory.get_node() + + # Positionally, an options array is a valid 3rd argument, covering every + # option type: string, int, bool and flag (which has no '='). + l1.rpc.call( + "plugin", + [ + "start", + plugin_path, + [ + "test-dynamic-config=Test options array", + "test-dynamic-int=42", + "test-dynamic-bool=false", + "test-dynamic-flag", + ], + ], + ) + assert l1.rpc.dynamic_option_report() == { + "test-dynamic-config": "Test options array", + "test-dynamic-int": 42, + "test-dynamic-bool": False, + "test-dynamic-flag": True, + } + l1.rpc.plugin_stop(plugin_path) + + l1.rpc.call( + "plugin", + { + "subcommand": "start", + "plugin": plugin_path, + "options": [ + "test-dynamic-config=Test options array", + "test-dynamic-int=42", + "test-dynamic-bool=false", + "test-dynamic-flag", + ], + }, + ) + assert l1.rpc.dynamic_option_report() == { + "test-dynamic-config": "Test options array", + "test-dynamic-int": 42, + "test-dynamic-bool": False, + "test-dynamic-flag": True, + } + l1.rpc.plugin_stop(plugin_path) + + l1.rpc.call( + "plugin", + { + "subcommand": "start", + "plugin": plugin_path, + "test-dynamic-config": "Test options array", + "test-dynamic-int": 42, + "test-dynamic-bool": False, + "test-dynamic-flag": None, + }, + ) + assert l1.rpc.dynamic_option_report() == { + "test-dynamic-config": "Test options array", + "test-dynamic-int": 42, + "test-dynamic-bool": False, + "test-dynamic-flag": None, + } + l1.rpc.plugin_stop(plugin_path) + + # Entries must be strings, not e.g. numbers. + with pytest.raises(RpcError, match="options array entries must be strings"): + l1.rpc.call("plugin", ["start", plugin_path, [42]]) + + # ...but any trailing positional args are ambiguous, so must be rejected. + with pytest.raises(RpcError, match="Extra parameters must be in object"): + l1.rpc.call( + "plugin", + [ + "start", + plugin_path, + ["test-dynamic-config=Test options array"], + "test-dynamic-config=yikes", + ], + ) + + # The same applies if there is no options array at all. + with pytest.raises(RpcError, match="Extra parameters must be in object"): + l1.rpc.call("plugin", ["start", plugin_path, None, "test-dynamic-config=yikes"]) + + # ...and flattened keyword options cannot be mixed with an options array. + with pytest.raises(RpcError, match="Cannot mix"): + l1.rpc.call( + "plugin", + { + "subcommand": "start", + "plugin": plugin_path, + "options": ["test-dynamic-config=Test options array"], + "test-dynamic-config": "yikes", + }, + ) + + def test_pyln_request_notify(node_factory): """Test that pyln-client plugins can send notifications. """ @@ -4654,10 +4756,10 @@ def test_dynamic_option_python_plugin(node_factory): assert result["configs"]["test-dynamic-config"]["value_str"] == "initial" - assert l1.rpc.dynamic_option_report() == {'test-dynamic-config': 'initial'} + assert l1.rpc.dynamic_option_report()['test-dynamic-config'] == 'initial' result = l1.rpc.setconfig("test-dynamic-config", "changed") assert result["config"]["value_str"] == "changed" - assert l1.rpc.dynamic_option_report() == {'test-dynamic-config': 'changed'} + assert l1.rpc.dynamic_option_report()['test-dynamic-config'] == 'changed' l1.daemon.wait_for_log( 'dynamic_option.py:.*Setting config test-dynamic-config to changed' @@ -4667,7 +4769,7 @@ def test_dynamic_option_python_plugin(node_factory): l1.rpc.setconfig("test-dynamic-config", "bad value") # Does not alter value! - assert l1.rpc.dynamic_option_report() == {'test-dynamic-config': 'changed'} + assert l1.rpc.dynamic_option_report()['test-dynamic-config'] == 'changed' def test_renepay_not_important(node_factory):