From 5e0a5c91116eb02af1c7eb8adbed983ae4adda17 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 18 Jan 2018 14:27:16 +1030 Subject: [PATCH] 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: #477 Signed-off-by: Rusty Russell --- doc/lightning-delinvoice.7 | 12 +++++++----- doc/lightning-delinvoice.7.txt | 12 +++++++----- lightningd/invoice.c | 18 +++++++++++++++--- tests/test_lightningd.py | 15 +++++++++++++++ 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/doc/lightning-delinvoice.7 b/doc/lightning-delinvoice.7 index 1c058f18f..2cb2b232f 100644 --- 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 -.\" 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 index 3f2dd9f6c..670750c1b 100644 --- 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 index f0d597f22..9f73c3f85 100644 --- 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 index 4e5204274..c50db0185 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -369,6 +369,21 @@ class LightningDTests(BaseLightningDTests): 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()