lightningd: handle duplicate watches on the same thing correctly.

Our hash tables allow duplicate keys, and we use that in a few places.
However, the get() function only returns the first, so it's not a good
idea with such hash tables.

Another patch fixes this at a deeper level (using different hash table
types depending on whether this table can have duplicates), but this
is the minimal fix for existing code.

This may be the cause behind us occasionally missing onchain events:

Fixes: #7460
Fixes: #7377
Fixes: #7118
Fixes: #6951

This fixes them in future: fixing them now will require something else.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Fixed: lightningd: occasionally we could miss transaction outputs (not telling gossipd, or even onchaind)

Author: rustyrussell

lightningd/chaintopology.c

@@ -55,20 +55,22 @@ static void filter_block_txs(struct chain_topology *topo, struct block *b)
 	/* Now we see if any of those txs are interesting. */
 	const size_t num_txs = tal_count(b->full_txs);
 	for (i = 0; i < num_txs; i++) {
-		const struct bitcoin_tx *tx = b->full_txs[i];
+		struct bitcoin_tx *tx = b->full_txs[i];
 		struct bitcoin_txid txid;
 		size_t j;
 		bool is_coinbase = i == 0;
 
 		/* Tell them if it spends a txo we care about. */
 		for (j = 0; j < tx->wtx->num_inputs; j++) {
 			struct bitcoin_outpoint out;
-			struct txowatch *txo;
+			struct txowatch_hash_iter it;
+
 			bitcoin_tx_input_get_txid(tx, j, &out.txid);
 			out.n = tx->wtx->inputs[j].index;
 
-			txo = txowatch_hash_get(topo->txowatches, &out);
-			if (txo) {
+			for (struct txowatch *txo = txowatch_hash_getfirst(topo->txowatches, &out, &it);
+			     txo;
+			     txo = txowatch_hash_getnext(topo->txowatches, &out, &it)) {
 				wallet_transaction_add(topo->ld->wallet,
 						       tx->wtx, b->height, i);
 				txowatch_fire(txo, tx, j, b);
@@ -104,7 +106,7 @@ static void filter_block_txs(struct chain_topology *topo, struct block *b)
 					       tx->wtx, b->height, i);
 		}
 
-		txwatch_inform(topo, &txid, tx);
+		txwatch_inform(topo, &txid, take(tx));
 	}
 	b->full_txs = tal_free(b->full_txs);
 	b->txids = tal_free(b->txids);

lightningd/watch.c

@@ -30,6 +30,7 @@
  * WE ASSUME NO MALLEABILITY!  This requires segregated witness.
  */
 #include "config.h"
+#include <bitcoin/psbt.h>
 #include <lightningd/chaintopology.h>
 #include <lightningd/channel.h>
 #include <lightningd/lightningd.h>
@@ -235,12 +236,13 @@ void txwatch_fire(struct chain_topology *topo,
 		  const struct bitcoin_txid *txid,
 		  unsigned int depth)
 {
-	struct txwatch *txw;
+	struct txwatch_hash_iter it;
 
-	txw = txwatch_hash_get(topo->txwatches, txid);
-
-	if (txw)
+	for (struct txwatch *txw = txwatch_hash_getfirst(topo->txwatches, txid, &it);
+	     txw;
+	     txw = txwatch_hash_getnext(topo->txwatches, txid, &it)) {
 		txw_fire(txw, txid, depth);
+	}
 }
 
 void txowatch_fire(const struct txowatch *txow,
@@ -295,12 +297,22 @@ void watch_topology_changed(struct chain_topology *topo)
 
 void txwatch_inform(const struct chain_topology *topo,
 		    const struct bitcoin_txid *txid,
-		    const struct bitcoin_tx *tx_may_steal)
+		    struct bitcoin_tx *tx TAKES)
 {
-	struct txwatch *txw;
-
-	txw = txwatch_hash_get(topo->txwatches, txid);
+	struct txwatch_hash_iter it;
+
+	for (struct txwatch *txw = txwatch_hash_getfirst(topo->txwatches, txid, &it);
+	     txw;
+	     txw = txwatch_hash_getnext(topo->txwatches, txid, &it)) {
+		if (txw->tx)
+			continue;
+		/* FIXME: YUCK!  These don't have PSBTs attached */
+		if (!tx->psbt)
+			tx->psbt = new_psbt(tx, tx->wtx);
+		txw->tx = clone_bitcoin_tx(txw, tx);
+	}
 
-	if (txw && !txw->tx)
-		txw->tx = tal_steal(txw, tx_may_steal);
+	/* If we don't clone above, handle take() now */
+	if (taken(tx))
+		tal_free(tx);
 }

lightningd/watch.h

@@ -89,10 +89,9 @@ void txowatch_fire(const struct txowatch *txow,
 bool watching_txid(const struct chain_topology *topo,
 		   const struct bitcoin_txid *txid);
 
-/* FIXME: Implement bitcoin_tx_dup() so we tx arg can be TAKEN */
 void txwatch_inform(const struct chain_topology *topo,
 		    const struct bitcoin_txid *txid,
-		    const struct bitcoin_tx *tx_may_steal);
+		    struct bitcoin_tx *tx TAKES);
 
 void watch_topology_changed(struct chain_topology *topo);
 #endif /* LIGHTNING_LIGHTNINGD_WATCH_H */