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 <rusty@rustcorp.com.au>

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

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

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

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

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

lightningd/jsonrpc.c

@@ -55,12 +55,16 @@ struct command_result *command_param_failed(void)
 	return &param_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 &param_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 == &param_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;
 }
 

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)
 {

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)

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