plugin/pay: rename 'description' to 'label', deprecate 'description'.

This is the same deprecation, but one level up.  For the moment, we
still support invoices with a `h` field (where description will be
necessary) but that will be removed once this option is removed.

Note that I just changed pylightning without backwards compatibility,
since the field was unlikely to be used, but we could do something
more complex here?

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

CHANGELOG.md

@@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - JSON API: `help` with a `command` argument gives a JSON array, like other commands.
 - JSON API: `sendpay` `description` parameter is renamed `label`.
 - build: we'll use the system libbase58 and libsodium if found suitable.
+- JSON API: `pay` now takes an optional `label` parameter for labelling payments, in place of never-used `description`.
 
 ### Deprecated
 
@@ -52,6 +53,7 @@ fields for your own sanity checking, and that you similarly
 provide appropriate suffixes for JSON input fields.
 
 - JSON API: `short_channel_id` fields in JSON commands with `:` separators (use `x` instead).
+- JSON API: `pay` `description` is deprecated, as is support for BOLT11 strings using `h`.
 - JSON API: `sendpay` parameter `description` and `waitsendpay` and `sendpay` output fields `description` (now `label`).
 
 ### Removed

contrib/pylightning/lightning/lightning.py

@@ -464,19 +464,19 @@ class LightningRpc(UnixDomainSocketRpc):
         }
         return self.call("waitsendpay", payload)
 
-    def pay(self, bolt11, msatoshi=None, description=None, riskfactor=None):
+    def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None, description=None):
         """
         Send payment specified by {bolt11} with {msatoshi}
-        (ignored if {bolt11} has an amount),
-
-        {description} (required if {bolt11} uses description hash)
+        (ignored if {bolt11} has an amount), optional {label}
         and {riskfactor} (default 1.0)
         """
         payload = {
             "bolt11": bolt11,
             "msatoshi": msatoshi,
+            "label": label,
+            "riskfactor": riskfactor,
+            # Deprecated.
             "description": description,
-            "riskfactor": riskfactor
         }
         return self.call("pay", payload)
 

doc/lightning-pay.7

@@ -31,12 +31,12 @@
 lightning-pay \- Command for sending a payment to a BOLT11 invoice
 .SH "SYNOPSIS"
 .sp
-\fBpay\fR \fIbolt11\fR [\fImsatoshi\fR] [\fIdescription\fR] [\fIriskfactor\fR] [\fImaxfeepercent\fR] [\fIretry_for\fR] [\fImaxdelay\fR] [\fIexemptfee\fR]
+\fBpay\fR \fIbolt11\fR [\fImsatoshi\fR] [\fIlabel\fR] [\fIriskfactor\fR] [\fImaxfeepercent\fR] [\fIretry_for\fR] [\fImaxdelay\fR] [\fIexemptfee\fR]
 .SH "DESCRIPTION"
 .sp
 The \fBpay\fR RPC command attempts to find a route to the given destination, and send the funds it asks for\&. If the \fIbolt11\fR does not contain an amount, \fImsatoshi\fR is required, otherwise if it is specified it must be \fInull\fR\&. \fImsatoshi\fR is in millisatoshi precision; it can be a whole number, or a whole number with suffix \fImsat\fR or \fIsat\fR, or a three decimal point number with suffix \fIsat\fR, or an 1 to 11 decimal point number suffixed by \fIbtc\fR\&.
 .sp
