Code refactoring and require status parameter

Signed-off-by: Vincenzo Palazzo <[email protected]>

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. 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.
 
-- *payment\_hash*: The unique identifier of a payment. Find it, you can run **listpays** or **listsendpays**;
-- *status*: Expected status of the payment. Valid values are *complete* or *failed*. 
-Only deletes 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,7 +31,7 @@ EXAMPLE JSON REQUEST
 RETURN VALUE
 ------------
 
-On success, the command will return a payment object, such as the **listsendpays**. If the payment is aa MPP (multi part payment) the command return a list of 
+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. If the lightning process fails before responding, the

lightningd/pay.c

@@ -1542,19 +1542,20 @@ 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);
 
-	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:
+		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);
 

tests/test_pay.py

@@ -3263,46 +3263,36 @@ 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 l1 -> l2 with a channel of 10 ** 5 sat!
+    l1, l2 = node_factory.line_graph(2, fundamount=10**5, wait_for_announce=True)
 
     with pytest.raises(RpcError):
         l2.rpc.delpay()
 
-    # invoice unpaid
+    # 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 unpaid with wrong status (pending status is a illegal input)
+    wait_for(lambda: l2.rpc.listpays(inv['bolt11'])['status'] == 'complete')
+
+    # 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')
 
@@ -3318,10 +3308,8 @@ def test_delpay_payment_split(node_factory, bitcoind):
     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'])

wallet/wallet.c

@@ -2669,7 +2669,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);

wallet/wallet.h

@@ -1032,8 +1032,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