diff --git a/contrib/pyln-client/pyln/client/plugin.py b/contrib/pyln-client/pyln/client/plugin.py index dbc1cbdd0bd9..0446300a625e 100644 --- a/contrib/pyln-client/pyln/client/plugin.py +++ b/contrib/pyln-client/pyln/client/plugin.py @@ -250,8 +250,8 @@ def add_option(self, name, default, description, opt_type="string"): "Name {} is already used by another option".format(name) ) - if opt_type not in ["string", "int", "bool"]: - raise ValueError('{} not in supported type set (string, int, bool)') + if opt_type not in ["string", "int", "bool", "flag"]: + raise ValueError('{} not in supported type set (string, int, bool, flag)') self.options[name] = { 'name': name, @@ -261,6 +261,15 @@ def add_option(self, name, default, description, opt_type="string"): 'value': None, } + def add_flag_option(self, name, description): + """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") + def get_option(self, name): if name not in self.options: raise ValueError("No option with name {} registered".format(name)) diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index 089411d88e4f..0d3b0f5f5f90 100644 --- a/doc/PLUGINS.md +++ b/doc/PLUGINS.md @@ -104,7 +104,7 @@ this example: The `options` will be added to the list of command line options that `lightningd` accepts. The above will add a `--greeting` option with a default value of `World` and the specified description. *Notice that -currently string, (unsigned) integers, and bool options are supported.* +currently string, integers, bool, and flag options are supported.* The `rpcmethods` are methods that will be exposed via `lightningd`'s JSON-RPC over Unix-Socket interface, just like the builtin @@ -139,6 +139,48 @@ methods, such as `help` and `getinfo`, as well as methods registered by other plugins. If there is a conflict then `lightningd` will report an error and exit. +#### Types of Options + +There are currently four supported option 'types': + - string: a string + - bool: a boolean + - int: parsed as a signed integer (64-bit) + - flag: no-arg flag option. Is boolean under the hood. Defaults to false. + +Nota bene: if a `flag` type option is not set, it will not appear +in the options set that is passed to the plugin. + +Here's an example option set, as sent in response to `getmanifest` + +```json + "options": [ + { + "name": "greeting", + "type": "string", + "default": "World", + "description": "What name should I call you?" + }, + { + "name": "run-hot", + "type": "flag", + "default": None, // defaults to false + "description": "If set, overclocks plugin" + }, + { + "name": "is_online", + "type": "bool", + "default": false, + "description": "Set to true if plugin can use network" + }, + { + "name": "service-port", + "type": "int", + "default": 6666, + "description": "Port to use to connect to 3rd-party service" + } + ], +``` + ### The `init` method The `init` method is required so that `lightningd` can pass back the diff --git a/lightningd/options.c b/lightningd/options.c index d76b30f6feb9..b9631b3b175f 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -1272,7 +1272,8 @@ static void add_config(struct lightningd *ld, json_add_opt_log_levels(response, ld->log); } else if (opt->cb_arg == (void *)opt_add_plugin_dir || opt->cb_arg == (void *)opt_disable_plugin - || opt->cb_arg == (void *)plugin_opt_set) { + || opt->cb_arg == (void *)plugin_opt_set + || opt->cb_arg == (void *)plugin_opt_flag_set) { /* FIXME: We actually treat it as if they specified * --plugin for each one, so ignore these */ #if DEVELOPER diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 04948a0ce41a..6abd0e0637ae 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -464,6 +464,13 @@ struct io_plan *plugin_stdout_conn_init(struct io_conn *conn, plugin_read_json, plugin); } +char *plugin_opt_flag_set(struct plugin_opt *popt) +{ + /* A set flag is a true */ + *popt->value->as_bool = true; + return NULL; +} + char *plugin_opt_set(const char *arg, struct plugin_opt *popt) { char *endp; @@ -490,7 +497,7 @@ char *plugin_opt_set(const char *arg, struct plugin_opt *popt) if (streq(arg, "true") || streq(arg, "True") || streq(arg, "1")) { *popt->value->as_bool = true; } else if (streq(arg, "false") || streq(arg, "False") - || streq(arg, "0") || streq(arg, "")) { + || streq(arg, "0")) { *popt->value->as_bool = false; } else return tal_fmt(tmpctx, "%s does not parse as type %s", @@ -551,15 +558,29 @@ static bool plugin_opt_add(struct plugin *plugin, const char *buffer, popt, "%.*s (default: %s)", desctok->end - desctok->start, buffer + desctok->start, *popt->value->as_bool ? "true" : "false"); } + } else if (json_tok_streq(buffer, typetok, "flag")) { + popt->type = "flag"; + popt->value->as_bool = talz(popt->value, bool); + popt->description = json_strdup(popt, buffer, desctok); + /* We default flags to false, the default token is ignored */ + *popt->value->as_bool = false; + } else { - plugin_kill(plugin, "Only \"string\", \"int\", and \"bool\" options are supported"); + plugin_kill(plugin, "Only \"string\", \"int\", \"bool\", and \"flag\" options are supported"); return false; } if (!defaulttok) popt->description = json_strdup(popt, buffer, desctok); list_add_tail(&plugin->plugin_opts, &popt->list); - opt_register_arg(popt->name, plugin_opt_set, NULL, popt, - popt->description); + + if (streq(popt->type, "flag")) + opt_register_noarg(popt->name, plugin_opt_flag_set, popt, + popt->description); + + else + opt_register_arg(popt->name, plugin_opt_set, NULL, popt, + popt->description); + return true; } @@ -1127,6 +1148,11 @@ plugin_populate_init_request(struct plugin *plugin, struct jsonrpc_request *req) /* Trim the `--` that we added before */ name = opt->name + 2; if (opt->value->as_bool) { + /* We don't include 'flag' types if they're not + * flagged on */ + if (streq(opt->type, "flag") && !*opt->value->as_bool) + continue; + json_add_bool(req->stream, name, *opt->value->as_bool); if (!deprecated_apis) continue; diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 350115adad90..d0223aa685cb 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -254,6 +254,12 @@ void plugin_request_send(struct plugin *plugin, */ char *plugin_opt_set(const char *arg, struct plugin_opt *popt); +/** + * Callback called when plugin flag-type options.It just stores + * the value in the plugin_opt + */ +char *plugin_opt_flag_set(struct plugin_opt *popt); + /** * Helpers to initialize a connection to a plugin; we read from their * stdout, and write to their stdin. diff --git a/tests/plugins/options.py b/tests/plugins/options.py index cb08d0b0bb8b..6eb7bb6c52b1 100755 --- a/tests/plugins/options.py +++ b/tests/plugins/options.py @@ -17,4 +17,5 @@ def init(configuration, options, plugin): plugin.add_option('str_opt', 'i am a string', 'an example string option') plugin.add_option('int_opt', 7, 'an example int type option', opt_type='int') plugin.add_option('bool_opt', True, 'an example bool type option', opt_type='bool') +plugin.add_flag_option('flag_opt', 'an example flag type option') plugin.run() diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 440b0dd4575c..2397f8d6e958 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -58,19 +58,23 @@ def test_option_types(node_factory): 'bool_opt': True, }) - n.daemon.is_in_log(r"option str_opt ok ") - n.daemon.is_in_log(r"option int_opt 22 ") - n.daemon.is_in_log(r"option bool_opt True ") + assert n.daemon.is_in_log(r"option str_opt ok ") + assert n.daemon.is_in_log(r"option int_opt 22 ") + assert n.daemon.is_in_log(r"option bool_opt True ") + # flag options aren't passed through if not flagged on + assert not n.daemon.is_in_log(r"option flag_opt") n.stop() # A blank bool_opt should default to false n = node_factory.get_node(options={ 'plugin': plugin_path, 'str_opt': 'ok', 'int_opt': 22, - 'bool_opt': '', + 'bool_opt': 'true', + 'flag_opt': None, }) - n.daemon.is_in_log(r"option bool_opt False ") + assert n.daemon.is_in_log(r"option bool_opt True ") + assert n.daemon.is_in_log(r"option flag_opt True ") n.stop() # What happens if we give it a bad bool-option? @@ -97,19 +101,36 @@ def test_option_types(node_factory): assert not n.daemon.running assert n.daemon.is_in_stderr('--int_opt: notok does not parse as type int') + # Flag opts shouldn't allow any input + n = node_factory.get_node(options={ + 'plugin': plugin_path, + 'str_opt': 'ok', + 'int_opt': 11, + 'bool_opt': 1, + 'flag_opt': True, + }, may_fail=True, expect_fail=True) + + # the node should fail to start, and we get a stderr msg + assert not n.daemon.running + assert n.daemon.is_in_stderr("--flag_opt: doesn't allow an argument") + plugin_path = os.path.join(os.getcwd(), 'tests/plugins/options.py') n = node_factory.get_node(options={ 'plugin': plugin_path, 'str_opt': 'ok', 'int_opt': 22, 'bool_opt': 1, + 'flag_opt': None, + 'allow-deprecated-apis': True }) - n.daemon.is_in_log(r"option str_opt ok ") - n.daemon.is_in_log(r"option int_opt 22 ") - n.daemon.is_in_log(r"option int_opt 22 ") - n.daemon.is_in_log(r"option bool_opt True ") - n.daemon.is_in_log(r"option bool_opt true ") + # because of how the python json parser works, since we're adding the deprecated + # string option after the 'typed' option in the JSON, the string option overwrites + # the earlier typed option in JSON parsing, resulting in a option set of only strings + assert n.daemon.is_in_log(r"option str_opt ok ") + assert n.daemon.is_in_log(r"option int_opt 22 ") + assert n.daemon.is_in_log(r"option bool_opt 1 ") + assert n.daemon.is_in_log(r"option flag_opt True ") n.stop()