onchaind: wire up dev_memleak.

For onchaind we need to remove globals from memleak consideration;
we also change the htlc pointer to an htlc copy, which simplifies
things as well.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/onchain_control.c

@@ -327,6 +327,8 @@ static unsigned int onchain_msg(struct subd *sd, const u8 *msg, const int *fds U
 	case WIRE_ONCHAIN_DEPTH:
 	case WIRE_ONCHAIN_HTLC:
 	case WIRE_ONCHAIN_KNOWN_PREIMAGE:
+	case WIRE_ONCHAIN_DEV_MEMLEAK:
+	case WIRE_ONCHAIN_DEV_MEMLEAK_REPLY:
 		break;
 	}
 

lightningd/peer_control.c

@@ -1399,23 +1399,41 @@ AUTODATA(json_command, &dev_forget_channel_command);
 
 /* Mutual recursion */
 static void peer_memleak_req_next(struct command *cmd, struct channel *prev);
+static void peer_memleak_req_done(struct subd *subd, bool found_leak,
+				  struct command *cmd)
+{
+	struct channel *c = subd->channel;
+
+	if (found_leak)
+		peer_memleak_done(cmd, subd);
+	else
+		peer_memleak_req_next(cmd, c);
+}
+
 static void channeld_memleak_req_done(struct subd *channeld,
 				      const u8 *msg, const int *fds UNUSED,
 				      struct command *cmd)
 {
-	struct channel *c = channeld->channel;
 	bool found_leak;
 
 	if (!fromwire_channel_dev_memleak_reply(msg, &found_leak)) {
 		command_fail(cmd, LIGHTNINGD, "Bad channel_dev_memleak");
 		return;
 	}
+	peer_memleak_req_done(channeld, found_leak, cmd);
+}
+
+static void onchaind_memleak_req_done(struct subd *onchaind,
+				      const u8 *msg, const int *fds UNUSED,
+				      struct command *cmd)
+{
+	bool found_leak;
 
-	if (found_leak) {
-		peer_memleak_done(cmd, channeld);
+	if (!fromwire_onchain_dev_memleak_reply(msg, &found_leak)) {
+		command_fail(cmd, LIGHTNINGD, "Bad onchain_dev_memleak");
 		return;
 	}
-	peer_memleak_req_next(cmd, c);
+	peer_memleak_req_done(onchaind, found_leak, cmd);
 }
 
 static void peer_memleak_req_next(struct command *cmd, struct channel *prev)
@@ -1437,7 +1455,6 @@ static void peer_memleak_req_next(struct command *cmd, struct channel *prev)
 			if (prev != NULL)
 				continue;
 
-			/* FIXME: handle onchaind here */
 			/* FIXME: handle closingd here */
 			if (streq(c->owner->name, "lightning_channeld")) {
 				subd_req(c, c->owner,
@@ -1445,6 +1462,12 @@ static void peer_memleak_req_next(struct command *cmd, struct channel *prev)
 					 -1, 0, channeld_memleak_req_done, cmd);
 				return;
 			}
+			if (streq(c->owner->name, "lightning_onchaind")) {
+				subd_req(c, c->owner,
+					 take(towire_onchain_dev_memleak(NULL)),
+					 -1, 0, onchaind_memleak_req_done, cmd);
+				return;
+			}
 		}
 	}
 	peer_memleak_done(cmd, NULL);

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

@@ -85,6 +85,9 @@ bool fromwire_hsm_sign_commitment_tx_reply(const void *p UNNEEDED, secp256k1_ecd
 /* Generated stub for fromwire_hsm_sign_invoice_reply */
 bool fromwire_hsm_sign_invoice_reply(const void *p UNNEEDED, secp256k1_ecdsa_recoverable_signature *sig UNNEEDED)
 { fprintf(stderr, "fromwire_hsm_sign_invoice_reply called!\n"); abort(); }
+/* Generated stub for fromwire_onchain_dev_memleak_reply */
+bool fromwire_onchain_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED)
+{ fprintf(stderr, "fromwire_onchain_dev_memleak_reply called!\n"); abort(); }
 /* Generated stub for get_block_height */
 u32 get_block_height(const struct chain_topology *topo UNNEEDED)
 { fprintf(stderr, "get_block_height called!\n"); abort(); }
