WIP: lightningd/plugin: add field early_conf, allow plugin early (sync) config or abort

contrib/pylightning/lightning/plugin.py

@@ -99,7 +99,7 @@ class Plugin(object):
 
     """
 
-    def __init__(self, stdout=None, stdin=None, autopatch=True, dynamic=True):
+    def __init__(self, stdout=None, stdin=None, autopatch=True, dynamic=True, early_conf=False):
         self.methods = {'init': Method('init', self._init, MethodType.RPCMETHOD)}
         self.options = {}
 
@@ -121,8 +121,9 @@ def __init__(self, stdout=None, stdin=None, autopatch=True, dynamic=True):
         self.rpc_filename = None
         self.lightning_dir = None
         self.rpc = None
-        self.startup = True
+        self.startup = None
         self.dynamic = dynamic
+        self.early_conf = early_conf
         self.child_init = None
 
         self.write_lock = RLock()
@@ -527,7 +528,8 @@ def _getmanifest(self, **kwargs):
             'rpcmethods': methods,
             'subscriptions': list(self.subscriptions.keys()),
             'hooks': hooks,
-            'dynamic': self.dynamic
+            'dynamic': self.dynamic,
+            'early_conf': self.early_conf
         }
 
     def _init(self, options, configuration, request):

lightningd/lightningd.c

@@ -686,6 +686,9 @@ int main(int argc, char *argv[])
 	/*~ Make sure we can reach the subdaemons, and versions match. */
 	test_subdaemons(ld);
 
+	/* Some *restricted* plugins may init (or abort) early before db touched */
+	plugins_early_config(ld->plugins);
+
 	/*~ Our "wallet" code really wraps the db, which is more than a simple
 	 * bitcoin wallet (though it's that too).  It also stores channel
 	 * states, invoices, payments, blocks and bitcoin transactions. */

lightningd/plugin.c

@@ -32,6 +32,8 @@
  * willing to wait. Plugins shouldn't do any initialization in the
  * `getmanifest` call anyway, that's what `init `is for. */
 #define PLUGIN_MANIFEST_TIMEOUT 60
+/* Timeout for the init response of early_config plugins */
+#define PLUGIN_CONFIG_TIMEOUT 60
 
 struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book,
 			    struct lightningd *ld)
@@ -796,6 +798,12 @@ static bool plugin_hooks_add(struct plugin *plugin, const char *buffer,
 	return true;
 }
 
+static void plugin_early_conf_timeout(struct plugin *plugin)
+{
+	log_broken(plugin->log, "The early_conf plugin failed to respond to \"init\" in time, terminating.");
+	fatal("Can't recover from plugin failure, terminating.");
+}
+
 static void plugin_manifest_timeout(struct plugin *plugin)
 {
 	log_broken(plugin->log, "The plugin failed to respond to \"getmanifest\" in time, terminating.");
@@ -810,8 +818,8 @@ static void plugin_manifest_cb(const char *buffer,
 			       const jsmntok_t *idtok,
 			       struct plugin *plugin)
 {
-	const jsmntok_t *resulttok, *dynamictok;
-	bool dynamic_plugin;
+	const jsmntok_t *resulttok, *dynamictok, *earlyconftok;
+	bool dynamic_plugin, earlyconf_plugin;
 
 	/* Check if all plugins have replied to getmanifest, and break
 	 * if they have and this is the startup init */
@@ -831,14 +839,28 @@ static void plugin_manifest_cb(const char *buffer,
 	dynamictok = json_get_member(buffer, resulttok, "dynamic");
 	if (dynamictok && json_to_bool(buffer, dynamictok, &dynamic_plugin))
 		plugin->dynamic = dynamic_plugin;
+	else
+		plugin->dynamic = true;
 
-	if (!plugin_opts_add(plugin, buffer, resulttok) ||
-	    !plugin_rpcmethods_add(plugin, buffer, resulttok) ||
-	    !plugin_subscriptions_add(plugin, buffer, resulttok) ||
-	    !plugin_hooks_add(plugin, buffer, resulttok))
-		plugin_kill(
-		    plugin,
-		    "Failed to register options, methods, hooks, or subscriptions.");
+	earlyconftok = json_get_member(buffer, resulttok, "early_conf");
+	if (earlyconftok && json_to_bool(buffer, earlyconftok, &earlyconf_plugin))
+		plugin->early_config = earlyconf_plugin;
+	else
+		plugin->early_config = false;
+
+	/* A previously unloaded plugin can indicate us it doesn't want to be
+	 * started-via-RPC, then kill it and don't registers hooks etc.*/
+	if (!plugin->plugins->startup
+			&& (!plugin->dynamic || plugin->early_config)) {
+		plugin_kill(plugin, "because plugin config%s%s",
+				     !plugin->dynamic ? " dynamic=True" : "",
+				     plugin->early_config ? " early_conf=True" : "");
+	} else if (!plugin_opts_add(plugin, buffer, resulttok) ||
+			   !plugin_rpcmethods_add(plugin, buffer, resulttok) ||
+			   !plugin_subscriptions_add(plugin, buffer, resulttok) ||
+			   !plugin_hooks_add(plugin, buffer, resulttok))
+		plugin_kill( plugin,
+			"Failed to register options, methods, hooks, or subscriptions.");
 
 	/* If all plugins have replied to getmanifest and this is not
 	 * the startup init, configure them */
@@ -1007,6 +1029,7 @@ void plugins_start(struct plugins *plugins, const char *dev_plugin_debug)
 void plugins_init(struct plugins *plugins, const char *dev_plugin_debug)
 {
 	plugins->pending_manifests = 0;
+	plugins->pending_early_configs = 0;
 	uintmap_init(&plugins->pending_requests);
 
 	plugins_add_default_dir(plugins,
@@ -1026,6 +1049,44 @@ static void plugin_config_cb(const char *buffer,
 			     const jsmntok_t *idtok,
 			     struct plugin *plugin)
 {
+	const jsmntok_t *errortok;
+	/* An init-error during startup from an `early_conf` plugin will crash
+	 * lightningd, outside startup it will kill the plugin (any type). */
+	errortok = json_get_member(buffer, toks, "error");
+	/* FIXME: shouldn't this be a JSMN_OBJECT ? */
+	if (errortok && errortok->type != JSMN_STRING) {
+		/* FIXME: should `static` plugins be treated same way?*/
+		if (plugin->plugins->startup && plugin->early_config)
+			fatal("init error at early config of plugin '%s':"
+				  " \"error\" result is not an object: %.*s", plugin->cmd,
+						    toks[0].end - toks[0].start,
+						    buffer + toks[0].start);
+
+		/* Plugins that don't like to be started at run-time, can return an
+		 * init error ... and be killed cleanly */
+		plugin_kill(plugin,
+			    "\"error\" result is not an object: %.*s",
+			    toks[0].end - toks[0].start,
+			    buffer + toks[0].start);
+	}
+
+	/* TODO: add error code + message
+	 * TODO: check json `id` */
+	if (plugin->plugins->startup && plugin->early_config) {
+		if (errortok)
+			fatal("init error at early config of plugin '%s'", plugin->cmd);
+		plugin->plugins->pending_early_configs--;
+		tal_free(plugin->timeout_timer);
+
+		/* This will break the io_loop in plugins_early_config */
+		if (plugin->plugins->pending_early_configs == 0)
+			io_break(plugin->plugins);
+	}
+
+	if (errortok) {
+		plugin_kill(plugin, "error initializing plugin");
+		return;
+	}
 	plugin->plugin_state = CONFIGURED;
 }
 
@@ -1061,6 +1122,31 @@ static void plugin_config(struct plugin *plugin)
 
 	jsonrpc_request_end(req);
 	plugin_request_send(plugin, req);
+
+	/* Wait for early_config plugin's `init` response during startup, so they
+	 * can do their thing or abort at an early stage */
+	if (plugin->plugins->startup && plugin->early_config) {
+		plugin->plugins->pending_early_configs++;
+		/* FIXME assert plugin->timeout_timer == NULL here ? */
+		plugin->timeout_timer = new_reltimer(plugin->plugins->ld->timers,
+				plugin, time_from_sec(PLUGIN_CONFIG_TIMEOUT),
+				plugin_early_conf_timeout, plugin);
+	}
+}
+
+void plugins_early_config(struct plugins *plugins)
+{
+	struct plugin *p;
+	list_for_each(&plugins->plugins, p, list) {
+		if (p->early_config && p->plugin_state == UNCONFIGURED) {
+			p->plugin_state = CONFIGURING;
+			plugin_config(p);
+		}
+	}
+	/* At start-up, wait for early_config plugins to initialize */
+	if (plugins->pending_early_configs > 0)
+		io_loop_with_timers(plugins->ld);
+
 }
 
 void plugins_config(struct plugins *plugins)

lightningd/plugin.h

@@ -32,6 +32,8 @@ struct plugin {
 	/* If this plugin can be restarted without restarting lightningd */
 	bool dynamic;
 	bool signal_startup;
+	/* Plugin initializes early, blocking lightningd, but is very restricted */
+	bool early_config;
 
 	/* Stuff we read */
 	char *buffer;
@@ -65,6 +67,7 @@ struct plugin {
 struct plugins {
 	struct list_head plugins;
 	size_t pending_manifests;
+	size_t pending_early_configs;
 	bool startup;
 
 	/* Currently pending requests by their request ID */
@@ -159,6 +162,14 @@ void PRINTF_FMT(2,3) plugin_kill(struct plugin *plugin, char *fmt, ...);
  * incoming JSON-RPC calls and messages.
  */
 void plugins_config(struct plugins *plugins);
+
+/**
+ * Sync'ed version of above, only for some very *limited* plugins that have set
+ * `early_config` in their getmanifest response to indicate they want to be
+ * initialized early.
+ */
+void plugins_early_config(struct plugins *plugins);
+
 /**
  * Add the plugin option and their respective options to listconfigs.
  *

lightningd/test/run-find_my_abspath.c

@@ -140,6 +140,9 @@ void per_peer_state_set_fds_arr(struct per_peer_state *pps UNNEEDED, const int *
 /* Generated stub for plugins_config */
 void plugins_config(struct plugins *plugins UNNEEDED)
 { fprintf(stderr, "plugins_config called!\n"); abort(); }
+/* Generated stub for plugins_early_config */
+void plugins_early_config(struct plugins *plugins UNNEEDED)
+{ fprintf(stderr, "plugins_early_config called!\n"); abort(); }
 /* Generated stub for plugins_init */
 void plugins_init(struct plugins *plugins UNNEEDED, const char *dev_plugin_debug UNNEEDED)
 { fprintf(stderr, "plugins_init called!\n"); abort(); }

tests/test_plugin.py

@@ -146,12 +146,16 @@ def test_plugin_command(node_factory):
     cmd = [hlp for hlp in n.rpc.help()["help"] if "hello" in hlp["command"]]
     assert(len(cmd) == 0)
 
+    # Test that a static plugin is killed when started via RPC
+    n.rpc.plugin_start(plugin=os.path.join(os.getcwd(), "tests/plugins/static.py"))
+    n.daemon.wait_for_log(r"plugin-static.py Killing plugin: because plugin config dynamic=True")
+    assert not n.daemon.is_in_log(r"Static plugin initialized.", start=0)
+
     # Test that we cannot stop a plugin with 'dynamic' set to False in
     # getmanifest
-    n.rpc.plugin_start(plugin=os.path.join(os.getcwd(), "tests/plugins/static.py"))
-    n.daemon.wait_for_log(r"Static plugin initialized.")
+    n2 = node_factory.get_node(options={"plugin": os.path.join(os.getcwd(), "tests/plugins/static.py")})
     with pytest.raises(RpcError, match=r"plugin cannot be managed when lightningd is up"):
-        n.rpc.plugin_stop(plugin="static.py")
+        n2.rpc.plugin_stop(plugin="static.py")
 
 
 def test_plugin_disable(node_factory):