Merge pull request #8672 from mohamedawnallah/makeDeleteAllPaymentsArgsRequired

rpcserver: Make sure the arguments are provided when calling `DeleteAllPayments` RPC

Author: guggero

cmd/lncli/cmd_payments.go

@@ -1783,6 +1783,7 @@ func deletePayments(ctx *cli.Context) error {
 			what)
 		_, err = client.DeleteAllPayments(
 			ctxc, &lnrpc.DeleteAllPaymentsRequest{
+				AllPayments:        includeNonFailed,
 				FailedPaymentsOnly: !includeNonFailed,
 				FailedHtlcsOnly:    failedHTLCsOnly,
 			},

docs/release-notes/release-notes-0.18.0.md

@@ -355,6 +355,12 @@ bitcoin peers' feefilter values into account](https://github.com/lightningnetwor
   `budget`, and `deadline_height`, the fields `force`, `requested_conf_target`,
   and `next_broadcast_height` are deprecated.
 
+* [Delete All Payments RPC](https://github.com/lightningnetwork/lnd/pull/8672)
+  adds `all_payments` option to the `DeleteAllPayments` RPC. This update
+  ensures that the arguments are provided when calling `DeleteAllPayments` RPC,
+  whether through gRPC or the REST API, due to the destructive nature of the
+  operation.
+
 ## lncli Updates
 
 * [Documented all available `lncli`

lnrpc/lightning.pb.go

@@ -4231,6 +4231,10 @@ message DeleteAllPaymentsRequest {
     Only delete failed HTLCs from payments, not the payment itself.
     */
     bool failed_htlcs_only = 2;
+
+    // Delete all payments. NOTE: Using this option requires careful
+    // consideration as it is a destructive operation.
+    bool all_payments = 3;
 }
 
 message DeletePaymentResponse {

lnrpc/lightning.proto

@@ -2268,6 +2268,13 @@
             "in": "query",
             "required": false,
             "type": "boolean"
+          },
+          {
+            "name": "all_payments",
+            "description": "Delete all payments. NOTE: Using this option requires careful\nconsideration as it is a destructive operation.",
+            "in": "query",
+            "required": false,
+            "type": "boolean"
           }
         ],
         "tags": [

lnrpc/lightning.swagger.json

@@ -70,7 +70,7 @@ func (h *HarnessRPC) DeleteAllPayments() {
 	ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout)
 	defer cancel()
 
-	req := &lnrpc.DeleteAllPaymentsRequest{}
+	req := &lnrpc.DeleteAllPaymentsRequest{AllPayments: true}
 	_, err := h.LN.DeleteAllPayments(ctxt, req)
 	h.NoError(err, "DeleteAllPayments")
 }

lntest/rpc/lnd.go

@@ -6763,6 +6763,27 @@ func (r *rpcServer) DeleteAllPayments(ctx context.Context,
 	req *lnrpc.DeleteAllPaymentsRequest) (
 	*lnrpc.DeleteAllPaymentsResponse, error) {
 
+	switch {
+	// Since this is a destructive operation, at least one of the options
+	// must be set to true.
+	case !req.AllPayments && !req.FailedPaymentsOnly &&
+		!req.FailedHtlcsOnly:
+
+		return nil, fmt.Errorf("at least one of the options " +
+			"`all_payments`, `failed_payments_only`, or " +
+			"`failed_htlcs_only` must be set to true")
+
+	// `all_payments` cannot be true with `failed_payments_only` or
+	// `failed_htlcs_only`. `all_payments` includes all records, making
+	// these options contradictory.
+	case req.AllPayments &&
+		(req.FailedPaymentsOnly || req.FailedHtlcsOnly):
+
+		return nil, fmt.Errorf("`all_payments` cannot be set to true " +
+			"while either `failed_payments_only` or " +
+			"`failed_htlcs_only` is also set to true")
+	}
+
 	rpcsLog.Infof("[DeleteAllPayments] failed_payments_only=%v, "+
 		"failed_htlcs_only=%v", req.FailedPaymentsOnly,
 		req.FailedHtlcsOnly)