@@ -386,6 +389,9 @@ u8 *towire_hsm_sign_commitment_tx(const tal_t *ctx UNNEEDED, const struct pubkey
 /* Generated stub for towire_hsm_sign_invoice */
 u8 *towire_hsm_sign_invoice(const tal_t *ctx UNNEEDED, const u8 *u5bytes UNNEEDED, const u8 *hrp UNNEEDED)
 { fprintf(stderr, "towire_hsm_sign_invoice called!\n"); abort(); }
+/* Generated stub for towire_onchain_dev_memleak */
+u8 *towire_onchain_dev_memleak(const tal_t *ctx UNNEEDED)
+{ fprintf(stderr, "towire_onchain_dev_memleak called!\n"); abort(); }
 /* Generated stub for txfilter_add_scriptpubkey */
 void txfilter_add_scriptpubkey(struct txfilter *filter UNNEEDED, const u8 *script TAKES UNNEEDED)
 { fprintf(stderr, "txfilter_add_scriptpubkey called!\n"); abort(); }

onchaind/onchain_wire.csv

@@ -90,3 +90,9 @@ onchain_add_utxo,,prev_out_index,u32
 onchain_add_utxo,,per_commit_point,struct pubkey
 onchain_add_utxo,,value,u64
 onchain_add_utxo,,blockheight,u32
+
+# master -> onchaind: do you have a memleak?
+onchain_dev_memleak,5033
+
+onchain_dev_memleak_reply,5133
+onchain_dev_memleak_reply,,leak,bool

onchaind/onchaind.c

@@ -8,6 +8,7 @@
 #include <common/initial_commit_tx.h>
 #include <common/key_derive.h>
 #include <common/keyset.h>
+#include <common/memleak.h>
 #include <common/peer_billboard.h>
 #include <common/status.h>
 #include <common/subdaemon.h>
@@ -91,8 +92,8 @@ struct tracked_output {
 	u64 satoshi;
 	enum output_type output_type;
 
-	/* If it is an HTLC, these are non-NULL */
-	const struct htlc_stub *htlc;
+	/* If it is an HTLC, this is set, wscript is non-NULL. */
+	struct htlc_stub htlc;
 	const u8 *wscript;
 
 	/* If it's an HTLC off our unilateral, this is their sig for htlc_tx */
@@ -397,8 +398,9 @@ static struct tracked_output *
 	out->output_type = output_type;
 	out->proposal = NULL;
 	out->resolved = NULL;
-	out->htlc = htlc;
-	out->wscript = wscript;
+	if (htlc)
+		out->htlc = *htlc;
+	out->wscript = tal_steal(out, wscript);
 	out->remote_htlc_sig = remote_htlc_sig;
 
 	*tal_arr_expand(outs) = out;
@@ -811,14 +813,14 @@ static void handle_htlc_onchain_fulfill(struct tracked_output *out,
 	sha256(&sha, &preimage, sizeof(preimage));
 	ripemd160(&ripemd, &sha, sizeof(sha));
 
-	if (!ripemd160_eq(&ripemd, &out->htlc->ripemd))
+	if (!ripemd160_eq(&ripemd, &out->htlc.ripemd))
 		status_failed(STATUS_FAIL_INTERNAL_ERROR,
 			      "%s/%s spent with bad preimage %s (ripemd not %s)",
 			      tx_type_name(out->tx_type),
 			      output_type_name(out->output_type),
 			      type_to_string(tmpctx, struct preimage, &preimage),
 			      type_to_string(tmpctx, struct ripemd160,
-					     &out->htlc->ripemd));
+					     &out->htlc.ripemd));
 
 	/* Tell master we found a preimage. */
 	status_trace("%s/%s gave us preimage %s",
@@ -1035,7 +1037,7 @@ static void update_resolution_depth(struct tracked_output *out, u32 depth)
 			     tx_type_name(out->tx_type),
 			     output_type_name(out->output_type),
 			     depth);
-		msg = towire_onchain_htlc_timeout(out, out->htlc);
+		msg = towire_onchain_htlc_timeout(out, &out->htlc);
 		wire_sync_write(REQ_FD, take(msg));
 	}
 	out->resolved->depth = depth;
