sendpay: allow route to contain both amount_msat and (deprecated) msatoshi.

Since it's a deprecation, we simply ignore one, rather than properly
checking they match etc.

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

common/param.c

@@ -36,6 +36,12 @@ static bool param_add(struct param **params,
 	return true;
 }
 
+/* FIXME: To support the deprecated p_req_dup_ok */
+static bool is_required(enum param_style style)
+{
+	return style == PARAM_REQUIRED || style == PARAM_REQUIRED_ALLOW_DUPS;
+}
+
 static struct command_result *make_callback(struct command *cmd,
 					     struct param *def,
 					     const char *buffer,
@@ -57,7 +63,7 @@ static struct command_result *post_check(struct command *cmd,
 	struct param *last = first + tal_count(params);
 
 	/* Make sure required params were provided. */
-	while (first != last && first->style == PARAM_REQUIRED) {
+	while (first != last && is_required(first->style)) {
 		if (!first->is_set) {
 			return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 					    "missing required parameter: %s",
@@ -145,6 +151,8 @@ static struct command_result *parse_by_name(struct command *cmd,
 			struct command_result *res;
 
 			if (p->is_set) {
+				if (p->style == PARAM_REQUIRED_ALLOW_DUPS)
+					continue;
 				return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 						    "duplicate json names: %s",
 						    p->name);
@@ -182,7 +190,7 @@ static int comp_by_arg(const struct param *a, const struct param *b,
 static int comp_req_order(const struct param *a, const struct param *b,
 			  void *unused)
 {
-	if (a->style != PARAM_REQUIRED && b->style == PARAM_REQUIRED)
+	if (!is_required(a->style) && is_required(b->style))
 		return 0;
 	return 1;
 }
@@ -249,7 +257,7 @@ static char *param_usage(const tal_t *ctx,
 		int len = strcspn(params[i].name, "|");
 		if (i != 0)
 			tal_append_fmt(&usage, " ");
-		if (params[i].style == PARAM_REQUIRED)
+		if (is_required(params[i].style))
 			tal_append_fmt(&usage, "%.*s", len, params[i].name);
 		else
 			tal_append_fmt(&usage, "[%.*s]", len, params[i].name);

common/param.h

@@ -70,6 +70,7 @@ const char *param_subcommand(struct command *cmd, const char *buffer,
 
 enum param_style {
 	PARAM_REQUIRED,
+	PARAM_REQUIRED_ALLOW_DUPS,
 	PARAM_OPTIONAL,
 	PARAM_OPTIONAL_WITH_DEFAULT,
 };
@@ -103,6 +104,20 @@ enum param_style {
 				  (const jsmntok_t *)NULL,      \
 				  (arg)) == (struct command_result *)NULL); })
 
+/*
+ * Add an required parameter, like p_req, but ignore duplicates.
+ */
+#define p_req_dup_ok(name, cbx, arg)                            \
+	      name"",                                           \
+	      PARAM_REQUIRED_ALLOW_DUPS,                        \
+	      (param_cbx)(cbx),                                 \
+	      ({ *arg = NULL;                                   \
+		 (arg) + 0*sizeof((cbx)((struct command *)NULL, \
+		                  (const char *)NULL,           \
+				  (const char *)NULL,           \
+				  (const jsmntok_t *)NULL,      \
+				  (arg)) == (struct command_result *)NULL); })
+
 /*
  * Add an optional parameter.  *arg is set to @def if it isn't found.
  * name can be <normal>|<deprecated> if it's been renamed.

lightningd/pay.c

@@ -1375,7 +1375,8 @@ static struct command_result *param_route_hops(struct command *cmd,
 		int *ignored;
 
 		if (!param(cmd, buffer, t,
-			   p_req("amount_msat|msatoshi", param_msat, &amount_msat),
+			   /* deprecated: getroute gives both, so we allow both! */
+			   p_req_dup_ok("amount_msat|msatoshi", param_msat, &amount_msat),
 			   p_req("id", param_node_id, &id),
 			   p_req("delay", param_number, &delay),
 			   p_req("channel", param_short_channel_id, &channel),

tests/test_pay.py

@@ -5315,7 +5315,6 @@ def test_pay_middle_fail(node_factory, bitcoind, executor):
         l1.rpc.waitsendpay('00' * 32)
 
 
-@pytest.mark.xfail(strict=True)
 def test_sendpay_dual_amounts(node_factory):
     """Test that handing *both* msatoshi and amount_msat to sendpay works"""
     l1 = node_factory.get_node(options={'allow-deprecated-apis': True})