lightningd: allow delpay to delete a specific payment.

This is actually what the autoclean plugin wants, especially since you can't otherwise delete a payment which has failed then succeeded. But insist on neither or both being specified, at least for now. Changelog-Added: JSON-RPC: `delpay` takes optional `groupid` and `partid` parameters to specify exactly what payment to delete. Signed-off-by: Rusty Russell <[email protected]>
okeygo · Sep 22, 2022 · f52ff07 · f52ff07
1 parent 0509531
commit f52ff07
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 23 deletions.
diff --git a/doc/lightning-delpay.7.md b/doc/lightning-delpay.7.md
@@ -4,15 +4,18 @@ lightning-delpay -- Command for removing a completed or failed payment
 SYNOPSIS
 --------
 
-**delpay** *payment\_hash* *status*
+**delpay** *payment\_hash* *status* [*partid* *groupid*]
 
 DESCRIPTION
 -----------
 
-The **delpay** RPC command deletes a payment with the given `payment_hash` if its status is either `complete` or `failed`. Deleting a `pending` payment is an error.
+The **delpay** RPC command deletes a payment with the given `payment_hash` if its status is either `complete` or `failed`. Deleting a `pending` payment is an error.  If *partid* and *groupid* are not specified, all payment parts are deleted.
 
 - *payment\_hash*: The unique identifier of a payment.
 - *status*: Expected status of the payment. 
+- *partid*: Specific partid to delete (must be paired with *groupid*)
+- *groupid*: Specific groupid to delete (must be paired with *partid*)
+
 Only deletes if the payment status matches.
 
 EXAMPLE JSON REQUEST

diff --git a/doc/schemas/delpay.request.json b/doc/schemas/delpay.request.json
@@ -0,0 +1,30 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "type": "object",
+  "required": [
+    "payment_hash",
+    "status"
+  ],
+  "additionalProperties": false,
+  "properties": {
+    "payment_hash": {
+      "type": "hex",
+      "description": "the hash of the *payment_preimage* which will prove payment",
+      "maxLength": 64,
+      "minLength": 64
+    },
+    "status": {
+      "type": "string",
+      "enum": [
+        "complete",
+        "failed"
+      ]
+    },
+    "partid": {
+      "type": "u64"
+    },
+    "groupid": {
+      "type": "u64"
+    }
+  }
+}
diff --git a/lightningd/pay.c b/lightningd/pay.c
@@ -1633,6 +1633,29 @@ static const struct json_command listsendpays_command = {
 };
 AUTODATA(json_command, &listsendpays_command);
 
+static struct command_result *
+param_payment_status_nopending(struct command *cmd,
+			       const char *name,
+			       const char *buffer,
+			       const jsmntok_t *tok,
+			       enum wallet_payment_status **status)
+{
+	struct command_result *res;
+
+	res = param_payment_status(cmd, name, buffer, tok, status);
+	if (res)
+		return res;
+
+	switch (**status) {
+	case PAYMENT_COMPLETE:
+	case PAYMENT_FAILED:
+		break;
+	case PAYMENT_PENDING:
+		return command_fail_badparam(cmd, name, buffer, tok,
+					     "Cannot delete pending status");
+	}
+	return NULL;
+}
 
 static struct command_result *json_delpay(struct command *cmd,
 						const char *buffer,
@@ -1643,29 +1666,35 @@ static struct command_result *json_delpay(struct command *cmd,
 	const struct wallet_payment **payments;
 	enum wallet_payment_status *status;
 	struct sha256 *payment_hash;
+	u64 *groupid, *partid;
+	bool found;
 
 	if (!param(cmd, buffer, params,
-		p_req("payment_hash", param_sha256, &payment_hash),
-		p_req("status", param_payment_status, &status),
-		NULL))
+		   p_req("payment_hash", param_sha256, &payment_hash),
+		   p_req("status", param_payment_status_nopending, &status),
+		   p_opt("partid", param_u64, &partid),
+		   p_opt("groupid", param_u64, &groupid),
+		   NULL))
 		return command_param_failed();
 
-	switch (*status) {
-		case PAYMENT_COMPLETE:
-		case PAYMENT_FAILED:
-			break;
-		case PAYMENT_PENDING:
-			return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Invalid status: %s",
-				payment_status_to_string(*status));
-	}
+	if ((partid != NULL) != (groupid != NULL))
+		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+				    "Must set both partid and groupid, or neither");
 
 	payments = wallet_payment_list(cmd, cmd->ld->wallet, payment_hash);
 
 	if (tal_count(payments) == 0)
 		return command_fail(cmd, PAY_NO_SUCH_PAYMENT, "Unknown payment with payment_hash: %s",
 				    type_to_string(tmpctx, struct sha256, payment_hash));
 
