From ceeb5503cc5759c63a75b480413e1f742f8ba1d5 Mon Sep 17 00:00:00 2001 From: darosior Date: Fri, 31 Jan 2020 19:13:59 +0100 Subject: [PATCH] libplugin: fix 'dynamic' field in getmanifest As a separated commit because it was pre-existent (changelog + xfail test). This also fix a logical problem in lightningd/plugin_control: we were assuming a plugin started with 'plugin start' but which did not comport a 'dynamic' entry in its manifest to be dynamic, though it should have been treated as static. Changelog-fixed: plugins: Dynamic C plugins can now be managed when lightningd is up --- lightningd/plugin.c | 9 +++++---- lightningd/plugin_control.c | 2 +- plugins/libplugin.c | 3 +-- tests/test_plugin.py | 1 - 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index c82f8d710..6d0a36fc6 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -73,6 +73,7 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES) p->js_arr = tal_arr(p, struct json_stream *, 0); p->used = 0; p->subscriptions = NULL; + p->dynamic = false; p->log = new_log(p, plugins->log_book, NULL, "plugin-%s", path_basename(tmpctx, p->cmd)); @@ -810,22 +811,22 @@ static void plugin_manifest_timeout(struct plugin *plugin) fatal("Can't recover from plugin failure, terminating."); } - bool plugin_parse_getmanifest_response(const char *buffer, const jsmntok_t *toks, const jsmntok_t *idtok, struct plugin *plugin) { const jsmntok_t *resulttok, *dynamictok; - bool dynamic_plugin; resulttok = json_get_member(buffer, toks, "result"); if (!resulttok || resulttok->type != JSMN_OBJECT) return false; dynamictok = json_get_member(buffer, resulttok, "dynamic"); - if (dynamictok && json_to_bool(buffer, dynamictok, &dynamic_plugin)) - plugin->dynamic = dynamic_plugin; + if (dynamictok && !json_to_bool(buffer, dynamictok, &plugin->dynamic)) + plugin_kill(plugin, "Bad 'dynamic' field ('%.*s')", + json_tok_full_len(dynamictok), + json_tok_full(buffer, dynamictok)); if (!plugin_opts_add(plugin, buffer, resulttok) || !plugin_rpcmethods_add(plugin, buffer, resulttok) || diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 3e6b0833a..18d4f2611 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -115,7 +115,7 @@ static struct command_result *plugin_start(struct dynamic_plugin *dp) struct jsonrpc_request *req; struct plugin *p = dp->plugin; - p->dynamic = true; + p->dynamic = false; p_cmd = tal_arrz(NULL, char *, 2); p_cmd[0] = p->cmd; /* In case the plugin create files, this is a better default. */ diff --git a/plugins/libplugin.c b/plugins/libplugin.c index bdb878171..99c38707c 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -591,8 +591,7 @@ handle_getmanifest(struct command *getmanifest_cmd) json_add_string(params, NULL, p->hook_subs[i].name); json_array_end(params); - json_add_string(params, "dynamic", - p->restartability == PLUGIN_RESTARTABLE ? "true" : "false"); + json_add_bool(params, "dynamic", p->restartability == PLUGIN_RESTARTABLE); return command_finished(getmanifest_cmd, params); } diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 55dd82624..e69ba0890 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -791,7 +791,6 @@ def test_rpc_command_hook(node_factory): l1.rpc.plugin_stop('rpc_command.py') -@pytest.mark.xfail(strict=True) def test_libplugin(node_factory): """Sanity checks for plugins made with libplugin""" plugin = os.path.join(os.getcwd(), "tests/plugins/test_libplugin")