pay: listpays groups by payment_hash and groupid

Fixes ElementsProject#4482
Fixes ElementsProject#4481

Changelog-Added: pay: Payment attempts are now grouped by the pay command that initiated them
Changelog-Fixed: pay: `listpays` returns payments orderd by their creation date
Changelog-Fixed: pay: `listpays` no longer groups attempts from multiple attempts to pay an invoice

lightningd/htlc_end.c

@@ -277,6 +277,7 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
 			      const struct pubkey *blinding,
 			      bool am_origin,
 			      u64 partid,
+			      u64 groupid,
 			      struct htlc_in *in)
 {
 	struct htlc_out *hout = tal(ctx, struct htlc_out);
@@ -303,8 +304,10 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
 	else
 		hout->blinding = NULL;
 	hout->am_origin = am_origin;
-	if (am_origin)
+	if (am_origin) {
 		hout->partid = partid;
+		hout->groupid = groupid;
+	}
 	hout->in = NULL;
 	if (in)
 		htlc_out_connect_htlc_in(hout, in);

lightningd/htlc_end.h

@@ -91,6 +91,9 @@ struct htlc_out {
 	/* If am_origin, this is the partid of the payment. */
 	u64 partid;
 
+	/* Is this is part of a group of HTLCs, which group is it? */
+	u64 groupid;
+
 	/* Where it's from, if not going to us. */
 	struct htlc_in *in;
 
@@ -163,6 +166,7 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
 			      const struct pubkey *blinding,
 			      bool am_origin,
 			      u64 partid,
+			      u64 groupid,
 			      struct htlc_in *in);
 
 void connect_htlc_in(struct htlc_in_map *map, struct htlc_in *hin);

lightningd/pay.c

@@ -323,7 +323,7 @@ void payment_succeeded(struct lightningd *ld, struct htlc_out *hout,
 	struct wallet_payment *payment;
 
 	wallet_payment_set_status(ld->wallet, &hout->payment_hash,
-				  hout->partid,
+				  hout->partid, hout->groupid,
 				  PAYMENT_COMPLETE, rval);
 	payment = wallet_payment_by_hash(tmpctx, ld->wallet,
 					 &hout->payment_hash,
@@ -627,7 +627,7 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
 	/* Save to DB */
 	payment_store(ld, payment);
 	wallet_payment_set_status(ld->wallet, &hout->payment_hash,
-				  hout->partid,
+				  hout->partid, hout->groupid,
 				  PAYMENT_FAILED, NULL);
 	wallet_payment_set_failinfo(ld->wallet,
 				    &hout->payment_hash,
@@ -767,6 +767,7 @@ static const u8 *send_onion(const tal_t *ctx, struct lightningd *ld,
 			    const struct sha256 *payment_hash,
 			    const struct pubkey *blinding,
 			    u64 partid,
+			    u64 groupid,
 			    struct channel *channel,
 			    struct htlc_out **hout)
 {
@@ -776,8 +777,8 @@ static const u8 *send_onion(const tal_t *ctx, struct lightningd *ld,
 	base_expiry = get_block_height(ld->topology) + 1;
 	onion = serialize_onionpacket(tmpctx, packet);
 	return send_htlc_out(ctx, channel, first_hop->amount,
-			     base_expiry + first_hop->delay,
-			     payment_hash, blinding, partid, onion, NULL, hout,
+			     base_expiry + first_hop->delay, payment_hash,
+			     blinding, partid, groupid, onion, NULL, hout,
 			     &dont_care_about_channel_update);
 }
 
@@ -1010,7 +1011,7 @@ send_payment_core(struct lightningd *ld,
 	}
 
 	failmsg = send_onion(tmpctx, ld, packet, first_hop, rhash, NULL, partid,
-			      channel, &hout);
+			     group, channel, &hout);
 
 	if (failmsg) {
 		fail = immediate_routing_failure(cmd, ld,

lightningd/peer_htlcs.c

@@ -621,6 +621,7 @@ const u8 *send_htlc_out(const tal_t *ctx,
 			const struct sha256 *payment_hash,
 			const struct pubkey *blinding,
 			u64 partid,
+			u64 groupid,
 			const u8 *onion_routing_packet,
 			struct htlc_in *in,
 			struct htlc_out **houtp,
@@ -654,7 +655,7 @@ const u8 *send_htlc_out(const tal_t *ctx,
 	*houtp = new_htlc_out(out->owner, out, amount, cltv,
 			      payment_hash, onion_routing_packet,
 			      blinding, in == NULL,
-			      partid, in);
+			      partid, groupid, in);
 	tal_add_destructor(*houtp, destroy_hout_subd_died);
 
 	/* Give channel 30 seconds to commit this htlc. */
@@ -766,8 +767,8 @@ static void forward_htlc(struct htlc_in *hin,
 
 	failmsg = send_htlc_out(tmpctx, next, amt_to_forward,
 				outgoing_cltv_value, &hin->payment_hash,
-				next_blinding, 0, next_onion, hin,
-				&hout, &needs_update_appended);
+				next_blinding, 0 /* partid */, 0 /* groupid */,
+				next_onion, hin, &hout, &needs_update_appended);
 	if (!failmsg)
 		return;
 

lightningd/peer_htlcs.h

@@ -46,6 +46,7 @@ const u8 *send_htlc_out(const tal_t *ctx,
 			const struct sha256 *payment_hash,
 			const struct pubkey *blinding,
 			u64 partid,
+			u64 groupid,
 			const u8 *onion_routing_packet,
 			struct htlc_in *in,
 			struct htlc_out **houtp,

plugins/pay.c

@@ -1646,6 +1646,13 @@ static bool attempt_ongoing(const struct sha256 *payment_hash)
 	return false;
 }
 
+/* A unique key for each payment attempt, even if the same invoice was
+ * attempted multiple times. */
+struct pay_sort_key {
+	const struct sha256 *payment_hash;
+	u64 groupid;
+};
+
 /* We consolidate multi-part payments into a single entry. */
 struct pay_mpp {
 	/* payment_hash from the invoice and lookup key */
@@ -1676,32 +1683,31 @@ struct pay_mpp {
 
 	/* The destination of the payment, if specified. */
 	const jsmntok_t *destination;
-};
 
-static const struct sha256 *pay_mpp_key(const struct pay_mpp *pm)
-{
-	return pm->payment_hash;
-}
+	/* Which sendpay group is this? Necessary for invoices that have been
+	 * attempted multiple times. */
+	struct pay_sort_key sortkey;
+};
 
-static size_t pay_mpp_hash(const struct sha256 *payment_hash)
+static const struct pay_sort_key *pay_mpp_key(const struct pay_mpp *pm)
 {
-	return siphash24(siphash_seed(), payment_hash, sizeof(struct sha256));
+	return &pm->sortkey;
 }
 
-static bool pay_mpp_eq(const struct pay_mpp *pm, const struct sha256 *payment_hash)
+static size_t pay_mpp_hash(const struct pay_sort_key *key)
 {
-	return memcmp(pm->payment_hash, payment_hash, sizeof(struct sha256)) == 0;
+	struct siphash24_ctx ctx;
+	siphash24_init(&ctx, siphash_seed());
+	siphash24_update(&ctx, key->payment_hash, sizeof(struct sha256));
+	siphash24_update(&ctx, &key->groupid, sizeof(u64));
+	return siphash24_done(&ctx);
 }
 
-static int cmp_pay_mpp(const struct pay_mpp *a,
-		       const struct pay_mpp *b,
-		       void *unused UNUSED)
+static bool pay_mpp_eq(const struct pay_mpp *pm, const struct pay_sort_key *key)
 {
-	if (a->timestamp < b->timestamp)
-		return -1;
-	if (a->timestamp == b->timestamp)
-		return 0;
-	return 1;
+	return memcmp(pm->sortkey.payment_hash, key->payment_hash,
+		      sizeof(struct sha256)) == 0 &&
+		      pm->sortkey.groupid == key->groupid;
 }
 
 HTABLE_DEFINE_TYPE(struct pay_mpp, pay_mpp_key, pay_mpp_hash, pay_mpp_eq,
@@ -1804,9 +1810,8 @@ static struct command_result *listsendpays_done(struct command *cmd,
 	const jsmntok_t *t, *arr;
 	struct json_stream *ret;
 	struct pay_map pay_map;
-	struct pay_map_iter it;
 	struct pay_mpp *pm;
-	struct pay_mpp *pays;
+	struct pay_sort_key *order = tal_arr(tmpctx, struct pay_sort_key, 0);
 
 	pay_map_init(&pay_map);
 
@@ -1816,10 +1821,12 @@ static struct command_result *listsendpays_done(struct command *cmd,
 				    "Unexpected non-array result from listsendpays");
 
 	json_for_each_arr(i, t, arr) {
-		const jsmntok_t *status, *invstrtok, *hashtok, *createdtok;
+		const jsmntok_t *status, *invstrtok, *hashtok, *createdtok, *grouptok;
 		const char *invstr = invstring;
 		struct sha256 payment_hash;
 		u32 created_at;
+		u64 groupid;
+		struct pay_sort_key key;
 
 		invstrtok = json_get_member(buf, t, "bolt11");
 		if (!invstrtok)
@@ -1829,12 +1836,21 @@ static struct command_result *listsendpays_done(struct command *cmd,
 		assert(hashtok != NULL);
 		assert(createdtok != NULL);
 
+		grouptok = json_get_member(buf, t, "groupid");
+		if (grouptok != NULL)
+			json_to_u64(buf, grouptok, &groupid);
+		else
+			groupid = 0;
+
 		json_to_sha256(buf, hashtok, &payment_hash);
 		json_to_u32(buf, createdtok, &created_at);
 		if (invstrtok)
 			invstr = json_strdup(cmd, buf, invstrtok);
 
-		pm = pay_map_get(&pay_map, &payment_hash);
+		key.payment_hash = &payment_hash;
+		key.groupid = groupid;
+
+		pm = pay_map_get(&pay_map, &key);
 		if (!pm) {
 			pm = tal(cmd, struct pay_mpp);
 			pm->state = 0;
@@ -1847,7 +1863,12 @@ static struct command_result *listsendpays_done(struct command *cmd,
 			pm->amount = talz(pm, struct amount_msat);
 			pm->num_nonfailed_parts = 0;
 			pm->timestamp = created_at;
+			pm->sortkey.payment_hash = pm->payment_hash;
+			pm->sortkey.groupid = groupid;
 			pay_map_add(&pay_map, pm);
+			// 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);
 		}
 
 		status = json_get_member(buf, t, "status");
@@ -1866,26 +1887,14 @@ static struct command_result *listsendpays_done(struct command *cmd,
 		}
 	}
 
-
-	pays = tal_arr(NULL, struct pay_mpp, pay_map_count(&pay_map));
-	i = 0;
-	for (pm = pay_map_first(&pay_map, &it);
-	     pm;
-	     pm = pay_map_next(&pay_map, &it)) {
-		pays[i++] = *pm;
-	}
-	pay_map_clear(&pay_map);
-
-	asort(pays, tal_count(pays), cmp_pay_mpp, NULL);
-
-	/* Now we've collapsed and sorted them, provide summary. */
 	ret = jsonrpc_stream_success(cmd);
 	json_array_start(ret, "pays");
-
-	for (i = 0; i < tal_count(pays); i++)
-		add_new_entry(ret, buf, &pays[i]);
-	tal_free(pays);
-
+	for (size_t i = 0; i < tal_count(order); i++) {
+		pm = pay_map_get(&pay_map, &order[i]);
+		assert(pm != NULL);
+		add_new_entry(ret, buf, pm);
+	}
+	pay_map_clear(&pay_map);
 	json_array_end(ret);
 	return command_finished(cmd, ret);
 }

wallet/db.c

@@ -832,6 +832,9 @@ static struct migration dbmigrations[] = {
 	 ", 0"
 	 ", local_offer_id FROM temp_payments;"), NULL},
     {SQL("DROP TABLE temp_payments;"), NULL},
+    /* HTLCs also need to carry the groupid around so we can
+     * selectively update them. */
+    {SQL("ALTER TABLE channel_htlcs ADD groupid BIGINT NOT NULL DEFAULT 0;"), NULL},
 };
 
 /* Leak tracking. */

wallet/test/run-wallet.c

@@ -1820,6 +1820,7 @@ static bool test_payment_crud(struct lightningd *ld, const tal_t *ctx)
 	t->payment_preimage = NULL;
 	memset(&t->payment_hash, 1, sizeof(t->payment_hash));
 	t->partid = 0;
+	t->groupid = 0;
 
 	db_begin_transaction(w->db);
 	t2 = tal_dup(NULL, struct wallet_payment, t);
@@ -1839,8 +1840,8 @@ static bool test_payment_crud(struct lightningd *ld, const tal_t *ctx)
 	t->status = PAYMENT_COMPLETE;
 	t->payment_preimage = tal(w, struct preimage);
 	memset(t->payment_preimage, 2, sizeof(*t->payment_preimage));
-	wallet_payment_set_status(w, &t->payment_hash, t->partid, t->status,
-				  t->payment_preimage);
+	wallet_payment_set_status(w, &t->payment_hash, t->partid, t->groupid,
+				  t->status, t->payment_preimage);
 	t2 = wallet_payment_by_hash(ctx, w, &t->payment_hash, t->partid);
 	CHECK(t2 != NULL);
 	CHECK(t2->status == t->status);

wallet/wallet.c

@@ -2587,6 +2587,7 @@ static bool wallet_stmt2htlc_out(struct wallet *wallet,
 		}
 	} else {
 		out->partid = db_column_u64(stmt, 13);
+		out->groupid = db_column_u64(stmt, 14);
 		out->am_origin = true;
 	}
 
@@ -2697,6 +2698,7 @@ bool wallet_htlcs_load_out_for_channel(struct wallet *wallet,
 					     ", received_time"
 					     ", partid"
 					     ", localfailmsg"
+					     ", groupid"
 					     " FROM channel_htlcs"
 					     " WHERE direction = ?"
 					     " AND channel_id = ?"
@@ -3164,7 +3166,7 @@ wallet_payment_by_hash(const tal_t *ctx, struct wallet *wallet,
 
 void wallet_payment_set_status(struct wallet *wallet,
 			       const struct sha256 *payment_hash,
-			       u64 partid,
+			       u64 partid, u64 groupid,
 			       const enum wallet_payment_status newstatus,
 			       const struct preimage *preimage)
 {
@@ -3181,21 +3183,23 @@ void wallet_payment_set_status(struct wallet *wallet,
 
 	stmt = db_prepare_v2(wallet->db,
 			     SQL("UPDATE payments SET status=? "
-				 "WHERE payment_hash=? AND partid=?"));
+				 "WHERE payment_hash=? AND partid=? AND groupid=?"));
 
 	db_bind_int(stmt, 0, wallet_payment_status_in_db(newstatus));
 	db_bind_sha256(stmt, 1, payment_hash);
 	db_bind_u64(stmt, 2, partid);
+	db_bind_u64(stmt, 3, groupid);
 	db_exec_prepared_v2(take(stmt));
 
 	if (preimage) {
 		stmt = db_prepare_v2(wallet->db,
 				     SQL("UPDATE payments SET payment_preimage=? "
-					 "WHERE payment_hash=? AND partid=?"));
+					 "WHERE payment_hash=? AND partid=? AND groupid=?"));
 
 		db_bind_preimage(stmt, 0, preimage);
 		db_bind_sha256(stmt, 1, payment_hash);
 		db_bind_u64(stmt, 2, partid);
+		db_bind_u64(stmt, 3, groupid);
 		db_exec_prepared_v2(take(stmt));
 	}
 	if (newstatus != PAYMENT_PENDING) {
@@ -3205,9 +3209,10 @@ void wallet_payment_set_status(struct wallet *wallet,
 						  "     , route_nodes = NULL"
 						  "     , route_channels = NULL"
 						  " WHERE payment_hash = ?"
-						  " AND partid = ?;"));
+						  " AND partid = ? AND groupid=?;"));
 		db_bind_sha256(stmt, 0, payment_hash);
 		db_bind_u64(stmt, 1, partid);
+		db_bind_u64(stmt, 2, groupid);
 		db_exec_prepared_v2(take(stmt));
 	}
 }

wallet/wallet.h

@@ -1058,7 +1058,7 @@ wallet_payment_by_hash(const tal_t *ctx, struct wallet *wallet,
  */
 void wallet_payment_set_status(struct wallet *wallet,
 			       const struct sha256 *payment_hash,
-			       u64 partid,
+			       u64 partid, u64 groupid,
 			       const enum wallet_payment_status newstatus,
 			       const struct preimage *preimage);