jsonprc: make json_get_params() fail the command, for better error reporting.

We move it into jsonrpc where it belongs, and make it fail the command.
This means it can tell us exactly what was wrong.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/json.c

@@ -225,90 +225,6 @@ const jsmntok_t *json_delve(const char *buffer,
 	return tok;
 }
 
-bool json_get_params(const char *buffer, const jsmntok_t param[], ...)
-{
-	va_list ap;
-	const char **names;
-	size_t num_names;
-	 /* Uninitialized warnings on p and end */
-	const jsmntok_t **tokptr, *p = NULL, *end = NULL;
-
-	if (param->type == JSMN_ARRAY) {
-		if (param->size == 0)
-			p = NULL;
-		else
-			p = param + 1;
-		end = json_next(param);
-	} else if (param->type != JSMN_OBJECT)
-		return false;
-
-	num_names = 0;
-	names = tal_arr(NULL, const char *, num_names + 1);
-	va_start(ap, param);
-	while ((names[num_names] = va_arg(ap, const char *)) != NULL) {
-		tokptr = va_arg(ap, const jsmntok_t **);
-		bool compulsory = true;
-		if (names[num_names][0] == '?') {
-			names[num_names]++;
-			compulsory = false;
-		}
-		if (param->type == JSMN_ARRAY) {
-			*tokptr = p;
-			if (p) {
-				p = json_next(p);
-				if (p == end)
-					p = NULL;
-			}
-		} else {
-			*tokptr = json_get_member(buffer, param,
-						  names[num_names]);
-		}
-		/* Convert 'null' to NULL */
-		if (*tokptr
-		    && (*tokptr)->type == JSMN_PRIMITIVE
-		    && buffer[(*tokptr)->start] == 'n') {
-			*tokptr = NULL;
-		}
-		if (compulsory && !*tokptr) {
-			va_end(ap);
-			tal_free(names);
-			return false;
-		}
-		num_names++;
-		tal_resize(&names, num_names + 1);
-	}
-
-	va_end(ap);
-
-	/* Now make sure there aren't any params which aren't valid */
-	if (param->type == JSMN_ARRAY) {
-		if (param->size > num_names) {
-			tal_free(names);
-			return false;
-		}
-	} else {
-		const jsmntok_t *t;
-
-		end = json_next(param);
-
-		/* Find each parameter among the valid names */
-		for (t = param + 1; t < end; t = json_next(t+1)) {
-			bool found = false;
-			for (size_t i = 0; i < num_names; i++) {
-				if (json_tok_streq(buffer, t, names[i]))
-					found = true;
-			}
-			if (!found) {
-				tal_free(names);
-				return false;
-			}
-		}
-	}
-
-	tal_free(names);
-	return true;
-}
-
 static bool strange_chars(const char *str, size_t len)
 {
 	for (size_t i = 0; i < len; i++) {

common/json.h

@@ -46,15 +46,6 @@ bool json_tok_is_null(const char *buffer, const jsmntok_t *tok);
 /* Returns next token with same parent. */
 const jsmntok_t *json_next(const jsmntok_t *tok);
 
-/* Get the parameters (by position or name).  Followed by triples of
- * of const char *name, const jsmntok_t **ret_ptr, then NULL.
- *
- * If name starts with '?' it is optional (and will be set to NULL
- * if it's a literal 'null' or not present).
- * Otherwise false is returned.
- */
-bool json_get_params(const char *buffer, const jsmntok_t param[], ...);
-
 /* Get top-level member. */
 const jsmntok_t *json_get_member(const char *buffer, const jsmntok_t tok[],
 				 const char *label);

lightningd/chaintopology.c

@@ -556,10 +556,9 @@ void json_dev_broadcast(struct command *cmd,
 	jsmntok_t *enabletok;
 	bool enable;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "enable", &enabletok,
 			     NULL)) {
-		command_fail(cmd, "Need enable");
 		return;
 	}
 
@@ -606,12 +605,11 @@ static void json_dev_setfees(struct command *cmd,
 	struct chain_topology *topo = cmd->ld->topology;
 	struct json_result *response;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "?immediate", &ratetok[FEERATE_IMMEDIATE],
 			     "?normal", &ratetok[FEERATE_NORMAL],
 			     "?slow", &ratetok[FEERATE_SLOW],
 			     NULL)) {
-		command_fail(cmd, "Bad parameters");
 		return;
 	}
 

lightningd/dev_ping.c

@@ -45,12 +45,11 @@ static void json_dev_ping(struct command *cmd,
 	struct pubkey id;
 	struct subd *owner;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "id", &idtok,
 			     "len", &lentok,
 			     "pongbytes", &pongbytestok,
 			     NULL)) {
-		command_fail(cmd, "Need id, len and pongbytes");
 		return;
 	}
 

