onchaind: explicit ack for onchaind_spent method.

This means it always tells us explicitly whether to keep watching or not,
and we know it's processed it.

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

Author: rustyrussell

lightningd/onchain_control.c

@@ -218,21 +218,64 @@ static enum watch_result onchain_tx_watched(struct lightningd *ld,
 static void watch_tx_and_outputs(struct channel *channel,
 				 const struct bitcoin_tx *tx);
 
+static void replay_unwatch_txid(struct channel *channel,
+				const struct bitcoin_txid *txid)
+{
+	replay_tx_hash_delkey(channel->onchaind_replay_watches, txid);
+}
+
+static void onchaind_spent_reply(struct subd *onchaind, const u8 *msg,
+				 const int *fds,
+				 struct bitcoin_txid *txid)
+{
+	bool interested;
+	struct txwatch *txw;
+	struct channel *channel = onchaind->channel;
+
+	if (!fromwire_onchaind_spent_reply(msg, &interested))
+		channel_internal_error(channel, "Invalid onchaind_spent_reply %s",
+				       tal_hex(tmpctx, msg));
+
+	/* Only delete watch if it says it doesn't care */
+	if (interested)
+		return;
+
+	/* If we're doing replay: */
+	if (channel->onchaind_replay_watches) {
+		replay_unwatch_txid(channel, txid);
+		return;
+	}
+
+	/* Frees the txo watches, too: see watch_tx_and_outputs() */
+	txw = find_txwatch(channel->peer->ld->topology, txid,
+			   onchain_tx_watched, channel);
+	if (!txw)
+		log_unusual(channel->log, "Can't unwatch txid %s",
+			    fmt_bitcoin_txid(tmpctx, txid));
+	tal_free(txw);
+}
+
 /**
  * Notify onchaind that an output was spent and register new watches.
  */
 static void onchain_txo_spent(struct channel *channel, const struct bitcoin_tx *tx, size_t input_num, u32 blockheight)
 {
 	u8 *msg;
+	struct bitcoin_txid *txid;
 	/* Onchaind needs all inputs, since it uses those to compare
 	 * with existing spends (which can vary, with feerate changes). */
 	struct tx_parts *parts = tx_parts_from_wally_tx(tmpctx, tx->wtx,
 							-1, -1);
 
 	watch_tx_and_outputs(channel, tx);
 
+	/* Reply will need this if we want to unwatch */
+	txid = tal(NULL, struct bitcoin_txid);
+	bitcoin_txid(tx, txid);
+
 	msg = towire_onchaind_spent(channel, parts, input_num, blockheight);
-	subd_send_msg(channel->owner, take(msg));
+	subd_req(channel->owner, channel->owner, take(msg), -1, 0,
+		 onchaind_spent_reply, take(txid));
 
 }
 
@@ -304,12 +347,6 @@ static void replay_watch_tx(struct channel *channel,
 	replay_tx_hash_add(channel->onchaind_replay_watches, rtx);
 }
 
-static void replay_unwatch_txid(struct channel *channel,
-				const struct bitcoin_txid *txid)
-{
-	replay_tx_hash_delkey(channel->onchaind_replay_watches, txid);
-}
-
 /* We've finished replaying, turn any txs left into live watches */
 static void convert_replay_txs(struct channel *channel)
 {
@@ -372,31 +409,6 @@ static void replay_block(struct bitcoind *bitcoind,
 	bitcoind_getrawblockbyheight(channel, bitcoind, height + 1, replay_block, channel);
 }
 
-static void handle_onchain_unwatch_tx(struct channel *channel, const u8 *msg)
-{
-	struct bitcoin_txid txid;
-	struct txwatch *txw;
-
-	if (!fromwire_onchaind_unwatch_tx(msg, &txid)) {
-		channel_internal_error(channel, "Invalid onchain_unwatch_tx");
-		return;
-	}
-
-	/* If we're doing replay: */
-	if (channel->onchaind_replay_watches) {
-		replay_unwatch_txid(channel, &txid);
-		return;
-	}
-
-	/* Frees the txo watches, too: see watch_tx_and_outputs() */
-	txw = find_txwatch(channel->peer->ld->topology, &txid,
-			   onchain_tx_watched, channel);
-	if (!txw)
-		log_unusual(channel->log, "Can't unwatch txid %s",
-			    fmt_bitcoin_txid(tmpctx, &txid));
-	tal_free(txw);
-}
-
 static void handle_extracted_preimage(struct channel *channel, const u8 *msg)
 {
 	struct preimage preimage;
@@ -1545,10 +1557,6 @@ static unsigned int onchain_msg(struct subd *sd, const u8 *msg, const int *fds U
 		handle_onchain_init_reply(sd->channel, msg);
 		break;
 
-	case WIRE_ONCHAIND_UNWATCH_TX:
-		handle_onchain_unwatch_tx(sd->channel, msg);
-		break;
-
  	case WIRE_ONCHAIND_EXTRACTED_PREIMAGE:
 		handle_extracted_preimage(sd->channel, msg);
 		break;
@@ -1614,6 +1622,7 @@ static unsigned int onchain_msg(struct subd *sd, const u8 *msg, const int *fds U
 	case WIRE_ONCHAIND_SPEND_CREATED:
 	case WIRE_ONCHAIND_DEV_MEMLEAK:
 	case WIRE_ONCHAIND_DEV_MEMLEAK_REPLY:
+	case WIRE_ONCHAIND_SPENT_REPLY:
 		break;
 	}
 

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

@@ -939,7 +939,7 @@ struct subd_req *subd_req_(const tal_t *ctx UNNEEDED,
 	       const u8 *msg_out UNNEEDED,
 	       int fd_out UNNEEDED, size_t num_fds_in UNNEEDED,
 	       void (*replycb)(struct subd * UNNEEDED, const u8 * UNNEEDED, const int * UNNEEDED, void *) UNNEEDED,
-	       void *replycb_data UNNEEDED)
+	       void *replycb_data TAKES UNNEEDED)
 { fprintf(stderr, "subd_req_ called!\n"); abort(); }
 /* Generated stub for subd_send_fd */
 void subd_send_fd(struct subd *sd UNNEEDED, int fd UNNEEDED)

onchaind/onchaind.c

@@ -978,14 +978,6 @@ static void billboard_update(struct tracked_output **outs)
 		       output_type_name(best->output_type), best->depth);
 }
 
-static void unwatch_txid(const struct bitcoin_txid *txid)
-{
-	u8 *msg;
-
-	msg = towire_onchaind_unwatch_tx(NULL, txid);
-	wire_sync_write(REQ_FD, take(msg));
-}
-
 static void handle_htlc_onchain_fulfill(struct tracked_output *out,
 					const struct tx_parts *tx_parts,
 					const struct bitcoin_outpoint *htlc_outpoint)
@@ -1194,23 +1186,30 @@ static void onchain_annotate_txin(const struct bitcoin_txid *txid, u32 innum,
 				    tmpctx, txid, innum, type)));
 }
 
