diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index 84ae5ebf8..635335b3c 100644 --- a/doc/PLUGINS.md +++ b/doc/PLUGINS.md @@ -128,6 +128,10 @@ The `dynamic` indicates if the plugin can be managed after `lightningd` has been started. Critical plugins that should not be stopped should set it to false. +If a `disable` member exists, the plugin will be disabled and the contents +of this member is the reason why. This allows plugins to disable themselves +if they are not supported in this configuration. + The `featurebits` object allows the plugin to register featurebits that should be announced in a number of places in [the protocol][bolt9]. They can be used to signal support for custom protocol extensions to direct peers, remote nodes and in @@ -237,10 +241,11 @@ simple JSON object containing the options: } ``` -The plugin must respond to `init` calls, however the response can be -arbitrary and will currently be discarded by `lightningd`. JSON-RPC -commands were chosen over notifications in order not to force plugins -to implement notifications which are not that well supported. +The plugin must respond to `init` calls. The response should be a +valid JSON-RPC response to the `init`, but this is not currently +enforced. If the response is an object containing `result` which +contains `disable` then the plugin will be disabled and the contents +of this member is the reason why. The `startup` field allows a plugin to detect if it was started at `lightningd` startup (true), or at runtime (false). diff --git a/lightningd/plugin.c b/lightningd/plugin.c index b6d5ea869..461086761 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -1229,6 +1229,17 @@ static const char *plugin_parse_getmanifest_response(const char *buffer, json_tok_full_len(toks), json_tok_full(buffer, toks)); + /* Plugin can disable itself: returns why it's disabled. */ + tok = json_get_member(buffer, resulttok, "disable"); + if (tok) { + log_debug(plugin->log, "disabled itself: %.*s", + json_tok_full_len(tok), + json_tok_full(buffer, tok)); + /* Don't get upset if this was a built-in! */ + plugin->important = false; + return json_strdup(plugin, buffer, tok); + } + dynamictok = json_get_member(buffer, resulttok, "dynamic"); if (dynamictok && !json_to_bool(buffer, dynamictok, &plugin->dynamic)) { return tal_fmt(plugin, "Bad 'dynamic' field ('%.*s')", @@ -1572,6 +1583,19 @@ static void plugin_config_cb(const char *buffer, const jsmntok_t *idtok, struct plugin *plugin) { + const char *disable; + + /* Plugin can also disable itself at this stage. */ + if (json_scan(tmpctx, buffer, toks, "{result:{disable:%}}", + JSON_SCAN_TAL(tmpctx, json_strdup, &disable)) == NULL) { + log_debug(plugin->log, "disabled itself at init: %s", + disable); + /* Don't get upset if this was a built-in! */ + plugin->important = false; + plugin_kill(plugin, disable); + return; + } + plugin->plugin_state = INIT_COMPLETE; plugin->timeout_timer = tal_free(plugin->timeout_timer); if (plugin->start_cmd) { diff --git a/tests/plugins/Makefile b/tests/plugins/Makefile index daf35de32..94ba95762 100644 --- a/tests/plugins/Makefile +++ b/tests/plugins/Makefile @@ -1,10 +1,15 @@ -PLUGIN_TESTLIBPLUGIN_SRC := tests/plugins/test_libplugin.c +PLUGIN_TESTLIBPLUGIN_SRC := tests/plugins/test_libplugin.c PLUGIN_TESTLIBPLUGIN_OBJS := $(PLUGIN_TESTLIBPLUGIN_SRC:.c=.o) tests/plugins/test_libplugin: bitcoin/chainparams.o $(PLUGIN_TESTLIBPLUGIN_OBJS) $(PLUGIN_LIB_OBJS) $(PLUGIN_COMMON_OBJS) $(JSMN_OBJS) $(CCAN_OBJS) $(PLUGIN_TESTLIBPLUGIN_OBJS): $(PLUGIN_LIB_HEADER) +PLUGIN_TESTSELFDISABLE_AFTER_GETMANIFEST_SRC := tests/plugins/test_selfdisable_after_getmanifest.c +PLUGIN_TESTSELFDISABLE_AFTER_GETMANIFEST_OBJS := $(PLUGIN_TESTSELFDISABLE_AFTER_GETMANIFEST_SRC:.c=.o) + +tests/plugins/test_selfdisable_after_getmanifest: bitcoin/chainparams.o $(PLUGIN_TESTSELFDISABLE_AFTER_GETMANIFEST_OBJS) common/json.o common/json_stream.o common/setup.o common/utils.o $(JSMN_OBJS) $(CCAN_OBJS) + # Make sure these depend on everything. -ALL_TEST_PROGRAMS += tests/plugins/test_libplugin -ALL_C_SOURCES += $(PLUGIN_TESTLIBPLUGIN_SRC) +ALL_TEST_PROGRAMS += tests/plugins/test_libplugin tests/plugins/test_selfdisable_after_getmanifest +ALL_C_SOURCES += $(PLUGIN_TESTLIBPLUGIN_SRC) $(PLUGIN_TESTSELFDISABLE_AFTER_GETMANIFEST_SRC) diff --git a/tests/plugins/test_selfdisable_after_getmanifest.c b/tests/plugins/test_selfdisable_after_getmanifest.c new file mode 100644 index 000000000..e740bf0fc --- /dev/null +++ b/tests/plugins/test_selfdisable_after_getmanifest.c @@ -0,0 +1,42 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +/* Our normal frameworks don't (yet?) support custom post-manifest responses, + * so this is open-coded */ +int main(int argc, char *argv[]) +{ + char *buf; + int r, off; + const jsmntok_t *toks, *id; + + common_setup(argv[0]); + + buf = tal_arr(tmpctx, char, 100); + off = 0; + do { + r = read(STDIN_FILENO, buf + off, tal_bytelen(buf) - off); + if (r < 0) + err(1, "reading stdin"); + off += r; + if (off == tal_bytelen(buf)) + tal_resize(&buf, off * 2); + + toks = json_parse_simple(tmpctx, buf, off); + } while (!toks); + + /* Tell it we're disabled (reusing id). */ + id = json_get_member(buf, toks, "id"); + buf = tal_fmt(tmpctx, "{\"jsonrpc\":\"2.0\",\"id\":%.*s,\"result\":{\"disable\":\"Self-disable test after getmanifest\"} }", + json_tok_full_len(id), + json_tok_full(buf, id)); + write_all(STDOUT_FILENO, buf, strlen(buf)); + + common_shutdown(); + return 0; +} diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 4b7311638..519bc7f59 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2321,3 +2321,24 @@ def test_pyln_request_notify(node_factory): notifications = [] l1.rpc.countdown(10) assert notifications == [] + + +def test_self_disable(node_factory): + """Test that plugin can disable itself without penalty. + """ + plugin_path = os.path.join( + os.path.dirname(__file__), 'plugins/test_selfdisable' + ) + l1 = node_factory.get_node(options={'important-plugin': plugin_path}) + + # Could happen before it gets set up. + l1.daemon.logsearch_start = 0 + l1.daemon.wait_for_log('test_selfdisable: disabled itself: "Self-disable test after getmanifest"') + + assert plugin_path not in [p['name'] for p in l1.rpc.plugin_list()['plugins']] + + # Also works with dynamic load attempts + with pytest.raises(RpcError, match="Self-disable test after getmanifest"): + l1.rpc.plugin_start(plugin_path) + + # Now test the disable-in-init-response.