diff --git a/doc/lightning-delforward.7.md b/doc/lightning-delforward.7.md index 5ae6ee51b..c8ee5e2f3 100644 --- a/doc/lightning-delforward.7.md +++ b/doc/lightning-delforward.7.md @@ -20,6 +20,11 @@ has no effect on the running of your node. You cannot delete forwards which have status *offered* (i.e. are currently active). +Note: for **listforwards** entries without an *in_htlc_id* entry (no +longer created in v22.11, but can exist from older versions), a value +of 18446744073709551615 can be used, but then it will delete *all* +entries without *in_htlc_id* for this *in_channel* and *status*. + RETURN VALUE ------------ diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 61c038b82..d0ce63b2b 100644 --- a/doc/lightningd-config.5.md +++ b/doc/lightningd-config.5.md @@ -461,6 +461,11 @@ accepted, and ignored. How old invoices which were not paid (and cannot be) (`expired` in listinvoices `status`) before deletion (default 0, meaning never). +Note: prior to v22.11, forwards for channels which were closed were +not easily distinguishable. As a result, autoclean may delete more +than one of these at once, and then suffer failures when it fails to +delete the others. + ### Payment control options: * **disable-mpp** [plugin `pay`] diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 16625d172..25ca2f049 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -3010,8 +3010,14 @@ static struct command_result *json_delforward(struct command *cmd, NULL)) return command_param_failed(); +#ifdef COMPAT_V0121 + /* Special value used if in_htlc_id is missing */ + if (*htlc_id == HTLC_INVALID_ID) + htlc_id = NULL; +#endif + if (!wallet_forward_delete(cmd->ld->wallet, - chan_in, *htlc_id, *status)) + chan_in, htlc_id, *status)) return command_fail(cmd, DELFORWARD_NOT_FOUND, "Could not find that forward"); diff --git a/plugins/autoclean.c b/plugins/autoclean.c index 1f3de4e8c..81e50352a 100644 --- a/plugins/autoclean.c +++ b/plugins/autoclean.c @@ -345,7 +345,15 @@ static struct command_result *listforwards_done(struct command *cmd, req = del_request_start("delforward", cinfo, subsys); json_add_tok(req->js, "in_channel", inchan, buf); - json_add_tok(req->js, "in_htlc_id", inid, buf); + /* This can be missing if it was a forwards record from an old + * closed channel in version <= 0.12.1. This is a special value + * but we will delete them *all*, resulting in some failures! */ +#ifdef COMPAT_V0121 + if (!inid) + json_add_u64(req->js, "in_htlc_id", -1ULL); + else +#endif + json_add_tok(req->js, "in_htlc_id", inid, buf); json_add_tok(req->js, "status", status, buf); send_outreq(plugin, req); } diff --git a/tests/test_db.py b/tests/test_db.py index c08f5daeb..8b1ac2969 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -507,3 +507,10 @@ def test_db_forward_migrate(bitcoind, node_factory): assert l1.rpc.getinfo()['fees_collected_msat'] == 4 assert len(l1.rpc.listforwards()['forwards']) == 4 + + # Make sure autoclean can handle these! + l1.stop() + l1.daemon.opts['autoclean-succeededforwards-age'] = 2 + l1.daemon.opts['autoclean-cycle'] = 1 + l1.start() + wait_for(lambda: l1.rpc.listforwards()['forwards'] == []) diff --git a/wallet/wallet.c b/wallet/wallet.c index 452154e76..bb07fbd55 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -4694,7 +4694,7 @@ const struct forwarding *wallet_forwarded_payments_get(struct wallet *w, bool wallet_forward_delete(struct wallet *w, const struct short_channel_id *chan_in, - u64 htlc_id, + const u64 *htlc_id, enum forward_status state) { struct db_stmt *stmt; @@ -4703,21 +4703,32 @@ bool wallet_forward_delete(struct wallet *w, /* When deleting settled ones, we have to add to deleted_forward_fees! */ if (state == FORWARD_SETTLED) { /* Of course, it might not be settled: don't add if they're wrong! */ - stmt = db_prepare_v2(w->db, SQL("SELECT" - " in_msatoshi - out_msatoshi" - " FROM forwards " - " WHERE in_channel_scid = ?" - " AND in_htlc_id = ?" - " AND state = ?;")); - db_bind_scid(stmt, 0, chan_in); - db_bind_u64(stmt, 1, htlc_id); - db_bind_int(stmt, 2, wallet_forward_status_in_db(FORWARD_SETTLED)); + if (htlc_id) { + stmt = db_prepare_v2(w->db, SQL("SELECT" + " CAST(COALESCE(SUM(in_msatoshi - out_msatoshi), 0) AS BIGINT)" + " FROM forwards " + " WHERE in_channel_scid = ?" + " AND in_htlc_id = ?" + " AND state = ?;")); + db_bind_scid(stmt, 0, chan_in); + db_bind_u64(stmt, 1, *htlc_id); + db_bind_int(stmt, 2, wallet_forward_status_in_db(FORWARD_SETTLED)); + } else { + stmt = db_prepare_v2(w->db, SQL("SELECT" + " CAST(COALESCE(SUM(in_msatoshi - out_msatoshi), 0) AS BIGINT)" + " FROM forwards " + " WHERE in_channel_scid = ?" + " AND in_htlc_id IS NULL" + " AND state = ?;")); + db_bind_scid(stmt, 0, chan_in); + db_bind_int(stmt, 1, wallet_forward_status_in_db(FORWARD_SETTLED)); + } db_query_prepared(stmt); if (db_step(stmt)) { struct amount_msat deleted; - db_col_amount_msat(stmt, "in_msatoshi - out_msatoshi", &deleted); + db_col_amount_msat(stmt, "CAST(COALESCE(SUM(in_msatoshi - out_msatoshi), 0) AS BIGINT)", &deleted); deleted.millisatoshis += /* Raw: db access */ db_get_intvar(w->db, "deleted_forward_fees", 0); db_set_intvar(w->db, "deleted_forward_fees", @@ -4726,14 +4737,24 @@ bool wallet_forward_delete(struct wallet *w, tal_free(stmt); } - stmt = db_prepare_v2(w->db, - SQL("DELETE FROM forwards" - " WHERE in_channel_scid = ?" - " AND in_htlc_id = ?" - " AND state = ?")); - db_bind_scid(stmt, 0, chan_in); - db_bind_u64(stmt, 1, htlc_id); - db_bind_int(stmt, 2, wallet_forward_status_in_db(state)); + if (htlc_id) { + stmt = db_prepare_v2(w->db, + SQL("DELETE FROM forwards" + " WHERE in_channel_scid = ?" + " AND in_htlc_id = ?" + " AND state = ?")); + db_bind_scid(stmt, 0, chan_in); + db_bind_u64(stmt, 1, *htlc_id); + db_bind_int(stmt, 2, wallet_forward_status_in_db(state)); + } else { + stmt = db_prepare_v2(w->db, + SQL("DELETE FROM forwards" + " WHERE in_channel_scid = ?" + " AND in_htlc_id IS NULL" + " AND state = ?")); + db_bind_scid(stmt, 0, chan_in); + db_bind_int(stmt, 1, wallet_forward_status_in_db(state)); + } db_exec_prepared_v2(stmt); changed = db_count_changes(stmt) != 0; tal_free(stmt); diff --git a/wallet/wallet.h b/wallet/wallet.h index 917d7f2aa..4983e4e22 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -1389,7 +1389,7 @@ const struct forwarding *wallet_forwarded_payments_get(struct wallet *w, */ bool wallet_forward_delete(struct wallet *w, const struct short_channel_id *chan_in, - u64 htlc_id, + const u64 *htlc_id, enum forward_status state); /**