delforward: allow deletion of "unknown in_htlc_id" and fix autoclean to use it.

Note the caveats: we will delete *all* of them at once!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

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
 ------------
 

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`]

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");
 

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);
 		}

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'] == [])

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"
+		if (htlc_id) {
-						" in_msatoshi - out_msatoshi"
+			stmt = db_prepare_v2(w->db, SQL("SELECT"
-						" FROM forwards "
+							" CAST(COALESCE(SUM(in_msatoshi - out_msatoshi), 0) AS BIGINT)"
-						" WHERE in_channel_scid = ?"
+							" FROM forwards "
-						" AND in_htlc_id = ?"
+							" WHERE in_channel_scid = ?"
-						" AND state = ?;"));
+							" AND in_htlc_id = ?"
-		db_bind_scid(stmt, 0, chan_in);
+							" AND state = ?;"));
-		db_bind_u64(stmt, 1, htlc_id);
+			db_bind_scid(stmt, 0, chan_in);
-		db_bind_int(stmt, 2, wallet_forward_status_in_db(FORWARD_SETTLED));
+			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,
+	if (htlc_id) {
-			     SQL("DELETE FROM forwards"
+		stmt = db_prepare_v2(w->db,
-				 " WHERE in_channel_scid = ?"
+				     SQL("DELETE FROM forwards"
-				 " AND in_htlc_id = ?"
+					 " WHERE in_channel_scid = ?"
-				 " AND state = ?"));
+					 " AND in_htlc_id = ?"
-	db_bind_scid(stmt, 0, chan_in);
+					 " AND state = ?"));
-	db_bind_u64(stmt, 1, htlc_id);
+		db_bind_scid(stmt, 0, chan_in);
-	db_bind_int(stmt, 2, wallet_forward_status_in_db(state));
+		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);

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);
 
 /**