lightningd/gossip_control.c

@@ -235,10 +235,9 @@ static void json_listnodes(struct command *cmd, const char *buffer,
 	jsmntok_t *idtok = NULL;
 	struct pubkey *id = NULL;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "?id", &idtok,
 			     NULL)) {
-		command_fail(cmd, "Invalid arguments");
 		return;
 	}
 
@@ -302,13 +301,12 @@ static void json_getroute(struct command *cmd, const char *buffer, const jsmntok
 	double riskfactor;
 	struct lightningd *ld = cmd->ld;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "id", &idtok,
 			     "msatoshi", &msatoshitok,
 			     "riskfactor", &riskfactortok,
 			     "?cltv", &cltvtok,
 			     NULL)) {
-		command_fail(cmd, "Need id, msatoshi and riskfactor");
 		return;
 	}
 
@@ -396,10 +394,9 @@ static void json_listchannels(struct command *cmd, const char *buffer,
 	jsmntok_t *idtok;
 	struct short_channel_id *id = NULL;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "?short_channel_id", &idtok,
 			     NULL)) {
-		command_fail(cmd, "Invalid arguments");
 		return;
 	}
 

lightningd/invoice.c

@@ -108,13 +108,12 @@ static void json_invoice(struct command *cmd,
 	char *b11enc;
 	u64 expiry = 3600;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "msatoshi", &msatoshi,
 			     "label", &label,
 			     "description", &desc,
 			     "?expiry", &exp,
 			     NULL)) {
-		command_fail(cmd, "Need {msatoshi}, {label} and {description}");
 		return;
 	}
 
@@ -228,10 +227,9 @@ static void json_listinvoice_internal(struct command *cmd,
 	struct json_result *response = new_json_result(cmd);
 	struct wallet *wallet = cmd->ld->wallet;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "?label", &label,
 			     NULL)) {
-		command_fail(cmd, "Invalid arguments");
 		return;
 	}
 
@@ -284,11 +282,10 @@ static void json_delinvoice(struct command *cmd,
 	const char *label, *status, *actual_status;
 	struct wallet *wallet = cmd->ld->wallet;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "label", &labeltok,
 			     "status", &statustok,
 			     NULL)) {
-		command_fail(cmd, "Invalid arguments");
 		return;
 	}
 
@@ -340,10 +337,9 @@ static void json_waitanyinvoice(struct command *cmd,
 	u64 pay_index;
 	struct wallet *wallet = cmd->ld->wallet;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "?lastpay_index", &pay_indextok,
 			     NULL)) {
-		command_fail(cmd, "Invalid arguments");
 		return;
 	}
 
