From f62d7bbe45701f657f657365e48cfa8365edb43f Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Wed, 19 Aug 2020 12:38:50 +0200 Subject: [PATCH] Added additional fix to code formatting and English spelling. Signed-off-by: Vincenzo Palazzo --- doc/lightning-delpay.7 | 23 +++++------ doc/lightning-delpay.7.md | 21 +++++----- lightningd/pay.c | 31 +++++++------- tests/test_pay.py | 72 ++++++++------------------------- wallet/db_postgres_sqlgen.c | 2 +- wallet/db_sqlite3_sqlgen.c | 2 +- wallet/statements_gettextgen.po | 2 +- wallet/wallet.c | 2 +- wallet/wallet.h | 3 +- 9 files changed, 58 insertions(+), 100 deletions(-) diff --git a/doc/lightning-delpay.7 b/doc/lightning-delpay.7 index 93f63ce37..356bf9862 100644 --- a/doc/lightning-delpay.7 +++ b/doc/lightning-delpay.7 @@ -3,19 +3,18 @@ lightning-delpay - Command for removing a completed or failed payment .SH SYNOPSIS -\fBdelpay\fR \fIpayment_hash\fR [status] +\fBdelpay\fR \fIpayment_hash\fR \fIstatus\fR .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\. +The \fBdelpay\fR RPC command deletes a payment with the given \fBpayment_hash\fR if its status is either \fBcomplete\fR or \fBfailed\fR\. Deleting a \fBpending\fR payment is an error\. .RS .IP \[bu] -\fIpayment_hash\fR: Rapresents the unique identifier of a payment\. To find it, you can run \fBlistpays\fR or \fBlistsendpays\fR; +\fIpayment_hash\fR: The unique identifier of a payment\. .IP \[bu] -\fIstatus\fR is the expected status of the payment\. It can be \fIcomplete\fR or \fIfailed\fR\. -Only delete if the payment status matches\. If not specified, defaults to \fIcomplete\fR\. +\fIstatus\fR: Expected status of the payment\. +Only deletes if the payment status matches\. .RE .SH EXAMPLE JSON REQUEST @@ -34,11 +33,11 @@ Only delete if the payment status matches\. If not specified, defaults to \fIcom .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\. +If successful the command returns a payment object, in the same format as \fBlistsendpays\fR\. If the payment is a multi-part payment (MPP) the command return a list of +payments will be return -- one payment object for each partid\. -On failure, an error is returned and any payment is deleted\. If the lightning process fails before responding, the +On failure, an error is returned\. 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\. @@ -46,9 +45,9 @@ The following error codes may occur: .RS .IP \[bu] --32602: Some parameter missed or some parameter is malformed; +-32602: Parameter missed or malformed; .IP \[bu] -211: Payment with payment_hash have a wrong status\. To check the correct status run the command \fBpaystatus\fR; +211: Payment status mismatch\. Check the correct status via \fBpaystatus\fR; .IP \[bu] 208: Payment with payment_hash not found\. @@ -88,4 +87,4 @@ Vincenzo Palazzo \fI is mainly responsible\. Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:57aa95a96bb9be2cd66df3f71e9260985a53d7126862b7973179323236e3d113 +\" SHA256STAMP:b10dd430aaace8b9f3607e72c871e2a883934f9a51d5bc0068a024df5ef1d6ee diff --git a/doc/lightning-delpay.7.md b/doc/lightning-delpay.7.md index b578ef549..8c9a78f27 100644 --- a/doc/lightning-delpay.7.md +++ b/doc/lightning-delpay.7.md @@ -4,17 +4,16 @@ lightning-delpay -- Command for removing a completed or failed payment SYNOPSIS -------- -**delpay** *payment\_hash* \[status\] +**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. +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. -- *payment\_hash*: Rapresents the unique identifier of a payment. To find it, you can run **listpays** or **listsendpays**; -- *status* is the expected status of the payment. It can be *complete* or *failed*. -Only delete if the payment status matches. If not specified, defaults to *complete*. +- *payment\_hash*: The unique identifier of a payment. +- *status*: Expected status of the payment. +Only deletes if the payment status matches. EXAMPLE JSON REQUEST ------------ @@ -32,16 +31,16 @@ EXAMPLE JSON REQUEST 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. +If successful the command returns a payment object, in the same format as **listsendpays**. If the payment is a multi-part payment (MPP) the command return a list of +payments will be return -- one payment object for each partid. -On failure, an error is returned and any payment is deleted. If the lightning process fails before responding, the +On failure, an error is returned. 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**; +- -32602: Parameter missed or malformed; +- 211: Payment status mismatch. Check the correct status via **paystatus**; - 208: Payment with payment\_hash not found. EXAMPLE JSON RESPONSE diff --git a/lightningd/pay.c b/lightningd/pay.c index 40ae9b93e..a5ebdd431 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -64,9 +64,11 @@ static const char *payment_status_to_string(const enum wallet_payment_status sta return "complete"; case PAYMENT_FAILED: return "failed"; - default: + case PAYMENT_PENDING: return "pending"; } + //This should never happen + abort(); } @@ -1528,35 +1530,34 @@ static struct command_result *json_delpay(struct command *cmd, if (!param(cmd, buffer, params, p_req("payment_hash", param_sha256, &payment_hash), - p_opt("status", param_string, &status_str), + p_req("status", param_string, &status_str), NULL)) return command_param_failed(); - if (!status_str) - status_str = "complete"; - if (!string_to_payment_status(status_str, &status)) - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "Unrecognized status: %s", status_str); + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Unrecognized status: %s", status_str); - if (status == PAYMENT_PENDING) - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Invalid status: %s", - payment_status_to_string(status)); + 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)); + } 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", + 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", + 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)); + payment_status_to_string(status)); } } diff --git a/tests/test_pay.py b/tests/test_pay.py index 8cf725272..927d48985 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3265,46 +3265,38 @@ def test_mpp_presplit_routehint_conflict(node_factory, bitcoind): def test_delpay_argument_invalid(node_factory, bitcoind): """ - This test includes all possible combination of input error inside the + This test includes all possible combinations of input error inside the delpay command. """ - l1, l2 = node_factory.get_nodes(2) + # 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) with pytest.raises(RpcError): l2.rpc.delpay() - # invoice unpayed + # sanity check inv = l1.rpc.invoice(10 ** 5, 'inv', 'inv') - payment_hash = inv["payment_hash"] + payment_hash = "AA" * 32 with pytest.raises(RpcError): - l2.rpc.delpay(payment_hash) + l2.rpc.delpay(payment_hash, 'complete') + + l2.rpc.pay(inv['bolt11']) - # payment unpayed with wrong status (pending status is a illegal input) + wait_for(lambda: l2.rpc.listpays(inv['bolt11'])['pays'][0]['status'] == 'complete') + + payment_hash = inv['payment_hash'] + + # payment paid 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') @@ -3313,47 +3305,15 @@ def test_delpay_argument_invalid(node_factory, bitcoind): 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 + Test behavior of delpay with an 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} - ) + l1, l2, l3 = node_factory.line_graph(3, fundamount=10**5, + wait_for_announce=True) inv = l3.rpc.invoice(amt, 'lbl', 'desc') l1.rpc.pay(inv['bolt11']) diff --git a/wallet/db_postgres_sqlgen.c b/wallet/db_postgres_sqlgen.c index 4c092bc4e..48be39372 100644 --- a/wallet/db_postgres_sqlgen.c +++ b/wallet/db_postgres_sqlgen.c @@ -1654,4 +1654,4 @@ struct db_query db_postgres_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */ -// SHA256STAMP:849372d40a212ddb790eb4e3181c6b525dce234e48cda9b71b8909527bd48e33 +// SHA256STAMP:ab6ee42edcd9d69622233f8041ba8c592bedd2ff2e1f518d0f6aa3a7eb4cd3fb diff --git a/wallet/db_sqlite3_sqlgen.c b/wallet/db_sqlite3_sqlgen.c index 16eb53879..802884b90 100644 --- a/wallet/db_sqlite3_sqlgen.c +++ b/wallet/db_sqlite3_sqlgen.c @@ -1654,4 +1654,4 @@ struct db_query db_sqlite3_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */ -// SHA256STAMP:849372d40a212ddb790eb4e3181c6b525dce234e48cda9b71b8909527bd48e33 +// SHA256STAMP:ab6ee42edcd9d69622233f8041ba8c592bedd2ff2e1f518d0f6aa3a7eb4cd3fb diff --git a/wallet/statements_gettextgen.po b/wallet/statements_gettextgen.po index 7190d2502..efa59a11b 100644 --- a/wallet/statements_gettextgen.po +++ b/wallet/statements_gettextgen.po @@ -1089,4 +1089,4 @@ msgstr "" #: wallet/test/run-wallet.c:1359 msgid "INSERT INTO channels (id) VALUES (1);" msgstr "" -# SHA256STAMP:57b5cf2dddb562f248adabe071280e2a0f13075334451d6a91ba7d98f663b255 +# SHA256STAMP:2ed0b71f2bd0f22a8c9cdcd96a727e68f625b611557762272168fafa0240e114 diff --git a/wallet/wallet.c b/wallet/wallet.c index e95c4f59e..3f0c40f5b 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -2448,7 +2448,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); diff --git a/wallet/wallet.h b/wallet/wallet.h index 033dc14a1..008950bf4 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -973,8 +973,7 @@ void wallet_payment_delete(struct wallet *wallet, * 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); +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