-/* An output has been spent: see if it resolves something we care about. */
-static void output_spent(struct tracked_output ***outs,
+/* An output has been spent: see if it resolves something we care about.
+ * Return true if it's useful to know about, false to suppress this and any
+ * child transactions.
+ */
+static bool output_spent(struct tracked_output ***outs,
 			 const struct tx_parts *tx_parts,
 			 u32 input_num,
 			 u32 tx_blockheight)
 {
+	bool interesting;
+
 	for (size_t i = 0; i < tal_count(*outs); i++) {
 		struct tracked_output *out = (*outs)[i];
 		struct bitcoin_outpoint htlc_outpoint;
 
-		if (out->resolved)
-			continue;
-
 		if (!wally_tx_input_spends(tx_parts->inputs[input_num],
 					   &out->outpoint))
 			continue;
 
+		interesting = true;
+
+		if (out->resolved)
+			continue;
+
 		/* Was this our resolution? */
 		if (resolved_by_proposal(out, tx_parts)) {
 			/* If it's our htlc tx, we need to resolve that, too. */
@@ -1221,7 +1220,7 @@ static void output_spent(struct tracked_output ***outs,
 
 			record_coin_movements(out, tx_blockheight,
 					      &tx_parts->txid);
-			return;
+			return interesting;
 		}
 
 		htlc_outpoint.txid = tx_parts->txid;
@@ -1342,17 +1341,18 @@ static void output_spent(struct tracked_output ***outs,
 				      tx_type_name(out->tx_type),
 				      output_type_name(out->output_type));
 		}
-		return;
 	}
 
-	struct bitcoin_txid txid;
-	wally_tx_input_get_txid(tx_parts->inputs[input_num], &txid);
-	/* Not interesting to us, so unwatch the tx and all its outputs */
-	status_debug("Notified about tx %s output %u spend, but we don't care",
-		     fmt_bitcoin_txid(tmpctx, &txid),
-		     tx_parts->inputs[input_num]->index);
+	if (!interesting) {
+		struct bitcoin_txid txid;
+		wally_tx_input_get_txid(tx_parts->inputs[input_num], &txid);
 
-	unwatch_txid(&tx_parts->txid);
+		status_debug("Notified about tx %s output %u spend, but we don't care",
+			     fmt_bitcoin_txid(tmpctx, &txid),
+			     tx_parts->inputs[input_num]->index);
+	}
+
+	return interesting;
 }
 
 static void update_resolution_depth(struct tracked_output *out, u32 depth)
@@ -1610,12 +1610,16 @@ static void handle_onchaind_spent(struct tracked_output ***outs, const u8 *msg)
 {
 	struct tx_parts *tx_parts;
 	u32 input_num, tx_blockheight;
+	bool interesting;
 
 	if (!fromwire_onchaind_spent(msg, msg, &tx_parts, &input_num,
 				     &tx_blockheight))
 		master_badmsg(WIRE_ONCHAIND_SPENT, msg);
 
-	output_spent(outs, tx_parts, input_num, tx_blockheight);
+	interesting = output_spent(outs, tx_parts, input_num, tx_blockheight);
+
+	/* Tell lightningd if it was interesting */
+	wire_sync_write(REQ_FD, take(towire_onchaind_spent_reply(NULL, interesting)));
 }
 
 static void handle_onchaind_known_preimage(struct tracked_output ***outs,
@@ -1675,7 +1679,7 @@ static void wait_for_resolved(struct tracked_output **outs)
 
 		/* We send these, not receive! */
 		case WIRE_ONCHAIND_INIT_REPLY:
-		case WIRE_ONCHAIND_UNWATCH_TX:
+		case WIRE_ONCHAIND_SPENT_REPLY:
 		case WIRE_ONCHAIND_EXTRACTED_PREIMAGE:
 		case WIRE_ONCHAIND_MISSING_HTLC_OUTPUT:
 		case WIRE_ONCHAIND_HTLC_TIMEOUT:

onchaind/onchaind_wire.csv

@@ -67,15 +67,15 @@ msgdata,onchaind_spent,tx,tx_parts,
 msgdata,onchaind_spent,input_num,u32,
 msgdata,onchaind_spent,blockheight,u32,
 
+# onchaind->master: do we want to continue watching this?
+msgtype,onchaind_spent_reply,5104
+msgdata,onchaind_spent_reply,interested,bool,
+
 # master->onchaind: We will receive more than one of these, as depth changes.
 msgtype,onchaind_depth,5005
 msgdata,onchaind_depth,txid,bitcoin_txid,
 msgdata,onchaind_depth,depth,u32,
 
-# onchaind->master: We don't want to watch this tx, or its outputs
-msgtype,onchaind_unwatch_tx,5006
-msgdata,onchaind_unwatch_tx,txid,bitcoin_txid,
-
 # master->onchaind: We know HTLC preimage
 msgtype,onchaind_known_preimage,5007
 msgdata,onchaind_known_preimage,preimage,preimage,

onchaind/test/run-grind_feerate.c

@@ -334,9 +334,9 @@ u8 *towire_onchaind_spend_penalty(const tal_t *ctx UNNEEDED, const struct bitcoi
 /* Generated stub for towire_onchaind_spend_to_us */
 u8 *towire_onchaind_spend_to_us(const tal_t *ctx UNNEEDED, const struct bitcoin_outpoint *outpoint UNNEEDED, struct amount_sat outpoint_amount UNNEEDED, u32 minblock UNNEEDED, u64 commit_num UNNEEDED, const u8 *wscript UNNEEDED)
 { fprintf(stderr, "towire_onchaind_spend_to_us called!\n"); abort(); }
-/* Generated stub for towire_onchaind_unwatch_tx */
-u8 *towire_onchaind_unwatch_tx(const tal_t *ctx UNNEEDED, const struct bitcoin_txid *txid UNNEEDED)
-{ fprintf(stderr, "towire_onchaind_unwatch_tx called!\n"); abort(); }
+/* Generated stub for towire_onchaind_spent_reply */
+u8 *towire_onchaind_spent_reply(const tal_t *ctx UNNEEDED, bool interested UNNEEDED)
+{ fprintf(stderr, "towire_onchaind_spent_reply called!\n"); abort(); }
 /* Generated stub for towire_secp256k1_ecdsa_signature */
 void towire_secp256k1_ecdsa_signature(u8 **pptr UNNEEDED,
 			      const secp256k1_ecdsa_signature *signature UNNEEDED)

tests/test_closing.py

@@ -1751,7 +1751,7 @@ def test_onchain_first_commit(node_factory, bitcoind):
     l1.daemon.wait_for_log('onchaind complete, forgetting peer')
 
 
-def test_onchain_unwatch(node_factory, bitcoind):
+def test_onchain_unwatch(node_factory, bitcoind, chainparams):
     """Onchaind should not watch random spends"""
     # We track channel balances, to verify that accounting is ok.
     coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py')
@@ -1784,7 +1784,12 @@ def test_onchain_unwatch(node_factory, bitcoind):
     # Daemon gets told about wallet; says it doesn't care.
     l1.rpc.withdraw(l1.rpc.newaddr()['bech32'], 'all')
     bitcoind.generate_block(1)
-    l1.daemon.wait_for_log("but we don't care")
+
+    # We see *two* of these: one for anchor spend as well!
+    if chainparams['elements']:
+        l1.daemon.wait_for_log("but we don't care")
+    else:
+        l1.daemon.wait_for_logs(["but we don't care"] * 2)
 
     # And lightningd should respect that!
     assert not l1.daemon.is_in_log("Can't unwatch txid")

wallet/test/run-wallet.c

@@ -985,7 +985,7 @@ struct subd_req *subd_req_(const tal_t *ctx UNNEEDED,
 	       const u8 *msg_out UNNEEDED,
 	       int fd_out UNNEEDED, size_t num_fds_in UNNEEDED,
 	       void (*replycb)(struct subd * UNNEEDED, const u8 * UNNEEDED, const int * UNNEEDED, void *) UNNEEDED,
-	       void *replycb_data UNNEEDED)
+	       void *replycb_data TAKES UNNEEDED)
 { fprintf(stderr, "subd_req_ called!\n"); abort(); }
 /* Generated stub for subd_send_fd */
 void subd_send_fd(struct subd *sd UNNEEDED, int fd UNNEEDED)