lightningd: Added --subdaemon command to allow alternate subdaemons.

Changelog-Added: lightningd: Added --subdaemon command to allow alternate subdaemons.

[ Wow, that was mammoth; 44 comments over 12 commits. Feels almost unfair to squash it into one commit, so I wanted to note @ksedgwic's perseverence here! --RR ]

doc/lightningd-config.5

@@ -123,6 +123,19 @@ is only valid on the command-line, or in a configuration file specified
 by \fI--conf\fR\.
 
 
+ \fBsubdaemon\fR=\fISUBDAEMON\fR:\fIPATH\fR
+Specifies an alternate subdaemon binary\.
+Current subdaemons are \fIchanneld\fR, \fIclosingd\fR,
+\fIconnectd\fR, \fIgossipd\fR, \fIhsmd\fR, \fIonchaind\fR, and \fIopeningd\fR\.
+If the supplied path is relative the subdaemon binary is found in the
+working directory\. This option may be specified multiple times\.
+
+
+ So, \fBsubdaemon=hsmd:remote_signer\fR would use a
+hypothetical remote signing proxy instead of the standard \fIlightning_hsmd\fR
+binary\.
+
+
  \fBpid-file\fR=\fIPATH\fR
 Specify pid file to write to\.
 

doc/lightningd-config.5.md

@@ -107,6 +107,17 @@ Sets the working directory. All files (except *--conf* and
 is only valid on the command-line, or in a configuration file specified
 by *--conf*.
 
+ **subdaemon**=*SUBDAEMON*:*PATH*
+Specifies an alternate subdaemon binary.
+Current subdaemons are *channeld*, *closingd*,
+*connectd*, *gossipd*, *hsmd*, *onchaind*, and *openingd*.
+If the supplied path is relative the subdaemon binary is found in the
+working directory. This option may be specified multiple times.
+
+ So, **subdaemon=hsmd:remote_signer** would use a
+hypothetical remote signing proxy instead of the standard *lightning_hsmd*
+binary.
+
  **pid-file**=*PATH*
 Specify pid file to write to.
 

lightningd/lightningd.c

@@ -57,6 +57,7 @@
 /*~ This is common code: routines shared by one or more executables
  *  (separate daemons, or the lightning-cli program). */
 #include <common/daemon.h>
+#include <common/memleak.h>
 #include <common/timeout.h>
 #include <common/utils.h>
 #include <common/version.h>
@@ -72,6 +73,7 @@
 #include <lightningd/io_loop_with_timers.h>
 #include <lightningd/jsonrpc.h>
 #include <lightningd/log.h>
+#include <lightningd/memdump.h>
 #include <lightningd/onchain_control.h>
 #include <lightningd/options.h>
 #include <onchaind/onchain_wire.h>
@@ -81,6 +83,12 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+static void destroy_alt_subdaemons(struct lightningd *ld);
+#if DEVELOPER
+static void memleak_help_alt_subdaemons(struct htable *memtable,
+					struct lightningd *ld);
+#endif /* DEVELOPER */
+
 /*~ The core lightning object: it's passed everywhere, and is basically a
  * global variable.  This new_xxx pattern is something we'll see often:
  * it allocates and initializes a new structure, using *tal*, the hierarchical
@@ -248,6 +256,11 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	 */
 	ld->encrypted_hsm = false;
 
+	/* This is used to override subdaemons */
+	strmap_init(&ld->alt_subdaemons);
+	tal_add_destructor(ld, destroy_alt_subdaemons);
+	memleak_add_helper(ld, memleak_help_alt_subdaemons);
+
 	/*~ We change umask if we daemonize, but not if we don't. Initialize the
 	 * initial_umask anyway as we might rely on it later (`plugin start`). */
 	ld->initial_umask = umask(0);
@@ -278,6 +291,50 @@ static const char *subdaemons[] = {
 	"lightning_openingd"
 };
 
+/* Return true if called with a recognized subdaemon e.g. "hsmd" */
+bool is_subdaemon(const char *sdname)
+{
+	for (size_t i = 0; i < ARRAY_SIZE(subdaemons); i++)
+		/* Skip the "lightning_" prefix in the table */
+		if (streq(sdname, subdaemons[i] + strlen("lightning_")))
+			return true;
+	return false;
+}
+
+static void destroy_alt_subdaemons(struct lightningd *ld)
+{
+	strmap_clear(&ld->alt_subdaemons);
+}
+
+#if DEVELOPER
+static void memleak_help_alt_subdaemons(struct htable *memtable,
+					struct lightningd *ld)
+{
+	memleak_remove_strmap(memtable, &ld->alt_subdaemons);
+}
+#endif /* DEVELOPER */
+
+const char *subdaemon_path(const tal_t *ctx, const struct lightningd *ld, const char *name)
+{
+	/* Strip the leading "lightning_" before looking in alt_subdaemons.
+	 */
+	size_t pfxlen = strlen("lightning_");
+	assert(strlen(name) > pfxlen);
+	const char *short_name = tal_strdup(ctx, name + pfxlen);
+
+	/* Is there an alternate path for this subdaemon? */
+	const char *dpath;
+	const char *alt = strmap_get(&ld->alt_subdaemons, short_name);
+	if (alt) {
+		/* path_join will honor absolute paths as well. */
+		dpath = path_join(ctx, ld->daemon_dir, alt);
+	} else {
+		/* This subdaemon is found in the standard place. */
+		dpath = path_join(ctx, ld->daemon_dir, name);
+	}
+	return dpath;
+}
+
 /*~ Check we can run them, and check their versions */
 void test_subdaemons(const struct lightningd *ld)
 {
@@ -292,7 +349,6 @@ void test_subdaemons(const struct lightningd *ld)
 	 * ARRAY_SIZE will cause a compiler error if the argument is actually
 	 * a pointer, not an array. */
 	for (i = 0; i < ARRAY_SIZE(subdaemons); i++) {
-		int outfd;
 		/*~ CCAN's path module uses tal, so wants a context to
 		 * allocate from.  We have a magic convenience context
 		 * `tmpctx` for temporary allocations like this.
@@ -302,7 +358,8 @@ void test_subdaemons(const struct lightningd *ld)
 		 * can free `tmpctx` in that top-level loop after each event
 		 * is handled.
 		 */
-		const char *dpath = path_join(tmpctx, ld->daemon_dir, subdaemons[i]);
+		int outfd;
+		const char *dpath = subdaemon_path(tmpctx, ld, subdaemons[i]);
 		const char *verstring;
 		/*~ CCAN's pipecmd module is like popen for grownups: it
 		 * takes pointers to fill in stdin, stdout and stderr file

lightningd/lightningd.h

@@ -4,6 +4,7 @@
 #include <bitcoin/chainparams.h>
 #include <bitcoin/privkey.h>
 #include <ccan/container_of/container_of.h>
+#include <ccan/strmap/strmap.h>
 #include <ccan/time/time.h>
 #include <ccan/timer/timer.h>
 #include <lightningd/htlc_end.h>
@@ -77,6 +78,8 @@ struct config {
 	struct secret *keypass;
 };
 
+typedef STRMAP(const char *) alt_subdaemon_map;
+
 struct lightningd {
 	/* The directory to find all the subdaemons. */
 	const char *daemon_dir;
@@ -257,12 +260,20 @@ struct lightningd {
 
 	/* Outstanding waitblockheight commands.  */
 	struct list_head waitblockheight_commands;
+
+	alt_subdaemon_map alt_subdaemons;
 };
 
 /* Turning this on allows a tal allocation to return NULL, rather than aborting.
  * Use only on carefully tested code! */
 extern bool tal_oom_ok;
 
+/* Returns true if called with a recognized subdaemon, eg: "hsmd" */
+bool is_subdaemon(const char *sdname);
+
+/* Returns the path to the subdaemon. Considers alternate subdaemon paths. */
+const char *subdaemon_path(const tal_t *ctx, const struct lightningd *ld, const char *name);
+
 /* Check we can run subdaemons, and check their versions */
 void test_subdaemons(const struct lightningd *ld);
 

lightningd/options.c

@@ -215,6 +215,33 @@ static char *opt_add_addr(const char *arg, struct lightningd *ld)
 	return opt_add_addr_withtype(arg, ld, ADDR_LISTEN_AND_ANNOUNCE, true);
 }
 
+static char *opt_subdaemon(const char *arg, struct lightningd *ld)
+{
+	char *subdaemon;
+	char *sdpath;
+
+	/* example arg: "hsmd:remote_hsmd" */
+
+	size_t colonoff = strcspn(arg, ":");
+	if (!arg[colonoff])
+		return tal_fmt(NULL, "argument must contain ':'");
+
+	subdaemon = tal_strndup(ld, arg, colonoff);
+	if (!is_subdaemon(subdaemon))
+		return tal_fmt(NULL, "\"%s\" is not a subdaemon", subdaemon);
+
+	/* Make the value a tal-child of the subdaemon */
+	sdpath = tal_strdup(subdaemon, arg + colonoff + 1);
+
+	/* Remove any preexisting alt subdaemon mapping (and
+	 * implicitly, the sdpath). */
+	tal_free(strmap_del(&ld->alt_subdaemons, subdaemon, NULL));
+
+	strmap_add(&ld->alt_subdaemons, subdaemon, sdpath);
+
+	return NULL;
+}
+
 static char *opt_add_bind_addr(const char *arg, struct lightningd *ld)
 {
 	struct wireaddr_internal addr;
@@ -879,6 +906,16 @@ static void register_opts(struct lightningd *ld)
 			 "Set the file mode (permissions) for the "
 			 "JSON-RPC socket");
 
+	opt_register_arg("--subdaemon", opt_subdaemon, NULL,
+			 ld, "Arg specified as SUBDAEMON:PATH. "
+			 "Specifies an alternate subdaemon binary. "
+			 "If the supplied path is relative the subdaemon "
+			 "binary is found in the working directory. "
+			 "This option may be specified multiple times. "
+			 "For example, "
+			 "--subdaemon=hsmd:remote_signer "
+			 "would use a hypothetical remote signing subdaemon.");
+
 	opt_register_logging(ld);
 	opt_register_version();
 
@@ -1127,6 +1164,31 @@ static void json_add_opt_addrs(struct json_stream *response,
 	}
 }
 
+struct json_add_opt_alt_subdaemon_args {
+	const char *name0;
+	struct json_stream *response;
+};
+
+static bool json_add_opt_alt_subdaemon(const char *member,
+				       const char *value,
+				       struct json_add_opt_alt_subdaemon_args *argp)
+{
+	json_add_string(argp->response,
+			argp->name0,
+			tal_fmt(argp->name0, "%s:%s", member, value));
+	return true;
+}
+
+static void json_add_opt_subdaemons(struct json_stream *response,
+				    const char *name0,
+				    alt_subdaemon_map *alt_subdaemons)
+{
+	struct json_add_opt_alt_subdaemon_args args;
+	args.name0 = name0;
+	args.response = response;
+	strmap_iterate(alt_subdaemons, json_add_opt_alt_subdaemon, &args);
+}
+
 static void add_config(struct lightningd *ld,
 		       struct json_stream *response,
 		       const struct opt_table *opt,
@@ -1221,6 +1283,10 @@ static void add_config(struct lightningd *ld,
 					   ld->proposed_listen_announce,
 					   ADDR_ANNOUNCE);
 			return;
+		} else if (opt->cb_arg == (void *)opt_subdaemon) {
+			json_add_opt_subdaemons(response, name0,
+						    &ld->alt_subdaemons);
+			return;
 		} else if (opt->cb_arg == (void *)opt_add_proxy_addr) {
 			if (ld->proxyaddr)
 				answer = fmt_wireaddr(name0, ld->proxyaddr);

lightningd/subd.c

@@ -136,7 +136,7 @@ static void close_taken_fds(va_list *ap)
 }
 
 /* We use sockets, not pipes, because fds are bidir. */
-static int subd(const char *dir, const char *name,
+static int subd(const char *path, const char *name,
 		const char *debug_subdaemon,
 		int *msgfd, int dev_disconnect_fd,
 		bool io_logging,
@@ -202,7 +202,7 @@ static int subd(const char *dir, const char *name,
 				close(i);
 
 		num_args = 0;
-		args[num_args++] = path_join(NULL, dir, name);
+		args[num_args++] = tal_strdup(NULL, path);
 		if (io_logging)
 			args[num_args++] = "--log-io";
 #if DEVELOPER
@@ -649,7 +649,9 @@ static struct subd *new_subd(struct lightningd *ld,
 	disconnect_fd = ld->dev_disconnect_fd;
 #endif /* DEVELOPER */
 
-	sd->pid = subd(ld->daemon_dir, name, debug_subd,
+	const char *path = subdaemon_path(tmpctx, ld, name);
+
+	sd->pid = subd(path, name, debug_subd,
 		       &msg_fd, disconnect_fd,
 		       /* We only turn on subdaemon io logging if we're going
 			* to print it: too stressful otherwise! */

lightningd/test/run-find_my_abspath.c

@@ -133,6 +133,9 @@ bool log_status_msg(struct log *log UNNEEDED,
  		    const struct node_id *node_id UNNEEDED,
 		    const u8 *msg UNNEEDED)
 { fprintf(stderr, "log_status_msg called!\n"); abort(); }
+/* Generated stub for memleak_remove_strmap_ */
+void memleak_remove_strmap_(struct htable *memtable UNNEEDED, const struct strmap *m UNNEEDED)
+{ fprintf(stderr, "memleak_remove_strmap_ called!\n"); abort(); }
 /* Generated stub for new_log */
 struct log *new_log(const tal_t *ctx UNNEEDED, struct log_book *record UNNEEDED,
 		    const struct node_id *default_node_id UNNEEDED,