JSONRPC: delinvoice: have a status argument.

delinvoice was orginally documented to only allow deletion of unpaid invoices, but there might be reasons to delete paid ones or unexpired ones. But we have to avoid the race where someone pays as it's deleted: the easiest way is to have the caller tell us the status, and fail if it's wrong. Fixes: ElementsProject#477 Signed-off-by: Rusty Russell <[email protected]>
Jon0405 · Jan 19, 2018 · 5e0a5c9 · 5e0a5c9
1 parent 2443d45
commit 5e0a5c9
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 13 deletions.
diff --git a/doc/lightning-delinvoice.7 b/doc/lightning-delinvoice.7
@@ -2,12 +2,12 @@
 .\"     Title: lightning-delinvoice
 .\"    Author: [see the "AUTHOR" section]
 .\" Generator: DocBook XSL Stylesheets v1.79.1 <http://docbook.sf.net/>
-.\"      Date: 01/13/2018
+.\"      Date: 01/18/2018
 .\"    Manual: \ \&
 .\"    Source: \ \&
 .\"  Language: English
 .\"
-.TH "LIGHTNING\-DELINVOIC" "7" "01/13/2018" "\ \&" "\ \&"
+.TH "LIGHTNING\-DELINVOIC" "7" "01/18/2018" "\ \&" "\ \&"
 .\" -----------------------------------------------------------------
 .\" * Define some portability stuff
 .\" -----------------------------------------------------------------
@@ -28,13 +28,15 @@
 .\" * MAIN CONTENT STARTS HERE *
 .\" -----------------------------------------------------------------
 .SH "NAME"
-lightning-delinvoice \- Protocol for removing an unpaid invoice\&.
+lightning-delinvoice \- Protocol for removing an invoice\&.
 .SH "SYNOPSIS"
 .sp
-\fBdelinvoice\fR \fIlabel\fR
+\fBdelinvoice\fR \fIlabel\fR \fIstatus\fR
 .SH "DESCRIPTION"
 .sp
-The \fBdelinvoice\fR RPC command removes an unpaid invoice\&. The caller should be particularly aware of the error case caused by a payment just before this command is invoked!
+The \fBdelinvoice\fR RPC command removes an invoice with \fIstatus\fR as given in \fBlistinvoices\fR\&.
+.sp
+The caller should be particularly aware of the error case caused by the \fIstatus\fR chaning just before this command is invoked!
 .SH "RETURN VALUE"
 .sp
 On success, an invoice description will be returned as per lightning\-listinvoice(7)\&.

diff --git a/doc/lightning-delinvoice.7.txt b/doc/lightning-delinvoice.7.txt
@@ -4,17 +4,19 @@ LIGHTNING-DELINVOICE(7)
 
 NAME
 ----
-lightning-delinvoice - Protocol for removing an unpaid invoice.
+lightning-delinvoice - Protocol for removing an invoice.
 
 SYNOPSIS
 --------
-*delinvoice* 'label'
+*delinvoice* 'label' 'status'
 
 DESCRIPTION
 -----------
-The *delinvoice* RPC command removes an unpaid invoice.  The caller
-should be particularly aware of the error case caused by a payment
-just before this command is invoked!
+The *delinvoice* RPC command removes an invoice with 'status' as
+given in *listinvoices*.
+
+The caller should be particularly aware of the error case caused by 
+the 'status' chaning just before this command is invoked!
 
 RETURN VALUE
 ------------

diff --git a/lightningd/invoice.c b/lightningd/invoice.c
@@ -275,13 +275,14 @@ static void json_delinvoice(struct command *cmd,
 			    const char *buffer, const jsmntok_t *params)
 {
 	const struct invoice *i;
-	jsmntok_t *labeltok;
+	jsmntok_t *labeltok, *statustok;
 	struct json_result *response = new_json_result(cmd);
-	const char *label;
+	const char *label, *status, *actual_status;
 	struct wallet *wallet = cmd->ld->wallet;
 
 	if (!json_get_params(buffer, params,
 			     "label", &labeltok,
+			     "status", &statustok,
 			     NULL)) {
 		command_fail(cmd, "Invalid arguments");
 		return;
@@ -295,6 +296,17 @@ static void json_delinvoice(struct command *cmd,
 		return;
 	}
 
+	status = tal_strndup(cmd, buffer + statustok->start,
+			     statustok->end - statustok->start);
+	/* This is time-sensitive, so only call once; otherwise error msg
+	 * might not make sense if it changed! */
+	actual_status = invoice_status_str(i);
+	if (!streq(actual_status, status)) {
+		command_fail(cmd, "Invoice status is %s not %s",
+			     actual_status, status);
+		return;
+	}
+
 	/* Get invoice details before attempting to delete, as
 	 * otherwise the invoice will be freed. */
 	json_add_invoice(response, i, true);
@@ -313,7 +325,7 @@ static void json_delinvoice(struct command *cmd,
 static const struct json_command delinvoice_command = {
 	"delinvoice",
 	json_delinvoice,
-	"Delete unpaid invoice {label}))",
+	"Delete unpaid invoice {label} with {status}",
 	"Returns {label}, {payment_hash}, {msatoshi} (if set), {complete}, {pay_index} (if paid) and {expiry_time} on success. "
 };
 AUTODATA(json_command, &delinvoice_command);

diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py
@@ -369,6 +369,21 @@ def test_invoice_expiry(self):
         assert l2.rpc.listinvoices('test_pay')['invoices'][0]['status'] == 'expired'
         assert l2.rpc.listinvoices('test_pay')['invoices'][0]['expiry_time'] < time.time()
 
+        # Try deleting it.
+        self.assertRaisesRegex(ValueError,
+                               'Invoice status is expired not unpaid',
+                               l2.rpc.delinvoice,
+                               'test_pay', 'unpaid')
+        self.assertRaisesRegex(ValueError,
+                               'Invoice status is expired not paid',
+                               l2.rpc.delinvoice,
+                               'test_pay', 'paid')
+        l2.rpc.delinvoice('test_pay', 'expired')
+
+        self.assertRaisesRegex(ValueError,
+                               'Unknown invoice',
+                               l2.rpc.delinvoice,
+                               'test_pay', 'expired')
     def test_connect(self):
         l1,l2 = self.connect()