Dynamic plugin cleanups

doc/lightningd-config.5

@@ -528,9 +528,11 @@ normal effect\.
 
 
  \fBdisable-plugin\fR=\fIPLUGIN\fR
-If \fIPLUGIN\fR contains a /, plugins with the same path as \fIPLUGIN\fR are
-disabled\. Otherwise, any plugin with that base name is disabled,
-whatever directory it is in\.
+If \fIPLUGIN\fR contains a /, plugins with the same path as \fIPLUGIN\fR will
+not be loaded at startup\. Otherwise, no plugin with that base name will
+be loaded at startup, whatever directory it is in\.  This option is useful for
+disabling a single plugin inside a directory\.  You can still explicitly
+load plugins which have been disabled, using \fBlightning-plugin\fR(7) \fBstart\fR\.
 
 .SH BUGS
 

doc/lightningd-config.5.md

@@ -434,9 +434,11 @@ including the default built-in plugin directory. You can still add
 normal effect.
 
  **disable-plugin**=*PLUGIN*
-If *PLUGIN* contains a /, plugins with the same path as *PLUGIN* are
-disabled. Otherwise, any plugin with that base name is disabled,
-whatever directory it is in.
+If *PLUGIN* contains a /, plugins with the same path as *PLUGIN* will
+not be loaded at startup. Otherwise, no plugin with that base name will
+be loaded at startup, whatever directory it is in.  This option is useful for
+disabling a single plugin inside a directory.  You can still explicitly
+load plugins which have been disabled, using lightning-plugin(7) `start`.
 
 BUGS
 ----

lightningd/bitcoind.c

@@ -47,7 +47,7 @@ static void plugin_config_cb(const char *buffer,
 			     const jsmntok_t *idtok,
 			     struct plugin *plugin)
 {
-	plugin->plugin_state = CONFIGURED;
+	plugin->plugin_state = INIT_COMPLETE;
 	io_break(plugin);
 }
 
@@ -77,8 +77,9 @@ static void wait_plugin(struct bitcoind *bitcoind, const char *method,
 	 * before responding to `init`).
 	 * Note that lightningd/plugin will not send `init` to an already
 	 * configured plugin. */
-	if (p->plugin_state != CONFIGURED)
+	if (p->plugin_state == NEEDS_INIT)
 		config_plugin(p);
+
 	strmap_add(&bitcoind->pluginsmap, method, p);
 }
 

lightningd/jsonrpc.c

@@ -774,11 +774,6 @@ REGISTER_SINGLE_PLUGIN_HOOK(rpc_command,
 			    rpc_command_hook_serialize,
 			    struct rpc_command_hook_payload *);
 
-static void call_rpc_command_hook(struct rpc_command_hook_payload *p)
-{
-	plugin_hook_call_rpc_command(p->cmd->ld, p);
-}
-
 /* We return struct command_result so command_fail return value has a natural
  * sink; we don't actually use the result. */
 static struct command_result *
@@ -787,6 +782,7 @@ parse_request(struct json_connection *jcon, const jsmntok_t tok[])
 	const jsmntok_t *method, *id, *params;
 	struct command *c;
 	struct rpc_command_hook_payload *rpc_hook;
+	bool completed;
 
 	if (tok[0].type != JSMN_OBJECT) {
 		json_command_malformed(jcon, "null",
@@ -850,11 +846,15 @@ parse_request(struct json_connection *jcon, const jsmntok_t tok[])
 	/* Duplicate since we might outlive the connection */
 	rpc_hook->buffer = tal_dup_talarr(rpc_hook, char, jcon->buffer);
 	rpc_hook->request = tal_dup_talarr(rpc_hook, jsmntok_t, tok);
-	/* Prevent a race between was_pending and still_pending */
-	new_reltimer(c->ld->timers, rpc_hook, time_from_msec(1),
-	             call_rpc_command_hook, rpc_hook);
 
-	return command_still_pending(c);
+	db_begin_transaction(jcon->ld->wallet->db);
+	completed = plugin_hook_call_rpc_command(jcon->ld, rpc_hook);
+	db_commit_transaction(jcon->ld->wallet->db);
+
+	/* If it's deferred, mark it (otherwise, it's completed) */
+	if (!completed)
+		return command_still_pending(c);
+	return NULL;
 }
 
 /* Mutual recursion */

lightningd/lightningd.c

@@ -123,8 +123,8 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	 * is a nod to keeping it minimal and explicit: we need this code for
 	 * testing, but its existence means we're not actually testing the
 	 * same exact code users will be running. */
-	ld->dev_debug_subprocess = NULL;
 #if DEVELOPER
+	ld->dev_debug_subprocess = NULL;
 	ld->dev_disconnect_fd = -1;
 	ld->dev_subdaemon_fail = false;
 	ld->dev_allow_localhost = false;
@@ -808,7 +808,7 @@ int main(int argc, char *argv[])
 	/*~ Initialize all the plugins we just registered, so they can
 	 *  do their thing and tell us about themselves (including
 	 *  options registration). */
-	plugins_init(ld->plugins, ld->dev_debug_subprocess);
+	plugins_init(ld->plugins);
 
 	/*~ Handle options and config. */
 	handle_opts(ld, argc, argv);

lightningd/lightningd.h

@@ -200,15 +200,15 @@ struct lightningd {
 	 * if we are the fundee. */
 	u32 max_funding_unconfirmed;
 
-	/* If we want to debug a subdaemon/plugin. */
-	const char *dev_debug_subprocess;
-
 	/* RPC which asked us to shutdown, if non-NULL */
 	struct io_conn *stop_conn;
 	/* RPC response to send once we've shut down. */
 	const char *stop_response;
 
 #if DEVELOPER
+	/* If we want to debug a subdaemon/plugin. */
+	const char *dev_debug_subprocess;
+
 	/* If we have a --dev-disconnect file */
 	int dev_disconnect_fd;
 

lightningd/options.c

@@ -343,15 +343,18 @@ static char *opt_add_proxy_addr(const char *arg, struct lightningd *ld)
 
 static char *opt_add_plugin(const char *arg, struct lightningd *ld)
 {
-	plugin_register(ld->plugins, arg);
+	if (plugin_blacklisted(ld->plugins, arg)) {
+		log_info(ld->log, "%s: disabled via disable-plugin", arg);
+		return NULL;
+	}
+	plugin_register(ld->plugins, arg, NULL);
 	return NULL;
 }
 
 static char *opt_disable_plugin(const char *arg, struct lightningd *ld)
 {
-	if (plugin_remove(ld->plugins, arg))
-		return NULL;
-	return tal_fmt(NULL, "Could not find plugin %s", arg);
+	plugin_blacklist(ld->plugins, arg);
+	return NULL;
 }
 
 static char *opt_add_plugin_dir(const char *arg, struct lightningd *ld)
@@ -1275,8 +1278,9 @@ static void add_config(struct lightningd *ld,
 			json_add_opt_plugins(response, ld->plugins);
 		} else if (opt->cb_arg == (void *)opt_log_level) {
 			json_add_opt_log_levels(response, ld->log);
+		} else if (opt->cb_arg == (void *)opt_disable_plugin) {
+			json_add_opt_disable_plugins(response, ld->plugins);
 		} else if (opt->cb_arg == (void *)opt_add_plugin_dir
-			   || opt->cb_arg == (void *)opt_disable_plugin
 			   || opt->cb_arg == (void *)plugin_opt_set
 			   || opt->cb_arg == (void *)plugin_opt_flag_set) {
 			/* FIXME: We actually treat it as if they specified