From 8fef91247ddc83990485500224a67822b7f59eb0 Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Fri, 7 Aug 2020 19:21:33 +0200 Subject: [PATCH] delpay: refactoring logic inside the command and update command doc Changelog-Added: JSON-RPC: delpay a new method to delete the payment completed or failed. Signed-off-by: Vincenzo Palazzo --- common/jsonrpc_errors.h | 1 + doc/Makefile | 1 + doc/index.rst | 1 + doc/lightning-delpay.7 | 89 +++++++++++++++++++++++++++++++++ doc/lightning-delpay.7.md | 83 +++++++++++++++++++++++++++++++ lightningd/pay.c | 96 ++++++++++++++++++++++++++++++++++++ tests/test_pay.py | 101 ++++++++++++++++++++++++++++++++++++++ wallet/wallet.c | 13 ++++- wallet/wallet.h | 9 ++++ 9 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 doc/lightning-delpay.7 create mode 100644 doc/lightning-delpay.7.md diff --git a/common/jsonrpc_errors.h b/common/jsonrpc_errors.h index 64a3aa4b4f36..ccc7c791c54b 100644 --- a/common/jsonrpc_errors.h +++ b/common/jsonrpc_errors.h @@ -40,6 +40,7 @@ static const errcode_t PAY_INVOICE_EXPIRED = 207; static const errcode_t PAY_NO_SUCH_PAYMENT = 208; static const errcode_t PAY_UNSPECIFIED_ERROR = 209; static const errcode_t PAY_STOPPED_RETRYING = 210; +static const errcode_t PAY_STATUS_UNEXPECTED = 211; /* `fundchannel` or `withdraw` errors */ static const errcode_t FUND_MAX_EXCEEDED = 300; diff --git a/doc/Makefile b/doc/Makefile index 8c02044a5ed3..c3d44c3e70b7 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -16,6 +16,7 @@ MANPAGES := doc/lightning-cli.1 \ doc/lightning-decodepay.7 \ doc/lightning-delexpiredinvoice.7 \ doc/lightning-delinvoice.7 \ + doc/lightning-delpay.7 \ doc/lightning-dev-sendcustommsg.7 \ doc/lightning-disconnect.7 \ doc/lightning-feerates.7 \ diff --git a/doc/index.rst b/doc/index.rst index 95c286885a98..69e34e6f3dde 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -38,6 +38,7 @@ c-lightning Documentation lightning-decodepay lightning-delexpiredinvoice lightning-delinvoice + lightning-delpay lightning-dev-sendcustommsg lightning-disconnect lightning-feerates diff --git a/doc/lightning-delpay.7 b/doc/lightning-delpay.7 new file mode 100644 index 000000000000..2fa7d01f8ed7 --- /dev/null +++ b/doc/lightning-delpay.7 @@ -0,0 +1,89 @@ +.TH "LIGHTNING-DELPAY" "7" "" "" "lightning-delpay" +.SH NAME +lightning-delpay - Command for removing a completed or failed payment +.SH SYNOPSIS + +\fBdelpay\fR \fIpayment_hash\fR [status] + +.SH DESCRIPTION + +The \fBdelpay\fR RPC command removes a payment as given in \fBlistsendpays\fR or \fBlistpays\fR with a complete or failed +status\. However, the command doesn't permit to remove a pending payment\. + +.RS +.IP \[bu] +\fIpayment_hash\fR: Rapresents the unique identifier of a payment\. To find it, the caller can run \fBlistpays\fR or \fBlistsendpays\fR; +.IP \[bu] +\fIstatus\fR: Represents a payment status\. Otherwise, if it is not specified it is equal to \fIcomplete\fR\. + +.RE +.SH EXAMPLE JSON REQUEST +.nf +.RS +{ + "id": 82, + "method": "delpay", + "params": { + "payment_hash": "4fa2f1b001067ec06d7f95b8695b8acd9ef04c1b4d1110e3b94e1fa0687bb1e0", + "status": "complete" + } +} +.RE + +.fi +.SH RETURN VALUE + +On success, the command will return a payment object, such as the \fBlistsendpays\fR\. In addition, if the payment is a MPP (Multi part payment) the command return a list of +payments; a payment object for each partid\. + + +On failure, an error is returned and any payment is deleted\. If the lightning process fails before responding, the +caller should use \fBlightning-listsentpays\fR(7) or \fBlightning-listpays\fR(7) to query whether this payment was deleted or not\. + + +The following error codes may occur: + +.RS +.IP \[bu] +-32602: Some parameter missed or some parameter is malformed; +.IP \[bu] +211: Payment with payment_hash have a wrong status\. To check the correct status run the command \fBpaystatus\fR; +.IP \[bu] +208: Payment with payment_hash not found\. + +.RE +.SH EXAMPLE JSON RESPONSE +.nf +.RS +{ + "payments": [ + { + "id": 2, + "payment_hash": "8dfd6538eeb33811c9114a75f792a143728d7f05643f38c3d574d3097e8910c0", + "destination": "0219f8900ee78a89f050c24d8b69492954f9fdbabed753710845eb75d3a75a5880", + "msatoshi": 1000, + "amount_msat": "1000msat", + "msatoshi_sent": 1000, + "amount_sent_msat": "1000msat", + "created_at": 1596224858, + "status": "complete", + "payment_preimage": "35bd4e2b481a1a84a22215b5372672cf81460a671816960ddb206464359e1822", + "bolt11": "lntb10n1p0jga20pp53h7k2w8wkvuprjg3ff6l0y4pgdeg6lc9vsln3s74wnfsjl5fzrqqdqdw3jhxazldahx2xqyjw5qcqp2sp5wut5jnhr6n7jd5747ky2g5flmw7hgx9yjnqzu60ps2jf6f7tc0us9qy9qsqu2a0k37nckl62005p69xavlkydkvhnypk4dphffy4x09zltwh9437ad7xkl83tefdarzhu5t30ju5s56wlrg97qkx404pq3srfc425cq3ke9af" + } + ] +} +.RE + +.fi +.SH AUTHOR + +Vincenzo Palazzo \fI is mainly responsible\. + +.SH SEE ALSO + +\fBlightning-listpays\fR(7), \fBlightning-listsendpays\fR(7), \fBlightning-paystatus\fR(7)\. + +.SH RESOURCES + +Main web site: \fIhttps://github.com/ElementsProject/lightning\fR + diff --git a/doc/lightning-delpay.7.md b/doc/lightning-delpay.7.md new file mode 100644 index 000000000000..26fa855051b2 --- /dev/null +++ b/doc/lightning-delpay.7.md @@ -0,0 +1,83 @@ +lightning-delpay -- Command for removing a completed or failed payment +============================================================ + +SYNOPSIS +-------- + +**delpay** *payment_hash* \[status\] + +DESCRIPTION +----------- + +The **delpay** RPC command removes a payment as given in **listsendpays** or **listpays** with a complete or failed +status. However, the command doesn't permit to remove a pending payment. + +- *payment_hash*: Rapresents the unique identifier of a payment. To find it, the caller can run **listpays** or **listsendpays**; +- *status*: Represents a payment status. Otherwise, if it is not specified it is equal to *complete*. + +EXAMPLE JSON REQUEST +------------ +```json +{ + "id": 82, + "method": "delpay", + "params": { + "payment_hash": "4fa2f1b001067ec06d7f95b8695b8acd9ef04c1b4d1110e3b94e1fa0687bb1e0", + "status": "complete" + } +} +``` + +RETURN VALUE +------------ + +On success, the command will return a payment object, such as the **listsendpays**. In addition, if the payment is a MPP (Multi part payment) the command return a list of +payments; a payment object for each partid. + +On failure, an error is returned and any payment is deleted. If the lightning process fails before responding, the +caller should use lightning-listsentpays(7) or lightning-listpays(7) to query whether this payment was deleted or not. + +The following error codes may occur: + +- -32602: Some parameter missed or some parameter is malformed; +- 211: Payment with payment_hash have a wrong status. To check the correct status run the command **paystatus**; +- 208: Payment with payment_hash not found. + +EXAMPLE JSON RESPONSE +----- +```json +{ + "payments": [ + { + "id": 2, + "payment_hash": "8dfd6538eeb33811c9114a75f792a143728d7f05643f38c3d574d3097e8910c0", + "destination": "0219f8900ee78a89f050c24d8b69492954f9fdbabed753710845eb75d3a75a5880", + "msatoshi": 1000, + "amount_msat": "1000msat", + "msatoshi_sent": 1000, + "amount_sent_msat": "1000msat", + "created_at": 1596224858, + "status": "complete", + "payment_preimage": "35bd4e2b481a1a84a22215b5372672cf81460a671816960ddb206464359e1822", + "bolt11": "lntb10n1p0jga20pp53h7k2w8wkvuprjg3ff6l0y4pgdeg6lc9vsln3s74wnfsjl5fzrqqdqdw3jhxazldahx2xqyjw5qcqp2sp5wut5jnhr6n7jd5747ky2g5flmw7hgx9yjnqzu60ps2jf6f7tc0us9qy9qsqu2a0k37nckl62005p69xavlkydkvhnypk4dphffy4x09zltwh9437ad7xkl83tefdarzhu5t30ju5s56wlrg97qkx404pq3srfc425cq3ke9af" + } + ] +} + +``` + + +AUTHOR +------ + +Vincenzo Palazzo <> is mainly responsible. + +SEE ALSO +-------- + +lightning-listpays(7), lightning-listsendpays(7), lightning-paystatus(7). + +RESOURCES +--------- + +Main web site: diff --git a/lightningd/pay.c b/lightningd/pay.c index f9dc40f5715e..9bbaf3271635 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -42,6 +42,31 @@ struct sendpay_command { struct command *cmd; }; +static enum wallet_payment_status string_to_payment_status(const char *status_str) +{ + if (streq(status_str, "complete")) { + return PAYMENT_COMPLETE; + } else if (streq(status_str, "failed")) { + return PAYMENT_FAILED; + } else if (streq(status_str, "pending")) { + return PAYMENT_PENDING; + } + return -1; +} + +static const char *payment_status_to_string(const enum wallet_payment_status status) +{ + switch (status) { + case PAYMENT_COMPLETE: + return "complete"; + case PAYMENT_FAILED: + return "failed"; + default: + return "pending"; + } +} + + static void destroy_sendpay_command(struct sendpay_command *pc) { list_del(&pc->list); @@ -1496,6 +1521,77 @@ static const struct json_command listsendpays_command = { }; AUTODATA(json_command, &listsendpays_command); + +static struct command_result *json_delpay(struct command *cmd, + const char *buffer, + const jsmntok_t *obj UNNEEDED, + const jsmntok_t *params) +{ + struct json_stream *response; + const struct wallet_payment **payments; + const char *status_str; + enum wallet_payment_status status; + struct sha256 *payment_hash; + + if (!param(cmd, buffer, params, + p_req("payment_hash", param_sha256, &payment_hash), + p_opt("status", param_string, &status_str), + NULL)) + return command_param_failed(); + + if (!payment_hash) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "missing required parameter: payment_hash"); + + + status = status_str == NULL ? PAYMENT_COMPLETE : string_to_payment_status(status_str); + + if (status == -1) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "status insert %s wrong", status_str); + + if (status == PAYMENT_PENDING) + return command_fail(cmd, PAY_STATUS_UNEXPECTED, "invalid status: %s", + payment_status_to_string(status)); + + 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)); + + for (int i = 0; i < tal_count(payments); i++) { + 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), + payment_status_to_string(payments[i]->status), + payment_status_to_string(status)); + } + } + + wallet_payment_delete_by_hash(cmd->ld->wallet, payment_hash); + + response = json_stream_success(cmd); + json_array_start(response, "payments"); + for (int i = 0; i < tal_count(payments); i++) { + json_object_start(response, NULL); + json_add_payment_fields(response, payments[i]); + json_object_end(response); + } + json_array_end(response); + return command_success(cmd, response); +} + +static const struct json_command delpay_command = { + "delpay", + "payment", + json_delpay, + "Delete payment with {payment_hash} and {status}", +}; +AUTODATA(json_command, &delpay_command); + static struct command_result *json_createonion(struct command *cmd, const char *buffer, const jsmntok_t *obj UNNEEDED, diff --git a/tests/test_pay.py b/tests/test_pay.py index dc5d99364d14..de6f5ad4b65b 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3239,3 +3239,104 @@ def test_mpp_presplit_routehint_conflict(node_factory, bitcoind): inv = l3.rpc.invoice(Millisatoshi(2 * 10000 * 1000), 'i', 'i', exposeprivatechannels=True)['bolt11'] l1.rpc.pay(inv) + + +def test_delpay_argument_invalid(node_factory, bitcoind): + """ + This test includes all possible combination of input error inside the + delpay command. + """ + + l1, l2 = node_factory.get_nodes(2) + + with pytest.raises(RpcError): + l2.rpc.delpay() + + # invoice unpayed + inv = l1.rpc.invoice(10 ** 5, 'inv', 'inv') + payment_hash = inv["payment_hash"] + with pytest.raises(RpcError): + l2.rpc.delpay(payment_hash) + + # payment unpayed with wrong status (pending status is a illegal input) + with pytest.raises(RpcError): + l2.rpc.delpay(payment_hash, 'pending') + + with pytest.raises(RpcError): + l2.rpc.delpay(payment_hash, 'invalid_status') + + l2.rpc.connect(l1.info['id'], 'localhost', l1.port) + l2.fund_channel(l1, 10 ** 6) + + bitcoind.generate_block(6) + sync_blockheight(bitcoind, [l1, l2]) + + wait_for(lambda: len(l2.rpc.listchannels()['channels']) == 2) + + l2.rpc.pay(inv['bolt11']) + + with pytest.raises(RpcError): + l2.rpc.delpay(payment_hash, 'failed') + + with pytest.raises(RpcError): + l2.rpc.delpay(payment_hash, 'pending') + + assert len(l2.rpc.listpays()['pays']) == 1 + + # test if the node is still ready + payments = l2.rpc.delpay(payment_hash, 'complete') + + assert payments['payments'][0]['bolt11'] == inv['bolt11'] + assert len(payments['payments']) == 1 + assert len(l2.rpc.listpays()['pays']) == 0 + + +def test_delpay(node_factory, bitcoind): + """ + This unit test try to catch some error inside the command + delpay when it receives the correct input from the user + """ + + l1, l2 = node_factory.get_nodes(2) + + amount_sat = 10 ** 6 + + # create l2->l1 channel. + l2.fundwallet(amount_sat * 5) + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + l2.rpc.fundchannel(l1.info['id'], amount_sat * 3) + + # Let the channel confirm. + bitcoind.generate_block(6) + sync_blockheight(bitcoind, [l1, l2]) + + invl1 = l1.rpc.invoice(Millisatoshi(amount_sat * 2 * 1000), "j", "j") + l2.rpc.pay(invl1["bolt11"]) + + before_del_pay = l2.rpc.listpays() + + l2.rpc.delpay(invl1["payment_hash"]) + + after_del_pay = l2.rpc.listpays()["pays"] + assert len(after_del_pay) == (len(before_del_pay) - 1) + + +def test_delpay_payment_split(node_factory, bitcoind): + """ + This test test the correct bheaivord of the commmand delpay with a mpp + """ + MPP_TARGET_SIZE = 10**7 # Taken from libpluin-pay.c + amt = 5 * MPP_TARGET_SIZE + + l1, l2, l3 = node_factory.line_graph( + 3, fundamount=10**8, wait_for_announce=True, + opts={'wumbo': None} + ) + + inv = l3.rpc.invoice(amt, 'lbl', 'desc') + l1.rpc.pay(inv['bolt11']) + + assert len(l1.rpc.listpays()['pays']) == 1 + delpay_result = l1.rpc.delpay(inv['payment_hash'], 'complete')['payments'] + assert len(delpay_result) >= 5 + assert len(l1.rpc.listpays()['pays']) == 0 diff --git a/wallet/wallet.c b/wallet/wallet.c index 4cc99ce599b3..d9374ec6146c 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -2641,7 +2641,7 @@ void wallet_payment_store(struct wallet *wallet, if (payment->bolt11 != NULL) db_bind_text(stmt, 10, payment->bolt11); else - db_bind_null(stmt, 10); + db_bind_null(stmt, 10); db_bind_amount_msat(stmt, 11, &payment->total_msat); db_bind_u64(stmt, 12, payment->partid); @@ -2682,6 +2682,17 @@ void wallet_payment_delete(struct wallet *wallet, db_exec_prepared_v2(take(stmt)); } +void wallet_payment_delete_by_hash(struct wallet *wallet, + const struct sha256 *payment_hash) +{ + struct db_stmt *stmt; + 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)); +} + static struct wallet_payment *wallet_stmt2payment(const tal_t *ctx, struct db_stmt *stmt) { diff --git a/wallet/wallet.h b/wallet/wallet.h index ba51401c4c2d..61b1547a8286 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -1026,6 +1026,15 @@ void wallet_payment_delete(struct wallet *wallet, const struct sha256 *payment_hash, u64 partid); +/** + * wallet_payment_delete_by_hash - 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. + */ +void wallet_payment_delete_by_hash(struct wallet *wallet, + const struct sha256 *payment_hash); + /** * wallet_local_htlc_out_delete - Remove a local outgoing failed HTLC *