@@ -1123,7 +1125,7 @@ static void handle_preimage(struct tracked_output **outs,
 		if (outs[i]->output_type != THEIR_HTLC)
 			continue;
 
-		if (!ripemd160_eq(&outs[i]->htlc->ripemd, &ripemd))
+		if (!ripemd160_eq(&outs[i]->htlc.ripemd, &ripemd))
 			continue;
 
 		/* Too late? */
@@ -1194,6 +1196,55 @@ static void handle_preimage(struct tracked_output **outs,
 	}
 }
 
+#if DEVELOPER
+static void memleak_remove_globals(struct htable *memtable, const tal_t *topctx)
+{
+	/* memleak_scan_region is overkill if these are simple pointers to
+	 * objects which don't contain pointers, but it works. */
+	if (keyset)
+		memleak_scan_region(memtable, keyset, sizeof(*keyset));
+
+	if (remote_per_commitment_point)
+		memleak_scan_region(memtable, remote_per_commitment_point,
+				    sizeof(*remote_per_commitment_point));
+
+	if (remote_per_commitment_secret)
+		memleak_scan_region(memtable, remote_per_commitment_secret,
+				    sizeof(*remote_per_commitment_secret));
+
+	/* top-level context args */
+	memleak_scan_region(memtable, topctx, 0);
+
+	memleak_scan_region(memtable, missing_htlc_msgs,
+			    tal_bytelen(missing_htlc_msgs));
+}
+
+static bool handle_dev_memleak(struct tracked_output **outs, const u8 *msg)
+{
+	struct htable *memtable;
+	bool found_leak;
+
+	if (!fromwire_onchain_dev_memleak(msg))
+		return false;
+
+	memtable = memleak_enter_allocations(tmpctx, msg, msg);
+	/* Top-level context is parent of outs */
+	memleak_remove_globals(memtable, tal_parent(outs));
+	memleak_remove_referenced(memtable, outs);
+
+	found_leak = dump_memleak(memtable);
+	wire_sync_write(REQ_FD,
+			take(towire_onchain_dev_memleak_reply(NULL,
+							      found_leak)));
+	return true;
+}
+#else
+static bool handle_dev_memleak(struct tracked_output **outs, const u8 *msg)
+{
+	return false;
+}
+#endif /* !DEVELOPER */
+
 /* BOLT #5:
  *
  * A node:
@@ -1224,7 +1275,7 @@ static void wait_for_resolved(struct tracked_output **outs)
 			output_spent(&outs, tx, input_num, tx_blockheight);
 		else if (fromwire_onchain_known_preimage(msg, &preimage))
 			handle_preimage(outs, &preimage);
-		else
+		else if (!handle_dev_memleak(outs, msg))
 			master_badmsg(-1, msg);
 
 		billboard_update(outs);
@@ -1715,8 +1766,8 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx,
 			which_htlc = resolve_their_htlc(out, matches, htlcs,
 							htlc_scripts);
 		}
-		out->htlc = &htlcs[which_htlc];
-		out->wscript = htlc_scripts[which_htlc];
+		out->htlc = htlcs[which_htlc];
+		out->wscript = tal_steal(out, htlc_scripts[which_htlc]);
 
 		/* Each of these consumes one HTLC signature */
 		remote_htlc_sigs++;
