diff --git a/lightningd/configs.c b/lightningd/configs.c index 391c65634..829e10058 100644 --- a/lightningd/configs.c +++ b/lightningd/configs.c @@ -558,6 +558,9 @@ static struct command_result *setconfig_success(struct command *cmd, struct json_stream *response; const char **names, *confline; + if (command_check_only(cmd)) + return command_check_done(cmd); + names = opt_names_arr(tmpctx, ot); if (val) @@ -584,12 +587,14 @@ static struct command_result *json_setconfig(struct command *cmd, const char *val; char *err; - if (!param(cmd, buffer, params, - p_req("config", param_opt_dynamic_config, &ot), - p_opt("val", param_string, &val), - NULL)) + if (!param_check(cmd, buffer, params, + p_req("config", param_opt_dynamic_config, &ot), + p_opt("val", param_string, &val), + NULL)) return command_param_failed(); + log_debug(cmd->ld->log, "setconfig!"); + /* We don't handle DYNAMIC MULTI, at least yet! */ assert(!(ot->type & OPT_MULTI)); @@ -601,6 +606,9 @@ static struct command_result *json_setconfig(struct command *cmd, if (is_plugin_opt(ot)) return plugin_set_dynamic_opt(cmd, ot, NULL, setconfig_success); + /* FIXME: we don't have a check-only mode! */ + if (command_check_only(cmd)) + return command_check_done(cmd); err = ot->cb(ot->u.arg); } else { assert(ot->type & OPT_HASARG); @@ -611,6 +619,9 @@ static struct command_result *json_setconfig(struct command *cmd, if (is_plugin_opt(ot)) return plugin_set_dynamic_opt(cmd, ot, val, setconfig_success); + /* FIXME: we don't have a check-only mode! */ + if (command_check_only(cmd)) + return command_check_done(cmd); err = ot->cb_arg(val, ot->u.arg); } diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 0998cee04..f31670713 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -2377,12 +2377,25 @@ struct command_result *plugin_set_dynamic_opt(struct command *cmd, psr->optname = tal_strdup(psr, ot->names + 2); psr->success = success; - req = jsonrpc_request_start(cmd, "setconfig", - cmd->id, - plugin->non_numeric_ids, - command_log(cmd), - NULL, plugin_setconfig_done, - psr); + if (command_check_only(cmd)) { + /* If plugin doesn't support check, we can't check */ + if (!plugin->can_check) + return command_check_done(cmd); + req = jsonrpc_request_start(cmd, "check", + cmd->id, + plugin->non_numeric_ids, + command_log(cmd), + NULL, plugin_setconfig_done, + psr); + json_add_string(req->stream, "command_to_check", "setconfig"); + } else { + req = jsonrpc_request_start(cmd, "setconfig", + cmd->id, + plugin->non_numeric_ids, + command_log(cmd), + NULL, plugin_setconfig_done, + psr); + } json_add_string(req->stream, "config", psr->optname); if (psr->val) json_add_string(req->stream, "val", psr->val); diff --git a/plugins/autoclean.c b/plugins/autoclean.c index 0e4f9b7f4..b4b0abdd7 100644 --- a/plugins/autoclean.c +++ b/plugins/autoclean.c @@ -580,10 +580,11 @@ static const char *init(struct plugin *p, } static char *cycle_seconds_option(struct plugin *plugin, const char *arg, + bool check_only, void *unused) { - char *problem = u64_option(plugin, arg, &cycle_seconds); - if (problem) + char *problem = u64_option(plugin, arg, check_only, &cycle_seconds); + if (problem || check_only) return problem; /* If timer is not running right now, reset it to new cycle_seconds */ diff --git a/plugins/funder.c b/plugins/funder.c index 3ab1d6bb9..33ba248ad 100644 --- a/plugins/funder.c +++ b/plugins/funder.c @@ -1222,7 +1222,7 @@ param_funder_opt(struct command *cmd, const char *name, opt_str = tal_strndup(cmd, buffer + tok->start, tok->end - tok->start); - err = funding_option(cmd->plugin, opt_str, *opt); + err = funding_option(cmd->plugin, opt_str, false, *opt); if (err) return command_fail_badparam(cmd, name, buffer, tok, err); @@ -1242,7 +1242,7 @@ param_policy_mod(struct command *cmd, const char *name, arg_str = tal_strndup(cmd, buffer + tok->start, tok->end - tok->start); - err = u64_option(cmd->plugin, arg_str, *mod); + err = u64_option(cmd->plugin, arg_str, false, *mod); if (err) { tal_free(err); if (!parse_amount_sat(&sats, arg_str, strlen(arg_str))) @@ -1542,93 +1542,134 @@ const struct plugin_notification notifs[] = { }, }; -static char *option_channel_base(struct plugin *plugin, const char *arg, struct funder_policy *policy) +static char *option_channel_base(struct plugin *plugin, const char *arg, + bool check_only, struct funder_policy *policy) { struct amount_msat amt; + u32 cfmbm; if (!parse_amount_msat(&amt, arg, strlen(arg))) return tal_fmt(tmpctx, "Unable to parse amount '%s'", arg); - if (!policy->rates) - policy->rates = default_lease_rates(policy); - - if (!assign_overflow_u32(&policy->rates->channel_fee_max_base_msat, - amt.millisatoshis)) /* Raw: conversion */ + if (!assign_overflow_u32(&cfmbm, amt.millisatoshis)) /* Raw: conversion */ return tal_fmt(tmpctx, "channel_fee_max_base_msat overflowed"); + if (!check_only) { + if (!policy->rates) + policy->rates = default_lease_rates(policy); + policy->rates->channel_fee_max_base_msat = cfmbm; + } + return NULL; } static char * option_channel_fee_proportional_thousandths_max(struct plugin *plugin, const char *arg, + bool check_only, struct funder_policy *policy) { + u16 fptm; + char *problem = u16_option(plugin, arg, false, &fptm); + + if (problem || check_only) + return problem; + if (!policy->rates) policy->rates = default_lease_rates(policy); - return u16_option(plugin, arg, &policy->rates->channel_fee_max_proportional_thousandths); + policy->rates->channel_fee_max_proportional_thousandths = fptm; + return NULL; } -static char *amount_option(struct plugin *plugin, const char *arg, struct amount_sat *amt) +static char *amount_option(struct plugin *plugin, const char *arg, bool check_only, + struct amount_sat *amt) { - if (!parse_amount_sat(amt, arg, strlen(arg))) + struct amount_sat v; + if (!parse_amount_sat(&v, arg, strlen(arg))) return tal_fmt(tmpctx, "Unable to parse amount '%s'", arg); + if (!check_only) + *amt = v; return NULL; } static char *option_lease_fee_base(struct plugin *plugin, const char *arg, + bool check_only, struct funder_policy *policy) { struct amount_sat amt; + u32 lfbs; char *err; - if (!policy->rates) - policy->rates = default_lease_rates(policy); - err = amount_option(plugin, arg, &amt); + err = amount_option(plugin, arg, false, &amt); if (err) return err; - if (!assign_overflow_u32(&policy->rates->lease_fee_base_sat, - amt.satoshis)) /* Raw: conversion */ + if (!assign_overflow_u32(&lfbs, amt.satoshis)) /* Raw: conversion */ return tal_fmt(tmpctx, "lease_fee_base_sat overflowed"); + if (!check_only) { + if (!policy->rates) + policy->rates = default_lease_rates(policy); + + policy->rates->lease_fee_base_sat = lfbs; + } + return NULL; } static char *option_lease_fee_basis(struct plugin *plugin, const char *arg, + bool check_only, struct funder_policy *policy) { + u16 lfb; + char *problem = u16_option(plugin, arg, false, &lfb); + + if (problem || check_only) + return problem; + if (!policy->rates) policy->rates = default_lease_rates(policy); - return u16_option(plugin, arg, &policy->rates->lease_fee_basis); + policy->rates->lease_fee_basis = lfb; + return NULL; } static char *option_lease_weight_max(struct plugin *plugin, const char *arg, + bool check_only, struct funder_policy *policy) { + u16 fw; + char *problem = u16_option(plugin, arg, false, &fw); + + if (problem || check_only) + return problem; + if (!policy->rates) policy->rates = default_lease_rates(policy); - return u16_option(plugin, arg, &policy->rates->funding_weight); + policy->rates->funding_weight = fw; + return NULL; } static char *amount_sat_or_u64_option(struct plugin *plugin, - const char *arg, u64 *amt) + const char *arg, + bool check_only, + u64 *amt) { struct amount_sat sats; char *err; - err = u64_option(plugin, arg, amt); + err = u64_option(plugin, arg, false, &sats.satoshis); /* Raw: want sats below */ if (err) { tal_free(err); if (!parse_amount_sat(&sats, arg, strlen(arg))) return tal_fmt(tmpctx, "Unable to parse option '%s'", arg); - - *amt = sats.satoshis; /* Raw: convert to u64 */ } + if (!check_only) + *amt = sats.satoshis; /* Raw: convert to u64 */ + return NULL; } diff --git a/plugins/funder_policy.c b/plugins/funder_policy.c index 78d7d435b..5a1ae81d0 100644 --- a/plugins/funder_policy.c +++ b/plugins/funder_policy.c @@ -20,18 +20,22 @@ const char *funder_opt_name(enum funder_opt opt) abort(); } -char *funding_option(struct plugin *plugin, const char *arg, enum funder_opt *opt) +char *funding_option(struct plugin *plugin, const char *arg, bool check_only, enum funder_opt *opt) { + enum funder_opt v; if (streq(arg, "match")) - *opt = MATCH; + v = MATCH; else if (streq(arg, "available")) - *opt = AVAILABLE; + v = AVAILABLE; else if (streq(arg, "fixed")) - *opt = FIXED; + v = FIXED; else return tal_fmt(tmpctx, "'%s' is not a valid option" " (match, available, fixed)", arg); + + if (!check_only) + *opt = v; return NULL; } diff --git a/plugins/funder_policy.h b/plugins/funder_policy.h index be55955df..09da9bfac 100644 --- a/plugins/funder_policy.h +++ b/plugins/funder_policy.h @@ -94,7 +94,8 @@ const char *funder_policy_desc(const tal_t *ctx, const struct funder_policy *policy); /* Convert a cmdline option to a funding_opt */ -char *funding_option(struct plugin *plugin, const char *arg, enum funder_opt *opt); +char *funding_option(struct plugin *plugin, const char *arg, bool check_only, + enum funder_opt *opt); /* Check policy settings, return error if fails */ char *funder_check_policy(const struct funder_policy *policy); diff --git a/plugins/libplugin.c b/plugins/libplugin.c index d7657d15e..56804398f 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -39,6 +39,24 @@ struct jstream { struct json_stream *js; }; +/* Create an array of these, one for each --option you support. */ +struct plugin_option { + const char *name; + const char *type; + const char *description; + /* Handle an option. If dynamic, check_only may be set to check + * for validity (but must not make any changes!) */ + char *(*handle)(struct plugin *plugin, const char *str, bool check_only, + void *arg); + void *arg; + /* If true, this option requires --developer to be enabled */ + bool dev_only; + /* If it's deprecated from a particular release (or NULL) */ + const char *depr_start, *depr_end; + /* If true, allow setting after plugin has initialized */ + bool dynamic; +}; + struct plugin { /* lightningd interaction */ struct io_conn *stdin_conn; @@ -1342,7 +1360,7 @@ static struct command_result *handle_init(struct command *cmd, if (!popt) plugin_err(p, "lightningd specified unknown option '%s'?", name); - problem = popt->handle(p, json_strdup(tmpctx, buf, t+1), popt->arg); + problem = popt->handle(p, json_strdup(tmpctx, buf, t+1), false, popt->arg); if (problem) plugin_err(p, "option '%s': %s", popt->name, problem); } @@ -1366,82 +1384,82 @@ static struct command_result *handle_init(struct command *cmd, return command_success(cmd, json_out_obj(cmd, NULL, NULL)); } -char *u64_option(struct plugin *plugin, const char *arg, u64 *i) +char *u64_option(struct plugin *plugin, const char *arg, bool check_only, u64 *i) { char *endp; + u64 v; /* This is how the manpage says to do it. Yech. */ errno = 0; - *i = strtol(arg, &endp, 0); + v = strtoul(arg, &endp, 0); if (*endp || !arg[0]) return tal_fmt(tmpctx, "'%s' is not a number", arg); if (errno) return tal_fmt(tmpctx, "'%s' is out of range", arg); + if (!check_only) + *i = v; return NULL; } -char *u32_option(struct plugin *plugin, const char *arg, u32 *i) +char *u32_option(struct plugin *plugin, const char *arg, bool check_only, u32 *i) { - char *endp; u64 n; + char *problem = u64_option(plugin, arg, false, &n); - errno = 0; - n = strtoul(arg, &endp, 0); - if (*endp || !arg[0]) - return tal_fmt(tmpctx, "'%s' is not a number", arg); - if (errno) - return tal_fmt(tmpctx, "'%s' is out of range", arg); + if (problem) + return problem; - *i = n; - if (*i != n) + if ((u32)n != n) return tal_fmt(tmpctx, "'%s' is too large (overflow)", arg); + if (!check_only) + *i = n; return NULL; } -char *u16_option(struct plugin *plugin, const char *arg, u16 *i) +char *u16_option(struct plugin *plugin, const char *arg, bool check_only, u16 *i) { - char *endp; u64 n; + char *problem = u64_option(plugin, arg, false, &n); - errno = 0; - n = strtoul(arg, &endp, 0); - if (*endp || !arg[0]) - return tal_fmt(tmpctx, "'%s' is not a number", arg); - if (errno) - return tal_fmt(tmpctx, "'%s' is out of range", arg); + if (problem) + return problem; - *i = n; - if (*i != n) + if ((u16)n != n) return tal_fmt(tmpctx, "'%s' is too large (overflow)", arg); + if (!check_only) + *i = n; return NULL; } -char *bool_option(struct plugin *plugin, const char *arg, bool *i) +char *bool_option(struct plugin *plugin, const char *arg, bool check_only, bool *i) { if (!streq(arg, "true") && !streq(arg, "false")) return tal_fmt(tmpctx, "'%s' is not a bool, must be \"true\" or \"false\"", arg); - *i = streq(arg, "true"); + if (!check_only) + *i = streq(arg, "true"); return NULL; } -char *flag_option(struct plugin *plugin, const char *arg, bool *i) +char *flag_option(struct plugin *plugin, const char *arg, bool check_only, bool *i) { /* We only get called if the flag was provided, so *i should be false * by default */ - assert(*i == false); + assert(check_only || *i == false); if (!streq(arg, "true")) return tal_fmt(tmpctx, "Invalid argument '%s' passed to a flag", arg); - *i = true; + if (!check_only) + *i = true; return NULL; } -char *charp_option(struct plugin *plugin, const char *arg, char **p) +char *charp_option(struct plugin *plugin, const char *arg, bool check_only, char **p) { - *p = tal_strdup(NULL, arg); + if (!check_only) + *p = tal_strdup(NULL, arg); return NULL; } @@ -1837,6 +1855,8 @@ static void ld_command_handle(struct plugin *plugin, const char *config, *val, *problem; struct plugin_option *popt; struct command_result *ret; + bool check_only; + config = json_strdup(tmpctx, plugin->buffer, json_get_member(plugin->buffer, paramstok, "config")); popt = find_opt(plugin, config); @@ -1850,11 +1870,9 @@ static void ld_command_handle(struct plugin *plugin, "lightningd setconfig non-dynamic option '%s'?", config); } - /* FIXME: allow checking into handler! */ - if (command_check_only(cmd)) { - ret = command_check_done(cmd); - return; - } + + check_only = command_check_only(cmd); + plugin_log(plugin, LOG_DBG, "setconfig %s check_only=%i", config, check_only); valtok = json_get_member(plugin->buffer, paramstok, "val"); if (valtok) @@ -1862,12 +1880,16 @@ static void ld_command_handle(struct plugin *plugin, else val = "true"; - problem = popt->handle(plugin, val, popt->arg); + problem = popt->handle(plugin, val, check_only, popt->arg); if (problem) ret = command_fail(cmd, JSONRPC2_INVALID_PARAMS, "%s", problem); - else - ret = command_finished(cmd, jsonrpc_stream_success(cmd)); + else { + if (check_only) + ret = command_check_done(cmd); + else + ret = command_finished(cmd, jsonrpc_stream_success(cmd)); + } assert(ret == &complete); return; } @@ -2076,7 +2098,7 @@ static struct plugin *new_plugin(const tal_t *ctx, o.name = optname; o.type = va_arg(ap, const char *); o.description = va_arg(ap, const char *); - o.handle = va_arg(ap, char *(*)(struct plugin *, const char *str, void *arg)); + o.handle = va_arg(ap, char *(*)(struct plugin *, const char *str, bool check_only, void *arg)); o.arg = va_arg(ap, void *); o.dev_only = va_arg(ap, int); /* bool gets promoted! */ o.depr_start = va_arg(ap, const char *); diff --git a/plugins/libplugin.h b/plugins/libplugin.h index 3e24391d1..e294fbe52 100644 --- a/plugins/libplugin.h +++ b/plugins/libplugin.h @@ -74,21 +74,6 @@ struct plugin_command { bool dev_only; }; -/* Create an array of these, one for each --option you support. */ -struct plugin_option { - const char *name; - const char *type; - const char *description; - char *(*handle)(struct plugin *plugin, const char *str, void *arg); - void *arg; - /* If true, this option requires --developer to be enabled */ - bool dev_only; - /* If it's deprecated from a particular release (or NULL) */ - const char *depr_start, *depr_end; - /* If true, allow setting after plugin has initialized */ - bool dynamic; -}; - /* Create an array of these, one for each notification you subscribe to. */ struct plugin_notification { /* "*" means wildcard: notify me on everything (should be last!) */ @@ -424,7 +409,9 @@ void plugin_notify_progress(struct command *cmd, /* Simply exists to check that `set` to plugin_option* is correct type */ static inline void *plugin_option_cb_check(char *(*set)(struct plugin *plugin, - const char *arg, void *)) + const char *arg, + bool check_only, + void *)) { return set; } @@ -440,7 +427,7 @@ bool plugin_developer_mode(const struct plugin *plugin); plugin_option_cb_check(typesafe_cb_preargs(char *, void *, \ (set), (arg), \ struct plugin *, \ - const char *)), \ + const char *, bool)),\ (arg), \ (dev_only), \ (depr_start), \ @@ -460,12 +447,12 @@ bool plugin_developer_mode(const struct plugin *plugin); plugin_option_((name), (type), (description), (set), (arg), false, (depr_start), (depr_end), false) /* Standard helpers */ -char *u64_option(struct plugin *plugin, const char *arg, u64 *i); -char *u32_option(struct plugin *plugin, const char *arg, u32 *i); -char *u16_option(struct plugin *plugin, const char *arg, u16 *i); -char *bool_option(struct plugin *plugin, const char *arg, bool *i); -char *charp_option(struct plugin *plugin, const char *arg, char **p); -char *flag_option(struct plugin *plugin, const char *arg, bool *i); +char *u64_option(struct plugin *plugin, const char *arg, bool check_only, u64 *i); +char *u32_option(struct plugin *plugin, const char *arg, bool check_only, u32 *i); +char *u16_option(struct plugin *plugin, const char *arg, bool check_only, u16 *i); +char *bool_option(struct plugin *plugin, const char *arg, bool check_only, bool *i); +char *charp_option(struct plugin *plugin, const char *arg, bool check_only, char **p); +char *flag_option(struct plugin *plugin, const char *arg, bool check_only, bool *i); /* The main plugin runner: append with 0 or more plugin_option(), then NULL. */ void NORETURN LAST_ARG_NULL plugin_main(char *argv[], diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 7f2933749..1b80dc82d 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -3133,6 +3133,14 @@ def test_autoclean(node_factory): with pytest.raises(RpcError, match=r'is not a number'): l3.rpc.setconfig('autoclean-expiredinvoices-age', 'xxx') + # check gives the same answer. + with pytest.raises(RpcError, match=r'is not a number'): + l3.rpc.check('setconfig', config='autoclean-expiredinvoices-age', val='xxx') + + # check does not actually set! + l3.rpc.check('setconfig', config='autoclean-expiredinvoices-age', val=2) == {'command_to_check': 'setconfig'} + assert l3.rpc.autoclean_status()['autoclean']['expiredinvoices']['enabled'] is False + l3.rpc.setconfig('autoclean-expiredinvoices-age', 2) assert l3.rpc.autoclean_status()['autoclean']['expiredinvoices']['enabled'] is True assert l3.rpc.autoclean_status()['autoclean']['expiredinvoices']['age'] == 2