-If \fIbolt11\fR contains a description hash (\fIh\fR field) \fIdescription\fR is required, otherwise it is unused\&. The \fIriskfactor\fR is described in detail in lightning\-getroute(7), and defaults to 10\&. The \fImaxfeepercent\fR limits the money paid in fees, and defaults to 0\&.5\&. The maxfeepercent\*(Aq is a percentage of the amount that is to be paid\&. The `exemptfee option can be used for tiny payments which would be dominated by the fee leveraged by forwarding nodes\&. Setting exemptfee allows the maxfeepercent check to be skipped on fees that are smaller than exemptfee (default: 5000 millisatoshi)\&.
+The \fIlabel\fR field is used to attach a label to payments, and is returned in lightning\-listpays(7) and lightning\-listpayments(7)\&. The \fIriskfactor\fR is described in detail in lightning\-getroute(7), and defaults to 10\&. The \fImaxfeepercent\fR limits the money paid in fees, and defaults to 0\&.5\&. The maxfeepercent\*(Aq is a percentage of the amount that is to be paid\&. The `exemptfee option can be used for tiny payments which would be dominated by the fee leveraged by forwarding nodes\&. Setting exemptfee allows the maxfeepercent check to be skipped on fees that are smaller than exemptfee (default: 5000 millisatoshi)\&.
 .sp
 The response will occur when the payment fails or succeeds\&. Once a payment has succeeded, calls to \fBpay\fR with the same \fIbolt11\fR will succeed immediately\&.
 .sp

doc/lightning-pay.7.txt

@@ -8,7 +8,7 @@ lightning-pay - Command for sending a payment to a BOLT11 invoice
 
 SYNOPSIS
 --------
-*pay* 'bolt11' ['msatoshi'] ['description'] ['riskfactor'] ['maxfeepercent'] ['retry_for'] ['maxdelay'] ['exemptfee']
+*pay* 'bolt11' ['msatoshi'] ['label'] ['riskfactor'] ['maxfeepercent'] ['retry_for'] ['maxdelay'] ['exemptfee']
 
 DESCRIPTION
 -----------
@@ -21,8 +21,9 @@ a whole number with suffix 'msat' or 'sat', or a three decimal
 point number with suffix 'sat', or an 1 to 11 decimal point number
 suffixed by 'btc'.
 
-If 'bolt11' contains a description hash ('h' field) 'description' is
-required, otherwise it is unused.  The 'riskfactor' is described in detail
+The 'label' field is used to attach a label to payments, and is returned
+in lightning-listpays(7) and lightning-listpayments(7).
+The 'riskfactor' is described in detail
 in lightning-getroute(7), and defaults to 10.
 The 'maxfeepercent' limits the money paid in fees, and defaults to 0.5.
 The `maxfeepercent' is a percentage of the amount that is to be

plugins/pay.c

@@ -40,7 +40,7 @@ struct pay_status {
 	struct list_node list;
 
 	/* Description user provided (if any) */
-	const char *desc;
+	const char *label;
 	/* Amount they wanted to pay. */
 	struct amount_msat msat;
 	/* CLTV delay required by destination. */
@@ -79,7 +79,7 @@ struct pay_command {
 	const char *payment_hash;
 
 	/* Description, if any. */
-	const char *desc;
+	const char *label;
 
 	/* Chatty description of attempts. */
 	struct pay_status *ps;
@@ -505,8 +505,8 @@ static struct command_result *getroute_done(struct command *cmd,
 		return next_routehint(cmd, pc);
 	}
 
-	if (pc->desc)
-		json_desc = tal_fmt(pc, ", 'description': '%s'", pc->desc);
+	if (pc->label)
+		json_desc = tal_fmt(pc, ", 'label': '%s'", pc->label);
 	else
 		json_desc = "";
 
@@ -834,7 +834,7 @@ static struct pay_status *add_pay_status(struct pay_command *pc,
 
 	/* The pay_status outlives the pc, so it simply takes field ownership */
 	ps->dest = tal_steal(ps, pc->dest);
-	ps->desc = tal_steal(ps, pc->desc);
+	ps->label = tal_steal(ps, pc->label);
 	ps->msat = pc->msat;
 	ps->final_cltv = pc->final_cltv;
 	ps->bolt11 = tal_steal(ps, b11str);
@@ -853,7 +853,7 @@ static struct command_result *handle_pay(struct command *cmd,
 {
 	struct amount_msat *msat;
 	struct bolt11 *b11;
-	const char *b11str;
+	const char *b11str, *description_deprecated;
 	char *fail;
 	double *riskfactor;
 	unsigned int *retryfor;
@@ -862,22 +862,54 @@ static struct command_result *handle_pay(struct command *cmd,
 	unsigned int *maxdelay;
 	struct amount_msat *exemptfee;
 
-	setup_locale();
+	/* If params is array, label takes place of description.  For
+	 * keywords, its a separate parameter. */
+	if (!params || params->type == JSMN_ARRAY) {
+		if (!param(cmd, buf, params,
+			   p_req("bolt11", param_string, &b11str),
+			   p_opt("msatoshi", param_msat, &msat),
+			   p_opt("label", param_string, &pc->label),
+			   p_opt_def("riskfactor", param_double, &riskfactor, 10),
+			   p_opt_def("maxfeepercent", param_percent, &maxfeepercent, 0.5),
+			   p_opt_def("retry_for", param_number, &retryfor, 60),
+			   p_opt_def("maxdelay", param_number, &maxdelay,
+				     maxdelay_default),
+			   p_opt_def("exemptfee", param_msat, &exemptfee, AMOUNT_MSAT(5000)),
+			   NULL))
+			return NULL;
 
+		/* This works because bolt11_decode ignores unneeded descriptions */
+		if (deprecated_apis)
+			description_deprecated = pc->label;
+		else
+			description_deprecated = NULL;
+	} else {
+		/* If by keyword, treat description and label as
+		 * separate parameters. */
 		if (!param(cmd, buf, params,
 			   p_req("bolt11", param_string, &b11str),
 			   p_opt("msatoshi", param_msat, &msat),
-		   p_opt("description", param_string, &pc->desc),
+			   p_opt("description", param_string,
+				 &description_deprecated),
 			   p_opt_def("riskfactor", param_double, &riskfactor, 10),
 			   p_opt_def("maxfeepercent", param_percent, &maxfeepercent, 0.5),
 			   p_opt_def("retry_for", param_number, &retryfor, 60),
 			   p_opt_def("maxdelay", param_number, &maxdelay,
 				     maxdelay_default),
 			   p_opt_def("exemptfee", param_msat, &exemptfee, AMOUNT_MSAT(5000)),
+			   p_opt("label", param_string, &pc->label),
 			   NULL))
 		return NULL;
 
-	b11 = bolt11_decode(cmd, b11str, pc->desc, &fail);
+		if (description_deprecated && !deprecated_apis)
+			return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+					    "Deprecated parameter description, use label");
+		if (description_deprecated && pc->label)
+			return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+					    "Cannot specify both description and label");
+	}
+
+	b11 = bolt11_decode(cmd, b11str, description_deprecated, &fail);
 	if (!b11) {
 		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 				    "Invalid bolt11: %s", fail);
@@ -1034,8 +1066,8 @@ static struct command_result *handle_paystatus(struct command *cmd,
 			       ps->msat.millisatoshis, /* Raw: JSON */
 			       type_to_string(tmpctx, struct amount_msat,
 					      &ps->msat), ps->dest);
-		if (ps->desc)
-			tal_append_fmt(&ret, ", 'description': '%s'", ps->desc);
+		if (ps->label)
+			tal_append_fmt(&ret, ", 'label': '%s'", ps->label);
 		if (ps->routehint_modifications)
 			tal_append_fmt(&ret, ", 'routehint_modifications': '%s'",
 				       ps->routehint_modifications);
@@ -1090,5 +1122,6 @@ static const struct plugin_command commands[] = { {
 
 int main(int argc, char *argv[])
 {
+	setup_locale();
 	plugin_main(argv, init, commands, ARRAY_SIZE(commands));
 }

tests/test_pay.py

@@ -220,10 +220,10 @@ def test_pay_optional_args(node_factory):
     l1, l2 = node_factory.line_graph(2)
 
     inv1 = l2.rpc.invoice(123000, 'test_pay', 'desc')['bolt11']
-    l1.rpc.pay(inv1, description='desc')
+    l1.rpc.pay(inv1, label='desc')
     payment1 = l1.rpc.listpayments(inv1)['payments']
     assert len(payment1) and payment1[0]['msatoshi'] == 123000
-    assert payment1[0]['description'] == 'desc'
+    assert payment1[0]['label'] == 'desc'
 
     inv2 = l2.rpc.invoice(321000, 'test_pay2', 'description')['bolt11']
     l1.rpc.pay(inv2, riskfactor=5.0)
@@ -231,10 +231,10 @@ def test_pay_optional_args(node_factory):
     assert len(payment2) == 1 and payment2[0]['msatoshi'] == 321000
 
     anyinv = l2.rpc.invoice('any', 'any_pay', 'desc')['bolt11']
-    l1.rpc.pay(anyinv, description='desc', msatoshi='500')
+    l1.rpc.pay(anyinv, label='desc', msatoshi='500')
     payment3 = l1.rpc.listpayments(anyinv)['payments']
     assert len(payment3) == 1 and payment3[0]['msatoshi'] == 500
-    assert payment3[0]['description'] == 'desc'
+    assert payment3[0]['label'] == 'desc'
 
     # Should see 3 completed transactions
     assert len(l1.rpc.listpayments()['payments']) == 3
@@ -1288,7 +1288,7 @@ def test_pay_routeboost(node_factory, bitcoind):
     assert only_one(status['pay'])['msatoshi'] == 10**5
     assert only_one(status['pay'])['amount_msat'] == Millisatoshi(10**5)
     assert only_one(status['pay'])['destination'] == l4.info['id']
-    assert 'description' not in only_one(status['pay'])
+    assert 'label' not in only_one(status['pay'])
     assert 'routehint_modifications' not in only_one(status['pay'])
     assert 'local_exclusions' not in only_one(status['pay'])
     # First attempt will fail, then it will try route hint
@@ -1349,13 +1349,13 @@ def test_pay_routeboost(node_factory, bitcoind):
                                       'label': 'test_pay_routeboost5',
                                       'description': 'test_pay_routeboost5',
                                       'dev-routes': [routel3l4l5, routel3l5]})
-        l1.rpc.pay(inv['bolt11'], description="paying test_pay_routeboost5")
+        l1.rpc.pay(inv['bolt11'], label="paying test_pay_routeboost5")
 
         status = l1.rpc.call('paystatus', [inv['bolt11']])
         assert only_one(status['pay'])['bolt11'] == inv['bolt11']
         assert only_one(status['pay'])['msatoshi'] == 10**5
         assert only_one(status['pay'])['destination'] == l5.info['id']
-        assert only_one(status['pay'])['description'] == "paying test_pay_routeboost5"
+        assert only_one(status['pay'])['label'] == "paying test_pay_routeboost5"
         assert 'routehint_modifications' not in only_one(status['pay'])
         assert 'local_exclusions' not in only_one(status['pay'])
         attempts = only_one(status['pay'])['attempts']

tests/test_plugin.py

@@ -125,7 +125,7 @@ def test_pay_plugin(node_factory):
         l1.rpc.call('pay')
 
     # Make sure usage messages are present.
-    assert only_one(l1.rpc.help('pay')['help'])['command'] == 'pay bolt11 [msatoshi] [description] [riskfactor] [maxfeepercent] [retry_for] [maxdelay] [exemptfee]'
+    assert only_one(l1.rpc.help('pay')['help'])['command'] == 'pay bolt11 [msatoshi] [label] [riskfactor] [maxfeepercent] [retry_for] [maxdelay] [exemptfee]'
     assert only_one(l1.rpc.help('paystatus')['help'])['command'] == 'paystatus [bolt11]'