@@ -389,8 +385,7 @@ static void json_waitinvoice(struct command *cmd,
 	jsmntok_t *labeltok;
 	const char *label = NULL;
 
-	if (!json_get_params(buffer, params, "label", &labeltok, NULL)) {
-		command_fail(cmd, "Missing {label}");
+	if (!json_get_params(cmd, buffer, params, "label", &labeltok, NULL)) {
 		return;
 	}
 
@@ -427,11 +422,10 @@ static void json_decodepay(struct command *cmd,
 	struct json_result *response;
         char *str, *desc, *fail;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "bolt11", &bolt11tok,
 			     "?description", &desctok,
 			     NULL)) {
-		command_fail(cmd, "Need bolt11 string");
 		return;
 	}
 

lightningd/jsonrpc.c

@@ -155,8 +155,7 @@ static void json_getlog(struct command *cmd,
 	struct log_book *lr = cmd->ld->log_book;
 	jsmntok_t *level;
 
-	if (!json_get_params(buffer, params, "?level", &level, NULL)) {
-		command_fail(cmd, "Invalid arguments");
+	if (!json_get_params(cmd, buffer, params, "?level", &level, NULL)) {
 		return;
 	}
 
@@ -206,10 +205,9 @@ static void json_rhash(struct command *cmd,
 	jsmntok_t *secrettok;
 	struct sha256 secret;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "secret", &secrettok,
 			     NULL)) {
-		command_fail(cmd, "Need secret");
 		return;
 	}
 
@@ -298,8 +296,7 @@ static void json_help(struct command *cmd,
 	struct json_command **cmdlist = get_cmdlist();
 	jsmntok_t *cmdtok;
 
-	if (!json_get_params(buffer, params, "?command", &cmdtok, NULL)) {
-		command_fail(cmd, "Invalid arguments");
+	if (!json_get_params(cmd, buffer, params, "?command", &cmdtok, NULL)) {
 		return;
 	}
 
@@ -626,13 +623,6 @@ static void parse_request(struct json_connection *jcon, const jsmntok_t tok[])
 		return;
 	}
 
-	if (params->type != JSMN_ARRAY && params->type != JSMN_OBJECT) {
-		command_fail_detailed(jcon->current,
-				      JSONRPC2_INVALID_PARAMS, NULL,
-				      "Expected array or object for params");
-		return;
-	}
-
 	db_begin_transaction(jcon->ld->wallet->db);
 	cmd->dispatch(jcon->current, jcon->buffer, params);
 	db_commit_transaction(jcon->ld->wallet->db);
@@ -642,6 +632,107 @@ static void parse_request(struct json_connection *jcon, const jsmntok_t tok[])
 		assert(jcon->current->pending);
 }
 
+bool json_get_params(struct command *cmd,
+		     const char *buffer, const jsmntok_t param[], ...)
+{
+	va_list ap;
+	const char **names;
+	size_t num_names;
+	 /* Uninitialized warnings on p and end */
+	const jsmntok_t **tokptr, *p = NULL, *end = NULL;
+
+	if (param->type == JSMN_ARRAY) {
+		if (param->size == 0)
+			p = NULL;
+		else
+			p = param + 1;
+		end = json_next(param);
+	} else if (param->type != JSMN_OBJECT) {
+		command_fail_detailed(cmd, JSONRPC2_INVALID_PARAMS, NULL,
+				      "Expected array or object for params");
+		return false;
+	}
+
+	num_names = 0;
+	names = tal_arr(cmd, const char *, num_names + 1);
+	va_start(ap, param);
+	while ((names[num_names] = va_arg(ap, const char *)) != NULL) {
+		tokptr = va_arg(ap, const jsmntok_t **);
+		bool compulsory = true;
+		if (names[num_names][0] == '?') {
+			names[num_names]++;
+			compulsory = false;
+		}
+		if (param->type == JSMN_ARRAY) {
+			*tokptr = p;
+			if (p) {
+				p = json_next(p);
+				if (p == end)
+					p = NULL;
+			}
+		} else {
+			*tokptr = json_get_member(buffer, param,
+						  names[num_names]);
+		}
+		/* Convert 'null' to NULL */
+		if (*tokptr
+		    && (*tokptr)->type == JSMN_PRIMITIVE
+		    && buffer[(*tokptr)->start] == 'n') {
+			*tokptr = NULL;
+		}
+		if (compulsory && !*tokptr) {
+			va_end(ap);
+			command_fail_detailed(cmd, JSONRPC2_INVALID_PARAMS, NULL,
+					      "Missing '%s' parameter",
+					      names[num_names]);
+			tal_free(names);
+			return false;
+		}
+		num_names++;
+		tal_resize(&names, num_names + 1);
+	}
+
+	va_end(ap);
+
+	/* Now make sure there aren't any params which aren't valid */
+	if (param->type == JSMN_ARRAY) {
+		if (param->size > num_names) {
+			tal_free(names);
+			command_fail_detailed(cmd, JSONRPC2_INVALID_PARAMS, NULL,
+					      "Too many parameters:"
+					      " got %u, expected %zu",
+					      param->size, num_names);
+			return false;
+		}
+	} else {
+		const jsmntok_t *t;
+
+		end = json_next(param);
+
+		/* Find each parameter among the valid names */
+		for (t = param + 1; t < end; t = json_next(t+1)) {
+			bool found = false;
+			for (size_t i = 0; i < num_names; i++) {
+				if (json_tok_streq(buffer, t, names[i]))
+					found = true;
+			}
+			if (!found) {
+				tal_free(names);
+				command_fail_detailed(cmd,
+						      JSONRPC2_INVALID_PARAMS,
+						      NULL,
+						      "Unknown parameter '%.*s'",
+						      t->end - t->start,
+						      buffer + t->start);
+				return false;
+			}
+		}
+	}
+
+	tal_free(names);
+	return true;
+}
+
 static struct io_plan *write_json(struct io_conn *conn,
 				  struct json_connection *jcon)
 {

lightningd/jsonrpc.h

@@ -56,6 +56,16 @@ struct json_command {
 	const char *verbose;
 };
 
+/* Get the parameters (by position or name).  Followed by triples of
+ * of const char *name, const jsmntok_t **ret_ptr, then NULL.
+ *
+ * If name starts with '?' it is optional (and will be set to NULL
+ * if it's a literal 'null' or not present).
+ * Otherwise false is returned, and command_fail already called.
+ */
+bool json_get_params(struct command *cmd,
+		     const char *buffer, const jsmntok_t param[], ...);
+
 struct json_result *null_response(const tal_t *ctx);
 void command_success(struct command *cmd, struct json_result *response);
 void PRINTF_FMT(2, 3) command_fail(struct command *cmd, const char *fmt, ...);

lightningd/options.c

@@ -774,8 +774,7 @@ static void json_listconfigs(struct command *cmd,
 	jsmntok_t *configtok;
 	bool found = false;
 
-	if (!json_get_params(buffer, params, "?config", &configtok, NULL)) {
-		command_fail(cmd, "Invalid arguments");
+	if (!json_get_params(cmd, buffer, params, "?config", &configtok, NULL)) {
 		return;
 	}
 

lightningd/pay.c

@@ -432,11 +432,10 @@ static void json_sendpay(struct command *cmd,
 	struct sha256 rhash;
 	struct route_hop *route;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "route", &routetok,
 			     "rhash", &rhashtok,
 			     NULL)) {
-		command_fail(cmd, "Need route and rhash");
 		return;
 	}
 
@@ -553,13 +552,12 @@ static void json_pay(struct command *cmd,
 	char *fail, *b11str, *desc;
 	u8 *req;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "bolt11", &bolt11tok,
 			     "?msatoshi", &msatoshitok,
 			     "?description", &desctok,
 			     "?riskfactor", &riskfactortok,
 			     NULL)) {
-		command_fail(cmd, "Need bolt11 string");
 		return;
 	}
 
@@ -632,11 +630,10 @@ static void json_listpayments(struct command *cmd, const char *buffer,
 	jsmntok_t *bolt11tok, *rhashtok;
 	struct sha256 *rhash = NULL;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "?bolt11", &bolt11tok,
 			     "?payment_hash", &rhashtok,
 			     NULL)) {
-		command_fail(cmd, "Invalid parameters");
 		return;
 	}
 

lightningd/peer_control.c

@@ -742,12 +742,11 @@ static void json_connect(struct command *cmd,
 	struct wireaddr addr;
 	u8 *msg;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "id", &idtok,
 			     "?host", &hosttok,
 			     "?port", &porttok,
 			     NULL)) {
-		command_fail(cmd, "Need id to connect");
 		return;
 	}
 
@@ -949,11 +948,10 @@ static void json_listpeers(struct command *cmd,
 
 	gpa->cmd = cmd;
 	gpa->specific_id = NULL;
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "?id", &idtok,
 			     "?level", &leveltok,
 			     NULL)) {
-		command_fail(cmd, "Invalid arguments");
 		return;
 	}
 
