lightningd: fix occasional missing txid detection.

I was working on rewriting our (somewhat chaotic) tx watching code
for 0.7.2, when I found this bug: we don't always notice the funding
tx in corner cases where more than one block is detected at
once.

This is just the one commit needed to fix the problem: it has some
unnecessary changes, but I'd prefer not to diverge too far from my
cleanup-txwatch branch.

Fixes: #2352
Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

lightningd/chaintopology.c

@@ -4,6 +4,7 @@
 #include "bitcoin/tx.h"
 #include "bitcoind.h"
 #include "chaintopology.h"
+#include "channel.h"
 #include "jsonrpc.h"
 #include "lightningd.h"
 #include "log.h"
@@ -100,6 +101,8 @@ static void filter_block_txs(struct chain_topology *topo, struct block *b)
 			wallet_transaction_add(topo->ld->wallet,
 					       tx, b->height, i);
 		}
+
+		txwatch_inform(topo, &txid, tx);
 	}
 	b->full_txs = tal_free(b->full_txs);
 }
@@ -240,8 +243,26 @@ void broadcast_tx(struct chain_topology *topo,
 static enum watch_result closeinfo_txid_confirmed(struct lightningd *ld,
 						  struct channel *channel,
 						  const struct bitcoin_txid *txid,
+						  const struct bitcoin_tx *tx,
 						  unsigned int depth)
 {
+	/* Sanity check. */
+	if (tx != NULL) {
+		struct bitcoin_txid txid2;
+
+		bitcoin_txid(tx, &txid2);
+		if (!bitcoin_txid_eq(txid, &txid2)) {
+			channel_internal_error(channel, "Txid for %s is not %s",
+					       type_to_string(tmpctx,
+							      struct bitcoin_tx,
+							      tx),
+					       type_to_string(tmpctx,
+							      struct bitcoin_txid,
+							      txid));
+			return DELETE_WATCH;
+		}
+	}
+
 	/* We delete ourselves first time, so should not be reorged out!! */
 	assert(depth > 0);
 	/* Subtle: depth 1 == current block. */

lightningd/onchain_control.c

@@ -79,17 +79,31 @@ static void onchain_tx_depth(struct channel *channel,
 static enum watch_result onchain_tx_watched(struct lightningd *ld,
 					    struct channel *channel,
 					    const struct bitcoin_txid *txid,
+					    const struct bitcoin_tx *tx,
 					    unsigned int depth)
 {
 	u32 blockheight = get_block_height(ld->topology);
+
+	if (tx != NULL) {
+		struct bitcoin_txid txid2;
+
+		bitcoin_txid(tx, &txid2);
+		if (!bitcoin_txid_eq(txid, &txid2)) {
+			channel_internal_error(channel, "Txid for %s is not %s",
+					       type_to_string(tmpctx,
+							      struct bitcoin_tx,
+							      tx),
+					       type_to_string(tmpctx,
+							      struct bitcoin_txid,
+							      txid));
+			return DELETE_WATCH;
+		}
+	}
+
 	if (depth == 0) {
 		log_unusual(channel->log, "Chain reorganization!");
 		channel_set_owner(channel, NULL, false);
 
-		/* FIXME!
-		topology_rescan(peer->ld->topology, peer->funding_txid);
-		*/
-
 		/* We will most likely be freed, so this is a noop */
 		return KEEP_WATCHING;
 	}

lightningd/peer_control.c

@@ -945,14 +945,48 @@ void peer_connected(struct lightningd *ld, const u8 *msg,
 	plugin_hook_call_peer_connected(ld, hook_payload, hook_payload);
 }
 
+/* FIXME: Unify our watch code so we get notified by txout, instead, like
+ * the wallet code does. */
+static bool check_funding_tx(const struct bitcoin_tx *tx,
+			     const struct channel *channel)
+{
+	u8 *wscript;
+
+	if (channel->funding_outnum >= tx->wtx->num_outputs)
+		return false;
+
+	if (!amount_sat_eq(bitcoin_tx_output_get_amount(tx,
+							channel->funding_outnum),
+			   channel->funding))
+		return false;
+
+	wscript = bitcoin_redeem_2of2(tmpctx,
+				      &channel->local_funding_pubkey,
+				      &channel->channel_info.remote_fundingkey);
+	return scripteq(scriptpubkey_p2wsh(tmpctx, wscript),
+			bitcoin_tx_output_get_script(tmpctx, tx,
+						     channel->funding_outnum));
+}
+
 static enum watch_result funding_depth_cb(struct lightningd *ld,
 					   struct channel *channel,
 					   const struct bitcoin_txid *txid,
+					   const struct bitcoin_tx *tx,
 					   unsigned int depth)
 {
 	const char *txidstr;
 	struct short_channel_id scid;
 
+	/* Sanity check */
+	if (tx && !check_funding_tx(tx, channel)) {
+		channel_internal_error(channel, "Bad tx %s: %s",
+				       type_to_string(tmpctx,
+						      struct bitcoin_txid, txid),
+				       type_to_string(tmpctx,
+						      struct bitcoin_tx, tx));
+		return DELETE_WATCH;
+	}
+
 	txidstr = type_to_string(tmpctx, struct bitcoin_txid, txid);
 	log_debug(channel->log, "Funding tx %s depth %u of %u",
 		  txidstr, depth, channel->minimum_depth);

lightningd/peer_htlcs.h

@@ -9,7 +9,9 @@
 
 struct channel;
 struct htlc_in;
+struct htlc_in_map;
 struct htlc_out;
+struct htlc_out_map;
 struct htlc_stub;
 struct lightningd;
 

lightningd/test/run-invoice-select-inchan.c

@@ -578,6 +578,7 @@ struct txwatch *watch_txid(const tal_t *ctx UNNEEDED,
 			   enum watch_result (*cb)(struct lightningd *ld UNNEEDED,
 						   struct channel *channel UNNEEDED,
 						   const struct bitcoin_txid * UNNEEDED,
+						   const struct bitcoin_tx * UNNEEDED,
 						   unsigned int depth))
 { fprintf(stderr, "watch_txid called!\n"); abort(); }
 /* Generated stub for watch_txo */

lightningd/watch.c

@@ -63,12 +63,17 @@ struct txwatch {
 
 	/* Transaction to watch. */
 	struct bitcoin_txid txid;
+
+	/* May be NULL if we haven't seen it yet. */
+	const struct bitcoin_tx *tx;
+
 	unsigned int depth;
 
 	/* A new depth (0 if kicked out, otherwise 1 = tip, etc.) */
 	enum watch_result (*cb)(struct lightningd *ld,
 				struct channel *channel,
 				const struct bitcoin_txid *txid,
+				const struct bitcoin_tx *tx,
 				unsigned int depth);
 };
 
@@ -124,7 +129,8 @@ struct txwatch *watch_txid(const tal_t *ctx,
 			   const struct bitcoin_txid *txid,
 			   enum watch_result (*cb)(struct lightningd *ld,
 						   struct channel *channel,
-						   const struct bitcoin_txid *,
+						   const struct bitcoin_txid *txid,
+						   const struct bitcoin_tx *tx,
 						   unsigned int depth))
 {
 	struct txwatch *w;
@@ -133,6 +139,7 @@ struct txwatch *watch_txid(const tal_t *ctx,
 	w->topo = topo;
 	w->depth = 0;
 	w->txid = *txid;
+	w->tx = NULL;
 	w->channel = channel;
 	w->cb = cb;
 
@@ -173,11 +180,13 @@ struct txwatch *watch_tx(const tal_t *ctx,
 			 enum watch_result (*cb)(struct lightningd *ld,
 						 struct channel *channel,
 						 const struct bitcoin_txid *,
+						 const struct bitcoin_tx *,
 						 unsigned int depth))
 {
 	struct bitcoin_txid txid;
 
 	bitcoin_txid(tx, &txid);
+	/* FIXME: Save populate txwatch->tx here, too! */
 	return watch_txid(ctx, topo, channel, &txid, cb);
 }
 
@@ -227,7 +236,8 @@ static bool txw_fire(struct txwatch *txw,
 		  type_to_string(tmpctx, struct bitcoin_txid, &txw->txid),
 		  depth ? "" : " REORG");
 	txw->depth = depth;
-	r = txw->cb(txw->topo->bitcoind->ld, txw->channel, txid, txw->depth);
+	r = txw->cb(txw->topo->bitcoind->ld, txw->channel, txid, txw->tx,
+		    txw->depth);
 	switch (r) {
 	case DELETE_WATCH:
 		tal_free(txw);
@@ -295,3 +305,15 @@ void watch_topology_changed(struct chain_topology *topo)
 		}
 	} while (needs_rerun);
 }
+
+void txwatch_inform(const struct chain_topology *topo,
+		    const struct bitcoin_txid *txid,
+		    const struct bitcoin_tx *tx_may_steal)
+{
+	struct txwatch *txw;
+
+	txw = txwatch_hash_get(&topo->txwatches, txid);
+
+	if (txw && !txw->tx)
+		txw->tx = tal_steal(txw, tx_may_steal);
+}

lightningd/watch.h

@@ -47,6 +47,7 @@ struct txwatch *watch_txid(const tal_t *ctx,
 			   enum watch_result (*cb)(struct lightningd *ld,
 						   struct channel *channel,
 						   const struct bitcoin_txid *,
+						   const struct bitcoin_tx *,
 						   unsigned int depth));
 
 struct txwatch *watch_tx(const tal_t *ctx,
@@ -56,6 +57,7 @@ struct txwatch *watch_tx(const tal_t *ctx,
 			 enum watch_result (*cb)(struct lightningd *ld,
 						 struct channel *channel,
 						 const struct bitcoin_txid *,
+						 const struct bitcoin_tx *,
 						 unsigned int depth));
 
 struct txowatch *watch_txo(const tal_t *ctx,
@@ -83,5 +85,10 @@ 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);
+
 void watch_topology_changed(struct chain_topology *topo);
 #endif /* LIGHTNING_LIGHTNINGD_WATCH_H */

wallet/test/run-wallet.c

@@ -569,6 +569,7 @@ struct txwatch *watch_txid(const tal_t *ctx UNNEEDED,
 			   enum watch_result (*cb)(struct lightningd *ld UNNEEDED,
 						   struct channel *channel UNNEEDED,
 						   const struct bitcoin_txid * UNNEEDED,
+						   const struct bitcoin_tx * UNNEEDED,
 						   unsigned int depth))
 { fprintf(stderr, "watch_txid called!\n"); abort(); }
 /* Generated stub for watch_txo */