libplugin: allow check setconfig on all dynamic options.

We need to pass through setconfig in check mode, then we need to have libplugin
add (and use!) a `check_only` parameter to all option setting.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `check` `setconfig` on plugin options can now check the config value would be accepted.
This commit is contained in:
Rusty Russell 2024-04-04 14:06:12 +10:30
parent 03db143216
commit dcbdb85497
9 changed files with 189 additions and 101 deletions

View File

@ -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);
}

View File

@ -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);

View File

@ -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 */

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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);

View File

@ -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 *);

View File

@ -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[],

View File

@ -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