watch: make it easier for them to self-delete.

Rather than keeping a pointer so they can free themselves, make it explicit. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
9 years ago · b8571c1ac8
3 changed files with 130 additions and 85 deletions
--- a/daemon/peer.c
+++ b/daemon/peer.c
@ -1154,7 +1154,8 @@ struct anchor_watch {
 	struct oneshot *timer;
 };

-static void anchor_depthchange(struct peer *peer, unsigned int depth,
+static enum watch_result anchor_depthchange(struct peer *peer,
+					    unsigned int depth,
 					    const struct sha256_double *txid,
 					    void *unused)
 {
@ -1175,6 +1176,7 @@ static void anchor_depthchange(struct peer *peer, unsigned int depth,

 	/* Since this gets called on every new block, check HTLCs here. */
 	check_htlc_expiry(peer);
+	return KEEP_WATCHING;
 }

 /* Yay, segwit!  We can just compare txids, even though we don't have both
@ -1474,7 +1476,7 @@ static void resolve_cheating(struct peer *peer)
 	broadcast_tx(peer, steal_tx);
 }

-static void our_htlc_spent(struct peer *peer,
+static enum watch_result our_htlc_spent(struct peer *peer,
 					const struct bitcoin_tx *tx,
 					size_t input_num,
 					ptrint_t *pi)
@ -1503,7 +1505,8 @@ static void our_htlc_spent(struct peer *peer,
 	
 	/* Our timeout tx has all-zeroes, so we can distinguish it. */
 	if (memeqzero(tx->input[input_num].witness[1], sizeof(preimage)))
-		return;
+		/* They might try to race us. */
+		return KEEP_WATCHING;

 	memcpy(&preimage, tx->input[input_num].witness[1], sizeof(preimage));
 	sha256(&sha, &preimage, sizeof(preimage));
@ -1526,9 +1529,10 @@ static void our_htlc_spent(struct peer *peer,
 	 * preimage; the knowledge is not revocable.
 	 */
 	peer->closing_onchain.resolved[i] = irrevocably_resolved(peer);
+	return DELETE_WATCH;
 }

-static void our_htlc_depth(struct peer *peer,
+static enum watch_result our_htlc_depth(struct peer *peer,
 					unsigned int depth,
 					const struct sha256_double *txid,
 					bool our_commit,
@ -1539,7 +1543,7 @@ static void our_htlc_depth(struct peer *peer,

 	/* Must be in a block. */
 	if (depth == 0)
-		return;
+		return KEEP_WATCHING;

 	height = get_block_height(peer->dstate);
 	h = htlc_by_index(peer->closing_onchain.ci, i);
@ -1554,11 +1558,11 @@ static void our_htlc_depth(struct peer *peer,
 	 */

 	if (height < abs_locktime_to_blocks(&h->expiry))
-		return;
+		return KEEP_WATCHING;

 	if (our_commit) {
 		if (depth < rel_locktime_to_blocks(&peer->remote.locktime))
-			return;
+			return KEEP_WATCHING;
 	}

 	/* BOLT #onchain:
@ -1566,27 +1570,29 @@ static void our_htlc_depth(struct peer *peer,
 	 * If the output has *timed out* and not been *resolved*, the node
 	 * MUST *resolve* the output by spending it.
 	 */
+	/* FIXME: we should simply delete this watch if HTLC is fulfilled. */
 	if (!peer->closing_onchain.resolved[i]) {
 		peer->closing_onchain.resolved[i]
 			= htlc_timeout_tx(peer, peer->closing_onchain.ci, i);
 		broadcast_tx(peer, peer->closing_onchain.resolved[i]);
 	}
+	return DELETE_WATCH;
 }

-static void our_htlc_depth_ourcommit(struct peer *peer,
+static enum watch_result our_htlc_depth_ourcommit(struct peer *peer,
 						  unsigned int depth,
 						  const struct sha256_double *txid,
 						  ptrint_t *i)
 {
-	our_htlc_depth(peer, depth, txid, true, ptr2int(i));
+	return our_htlc_depth(peer, depth, txid, true, ptr2int(i));
 }

-static void our_htlc_depth_theircommit(struct peer *peer,
+static enum watch_result our_htlc_depth_theircommit(struct peer *peer,
 						    unsigned int depth,
 						    const struct sha256_double *txid,
 						    ptrint_t *i)
 {
-	our_htlc_depth(peer, depth, txid, false, ptr2int(i));
+	return our_htlc_depth(peer, depth, txid, false, ptr2int(i));
 }

 static void resolve_our_htlcs(struct peer *peer,
@ -1644,7 +1650,7 @@ void our_htlc_fulfilled(struct peer *peer, struct htlc *htlc,
 	}
 }

-static void their_htlc_depth(struct peer *peer,
+static enum watch_result their_htlc_depth(struct peer *peer,
 					  unsigned int depth,
 					  const struct sha256_double *txid,
 					  ptrint_t *pi)
@ -1655,7 +1661,7 @@ static void their_htlc_depth(struct peer *peer,

 	/* Must be in a block. */
 	if (depth == 0)
-		return;
+		return KEEP_WATCHING;

 	height = get_block_height(peer->dstate);
 	h = htlc_by_index(peer->closing_onchain.ci, i);
@ -1667,9 +1673,10 @@ static void their_htlc_depth(struct peer *peer,
 	 */

 	if (height < abs_locktime_to_blocks(&h->expiry))
-		return;
+		return KEEP_WATCHING;

 	peer->closing_onchain.resolved[i] = irrevocably_resolved(peer);
+	return DELETE_WATCH;
 }

 static void resolve_their_htlcs(struct peer *peer,
@ -1691,18 +1698,16 @@ static void resolve_their_htlcs(struct peer *peer,
 	}	
 }

-static void our_main_output_depth(struct peer *peer,
+static enum watch_result our_main_output_depth(struct peer *peer,
 					       unsigned int depth,
 					       const struct sha256_double *txid,
 					       void *unused)
 {
 	/* Not past CSV timeout? */
 	if (depth < rel_locktime_to_blocks(&peer->remote.locktime))
-		return;
+		return KEEP_WATCHING;

-	/* Already done?  (FIXME: Delete after first time) */
-	if (peer->closing_onchain.resolved[0])
-		return;
+	assert(!peer->closing_onchain.resolved[0]);

 	/* BOLT #onchain:
 	 *
@ -1715,6 +1720,7 @@ static void our_main_output_depth(struct peer *peer,
 	 */
 	peer->closing_onchain.resolved[0] = bitcoin_spend_ours(peer);
 	broadcast_tx(peer, peer->closing_onchain.resolved[0]);
+	return DELETE_WATCH;
 }

 /* BOLT #onchain:
@ -1832,7 +1838,7 @@ static void resolve_mutual_close(struct peer *peer)
 }

 /* Called every time the tx spending the funding tx changes depth. */
-static void check_for_resolution(struct peer *peer,
+static enum watch_result check_for_resolution(struct peer *peer,
 					      unsigned int depth,
 					      const struct sha256_double *txid,
 					      void *unused)
@ -1848,7 +1854,7 @@ static void check_for_resolution(struct peer *peer,
 	 */
 	for (i = 0; i < n; i++)
 		if (!peer->closing_onchain.resolved[i])
-			return;
+			return KEEP_WATCHING;

 	/* BOLT #onchain:
 	 *
@ -1857,14 +1863,14 @@ static void check_for_resolution(struct peer *peer,
 	 * 100 deep on the most-work blockchain.
 	 */
 	if (depth < forever)
-		return;
+		return KEEP_WATCHING;

 	for (i = 0; i < n; i++) {
 		struct sha256_double txid;

 		bitcoin_txid(peer->closing_onchain.resolved[i], &txid);
 		if (get_tx_depth(peer->dstate, &txid) < forever)
-			return;
+			return KEEP_WATCHING;
 	}

 	/* BOLT #onchain:
@ -1880,11 +1886,13 @@ static void check_for_resolution(struct peer *peer,
 		io_break(peer);
 	else
 		io_wake(peer);
+
+	return DELETE_WATCH;
 }
 	    
 /* We assume the tx is valid!  Don't do a blockchain.info and feed this
 * invalid transactions! */
-static void anchor_spent(struct peer *peer,
+static enum watch_result anchor_spent(struct peer *peer,
 				      const struct bitcoin_tx *tx,
 				      size_t input_num,
 				      void *unused)
@ -1943,7 +1951,7 @@ static void anchor_spent(struct peer *peer,
 			       "anchor_spent");
 		/* No longer call into the state machine. */
 		peer->anchor.watches->depthok = INPUT_NONE;
-		return;
+		return DELETE_WATCH;
 	}

 	/* BOLT #onchain:
@ -1970,6 +1978,7 @@ static void anchor_spent(struct peer *peer,

 	/* No longer call into the state machine. */
 	peer->anchor.watches->depthok = INPUT_NONE;
+	return KEEP_WATCHING;
 }

 static void anchor_timeout(struct anchor_watch *w)
--- a/daemon/watch.c
+++ b/daemon/watch.c
@ -88,8 +88,9 @@ static void destroy_txwatch(struct txwatch *w)
 struct txwatch *watch_txid_(const tal_t *ctx,
 			    struct peer *peer,
 			    const struct sha256_double *txid,
-			    void (*cb)(struct peer *peer, unsigned int depth,
-				       const struct sha256_double *txid,
+			    enum watch_result (*cb)(struct peer *peer,
+						    unsigned int depth,
+						    const struct sha256_double *,
 						    void *arg),
 			    void *cb_arg)
 {
@ -118,8 +119,9 @@ bool watching_txid(struct lightningd_state *dstate,
 struct txwatch *watch_tx_(const tal_t *ctx,
 			  struct peer *peer,
 			  const struct bitcoin_tx *tx,
-			  void (*cb)(struct peer *peer, unsigned int depth,
-				     const struct sha256_double *txid,
+			  enum watch_result (*cb)(struct peer *peer,
+						  unsigned int depth,
+						  const struct sha256_double *,
 						  void *arg),
 			  void *cb_arg)
 {
@ -133,7 +135,7 @@ struct txowatch *watch_txo_(const tal_t *ctx,
 			    struct peer *peer,
 			    const struct sha256_double *txid,
 			    unsigned int output,
-			    void (*cb)(struct peer *peer,
+			    enum watch_result (*cb)(struct peer *peer,
 						    const struct bitcoin_tx *tx,
 						    size_t input_num,
 						    void *),
@ -160,6 +162,7 @@ void txwatch_fire(struct lightningd_state *dstate,
 	struct txwatch *txw = txwatch_hash_get(&dstate->txwatches, txid);

 	if (txw && depth != txw->depth) {
+		enum watch_result r;
 		log_debug(txw->peer->log,
 			  "Got depth change %u for %02x%02x%02x...\n",
 			  txw->depth,
@ -167,7 +170,15 @@ void txwatch_fire(struct lightningd_state *dstate,
 			  txw->txid.sha.u.u8[1],
 			  txw->txid.sha.u.u8[2]);
 		txw->depth = depth;
-		txw->cb(txw->peer, txw->depth, &txw->txid, txw->cbdata);
+		r = txw->cb(txw->peer, txw->depth, &txw->txid, txw->cbdata);
+		switch (r) {
+		case DELETE_WATCH:
+			tal_free(txw);
+			return;
+		case KEEP_WATCHING:
+			return;
+		}
+		fatal("txwatch callback %p returned %i\n", txw->cb, r);
 	}
 }

@ -177,6 +188,7 @@ void txowatch_fire(struct lightningd_state *dstate,
 		   size_t input_num)
 {
 	struct sha256_double txid;
+	enum watch_result r;

 	bitcoin_txid(tx, &txid);
 	log_debug(txow->peer->log,
@ -189,7 +201,15 @@ void txowatch_fire(struct lightningd_state *dstate,
 		  txid.sha.u.u8[1],
 		  txid.sha.u.u8[2],
 		  txid.sha.u.u8[3]);
-	txow->cb(txow->peer, tx, input_num, txow->cbdata);
+	r = txow->cb(txow->peer, tx, input_num, txow->cbdata);
+	switch (r) {
+	case DELETE_WATCH:
+		tal_free(txow);
+		return;
+	case KEEP_WATCHING:
+		return;
+	}
+	fatal("txowatch callback %p returned %i\n", txow->cb, r);
 }

 void watch_topology_changed(struct lightningd_state *dstate)
@ -208,9 +228,18 @@ again:

 		depth = get_tx_depth(dstate, &w->txid);
 		if (depth != w->depth) {
+			enum watch_result r;
 			w->depth = depth;
-			w->cb(w->peer, w->depth, &w->txid, w->cbdata);
 			needs_rerun = true;
+			r = w->cb(w->peer, w->depth, &w->txid, w->cbdata);
+			switch (r) {
+			case DELETE_WATCH:
+				tal_free(w);
+				continue;
+			case KEEP_WATCHING:
+				continue;
+			}
+			fatal("txwatch callback %p returned %i\n", w->cb, r);
 		}
 	}
 	if (needs_rerun)
--- a/daemon/watch.h
+++ b/daemon/watch.h
@ -11,6 +11,11 @@
 struct bitcoin_tx;
 struct lightningd_state;

+enum watch_result {
+	DELETE_WATCH = -1,
+	KEEP_WATCHING = -2
+};
+
 struct txwatch_output {
 	struct sha256_double txid;
 	unsigned int index;
@ -25,7 +30,7 @@ struct txowatch {
 	struct txwatch_output out;

 	/* A new tx. */
-	void (*cb)(struct peer *peer,
+	enum watch_result (*cb)(struct peer *peer,
 				const struct bitcoin_tx *tx,
 				size_t input_num,
 				void *cbdata);
@ -51,7 +56,7 @@ struct txwatch {
 	unsigned int depth;

 	/* A new depth (0 if kicked out, otherwise 1 = tip, etc.) */
-	void (*cb)(struct peer *peer, unsigned int depth,
+	enum watch_result (*cb)(struct peer *peer, unsigned int depth,
 				const struct sha256_double *txid,
 				void *cbdata);
 	void *cbdata;
@ -67,14 +72,15 @@ HTABLE_DEFINE_TYPE(struct txwatch, txwatch_keyof, txid_hash, txwatch_eq,
 struct txwatch *watch_txid_(const tal_t *ctx,
 			    struct peer *peer,
 			    const struct sha256_double *txid,
-			    void (*cb)(struct peer *peer, unsigned int depth,
-				       const struct sha256_double *txid,
+			    enum watch_result (*cb)(struct peer *peer,
+						    unsigned int depth,
+						    const struct sha256_double*,
 						    void *),
 			    void *cbdata);

 #define watch_txid(ctx, peer, txid, cb, cbdata)				\
 	watch_txid_((ctx), (peer), (txid),				\
-		    typesafe_cb_preargs(void, void *,			\
+		    typesafe_cb_preargs(enum watch_result, void *,	\
 					(cb), (cbdata),			\
 					struct peer *,			\
 					unsigned int depth,		\
@ -84,14 +90,15 @@ struct txwatch *watch_txid_(const tal_t *ctx,
 struct txwatch *watch_tx_(const tal_t *ctx,
 			  struct peer *peer,
 			  const struct bitcoin_tx *tx,
-			  void (*cb)(struct peer *peer, unsigned int depth,
-				     const struct sha256_double *txid,
+			  enum watch_result (*cb)(struct peer *peer,
+						  unsigned int depth,
+						  const struct sha256_double *,
 						  void *),
 			  void *cbdata);

 #define watch_tx(ctx, peer, tx, cb, cbdata)				\
 	watch_tx_((ctx), (peer), (tx),					\
-		  typesafe_cb_preargs(void, void *,			\
+		  typesafe_cb_preargs(enum watch_result, void *,	\
 				      (cb), (cbdata),			\
 				      struct peer *,			\
 				      unsigned int depth,		\
@ -102,7 +109,7 @@ struct txowatch *watch_txo_(const tal_t *ctx,
 			    struct peer *peer,
 			    const struct sha256_double *txid,
 			    unsigned int output,
-			    void (*cb)(struct peer *peer,
+			    enum watch_result (*cb)(struct peer *peer,
 						    const struct bitcoin_tx *tx,
 						    size_t input_num,
 						    void *),
@ -110,7 +117,7 @@ struct txowatch *watch_txo_(const tal_t *ctx,

 #define watch_txo(ctx, peer, txid, outnum, cb, cbdata)			\
 	watch_txo_((ctx), (peer), (txid), (outnum),			\
-		  typesafe_cb_preargs(void, void *,			\
+		   typesafe_cb_preargs(enum watch_result, void *,	\
 				      (cb), (cbdata),			\
 				      struct peer *,			\
 				      const struct bitcoin_tx *,	\