+	found = false;
 	for (int i = 0; i < tal_count(payments); i++) {
+		if (groupid && payments[i]->groupid != *groupid)
+			continue;
+		if (partid && payments[i]->partid != *partid)
+			continue;
+
+		found = true;
 		if (payments[i]->status != *status) {
 			return command_fail(cmd, PAY_STATUS_UNEXPECTED, "Payment with hash %s has %s status but it should be %s",
 					type_to_string(tmpctx, struct sha256, payment_hash),
@@ -1674,11 +1703,20 @@ static struct command_result *json_delpay(struct command *cmd,
 		}
 	}
 
-	wallet_payment_delete_by_hash(cmd->ld->wallet, payment_hash);
+	if (!found) {
+		return command_fail(cmd, PAY_NO_SUCH_PAYMENT,
+				    "No payment for that payment_hash with that partid and groupid");
+	}
+
+	wallet_payment_delete(cmd->ld->wallet, payment_hash, partid, groupid);
 
 	response = json_stream_success(cmd);
 	json_array_start(response, "payments");
 	for (int i = 0; i < tal_count(payments); i++) {
+		if (groupid && payments[i]->groupid != *groupid)
+			continue;
+		if (partid && payments[i]->partid != *partid)
+			continue;
 		json_object_start(response, NULL);
 		json_add_payment_fields(response, payments[i]);
 		json_object_end(response);

diff --git a/plugins/autoclean.c b/plugins/autoclean.c
@@ -271,10 +271,19 @@ static struct command_result *listsendpays_done(struct command *cmd,
 		if (paytime <= now - cinfo->subsystem_age[subsys]) {
 			struct out_req *req;
 			const jsmntok_t *phash = json_get_member(buf, t, "payment_hash");
+			const jsmntok_t *groupid = json_get_member(buf, t, "groupid");
+			const jsmntok_t *partidtok = json_get_member(buf, t, "partid");
+			u64 partid;
+			if (partidtok)
+				json_to_u64(buf, partidtok, &partid);
+			else
+				partid = 0;
 
 			req = del_request_start("delpay", cinfo, subsys);
 			json_add_tok(req->js, "payment_hash", phash, buf);
 			json_add_tok(req->js, "status", status, buf);
+			json_add_tok(req->js, "groupid", groupid, buf);
+			json_add_u64(req->js, "partid", partid);
 			send_outreq(plugin, req);
 		}
 	}

diff --git a/tests/test_pay.py b/tests/test_pay.py
@@ -4009,8 +4009,10 @@ def test_delpay_argument_invalid(node_factory, bitcoind):
     # Create the line graph l2 -> l1 with a channel of 10 ** 5 sat!
     l2, l1 = node_factory.line_graph(2, fundamount=10**5, wait_for_announce=True)
 
+    l2.rpc.check_request_schemas = False
     with pytest.raises(RpcError):
         l2.rpc.delpay()
+    l2.rpc.check_request_schemas = True
 
     # sanity check
     inv = l1.rpc.invoice(10 ** 5, 'inv', 'inv')
@@ -4025,11 +4027,13 @@ def test_delpay_argument_invalid(node_factory, bitcoind):
     payment_hash = inv['payment_hash']
 
     # payment paid with wrong status (pending status is a illegal input)
+    l2.rpc.check_request_schemas = False
     with pytest.raises(RpcError):
         l2.rpc.delpay(payment_hash, 'pending')
 
     with pytest.raises(RpcError):
         l2.rpc.delpay(payment_hash, 'invalid_status')
+    l2.rpc.check_request_schemas = True
 
     with pytest.raises(RpcError):
         l2.rpc.delpay(payment_hash, 'failed')

diff --git a/wallet/wallet.c b/wallet/wallet.c
@@ -3163,13 +3163,27 @@ u64 wallet_payment_get_groupid(struct wallet *wallet,
 	return groupid;
 }
 
-void wallet_payment_delete_by_hash(struct wallet *wallet,
-				   const struct sha256 *payment_hash)
+void wallet_payment_delete(struct wallet *wallet,
+			   const struct sha256 *payment_hash,
+			   const u64 *groupid,
+			   const u64 *partid)
 {
 	struct db_stmt *stmt;
-	stmt = db_prepare_v2(
-		wallet->db, SQL("DELETE FROM payments WHERE payment_hash = ?"));
-
+	if (groupid) {
+		assert(partid);
+		stmt = db_prepare_v2(wallet->db,
+				     SQL("DELETE FROM payments"
+					 " WHERE payment_hash = ?"
+					 "   AND groupid = ?"
+					 "   AND partid = ?"));
+		db_bind_u64(stmt, 1, *groupid);
+		db_bind_u64(stmt, 2, *partid);
+	} else {
+		assert(!partid);
+		stmt = db_prepare_v2(wallet->db,
+				     SQL("DELETE FROM payments"
+					 " WHERE payment_hash = ?"));
+	}
 	db_bind_sha256(stmt, 0, payment_hash);
 	db_exec_prepared_v2(take(stmt));
 }

diff --git a/wallet/wallet.h b/wallet/wallet.h
@@ -1089,12 +1089,16 @@ void wallet_payment_store(struct wallet *wallet,
 			  struct wallet_payment *payment TAKES);
 
 /**
- * wallet_payment_delete_by_hash - Remove a payment
+ * wallet_payment_delete - Remove a payment
  *
- * Removes the payment from the database by hash; if it is a MPP payment
- * it remove all parts with a single query.
+ * Removes the payment from the database by hash; groupid and partid
+ * may both be NULL to delete all entries, otherwise deletes only that
+ * group/partid.
  */
-void wallet_payment_delete_by_hash(struct wallet *wallet, const struct sha256 *payment_hash);
+void wallet_payment_delete(struct wallet *wallet,
+			   const struct sha256 *payment_hash,
+			   const u64 *groupid,
+			   const u64 *partid);
 
 /**
  * wallet_local_htlc_out_delete - Remove a local outgoing failed HTLC