From f52ff07558709bd1f7ed0cdca65c891d80b1a785 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 19 Sep 2022 10:24:26 +0930 Subject: [PATCH] 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 --- doc/lightning-delpay.7.md | 7 ++-- doc/schemas/delpay.request.json | 30 ++++++++++++++++ lightningd/pay.c | 62 ++++++++++++++++++++++++++------- plugins/autoclean.c | 9 +++++ tests/test_pay.py | 4 +++ wallet/wallet.c | 24 ++++++++++--- wallet/wallet.h | 12 ++++--- 7 files changed, 125 insertions(+), 23 deletions(-) create mode 100644 doc/schemas/delpay.request.json diff --git a/doc/lightning-delpay.7.md b/doc/lightning-delpay.7.md index 55c182c42d29..4568ebfbb9c0 100644 --- 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 new file mode 100644 index 000000000000..0b341c3041ec --- /dev/null +++ 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 index 5e6d37ffae75..07b2d09820fd 100644 --- 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,21 +1666,20 @@ 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); @@ -1665,7 +1687,14 @@ static struct command_result *json_delpay(struct command *cmd, 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 index dcbb5a64d5b3..1f3de4e8cc02 100644 --- 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 index dba8e6eb989d..465e0bedf1b8 100644 --- 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 index 8458f42bca98..a5f6b81a6f57 100644 --- 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 index d5315eb1e514..917d7f2aa9f9 100644 --- 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