From 17858c94902e505e491e16255a8b256bb4ed6b6b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 19 Sep 2022 10:19:52 +0930 Subject: [PATCH] lightningd: deprecated "delexpiredinvoice", put functionality in autoclean plugin. Signed-off-by: Rusty Russell Changelog-Deprecated: JSON-RPC: `delexpiredinvoice`: use `autoclean-once`. --- lightningd/invoice.c | 3 +- plugins/autoclean.c | 121 ++++++++++++++++++++++++++++++++++------- tests/test_invoices.py | 11 ---- tests/test_plugin.py | 7 ++- 4 files changed, 108 insertions(+), 34 deletions(-) diff --git a/lightningd/invoice.c b/lightningd/invoice.c index c239a67a9..459a80704 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -1482,7 +1482,8 @@ static const struct json_command delexpiredinvoice_command = { "delexpiredinvoice", "payment", json_delexpiredinvoice, - "Delete all expired invoices that expired as of given {maxexpirytime} (a UNIX epoch time), or all expired invoices if not specified" + "Delete all expired invoices that expired as of given {maxexpirytime} (a UNIX epoch time), or all expired invoices if not specified", + true /*deprecated*/ }; AUTODATA(json_command, &delexpiredinvoice_command); diff --git a/plugins/autoclean.c b/plugins/autoclean.c index b9b71b833..51ad7847e 100644 --- a/plugins/autoclean.c +++ b/plugins/autoclean.c @@ -1,6 +1,7 @@ #include "config.h" #include #include +#include #include #include #include @@ -37,11 +38,23 @@ static bool json_to_subsystem(const char *buffer, const jsmntok_t *tok, static u64 deprecated_cycle_seconds = UINT64_MAX; static u64 cycle_seconds = 3600; static u64 subsystem_age[NUM_SUBSYSTEM]; - +static size_t cleanup_reqs_remaining; +static struct plugin *plugin; static struct plugin_timer *cleantimer; static void do_clean(void *cb_arg); +/* Fatal failures */ +static struct command_result *cmd_failed(struct command *cmd, + const char *buf, + const jsmntok_t *result, + char *cmdname) +{ + plugin_err(plugin, "Failed '%s': '%.*s'", cmdname, + json_tok_full_len(result), + json_tok_full(buf, result)); +} + static const char *datastore_path(const tal_t *ctx, enum subsystem subsystem, const char *field) @@ -50,32 +63,101 @@ static const char *datastore_path(const tal_t *ctx, subsystem_to_str(subsystem), field); } -static struct command_result *ignore(struct command *timer, - const char *buf, - const jsmntok_t *result, - void *arg) +static struct command_result *set_next_timer(struct plugin *plugin) { - struct plugin *p = arg; - cleantimer = plugin_timer(p, time_from_sec(cycle_seconds), do_clean, p); - return timer_complete(p); + plugin_log(plugin, LOG_DBG, "setting next timer"); + cleantimer = plugin_timer(plugin, time_from_sec(cycle_seconds), do_clean, plugin); + return timer_complete(plugin); } -static void do_clean(void *cb_arg) +static struct command_result *del_done(struct command *cmd, + const char *buf, + const jsmntok_t *result, + ptrint_t *unused) { - struct plugin *p = cb_arg; + assert(cleanup_reqs_remaining != 0); + plugin_log(plugin, LOG_DBG, "delinvoice_done: %zu remaining", + cleanup_reqs_remaining-1); + if (--cleanup_reqs_remaining == 0) + return set_next_timer(plugin); + return command_still_pending(cmd); +} - if (!subsystem_age[EXPIREDINVOICES]) { - ignore(NULL, NULL, NULL, p); - return; +static struct command_result *del_failed(struct command *cmd, + const char *buf, + const jsmntok_t *result, + ptrint_t *subsystemp) +{ + plugin_log(plugin, LOG_UNUSUAL, "%s del failed: %.*s", + subsystem_to_str(ptr2int(subsystemp)), + json_tok_full_len(result), + json_tok_full(buf, result)); + assert(cleanup_reqs_remaining != 0); + if (--cleanup_reqs_remaining == 0) + return command_still_pending(cmd); + return set_next_timer(plugin); +} + +static struct command_result *listinvoices_done(struct command *cmd, + const char *buf, + const jsmntok_t *result, + char *unused) +{ + const jsmntok_t *t, *inv = json_get_member(buf, result, "invoices"); + size_t i; + u64 now = time_now().ts.tv_sec; + + json_for_each_arr(i, t, inv) { + const jsmntok_t *status = json_get_member(buf, t, "status"); + const jsmntok_t *time; + u64 invtime; + + if (json_tok_streq(buf, status, "expired")) + time = json_get_member(buf, t, "expires_at"); + else + continue; + + if (!json_to_u64(buf, time, &invtime)) { + plugin_err(plugin, "Bad time '%.*s'", + json_tok_full_len(time), + json_tok_full(buf, time)); + } + + if (invtime <= now - subsystem_age[EXPIREDINVOICES]) { + struct out_req *req; + const jsmntok_t *label = json_get_member(buf, t, "label"); + + req = jsonrpc_request_start(plugin, NULL, "delinvoice", + del_done, del_failed, + int2ptr(EXPIREDINVOICES)); + json_add_tok(req->js, "label", label, buf); + json_add_tok(req->js, "status", status, buf); + send_outreq(plugin, req); + plugin_log(plugin, LOG_DBG, "Expiring %.*s", + json_tok_full_len(label), json_tok_full(buf, label)); + cleanup_reqs_remaining++; + } } - /* FIXME: delexpiredinvoice should be in our plugin too! */ - struct out_req *req = jsonrpc_request_start(p, NULL, "delexpiredinvoice", - ignore, ignore, p); - json_add_u64(req->js, "maxexpirytime", - time_now().ts.tv_sec - subsystem_age[EXPIREDINVOICES]); + if (cleanup_reqs_remaining) + return command_still_pending(cmd); + return set_next_timer(plugin); +} - send_outreq(p, req); +static void do_clean(void *unused) +{ + struct out_req *req = NULL; + + assert(cleanup_reqs_remaining == 0); + if (subsystem_age[EXPIREDINVOICES] != 0) { + req = jsonrpc_request_start(plugin, NULL, "listinvoices", + listinvoices_done, cmd_failed, + (char *)"listinvoices"); + send_outreq(plugin, req); + } + + if (!req) + set_next_timer(plugin); } static struct command_result *json_autocleaninvoice(struct command *cmd, @@ -203,6 +285,7 @@ static struct command_result *json_autoclean(struct command *cmd, static const char *init(struct plugin *p, const char *buf UNUSED, const jsmntok_t *config UNUSED) { + plugin = p; if (deprecated_cycle_seconds != UINT64_MAX) { if (deprecated_cycle_seconds == 0) { plugin_log(p, LOG_DBG, "autocleaning not active"); diff --git a/tests/test_invoices.py b/tests/test_invoices.py index ef74d4040..dbb9ab7a4 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -411,7 +411,6 @@ def test_invoice_expiry(node_factory, executor): l2.rpc.invoice('any', 'inv1', 'description', 10) l2.rpc.invoice('any', 'inv2', 'description', 4) l2.rpc.invoice('any', 'inv3', 'description', 16) - creation = int(time.time()) # Check waitinvoice correctly waits w1 = executor.submit(l2.rpc.waitinvoice, 'inv1') @@ -437,16 +436,6 @@ def test_invoice_expiry(node_factory, executor): with pytest.raises(RpcError): w3.result() - # Test delexpiredinvoice - l2.rpc.delexpiredinvoice(maxexpirytime=creation + 8) - # only inv2 should have been deleted - assert len(l2.rpc.listinvoices()['invoices']) == 2 - assert len(l2.rpc.listinvoices('inv2')['invoices']) == 0 - # Test delexpiredinvoice all - l2.rpc.delexpiredinvoice() - # all invoices are expired and should be deleted - assert len(l2.rpc.listinvoices()['invoices']) == 0 - start = int(time.time()) inv = l2.rpc.invoice(amount_msat=123000, label='inv_s', description='description', expiry=1)['bolt11'] end = int(time.time()) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 987086a3d..d17c90d71 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2940,6 +2940,7 @@ def test_autoclean(node_factory): assert l1.rpc.autoclean_status('expiredinvoices')['autoclean']['expiredinvoices']['enabled'] is False l1.rpc.invoice(amount_msat=12300, label='inv1', description='description1', expiry=5) l1.rpc.invoice(amount_msat=12300, label='inv2', description='description2', expiry=20) + l1.rpc.invoice(amount_msat=12300, label='inv3', description='description3', expiry=20) l1.rpc.autoclean(subsystem='expiredinvoices', age=2) assert l1.rpc.autoclean_status()['autoclean']['expiredinvoices']['enabled'] is True assert l1.rpc.autoclean_status()['autoclean']['expiredinvoices']['age'] == 2 @@ -2964,7 +2965,7 @@ def test_autoclean(node_factory): assert l1.rpc.autoclean_status()['autoclean']['expiredinvoices']['enabled'] is False assert 'age' not in l1.rpc.autoclean_status()['autoclean']['expiredinvoices'] - # Same with inv2. + # Same with inv2/3 wait_for(lambda: only_one(l1.rpc.listinvoices('inv2')['invoices'])['status'] == 'expired') # Give it time to notice. @@ -2977,9 +2978,9 @@ def test_autoclean(node_factory): assert l1.rpc.autoclean_status()['autoclean']['expiredinvoices']['enabled'] is False assert 'age' not in l1.rpc.autoclean_status()['autoclean']['expiredinvoices'] - # Now enable: it will get autocleaned + # Now enable: they will get autocleaned l1.rpc.autoclean(subsystem='expiredinvoices', age=2) - wait_for(lambda: l1.rpc.listinvoices('inv2')['invoices'] == []) + wait_for(lambda: l1.rpc.listinvoices()['invoices'] == []) def test_block_added_notifications(node_factory, bitcoind):