wallet: make sure to watch all txids in transactions table.

We watch if they are to do with a channel, or have outputs going to us, but otherwise
we didn't, so we never updated the blockheight in the db.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `listtransactions` now correctly updates `blockheight` for txs created by `sendpsbt` which have no change outputs.

Author: rustyrussell

lightningd/chaintopology.c

@@ -347,6 +347,37 @@ static void watch_for_utxo_reconfirmation(struct chain_topology *topo,
 	}
 }
 
+static enum watch_result tx_confirmed(struct lightningd *ld,
+				      const struct bitcoin_txid *txid,
+				      const struct bitcoin_tx *tx,
+				      unsigned int depth,
+				      void *unused)
+{
+	/* We don't actually need to do anything here: the fact that we were
+	 * watching the tx made chaintopology.c update the transaction depth */
+	if (depth != 0)
+		return DELETE_WATCH;
+	return KEEP_WATCHING;
+}
+
+void watch_unconfirmed_txid(struct lightningd *ld,
+			    struct chain_topology *topo,
+			    const struct bitcoin_txid *txid)
+{
+	watch_txid(ld->wallet, topo, txid, tx_confirmed, NULL);
+}
+
+static void watch_for_unconfirmed_txs(struct lightningd *ld,
+				      struct chain_topology *topo)
+{
+	struct bitcoin_txid *txids;
+
+	txids = wallet_transactions_by_height(tmpctx, ld->wallet, 0);
+	log_debug(ld->log, "Got %zu unconfirmed transactions", tal_count(txids));
+	for (size_t i = 0; i < tal_count(txids); i++)
+		watch_unconfirmed_txid(ld, topo, &txids[i]);
+}
+
 /* Mutual recursion via timer. */
 static void next_updatefee_timer(struct chain_topology *topo);
 
@@ -1019,6 +1050,7 @@ static void remove_tip(struct chain_topology *topo)
 
 	/* This may have unconfirmed txs: reconfirm as we add blocks. */
 	watch_for_utxo_reconfirmation(topo, topo->ld->wallet);
+
 	block_map_del(topo->block_map, b);
 
 	/* These no longer exist, so gossipd drops any reference to them just
@@ -1469,6 +1501,11 @@ void setup_topology(struct chain_topology *topo)
 
 	/* May have unconfirmed txs: reconfirm as we add blocks. */
 	watch_for_utxo_reconfirmation(topo, topo->ld->wallet);
+
+	/* We usually watch txs because we have outputs coming to us, or they're
+	 * related to a channel.  But not if they're created by sendpsbt without any
+	 * outputs to us. */
+	watch_for_unconfirmed_txs(topo->ld, topo);
 	db_commit_transaction(topo->ld->wallet->db);
 
 	tal_free(local_ctx);

lightningd/chaintopology.h

@@ -9,6 +9,7 @@ struct command;
 struct lightningd;
 struct peer;
 struct txwatch;
+struct wallet;
 
 /* We keep the last three in case there are outliers (for min/max) */
 #define FEE_HISTORY_NUM 3
@@ -277,4 +278,11 @@ void topology_add_sync_waiter_(const tal_t *ctx,
 
 /* In channel_control.c */
 void notify_feerate_change(struct lightningd *ld);
+
+/* We want to update db when this txid is confirmed.  We always do this
+ * if it's related to a channel or incoming funds, but sendpsbt without
+ * change would be otherwise untracked. */
+void watch_unconfirmed_txid(struct lightningd *ld,
+			    struct chain_topology *topo,
+			    const struct bitcoin_txid *txid);
 #endif /* LIGHTNING_LIGHTNINGD_CHAINTOPOLOGY_H */

tests/test_wallet.py

@@ -1887,8 +1887,8 @@ def test_onchain_missing_no_p2tr_migrate(node_factory, bitcoind):
     l2.daemon.wait_for_log('Rescan finished! 1 outputs recovered')
 
 
-@pytest.mark.xfail(strict=True)
 @pytest.mark.parametrize("restart", [False, True])
+@unittest.skipIf(TEST_NETWORK != 'regtest', "elementsd doesn't support psbt features we need")
 def test_sendpsbt_confirm(node_factory, bitcoind, restart):
     """We should see our sendpsbt in wallet, and that it gets confirmed"""
     l1, l2 = node_factory.get_nodes(2)

wallet/wallet.c

@@ -5184,9 +5184,16 @@ struct bitcoin_txid *wallet_transactions_by_height(const tal_t *ctx,
 	struct db_stmt *stmt;
 	struct bitcoin_txid *txids = tal_arr(ctx, struct bitcoin_txid, 0);
 	int count = 0;
-	stmt = db_prepare_v2(
-	    w->db, SQL("SELECT id FROM transactions WHERE blockheight=?"));
-	db_bind_int(stmt, blockheight);
+
+	/* Note: blockheight=NULL is not the same as is NULL! */
+	if (blockheight == 0) {
+		stmt = db_prepare_v2(
+			w->db, SQL("SELECT id FROM transactions WHERE blockheight IS NULL"));
+	} else {
+		stmt = db_prepare_v2(
+			w->db, SQL("SELECT id FROM transactions WHERE blockheight=?"));
+		db_bind_int(stmt, blockheight);
+	}
 	db_query_prepared(stmt);
 
 	while (db_step(stmt)) {

wallet/walletrpc.c

@@ -942,7 +942,6 @@ static void maybe_notify_new_external_send(struct lightningd *ld,
 	wallet_save_chain_mvt(ld, take(mvt));
 }
 
-
 static void sendpsbt_done(struct bitcoind *bitcoind UNUSED,
 			  bool success, const char *msg,
 			  struct sending_psbt *sending)
@@ -974,10 +973,14 @@ static void sendpsbt_done(struct bitcoind *bitcoind UNUSED,
 	}
 
 	wallet_transaction_add(ld->wallet, sending->wtx, 0, 0);
+	wally_txid(sending->wtx, &txid);
 
 	/* Extract the change output and add it to the DB */
-	wallet_extract_owned_outputs(ld->wallet, sending->wtx, false, NULL);
-	wally_txid(sending->wtx, &txid);
+	if (wallet_extract_owned_outputs(ld->wallet, sending->wtx, false, NULL) == 0) {
+		/* If we're not watching it for selfish reasons (i.e. pure send to
+		 * others), make sure we're watching it so we can update depth in db */
+		watch_unconfirmed_txid(ld, ld->topology, &txid);
+	}
 
 	for (size_t i = 0; i < sending->psbt->num_outputs; i++)
 		maybe_notify_new_external_send(ld, &txid, i, sending->psbt);