wallet: make sure to watch all txids in transactions table.
What changed, and why it matters
This commit fixes a bookkeeping bug in Core Lightning's wallet. When a user broadcast a transaction created with `sendpsbt` that had no change output coming back to the node, the node did not track the transaction's confirmation status. As a result, `listtransactions` would never show the correct `blockheight` for that transaction. The fix makes the node explicitly watch all unconfirmed transactions in its database so their confirmation height gets updated. There is no direct security exploit here; it is a data-correctness and user-experience issue.
No immediate security response is required. Operators and integrators should update to a release containing this commit if accurate `listtransactions` blockheight data is needed for accounting or auditing of `sendpsbt` spends without change outputs. Reviewers may want to confirm that `wallet_transactions_by_height(..., 0)` correctly maps to `blockheight IS NULL` on all supported database backends and that the new watch is cleaned up after confirmation.
Security signals we found
Missing confirmation tracking for externally-directed transactions
Database `blockheight` field left stale for unowned-output spends
Fix adds explicit txid watches for all unconfirmed wallet transactions
Test previously marked xfail is now enabled and parameterized over restart
Evidence from the diff
The patch adds watch_unconfirmed_txid() and watch_for_unconfirmed_txs() in lightningd/chaintopology.c to register a txwatch for every transaction in the transactions table whose blockheight is NULL. Previously, transaction confirmation tracking relied on existing watches created because an output belonged to the wallet or because the tx was channel-related. A pure external spend via sendpsbt with no owned outputs therefore fell through the cracks. wallet_transaction_add() still records the tx at height 0, and now sendpsbt_done() explicitly calls watch_unconfirmed_txid() when wallet_extract_owned_outputs() returns zero. A test (test_sendpsbt_confirm) is re-enabled to verify the behavior across restarts.
Changed components
lightningd/chaintopology.clightningd/chaintopology.hwallet/wallet.cwallet/walletrpc.ctests/test_wallet.pyInspect captured patch +62 / −7
diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c
index a35e4d0..99e9920 100644
--- a/lightningd/chaintopology.c
+++ b/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);
diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h
index f6aa818..99d614b 100644
--- a/lightningd/chaintopology.h
+++ b/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 */
diff --git a/tests/test_wallet.py b/tests/test_wallet.py
index 436f14e..dbe1e09 100644
--- a/tests/test_wallet.py
+++ b/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)
diff --git a/wallet/wallet.c b/wallet/wallet.c
index d2dc7d0..363c083 100644
--- a/wallet/wallet.c
+++ b/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)) {
diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c
index 6155c89..a39f685 100644
--- a/wallet/walletrpc.c
+++ b/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);
Why this scored 24/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.