From f51a3d8ef7c9e02ad6673e0960a57c4669576a93 Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Mon, 4 Apr 2022 12:26:52 +0930
Subject: [PATCH] plugins/pay: always include `bolt11` (and `description`) in
 listpays.

We were setting it on the root, but that doesn't get handed to
sendpay.  Our schema doesn't *require* bolt11, either, so this was
missed (there could be a *bolt12* instead).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `listpays` always includes `bolt11` or `bolt12` field.
---
 plugins/keysend.c       |  2 ++
 plugins/libplugin-pay.c | 23 +++++++++++------------
 plugins/libplugin-pay.h |  4 ++++
 plugins/pay.c           |  7 +++++++
 tests/test_pay.py       |  3 +++
 5 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/plugins/keysend.c b/plugins/keysend.c
index 22ae85e3b..47f9440c1 100644
--- a/plugins/keysend.c
+++ b/plugins/keysend.c
@@ -186,6 +186,8 @@ static struct command_result *json_keysend(struct command *cmd, const char *buf,
 	p->min_final_cltv_expiry = 22;
 	p->features = NULL;
 	p->invstring = NULL;
+	/* Don't try to use invstring to hand to sendonion! */
+	p->invstring_used = true;
 	p->why = "Initial attempt";
 	p->constraints.cltv_budget = *maxdelay;
 	p->deadline = timeabs_add(time_now(), time_from_sec(*retryfor));
diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c
index b206ce759..e415cf99d 100644
--- a/plugins/libplugin-pay.c
+++ b/plugins/libplugin-pay.c
@@ -61,11 +61,10 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd,
 	p->failreason = NULL;
 	p->getroute->riskfactorppm = 10000000;
 	p->abort = false;
+	p->invstring_used = false;
 	p->route = NULL;
 	p->temp_exclusion = NULL;
 	p->failroute_retry = false;
-	p->invstring = NULL;
-	p->description = NULL;
 	p->routetxt = NULL;
 	p->max_htlcs = UINT32_MAX;
 	p->aborterror = NULL;
@@ -94,6 +93,8 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd,
 		p->local_id = parent->local_id;
 		p->local_offer_id = parent->local_offer_id;
 		p->groupid = parent->groupid;
+		p->invstring = parent->invstring;
+		p->description = parent->description;
 	} else {
 		assert(cmd != NULL);
 		p->partid = 0;
@@ -102,6 +103,7 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd,
 		p->channel_hints = tal_arr(p, struct channel_hint, 0);
 		p->excluded_nodes = tal_arr(p, struct node_id, 0);
 		p->id = next_id++;
+		p->description = NULL;
 		/* Caller must set this.  */
 		p->local_id = NULL;
 		p->local_offer_id = NULL;
@@ -1536,6 +1538,7 @@ static struct command_result *payment_createonion_success(struct command *cmd,
 	struct out_req *req;
 	struct route_hop *first = &p->route[0];
 	struct secret *secrets;
+	struct payment *root = payment_root(p);
 
 	p->createonion_response = json_to_createonion_response(p, buffer, toks);
 
@@ -1565,12 +1568,15 @@ static struct command_result *payment_createonion_success(struct command *cmd,
 	if (p->label)
 		json_add_string(req->js, "label", p->label);
 
-	if (p->invstring)
+	if (!root->invstring_used) {
 		/* FIXME: rename parameter to invstring */
 		json_add_string(req->js, "bolt11", p->invstring);
 
-	if (p->description)
-		json_add_string(req->js, "description", p->description);
+		if (p->description)
+			json_add_string(req->js, "description", p->description);
+
+		root->invstring_used = true;
+	}
 
 	if (p->destination)
 		json_add_node_id(req->js, "destination", p->destination);
@@ -3559,13 +3565,6 @@ static void presplit_cb(struct presplit_mod_data *d, struct payment *p)
 			struct payment *c =
 			    payment_new(p, NULL, p, p->modifiers);
 
-			/* Annotate the subpayments with the bolt11 string,
-			 * they'll be used when aggregating the payments
-			 * again. */
-			c->invstring = tal_strdup(c, p->invstring);
-			if (p->description)
-				c->description = tal_strdup(c, p->description);
-
 			/* Get ~ target, but don't exceed amt */
 			c->amount = fuzzed_near(target, amt);
 
diff --git a/plugins/libplugin-pay.h b/plugins/libplugin-pay.h
index 220972547..eb125acef 100644
--- a/plugins/libplugin-pay.h
+++ b/plugins/libplugin-pay.h
@@ -255,6 +255,10 @@ struct payment {
 	 * true. Set only on the root payment. */
 	bool abort;
 
+	/* We only set invstring/description on one of our sendpays per group,
+	 * so we track when we've done that. */
+	bool invstring_used;
+
 	/* Serialized bolt11/12 string, kept attachd to the root so we can filter
 	 * by the invoice. */
 	const char *invstring;
diff --git a/plugins/pay.c b/plugins/pay.c
index 610d369c1..51b632a03 100644
--- a/plugins/pay.c
+++ b/plugins/pay.c
@@ -480,6 +480,13 @@ static struct command_result *listsendpays_done(struct command *cmd,
 			// First time we see the groupid we add it to the order
 			// array, so we can retrieve them in the correct order.
 			tal_arr_expand(&order, pm->sortkey);
+		} else {
+			/* Not all payments have bolt11/bolt12 or
+			 * description, as an optimization */
+			if (!pm->invstring)
+				pm->invstring = tal_steal(pm, invstr);
+			if (!pm->description)
+				pm->description = json_get_member(buf, t, "description");
 		}
 
 		status = json_get_member(buf, t, "status");
diff --git a/tests/test_pay.py b/tests/test_pay.py
index e0914887e..672538205 100644
--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@@ -3799,6 +3799,9 @@ def test_mpp_adaptive(node_factory, bitcoind):
     pprint(p)
     pprint(l1.rpc.paystatus(inv))
 
+    # listpays() shows bolt11 string
+    assert 'bolt11' in only_one(l1.rpc.listpays()['pays'])
+
 
 def test_pay_fail_unconfirmed_channel(node_factory, bitcoind):
     '''