From cf72fb418e16a8766fc9ee8d158fef3af748fb18 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 4 Apr 2024 14:06:11 +1030 Subject: [PATCH] param: generalize check handling a little. We want to extend it to plugins, and we want it to be allowed to be async for more power, so rather than not completing the cmd if we're checking, do it in command_check_done() and call it. This is cleaner than the special case we had before, and allows check to us all the normal jsonrpc mechanisms, especially async requests (which we'll need if we want to hand check requests to plugins!). Signed-off-by: Rusty Russell --- common/json_command.h | 5 +++++ common/json_param.c | 8 ++++++-- common/test/run-json_filter.c | 4 ++++ common/test/run-json_remove.c | 4 ++++ common/test/run-param.c | 4 ++++ lightningd/jsonrpc.c | 34 ++++++++++------------------------ lightningd/jsonrpc.h | 5 ----- lightningd/plugin.c | 5 +++-- plugins/libplugin.c | 5 +++++ 9 files changed, 41 insertions(+), 33 deletions(-) diff --git a/common/json_command.h b/common/json_command.h index 163973020..33168ca1c 100644 --- a/common/json_command.h +++ b/common/json_command.h @@ -57,4 +57,9 @@ void command_set_usage(struct command *cmd, const char *usage); /* Also caller supplied: is this invoked simply to check parameters? */ bool command_check_only(const struct command *cmd); +/* To return after param_check() succeeds but we're still + * command_check_only(cmd). */ +struct command_result *command_check_done(struct command *cmd) + WARN_UNUSED_RESULT; + #endif /* LIGHTNING_COMMON_JSON_COMMAND_H */ diff --git a/common/json_param.c b/common/json_param.c index ab4c6e2e5..6b6196fe3 100644 --- a/common/json_param.c +++ b/common/json_param.c @@ -378,9 +378,13 @@ bool param(struct command *cmd, ret = param_core(cmd, buffer, tokens, ap); va_end(ap); - /* Always fail if we're just checking! */ - if (ret && command_check_only(cmd)) + /* Always "fail" if we're just checking! */ + if (ret && command_check_only(cmd)) { + /* We really do ignore result here! */ + if (command_check_done(cmd)) + ; ret = false; + } return ret; } diff --git a/common/test/run-json_filter.c b/common/test/run-json_filter.c index f8e024516..88659aa66 100644 --- a/common/test/run-json_filter.c +++ b/common/test/run-json_filter.c @@ -12,6 +12,10 @@ struct command; /* AUTOGENERATED MOCKS START */ +/* Generated stub for command_check_done */ +struct command_result *command_check_done(struct command *cmd) + +{ fprintf(stderr, "command_check_done called!\n"); abort(); } /* Generated stub for command_check_only */ bool command_check_only(const struct command *cmd UNNEEDED) { fprintf(stderr, "command_check_only called!\n"); abort(); } diff --git a/common/test/run-json_remove.c b/common/test/run-json_remove.c index cb1f72cf0..23a6f86d6 100644 --- a/common/test/run-json_remove.c +++ b/common/test/run-json_remove.c @@ -41,6 +41,10 @@ struct amount_asset amount_sat_to_asset(struct amount_sat *sat UNNEEDED, const u /* Generated stub for amount_tx_fee */ struct amount_sat amount_tx_fee(u32 fee_per_kw UNNEEDED, size_t weight UNNEEDED) { fprintf(stderr, "amount_tx_fee called!\n"); abort(); } +/* Generated stub for command_check_done */ +struct command_result *command_check_done(struct command *cmd) + +{ fprintf(stderr, "command_check_done called!\n"); abort(); } /* Generated stub for command_check_only */ bool command_check_only(const struct command *cmd UNNEEDED) { fprintf(stderr, "command_check_only called!\n"); abort(); } diff --git a/common/test/run-param.c b/common/test/run-param.c index 09350bee4..c2e6ed6c8 100644 --- a/common/test/run-param.c +++ b/common/test/run-param.c @@ -57,6 +57,10 @@ bool command_deprecated_in_ok(struct command *cmd, } /* AUTOGENERATED MOCKS START */ +/* Generated stub for command_check_done */ +struct command_result *command_check_done(struct command *cmd) + +{ fprintf(stderr, "command_check_done called!\n"); abort(); } /* Generated stub for command_dev_apis */ bool command_dev_apis(const struct command *cmd UNNEEDED) { fprintf(stderr, "command_dev_apis called!\n"); abort(); } diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index c35806f35..ef98b17b2 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -55,12 +55,16 @@ struct command_result *command_param_failed(void) return ¶m_failed; } -/* For our purposes, the same as command_param_failed: we examine - * cmd->mode to see if it's really done. */ +/* We immediately respond with success if we reach here. */ struct command_result *command_check_done(struct command *cmd) { + struct json_stream *response; + assert(cmd->mode == CMD_CHECK); - return ¶m_failed; + + response = json_stream_success(cmd); + json_add_string(response, "command_to_check", cmd->json_cmd->name); + return command_success(cmd, response); } struct command_result *command_its_complicated(const char *relationship_details @@ -600,12 +604,6 @@ struct command_result *command_raw_complete(struct command *cmd, if (cmd->jcon) tal_steal(cmd->jcon, result); - /* Don't free it here if we're doing `check` */ - if (command_check_only(cmd)) { - cmd->mode = CMD_CHECK_FAILED; - return command_param_failed(); - } - tal_free(cmd); return &complete; } @@ -1673,8 +1671,8 @@ static struct command_result *json_check(struct command *cmd, { jsmntok_t *mod_params; const jsmntok_t *name_tok; - struct json_stream *response; struct command_result *res; + struct lightningd *ld = cmd->ld; if (cmd->mode == CMD_USAGE) { mod_params = NULL; @@ -1696,22 +1694,10 @@ static struct command_result *json_check(struct command *cmd, cmd->mode = CMD_CHECK; /* Make *sure* it doesn't try to manip db! */ - db_set_readonly(cmd->ld->wallet->db, true); + db_set_readonly(ld->wallet->db, true); res = cmd->json_cmd->dispatch(cmd, buffer, mod_params, mod_params); - db_set_readonly(cmd->ld->wallet->db, false); + db_set_readonly(ld->wallet->db, false); - /* CMD_CHECK always makes it "fail" parameter parsing. */ - assert(res == ¶m_failed); - if (cmd->mode == CMD_CHECK_FAILED) { - tal_free(cmd); - return res; - } - - response = json_stream_success(cmd); - json_add_string(response, "command_to_check", cmd->json_cmd->name); - res = command_success(cmd, response); - /* CMD_CHECK means we don't get freed! */ - tal_free(cmd); return res; } diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h index 95723928b..015624743 100644 --- a/lightningd/jsonrpc.h +++ b/lightningd/jsonrpc.h @@ -151,11 +151,6 @@ struct logger *command_log(struct command *cmd); extern struct command_result *command_param_failed(void) WARN_UNUSED_RESULT; -/* To return after param_check() succeeds but we're still - * command_check_only(cmd). */ -struct command_result *command_check_done(struct command *cmd) - WARN_UNUSED_RESULT; - /* Wrapper for pending commands (ignores return) */ static inline void was_pending(const struct command_result *res) { diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 2110d86be..cbf516855 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -1309,8 +1309,9 @@ static struct command_result *plugin_rpcmethod_dispatch(struct command *cmd, struct jsonrpc_request *req; bool cmd_ok; - if (cmd->mode == CMD_CHECK) - return command_param_failed(); + /* FIXME: Pass through to plugins! */ + if (command_check_only(cmd)) + return command_check_done(cmd); plugin = find_plugin_for_command(cmd->ld, cmd->json_cmd->name); if (!plugin) diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 5b8cdfde6..9cb374cd7 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -601,6 +601,11 @@ bool command_check_only(const struct command *cmd) return false; } +struct command_result *command_check_done(struct command *cmd) +{ + abort(); +} + void command_set_usage(struct command *cmd, const char *usage TAKES) { usage = tal_strdup(NULL, usage);