@@ -2673,11 +2671,10 @@ static void json_fund_channel(struct command *cmd,
 	struct funding_channel *fc = tal(cmd, struct funding_channel);
 	u8 *msg;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "id", &peertok,
 			     "satoshi", &satoshitok,
 			     NULL)) {
-		command_fail(cmd, "Need id and satoshi");
 		return;
 	}
 
@@ -2732,10 +2729,9 @@ static void json_close(struct command *cmd,
 	jsmntok_t *peertok;
 	struct peer *peer;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "id", &peertok,
 			     NULL)) {
-		command_fail(cmd, "Need id");
 		return;
 	}
 
@@ -2849,10 +2845,9 @@ static void json_sign_last_tx(struct command *cmd,
 	struct json_result *response = new_json_result(cmd);
 	u8 *linear;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "id", &peertok,
 			     NULL)) {
-		command_fail(cmd, "Need id");
 		return;
 	}
 
@@ -2891,10 +2886,9 @@ static void json_dev_fail(struct command *cmd,
 	jsmntok_t *peertok;
 	struct peer *peer;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "id", &peertok,
 			     NULL)) {
-		command_fail(cmd, "Need id");
 		return;
 	}
 
@@ -2930,10 +2924,9 @@ static void json_dev_reenable_commit(struct command *cmd,
 	struct peer *peer;
 	u8 *msg;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "id", &peertok,
 			     NULL)) {
-		command_fail(cmd, "Need id");
 		return;
 	}
 

wallet/walletrpc.c

@@ -199,11 +199,10 @@ static void json_withdraw(struct command *cmd,
 	struct bitcoin_tx *tx;
 	bool withdraw_all = false;
 
-	if (!json_get_params(buffer, params,
+	if (!json_get_params(cmd, buffer, params,
 			     "destination", &desttok,
 			     "satoshi", &sattok,
 			     NULL)) {
-		command_fail(cmd, "Need destination and satoshi.");
 		return;
 	}