@@ -2217,8 +2268,8 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx,
 			which_htlc = resolve_their_htlc(out, matches, htlcs,
 							htlc_scripts);
 		}
-		out->htlc = &htlcs[which_htlc];
-		out->wscript = htlc_scripts[which_htlc];
+		out->htlc = htlcs[which_htlc];
+		out->wscript = tal_steal(out, htlc_scripts[which_htlc]);
 		htlc_scripts[which_htlc] = NULL;
 	}
 
@@ -2342,7 +2393,7 @@ int main(int argc, char *argv[])
 	missing_htlc_msgs = tal_arr(ctx, u8 *, 0);
 
 	msg = wire_sync_read(tmpctx, REQ_FD);
-	if (!fromwire_onchain_init(ctx, msg,
+	if (!fromwire_onchain_init(tmpctx, msg,
 				   &shachain,
 				   &funding_amount_satoshi,
 				   &old_remote_per_commit_point,
@@ -2370,11 +2421,13 @@ int main(int argc, char *argv[])
 	}
 
 	bitcoin_txid(tx, &txid);
+	/* We need to keep tx around, but there's only one: not really a leak */
+	tal_steal(ctx, notleak(tx));
 
 	/* FIXME: Filter as we go, don't load them all into mem! */
-	htlcs = tal_arr(ctx, struct htlc_stub, num_htlcs);
-	tell_if_missing = tal_arr(ctx, bool, num_htlcs);
-	tell_immediately = tal_arr(ctx, bool, num_htlcs);
+	htlcs = tal_arr(tmpctx, struct htlc_stub, num_htlcs);
+	tell_if_missing = tal_arr(htlcs, bool, num_htlcs);
+	tell_immediately = tal_arr(htlcs, bool, num_htlcs);
 	if (!htlcs || !tell_if_missing || !tell_immediately)
 		status_failed(STATUS_FAIL_INTERNAL_ERROR,
 			      "Can't allocate %"PRIu64" htlcs", num_htlcs);

onchaind/test/run-grind_feerate.c

@@ -33,6 +33,9 @@ bool fromwire_hsm_sign_tx_reply(const void *p UNNEEDED, secp256k1_ecdsa_signatur
 /* Generated stub for fromwire_onchain_depth */
 bool fromwire_onchain_depth(const void *p UNNEEDED, struct bitcoin_txid *txid UNNEEDED, u32 *depth UNNEEDED)
 { fprintf(stderr, "fromwire_onchain_depth called!\n"); abort(); }
+/* Generated stub for fromwire_onchain_dev_memleak */
+bool fromwire_onchain_dev_memleak(const void *p UNNEEDED)
+{ fprintf(stderr, "fromwire_onchain_dev_memleak called!\n"); abort(); }
 /* Generated stub for fromwire_onchain_htlc */
 bool fromwire_onchain_htlc(const void *p UNNEEDED, struct htlc_stub *htlc UNNEEDED, bool *tell_if_missing UNNEEDED, bool *tell_immediately UNNEEDED)
 { fprintf(stderr, "fromwire_onchain_htlc called!\n"); abort(); }
@@ -129,6 +132,9 @@ u8 *towire_onchain_all_irrevocably_resolved(const tal_t *ctx UNNEEDED)
 /* Generated stub for towire_onchain_broadcast_tx */
 u8 *towire_onchain_broadcast_tx(const tal_t *ctx UNNEEDED, const struct bitcoin_tx *tx UNNEEDED)
 { fprintf(stderr, "towire_onchain_broadcast_tx called!\n"); abort(); }
+/* Generated stub for towire_onchain_dev_memleak_reply */
+u8 *towire_onchain_dev_memleak_reply(const tal_t *ctx UNNEEDED, bool leak UNNEEDED)
+{ fprintf(stderr, "towire_onchain_dev_memleak_reply called!\n"); abort(); }
 /* Generated stub for towire_onchain_extracted_preimage */
 u8 *towire_onchain_extracted_preimage(const tal_t *ctx UNNEEDED, const struct preimage *preimage UNNEEDED)
 { fprintf(stderr, "towire_onchain_extracted_preimage called!\n"); abort(); }
@@ -152,6 +158,27 @@ bool wire_sync_write(int fd UNNEEDED, const void *msg TAKES UNNEEDED)
 { fprintf(stderr, "wire_sync_write called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
+#if DEVELOPER
+/* Generated stub for dump_memleak */
+bool dump_memleak(struct htable *memtable UNNEEDED)
+{ fprintf(stderr, "dump_memleak called!\n"); abort(); }
+/* Generated stub for memleak_enter_allocations */
+struct htable *memleak_enter_allocations(const tal_t *ctx UNNEEDED,
+					 const void *exclude1 UNNEEDED,
+					 const void *exclude2 UNNEEDED)
+{ fprintf(stderr, "memleak_enter_allocations called!\n"); abort(); }
+/* Generated stub for memleak_remove_referenced */
+void memleak_remove_referenced(struct htable *memtable UNNEEDED, const void *root UNNEEDED)
+{ fprintf(stderr, "memleak_remove_referenced called!\n"); abort(); }
+/* Generated stub for memleak_scan_region */
+void memleak_scan_region(struct htable *memtable UNNEEDED,
+			 const void *p UNNEEDED, size_t bytelen UNNEEDED)
+{ fprintf(stderr, "memleak_scan_region called!\n"); abort(); }
+/* Generated stub for notleak_ */
+void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
+{ fprintf(stderr, "notleak_ called!\n"); abort(); }
+#endif
+
 int main(int argc, char *argv[])
 {
 	setup_locale();

wallet/test/run-wallet.c

@@ -94,6 +94,9 @@ bool fromwire_gossip_get_channel_peer_reply(const tal_t *ctx UNNEEDED, const voi
 /* Generated stub for fromwire_hsm_sign_commitment_tx_reply */
 bool fromwire_hsm_sign_commitment_tx_reply(const void *p UNNEEDED, secp256k1_ecdsa_signature *sig UNNEEDED)
 { fprintf(stderr, "fromwire_hsm_sign_commitment_tx_reply called!\n"); abort(); }
+/* Generated stub for fromwire_onchain_dev_memleak_reply */
+bool fromwire_onchain_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED)
+{ fprintf(stderr, "fromwire_onchain_dev_memleak_reply called!\n"); abort(); }
 /* Generated stub for get_block_height */
 u32 get_block_height(const struct chain_topology *topo UNNEEDED)
 { fprintf(stderr, "get_block_height called!\n"); abort(); }
@@ -451,6 +454,9 @@ u8 *towire_gossip_get_channel_peer(const tal_t *ctx UNNEEDED, const struct short
 /* Generated stub for towire_hsm_sign_commitment_tx */
 u8 *towire_hsm_sign_commitment_tx(const tal_t *ctx UNNEEDED, const struct pubkey *peer_id UNNEEDED, u64 channel_dbid UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const struct pubkey *remote_funding_key UNNEEDED, u64 funding_amount UNNEEDED)
 { fprintf(stderr, "towire_hsm_sign_commitment_tx called!\n"); abort(); }
+/* Generated stub for towire_onchain_dev_memleak */
+u8 *towire_onchain_dev_memleak(const tal_t *ctx UNNEEDED)
+{ fprintf(stderr, "towire_onchain_dev_memleak called!\n"); abort(); }
 /* Generated stub for towire_onchain_known_preimage */
 u8 *towire_onchain_known_preimage(const tal_t *ctx UNNEEDED, const struct preimage *preimage UNNEEDED)
 { fprintf(stderr, "towire_onchain_known_preimage called!\n"); abort(); }