From 711353fb242741f78e739d237ceffd1d3193948a Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 27 Apr 2021 15:15:09 +0200 Subject: [PATCH 01/17] plugin: Add a list of notification topics registered by plugin We will eventually start emitting and dispatching custom notifications from plugins just like we dispatch internal notifications. In order to get reasonable error messages we need to make sure that the topics plugins are asking for were correctly registered. When doing this we don't really care about whether the plugin that registered the notification is still alive or not (it might have died, but subscribers should stay up and running), so we keep a list of all topics attached to the `struct plugins` which gathers global plugin information. --- contrib/pyln-client/pyln/client/plugin.py | 9 ++++++ lightningd/notification.c | 9 +++++- lightningd/notification.h | 2 +- lightningd/plugin.c | 34 ++++++++++++++++++----- lightningd/plugin.h | 4 +++ tests/plugins/custom_notifications.py | 21 ++++++++++++++ tests/test_plugin.py | 10 +++++++ 7 files changed, 80 insertions(+), 9 deletions(-) create mode 100755 tests/plugins/custom_notifications.py diff --git a/contrib/pyln-client/pyln/client/plugin.py b/contrib/pyln-client/pyln/client/plugin.py index 5d3095e7e4bf..a730fc4496f1 100644 --- a/contrib/pyln-client/pyln/client/plugin.py +++ b/contrib/pyln-client/pyln/client/plugin.py @@ -223,6 +223,7 @@ def __init__(self, stdout: Optional[io.TextIOBase] = None, } self.options: Dict[str, Dict[str, Any]] = {} + self.notification_topics: List[str] = [] def convert_featurebits( bits: Optional[Union[int, str, bytes]]) -> Optional[str]: @@ -420,6 +421,11 @@ def add_flag_option(self, name: str, description: str, self.add_option(name, None, description, opt_type="flag", deprecated=deprecated) + def add_notification_topic(self, topic: str): + """Announce that the plugin will emit notifications for the topic. + """ + self.notification_topics.append(topic) + def get_option(self, name: str) -> str: if name not in self.options: raise ValueError("No option with name {} registered".format(name)) @@ -898,6 +904,9 @@ def _getmanifest(self, **kwargs) -> JSONType: 'subscriptions': list(self.subscriptions.keys()), 'hooks': hooks, 'dynamic': self.dynamic, + 'notifications': [ + {"method": name} for name in self.notification_topics + ], } # Compact the features a bit, not important. diff --git a/lightningd/notification.c b/lightningd/notification.c index 7cb2ebea94f7..bcd4491d0e7c 100644 --- a/lightningd/notification.c +++ b/lightningd/notification.c @@ -18,12 +18,19 @@ static struct notification *find_notification_by_topic(const char* topic) return NULL; } -bool notifications_have_topic(const char *topic) +bool notifications_have_topic(const struct plugins *plugins, const char *topic) { struct notification *noti = find_notification_by_topic(topic); if (noti) return true; + /* Some plugin at some point announced it'd be emitting + * notifications to this topic. We don't care if it died, just + * that it was a valid topic at some point in time. */ + for (size_t i=0; inotification_topics); i++) + if (streq(plugins->notification_topics[i], topic)) + return true; + return false; } diff --git a/lightningd/notification.h b/lightningd/notification.h index 8ad8e9de0460..6736db912dd5 100644 --- a/lightningd/notification.h +++ b/lightningd/notification.h @@ -25,7 +25,7 @@ struct onionreply; struct wally_psbt; -bool notifications_have_topic(const char *topic); +bool notifications_have_topic(const struct plugins *plugins, const char *topic); struct notification { const char *topic; diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 5c68d86cdb75..ac0700fc5e51 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -76,6 +76,7 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, p->startup = true; p->json_cmds = tal_arr(p, struct command *, 0); p->blacklist = tal_arr(p, const char *, 0); + p->notification_topics = tal_arr(p, const char *, 0); p->shutdown = false; p->plugin_idx = 0; #if DEVELOPER @@ -103,6 +104,25 @@ void plugins_free(struct plugins *plugins) tal_free(plugins); } +/* Check that all the plugin's subscriptions are actually for known + * notification topics. Emit a warning if that's not the case, but + * don't kill the plugin. */ +static void plugin_check_subscriptions(struct plugins *plugins, + struct plugin *plugin) +{ + if (plugin->subscriptions == NULL) + return; + + for (size_t i = 0; i < tal_count(plugin->subscriptions); i++) { + const char *topic = plugin->subscriptions[i]; + if (!notifications_have_topic(plugins, topic)) + log_unusual( + plugin->log, + "topic '%s' is not a known notification topic", + topic); + } +} + /* Once they've all replied with their manifests, we can order them. */ static void check_plugins_manifests(struct plugins *plugins) { @@ -1136,14 +1156,12 @@ static const char *plugin_subscriptions_add(struct plugin *plugin, json_tok_full_len(s), json_tok_full(buffer, s)); } - topic = json_strdup(plugin, plugin->buffer, s); - - if (!notifications_have_topic(topic)) { - return tal_fmt( - plugin, - "topic '%s' is not a known notification topic", topic); - } + /* We add all subscriptions while parsing the + * manifest, without checking that they exist, since + * later plugins may also emit notifications of custom + * types that we don't know about yet. */ + topic = json_strdup(plugin, plugin->buffer, s); tal_arr_expand(&plugin->subscriptions, topic); } return NULL; @@ -1338,6 +1356,8 @@ static const char *plugin_parse_getmanifest_response(const char *buffer, if (!err) err = plugin_add_params(plugin); + plugin_check_subscriptions(plugin->plugins, plugin); + plugin->plugin_state = NEEDS_INIT; return err; } diff --git a/lightningd/plugin.h b/lightningd/plugin.h index f7954e5682e3..27e2ee777507 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -128,6 +128,10 @@ struct plugins { /* Whether builtin plugins should be overridden as unimportant. */ bool dev_builtin_plugins_unimportant; #endif /* DEVELOPER */ + + /* Notification topics that plugins have registered with us + * and that other plugins may subscribe to. */ + const char **notification_topics; }; /* The value of a plugin option, which can have different types. diff --git a/tests/plugins/custom_notifications.py b/tests/plugins/custom_notifications.py new file mode 100755 index 000000000000..9b32aa45efc9 --- /dev/null +++ b/tests/plugins/custom_notifications.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python3 +from pyln.client import Plugin + + +plugin = Plugin() + + +@plugin.subscribe("custom") +def on_custom_notification(val, plugin, **kwargs): + plugin.log("Got a custom notification {}".format(val)) + + +@plugin.method("emit") +def emit(plugin): + """Emit a simple string notification to topic "custom" + """ + plugin.notify("custom", "Hello world") + + +plugin.add_notification_topic("custom") +plugin.run() diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 9699de37c8db..ff0fdf8e5b0d 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2402,3 +2402,13 @@ def test_self_disable(node_factory): # Also works with dynamic load attempts with pytest.raises(RpcError, match="Disabled via selfdisable option"): l1.rpc.plugin_start(p2, selfdisable=True) + + +@pytest.mark.xfail(strict=True) +def test_custom_notification_topics(node_factory): + plugin = os.path.join( + os.path.dirname(__file__), "plugins", "custom_notifications.py" + ) + l1 = node_factory.get_node(options={'plugin': plugin}) + l1.rpc.emit() + l1.daemon.wait_for_log(r'Got a custom notification Hello world') From 79e4ff931bc035aec08f04de9f77acbe9e1c057e Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 27 Apr 2021 17:47:55 +0200 Subject: [PATCH 02/17] plugin: Move the notification subscription check into a second phase A plugin might subscribe to a notification topic that is only registered by another plugin later, so push the check to that consistency check phase where we do hook ordering as well. --- lightningd/plugin.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index ac0700fc5e51..d7d7327961bb 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -126,6 +126,7 @@ static void plugin_check_subscriptions(struct plugins *plugins, /* Once they've all replied with their manifests, we can order them. */ static void check_plugins_manifests(struct plugins *plugins) { + struct plugin *plugin; struct plugin **depfail; if (plugins_any_in_state(plugins, AWAITING_GETMANIFEST_RESPONSE)) @@ -141,6 +142,12 @@ static void check_plugins_manifests(struct plugins *plugins) "Cannot meet required hook dependencies"); } + /* Check that all the subscriptions are matched with real + * topics. */ + list_for_each(&plugins->plugins, plugin, list) { + plugin_check_subscriptions(plugin->plugins, plugin); + } + /* As startup, we break out once all getmanifest are returned */ if (plugins->startup) io_break(plugins); @@ -1356,8 +1363,6 @@ static const char *plugin_parse_getmanifest_response(const char *buffer, if (!err) err = plugin_add_params(plugin); - plugin_check_subscriptions(plugin->plugins, plugin); - plugin->plugin_state = NEEDS_INIT; return err; } From 96eb5589a575d8ea3a7d9f3ecebbba74ce4e8bd7 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 27 Apr 2021 18:13:01 +0200 Subject: [PATCH 03/17] plugin: Store the notification topics announced by the plugins --- lightningd/plugin.c | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index d7d7327961bb..e5c2868b6178 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -1273,6 +1273,44 @@ static void plugin_manifest_timeout(struct plugin *plugin) fatal("Can't recover from plugin failure, terminating."); } +static const char *plugin_notifications_add(const char *buffer, + const jsmntok_t *result, + struct plugin *plugin) +{ + char *name; + const jsmntok_t *method, *obj; + const jsmntok_t *notifications = + json_get_member(buffer, result, "notifications"); + + if (!notifications) + return NULL; + + if (notifications->type != JSMN_ARRAY) + return tal_fmt(plugin, + "\"result.notifications\" is not an array"); + + for (size_t i = 0; i < notifications->size; i++) { + obj = json_get_arr(notifications, i); + if (obj->type != JSMN_OBJECT) + return tal_fmt( + plugin, + "\"result.notifications[%zu]\" is not an object", + i); + + method = json_get_member(buffer, obj, "method"); + if (method == NULL || method->type != JSMN_STRING) + return tal_fmt(plugin, + "\"result.notifications[%zu].name\" " + "missing or not a string.", + i); + + name = json_strdup(plugin->plugins, buffer, method); + tal_arr_expand(&plugin->plugins->notification_topics, name); + } + + return NULL; +} + static const char *plugin_parse_getmanifest_response(const char *buffer, const jsmntok_t *toks, const jsmntok_t *idtok, @@ -1353,7 +1391,9 @@ static const char *plugin_parse_getmanifest_response(const char *buffer, } } - err = plugin_opts_add(plugin, buffer, resulttok); + err = plugin_notifications_add(buffer, resulttok, plugin); + if (!err) + err = plugin_opts_add(plugin, buffer, resulttok); if (!err) err = plugin_rpcmethods_add(plugin, buffer, resulttok); if (!err) From 09253838450c4a094876638566558692d79a95c8 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 27 Apr 2021 21:07:38 +0200 Subject: [PATCH 04/17] plugin: Implement custom notification dispatch for plugins Changelog-Added: plugin: Plugins may now send custom notifications that other plugins can subscribe to. --- lightningd/plugin.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index e5c2868b6178..c139a38dd40f 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -426,7 +426,8 @@ static const char *plugin_notification_handle(struct plugin *plugin, const jsmntok_t *toks) { const jsmntok_t *methtok, *paramstok; - + const char *methname; + struct jsonrpc_notification *n; methtok = json_get_member(plugin->buffer, toks, "method"); paramstok = json_get_member(plugin->buffer, toks, "params"); @@ -447,6 +448,27 @@ static const char *plugin_notification_handle(struct plugin *plugin, } else if (json_tok_streq(plugin->buffer, methtok, "message") || json_tok_streq(plugin->buffer, methtok, "progress")) { return plugin_notify_handle(plugin, methtok, paramstok); + } + + methname = json_strdup(tmpctx, plugin->buffer, methtok); + + if (notifications_have_topic(plugin->plugins, methname)) { + n = tal(NULL, struct jsonrpc_notification); + n->method = tal_steal(n, methname); + n->stream = new_json_stream(n, NULL, NULL); + json_object_start(n->stream, NULL); + json_add_string(n->stream, "jsonrpc", "2.0"); + json_add_string(n->stream, "method", methname); + + json_add_tok(n->stream, "params", paramstok, plugin->buffer); + + json_object_end(n->stream); /* closes '.' */ + + /* We guarantee to have \n\n at end of each response. */ + json_stream_append(n->stream, "\n\n", strlen("\n\n")); + + plugins_notify(plugin->plugins, take(n)); + return NULL; } else { return tal_fmt(plugin, "Unknown notification method %.*s", json_tok_full_len(methtok), From bdbfbb102c65b18980f2bc4e4eae135a98725124 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 15:58:46 +0200 Subject: [PATCH 05/17] plugin: Prevent plugins from registering native notification topics They may already have subscribers, and they may crash if presented with a malformed notification. --- lightningd/notification.c | 9 +++++++-- lightningd/notification.h | 4 ++++ lightningd/plugin.c | 8 ++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lightningd/notification.c b/lightningd/notification.c index bcd4491d0e7c..63d2bec60900 100644 --- a/lightningd/notification.c +++ b/lightningd/notification.c @@ -18,10 +18,15 @@ static struct notification *find_notification_by_topic(const char* topic) return NULL; } -bool notifications_have_topic(const struct plugins *plugins, const char *topic) +bool notifications_topic_is_native(const char *topic) { struct notification *noti = find_notification_by_topic(topic); - if (noti) + return noti != NULL; +} + +bool notifications_have_topic(const struct plugins *plugins, const char *topic) +{ + if (notifications_topic_is_native(topic)) return true; /* Some plugin at some point announced it'd be emitting diff --git a/lightningd/notification.h b/lightningd/notification.h index 6736db912dd5..452926a37dff 100644 --- a/lightningd/notification.h +++ b/lightningd/notification.h @@ -27,6 +27,10 @@ struct wally_psbt; bool notifications_have_topic(const struct plugins *plugins, const char *topic); +/* Is the provided notification topic native, i.e., provided by + * lightningd itself? */ +bool notifications_topic_is_native(const char *topic); + struct notification { const char *topic; /* the serialization interface */ diff --git a/lightningd/plugin.c b/lightningd/plugin.c index c139a38dd40f..907e0cf84b2b 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -1327,6 +1327,14 @@ static const char *plugin_notifications_add(const char *buffer, i); name = json_strdup(plugin->plugins, buffer, method); + + if (notifications_topic_is_native(name)) + return tal_fmt(plugin, + "plugin attempted to register a native " + "notification topic \"%s\", these may " + "however only be sent by lightningd", + name); + tal_arr_expand(&plugin->plugins->notification_topics, name); } From 03df5fbce6e74c45c403a505c095159f8942bfc0 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 16:18:40 +0200 Subject: [PATCH 06/17] plugin: Move list of notification topics to each plugin We want to ensure that plugins register their topics before sending any notification, so we need to remember which plugin registered which topics. --- lightningd/notification.c | 12 +++++++----- lightningd/plugin.c | 6 +++--- lightningd/plugin.h | 8 ++++---- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/lightningd/notification.c b/lightningd/notification.c index 63d2bec60900..df9b6deaebb7 100644 --- a/lightningd/notification.c +++ b/lightningd/notification.c @@ -26,15 +26,17 @@ bool notifications_topic_is_native(const char *topic) bool notifications_have_topic(const struct plugins *plugins, const char *topic) { + struct plugin *plugin; if (notifications_topic_is_native(topic)) return true; /* Some plugin at some point announced it'd be emitting - * notifications to this topic. We don't care if it died, just - * that it was a valid topic at some point in time. */ - for (size_t i=0; inotification_topics); i++) - if (streq(plugins->notification_topics[i], topic)) - return true; + * notifications to this topic. */ + list_for_each(&plugins->plugins, plugin, list) { + for (size_t i = 0; i < tal_count(plugin->notification_topics); i++) + if (streq(plugin->notification_topics[i], topic)) + return true; + } return false; } diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 907e0cf84b2b..57b9e5e9b6d9 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -76,7 +76,6 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, p->startup = true; p->json_cmds = tal_arr(p, struct command *, 0); p->blacklist = tal_arr(p, const char *, 0); - p->notification_topics = tal_arr(p, const char *, 0); p->shutdown = false; p->plugin_idx = 0; #if DEVELOPER @@ -246,6 +245,7 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES, p->plugin_state = UNCONFIGURED; p->js_arr = tal_arr(p, struct json_stream *, 0); p->used = 0; + p->notification_topics = tal_arr(p, const char *, 0); p->subscriptions = NULL; p->dynamic = false; p->index = plugins->plugin_idx++; @@ -1326,7 +1326,7 @@ static const char *plugin_notifications_add(const char *buffer, "missing or not a string.", i); - name = json_strdup(plugin->plugins, buffer, method); + name = json_strdup(plugin, buffer, method); if (notifications_topic_is_native(name)) return tal_fmt(plugin, @@ -1335,7 +1335,7 @@ static const char *plugin_notifications_add(const char *buffer, "however only be sent by lightningd", name); - tal_arr_expand(&plugin->plugins->notification_topics, name); + tal_arr_expand(&plugin->notification_topics, name); } return NULL; diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 27e2ee777507..e31e642b1abb 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -93,6 +93,10 @@ struct plugin { /* Parameters for dynamically-started plugins. */ const char *parambuf; const jsmntok_t *params; + + /* Notification topics that this plugin has registered with us + * and that other plugins may subscribe to. */ + const char **notification_topics; }; /** @@ -128,10 +132,6 @@ struct plugins { /* Whether builtin plugins should be overridden as unimportant. */ bool dev_builtin_plugins_unimportant; #endif /* DEVELOPER */ - - /* Notification topics that plugins have registered with us - * and that other plugins may subscribe to. */ - const char **notification_topics; }; /* The value of a plugin option, which can have different types. From d4c0fc05f7367f3da0d5ed3972ab76f7e3013a5a Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 16:23:06 +0200 Subject: [PATCH 07/17] plugin: Remember the shortname for a plugin We use it in a couple of places, so let's remember it for easier access. --- lightningd/plugin.c | 9 +++------ lightningd/plugin.h | 5 +++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 57b9e5e9b6d9..4f4e858669a8 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -240,6 +240,7 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES, p = tal(plugins, struct plugin); p->plugins = plugins; p->cmd = tal_strdup(p, path); + p->shortname = path_basename(p, p->cmd); p->start_cmd = start_cmd; p->plugin_state = UNCONFIGURED; @@ -250,8 +251,7 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES, p->dynamic = false; p->index = plugins->plugin_idx++; - p->log = new_log(p, plugins->log_book, NULL, "plugin-%s", - path_basename(tmpctx, p->cmd)); + p->log = new_log(p, plugins->log_book, NULL, "plugin-%s", p->shortname); p->methods = tal_arr(p, const char *, 0); list_head_init(&p->plugin_opts); @@ -1858,7 +1858,6 @@ void json_add_opt_plugins_array(struct json_stream *response, bool important) { struct plugin *p; - const char *plugin_name; struct plugin_opt *opt; const char *opt_name; @@ -1873,9 +1872,7 @@ void json_add_opt_plugins_array(struct json_stream *response, json_add_string(response, "path", p->cmd); /* FIXME: use executables basename until plugins can define their names */ - plugin_name = path_basename(NULL, p->cmd); - json_add_string(response, "name", plugin_name); - tal_free(plugin_name); + json_add_string(response, "name", p->shortname); if (!list_empty(&p->plugin_opts)) { json_object_start(response, "options"); diff --git a/lightningd/plugin.h b/lightningd/plugin.h index e31e642b1abb..9fae129cf649 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -39,8 +39,13 @@ enum plugin_state { * A plugin, exposed as a stub so we can pass it as an argument. */ struct plugin { + /* Must be first element in the struct otherwise we get false + * positives for leaks. */ struct list_node list; + /* The filename that can be used to refer to the plugin. */ + const char *shortname; + pid_t pid; char *cmd; struct io_conn *stdin_conn, *stdout_conn; From 8d30301e276e797fdf88de7aa1ba8545648ad7ac Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 16:40:35 +0200 Subject: [PATCH 08/17] plugin: Wrap custom notifications in a dict with additional origin This should allow us to differentiate the origin of the notification, and further prevent plugins from spoofing native notifications. --- lightningd/plugin.c | 17 ++++------------- tests/plugins/custom_notifications.py | 4 ++-- tests/test_plugin.py | 1 - 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 4f4e858669a8..41da67a3009c 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -453,19 +453,10 @@ static const char *plugin_notification_handle(struct plugin *plugin, methname = json_strdup(tmpctx, plugin->buffer, methtok); if (notifications_have_topic(plugin->plugins, methname)) { - n = tal(NULL, struct jsonrpc_notification); - n->method = tal_steal(n, methname); - n->stream = new_json_stream(n, NULL, NULL); - json_object_start(n->stream, NULL); - json_add_string(n->stream, "jsonrpc", "2.0"); - json_add_string(n->stream, "method", methname); - - json_add_tok(n->stream, "params", paramstok, plugin->buffer); - - json_object_end(n->stream); /* closes '.' */ - - /* We guarantee to have \n\n at end of each response. */ - json_stream_append(n->stream, "\n\n", strlen("\n\n")); + n = jsonrpc_notification_start(NULL, methname); + json_add_string(n->stream, "origin", plugin->shortname); + json_add_tok(n->stream, "payload", paramstok, plugin->buffer); + jsonrpc_notification_end(n); plugins_notify(plugin->plugins, take(n)); return NULL; diff --git a/tests/plugins/custom_notifications.py b/tests/plugins/custom_notifications.py index 9b32aa45efc9..5705439509de 100755 --- a/tests/plugins/custom_notifications.py +++ b/tests/plugins/custom_notifications.py @@ -6,8 +6,8 @@ @plugin.subscribe("custom") -def on_custom_notification(val, plugin, **kwargs): - plugin.log("Got a custom notification {}".format(val)) +def on_custom_notification(origin, payload, **kwargs): + plugin.log("Got a custom notification {} from plugin {}".format(payload, origin)) @plugin.method("emit") diff --git a/tests/test_plugin.py b/tests/test_plugin.py index ff0fdf8e5b0d..d2572d5cb89b 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2404,7 +2404,6 @@ def test_self_disable(node_factory): l1.rpc.plugin_start(p2, selfdisable=True) -@pytest.mark.xfail(strict=True) def test_custom_notification_topics(node_factory): plugin = os.path.join( os.path.dirname(__file__), "plugins", "custom_notifications.py" From 6a7e0017710c56df1e553486efd2dbac511a8352 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 17:21:14 +0200 Subject: [PATCH 09/17] plugin: Restrict plugin notifications only to announced topics We want to have well-behaved notifications that are clearly announced during the initialization, kill plugins that don't behave. --- lightningd/plugin.c | 37 +++++++++++++++++++++++++++++++------ plugins/libplugin.c | 2 ++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 41da67a3009c..587e3c9dc25c 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -417,6 +417,18 @@ static const char *plugin_notify_handle(struct plugin *plugin, return NULL; } +/* Check if the plugin is allowed to send a notification of the + * specified topic, i.e., whether the plugin has announced the topic + * correctly in its manifest. */ +static bool plugin_notification_allowed(const struct plugin *plugin, const char *topic) +{ + for (size_t i=0; inotification_topics); i++) + if (streq(plugin->notification_topics[i], topic)) + return true; + + return false; +} + /* Returns the error string, or NULL */ static const char *plugin_notification_handle(struct plugin *plugin, const jsmntok_t *toks) @@ -453,13 +465,26 @@ static const char *plugin_notification_handle(struct plugin *plugin, methname = json_strdup(tmpctx, plugin->buffer, methtok); if (notifications_have_topic(plugin->plugins, methname)) { - n = jsonrpc_notification_start(NULL, methname); - json_add_string(n->stream, "origin", plugin->shortname); - json_add_tok(n->stream, "payload", paramstok, plugin->buffer); - jsonrpc_notification_end(n); - plugins_notify(plugin->plugins, take(n)); - return NULL; + if (!plugin_notification_allowed(plugin, methname)) { + log_unusual( + plugin->log, + "Plugin attempted to send a notification to topic " + "\"%s\" it hasn't declared in its manifest, not " + "forwarding to subscribers.", + methname); + return NULL; + } else { + + n = jsonrpc_notification_start(NULL, methname); + json_add_string(n->stream, "origin", plugin->shortname); + json_add_tok(n->stream, "payload", paramstok, + plugin->buffer); + jsonrpc_notification_end(n); + + plugins_notify(plugin->plugins, take(n)); + return NULL; + } } else { return tal_fmt(plugin, "Unknown notification method %.*s", json_tok_full_len(methtok), diff --git a/plugins/libplugin.c b/plugins/libplugin.c index c318f5f2acae..9eb1c096e6b4 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -91,6 +91,8 @@ struct plugin { /* Location of the RPC filename in case we need to defer RPC * initialization or need to recover from a disconnect. */ const char *rpc_location; + + char **notification_topics; }; /* command_result is mainly used as a compile-time check to encourage you From 07a73ae70bb6e3682c48902e9fe79f884e55721e Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 17:28:10 +0200 Subject: [PATCH 10/17] cleanup: Make lnprototest run only with DEVELOPER=1 --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index 09b507eeec02..97ef098ecb8a 100644 --- a/Makefile +++ b/Makefile @@ -380,7 +380,11 @@ check-protos: $(ALL_PROGRAMS) ifeq ($(PYTEST),) @echo "py.test is required to run the protocol tests, please install using 'pip3 install -r requirements.txt', and rerun 'configure'."; false else +ifeq ($(DEVELOPER),1) @(cd external/lnprototest && PYTHONPATH=$(PYTHONPATH) LIGHTNING_SRC=../.. $(PYTEST) --runner lnprototest.clightning.Runner $(PYTEST_OPTS)) +else + @echo "lnprototest target requires DEVELOPER=1, skipping" +endif endif pytest: $(ALL_PROGRAMS) From 87d32bb637e329b99d797e36da8c8c4552d46d8c Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 17:28:27 +0200 Subject: [PATCH 11/17] libplugin: Add notification topics to plugin_main --- plugins/autoclean.c | 2 +- plugins/bcli.c | 2 +- plugins/fetchinvoice.c | 1 + plugins/keysend.c | 2 +- plugins/libplugin.c | 13 ++++++++++--- plugins/libplugin.h | 2 ++ plugins/offers.c | 2 +- plugins/pay.c | 1 + plugins/spender/main.c | 1 + plugins/txprepare.c | 2 +- tests/plugins/test_libplugin.c | 1 + 11 files changed, 21 insertions(+), 8 deletions(-) diff --git a/plugins/autoclean.c b/plugins/autoclean.c index 93b71923e05f..21f0c8d30c58 100644 --- a/plugins/autoclean.c +++ b/plugins/autoclean.c @@ -91,7 +91,7 @@ int main(int argc, char *argv[]) { setup_locale(); plugin_main(argv, init, PLUGIN_STATIC, true, NULL, commands, ARRAY_SIZE(commands), - NULL, 0, NULL, 0, + NULL, 0, NULL, 0, NULL, 0, plugin_option("autocleaninvoice-cycle", "string", "Perform cleanup of expired invoices every" diff --git a/plugins/bcli.c b/plugins/bcli.c index ea1ab00131b4..e19aee2e5de8 100644 --- a/plugins/bcli.c +++ b/plugins/bcli.c @@ -972,7 +972,7 @@ int main(int argc, char *argv[]) plugin_main(argv, init, PLUGIN_STATIC, false /* Do not init RPC on startup*/, NULL, commands, ARRAY_SIZE(commands), - NULL, 0, NULL, 0, + NULL, 0, NULL, 0, NULL, 0, plugin_option("bitcoin-datadir", "string", "-datadir arg for bitcoin-cli", diff --git a/plugins/fetchinvoice.c b/plugins/fetchinvoice.c index 311ba99b07b8..598aab0a3419 100644 --- a/plugins/fetchinvoice.c +++ b/plugins/fetchinvoice.c @@ -1389,6 +1389,7 @@ int main(int argc, char *argv[]) /* No notifications */ NULL, 0, hooks, ARRAY_SIZE(hooks), + NULL, 0, /* No options */ NULL); } diff --git a/plugins/keysend.c b/plugins/keysend.c index ef653344066f..69cf7e38be07 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -388,5 +388,5 @@ int main(int argc, char *argv[]) plugin_main(argv, init, PLUGIN_STATIC, true, &features, commands, ARRAY_SIZE(commands), NULL, 0, hooks, ARRAY_SIZE(hooks), - NULL); + NULL, 0, NULL); } diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 9eb1c096e6b4..8dbb7b2b17e2 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -92,7 +92,8 @@ struct plugin { * initialization or need to recover from a disconnect. */ const char *rpc_location; - char **notification_topics; + const char **notif_topics; + size_t num_notif_topics; }; /* command_result is mainly used as a compile-time check to encourage you @@ -1314,6 +1315,8 @@ static struct plugin *new_plugin(const tal_t *ctx, size_t num_notif_subs, const struct plugin_hook *hook_subs, size_t num_hook_subs, + const char **notif_topics, + size_t num_notif_topics, va_list ap) { const char *optname; @@ -1351,6 +1354,8 @@ static struct plugin *new_plugin(const tal_t *ctx, p->commands = commands; p->num_commands = num_commands; + p->notif_topics = notif_topics; + p->num_notif_topics = num_notif_topics; p->notif_subs = notif_subs; p->num_notif_subs = num_notif_subs; p->hook_subs = hook_subs; @@ -1383,6 +1388,8 @@ void plugin_main(char *argv[], size_t num_notif_subs, const struct plugin_hook *hook_subs, size_t num_hook_subs, + const char **notif_topics, + size_t num_notif_topics, ...) { struct plugin *plugin; @@ -1395,10 +1402,10 @@ void plugin_main(char *argv[], /* Note this already prints to stderr, which is enough for now */ daemon_setup(argv[0], NULL, NULL); - va_start(ap, num_hook_subs); + va_start(ap, num_notif_topics); plugin = new_plugin(NULL, init, restartability, init_rpc, features, commands, num_commands, notif_subs, num_notif_subs, hook_subs, - num_hook_subs, ap); + num_hook_subs, notif_topics, num_notif_topics, ap); va_end(ap); setup_command_usage(plugin); diff --git a/plugins/libplugin.h b/plugins/libplugin.h index fcda5baa7ada..710cd31a11b1 100644 --- a/plugins/libplugin.h +++ b/plugins/libplugin.h @@ -289,6 +289,8 @@ void NORETURN LAST_ARG_NULL plugin_main(char *argv[], size_t num_notif_subs, const struct plugin_hook *hook_subs, size_t num_hook_subs, + const char **notif_topics, + size_t num_notif_topics, ...); struct listpeers_channel { diff --git a/plugins/offers.c b/plugins/offers.c index 17117eef9df8..5bf5a5da1a42 100644 --- a/plugins/offers.c +++ b/plugins/offers.c @@ -722,5 +722,5 @@ int main(int argc, char *argv[]) setenv("TZ", "", 1); plugin_main(argv, init, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), NULL, 0, hooks, ARRAY_SIZE(hooks), - NULL); + NULL, 0, NULL); } diff --git a/plugins/pay.c b/plugins/pay.c index 189ebbbf7b87..f5ce5f4ee64a 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -2198,6 +2198,7 @@ int main(int argc, char *argv[]) setup_locale(); plugin_main(argv, init, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), NULL, 0, NULL, 0, + NULL, 0, plugin_option("disable-mpp", "flag", "Disable multi-part payments.", flag_option, &disablempp), diff --git a/plugins/spender/main.c b/plugins/spender/main.c index 86beee72358f..90264ca5346b 100644 --- a/plugins/spender/main.c +++ b/plugins/spender/main.c @@ -39,6 +39,7 @@ int main(int argc, char **argv) commands, tal_count(commands), notifs, tal_count(notifs), NULL, 0, + NULL, 0, /* Notification topics */ NULL); tal_free(owner); diff --git a/plugins/txprepare.c b/plugins/txprepare.c index 4300bfe23bd6..64e9b48322dc 100644 --- a/plugins/txprepare.c +++ b/plugins/txprepare.c @@ -553,5 +553,5 @@ int main(int argc, char *argv[]) { setup_locale(); plugin_main(argv, NULL, PLUGIN_RESTARTABLE, true, NULL, commands, - ARRAY_SIZE(commands), NULL, 0, NULL, 0, NULL); + ARRAY_SIZE(commands), NULL, 0, NULL, 0, NULL, 0, NULL); } diff --git a/tests/plugins/test_libplugin.c b/tests/plugins/test_libplugin.c index cbccaa2229d6..6197116d8303 100644 --- a/tests/plugins/test_libplugin.c +++ b/tests/plugins/test_libplugin.c @@ -146,6 +146,7 @@ int main(int argc, char *argv[]) plugin_main(argv, init, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), notifs, ARRAY_SIZE(notifs), hooks, ARRAY_SIZE(hooks), + NULL, 0, /* Notification topics we publish */ plugin_option("name", "string", "Who to say hello to.", From cdf53d3ae6107781899ddb084cb49e7da26b7345 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 18:27:44 +0200 Subject: [PATCH 12/17] libplugin: Add notification topics to the manifest response --- plugins/libplugin.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 8dbb7b2b17e2..7be9800b87f2 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -670,6 +670,14 @@ handle_getmanifest(struct command *getmanifest_cmd, json_add_bool(params, "dynamic", p->restartability == PLUGIN_RESTARTABLE); + json_array_start(params, "notifications"); + for (size_t i = 0; p->notif_topics && i < p->num_notif_topics; i++) { + json_object_start(params, NULL); + json_add_string(params, "method", p->notif_topics[i]); + json_object_end(params); + } + json_array_end(params); + return command_finished(getmanifest_cmd, params); } From aebb54a5f906831fab7168ff69c6d2868774ad4a Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 18:36:19 +0200 Subject: [PATCH 13/17] libplugin: Add functions to start and end notifications --- plugins/libplugin.c | 19 +++++++++++++++++++ plugins/libplugin.h | 7 +++++++ 2 files changed, 26 insertions(+) diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 7be9800b87f2..2bae5e5ae8c1 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -1028,6 +1028,25 @@ static void plugin_logv(struct plugin *p, enum log_level l, jsonrpc_finish_and_send(p, js); } +struct json_stream *plugin_notification_start(struct plugin *plugin, + const char *method) +{ + struct json_stream *js = new_json_stream(plugin, NULL, NULL); + + json_object_start(js, NULL); + json_add_string(js, "jsonrpc", "2.0"); + json_add_string(js, "method", method); + + json_object_start(js, "params"); + return js; +} +void plugin_notification_end(struct plugin *plugin, + struct json_stream *stream) +{ + json_object_end(stream); + jsonrpc_finish_and_send(plugin, stream); +} + struct json_stream *plugin_notify_start(struct command *cmd, const char *method) { struct json_stream *js = new_json_stream(cmd, NULL, NULL); diff --git a/plugins/libplugin.h b/plugins/libplugin.h index 710cd31a11b1..8dac32f53f3e 100644 --- a/plugins/libplugin.h +++ b/plugins/libplugin.h @@ -242,6 +242,13 @@ void plugin_log(struct plugin *p, enum log_level l, const char *fmt, ...) PRINTF struct json_stream *plugin_notify_start(struct command *cmd, const char *method); void plugin_notify_end(struct command *cmd, struct json_stream *js); +/* Send a notification for a custom notification topic. These are sent + * to lightningd and distributed to subscribing plugins. */ +struct json_stream *plugin_notification_start(struct plugin *plugins, + const char *method); +void plugin_notification_end(struct plugin *plugin, + struct json_stream *stream TAKES); + /* Convenience wrapper for notify "message" */ void plugin_notify_message(struct command *cmd, enum log_level level, From 799f8ce1e9e3ed08aebcb9f7e2ef4b7705152754 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 29 Apr 2021 11:18:19 +0200 Subject: [PATCH 14/17] plugin: Make unannounced notification topics no longer fatal Since plugins will start sending them soon, and they are likely to get it wrong sometimes, be a bit more lenient, warn them in the logs instead and then make sure it doesn't accidentally work anyway. --- lightningd/plugin.c | 29 +++++++++------------------ tests/plugins/custom_notifications.py | 14 +++++++++++++ tests/test_plugin.py | 12 +++++++++++ 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 587e3c9dc25c..cee3da116b1c 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -464,32 +464,21 @@ static const char *plugin_notification_handle(struct plugin *plugin, methname = json_strdup(tmpctx, plugin->buffer, methtok); - if (notifications_have_topic(plugin->plugins, methname)) { - - if (!plugin_notification_allowed(plugin, methname)) { - log_unusual( - plugin->log, + if (!plugin_notification_allowed(plugin, methname)) { + log_unusual(plugin->log, "Plugin attempted to send a notification to topic " "\"%s\" it hasn't declared in its manifest, not " "forwarding to subscribers.", methname); - return NULL; - } else { + } else if (notifications_have_topic(plugin->plugins, methname)) { + n = jsonrpc_notification_start(NULL, methname); + json_add_string(n->stream, "origin", plugin->shortname); + json_add_tok(n->stream, "payload", paramstok, plugin->buffer); + jsonrpc_notification_end(n); - n = jsonrpc_notification_start(NULL, methname); - json_add_string(n->stream, "origin", plugin->shortname); - json_add_tok(n->stream, "payload", paramstok, - plugin->buffer); - jsonrpc_notification_end(n); - - plugins_notify(plugin->plugins, take(n)); - return NULL; - } - } else { - return tal_fmt(plugin, "Unknown notification method %.*s", - json_tok_full_len(methtok), - json_tok_full(plugin->buffer, methtok)); + plugins_notify(plugin->plugins, take(n)); } + return NULL; } /* Returns the error string, or NULL */ diff --git a/tests/plugins/custom_notifications.py b/tests/plugins/custom_notifications.py index 5705439509de..092f2d2c2479 100755 --- a/tests/plugins/custom_notifications.py +++ b/tests/plugins/custom_notifications.py @@ -17,5 +17,19 @@ def emit(plugin): plugin.notify("custom", "Hello world") +@plugin.method("faulty-emit") +def faulty_emit(plugin): + """Emit a simple string notification to topic "custom" + """ + plugin.notify("ididntannouncethis", "Hello world") + + +@plugin.subscribe("ididntannouncethis") +def on_faulty_emit(origin, payload, **kwargs): + """We should never receive this as it gets dropped. + """ + plugin.log("Got the ididntannouncethis event") + + plugin.add_notification_topic("custom") plugin.run() diff --git a/tests/test_plugin.py b/tests/test_plugin.py index d2572d5cb89b..d7d91020f64e 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2411,3 +2411,15 @@ def test_custom_notification_topics(node_factory): l1 = node_factory.get_node(options={'plugin': plugin}) l1.rpc.emit() l1.daemon.wait_for_log(r'Got a custom notification Hello world') + + # And now make sure that we drop unannounced notifications + l1.rpc.faulty_emit() + l1.daemon.wait_for_log( + r"Plugin attempted to send a notification to topic .* not forwarding" + ) + time.sleep(1) + assert not l1.daemon.is_in_log(r'Got the ididntannouncethis event') + + # The plugin just dist what previously was a fatal mistake (emit + # an unknown notification), make sure we didn't kill it. + assert 'custom_notifications.py' in [p['name'] for p in l1.rpc.listconfigs()['plugins']] From 41b4cec425a28fd632eaa9433d819752261fcdfb Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Apr 2021 18:36:56 +0200 Subject: [PATCH 15/17] pay: Add notification for pay_success --- plugins/keysend.c | 6 +++++- plugins/libplugin-pay.c | 8 ++++++++ plugins/pay.c | 7 ++++++- tests/plugins/custom_notifications.py | 10 ++++++++++ tests/test_plugin.py | 7 ++++++- 5 files changed, 35 insertions(+), 3 deletions(-) diff --git a/plugins/keysend.c b/plugins/keysend.c index 69cf7e38be07..54f5ecfa14d5 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -377,6 +377,10 @@ static const struct plugin_hook hooks[] = { }, }; +static const char *notification_topics[] = { + "pay_success", +}; + int main(int argc, char *argv[]) { struct feature_set features; @@ -388,5 +392,5 @@ int main(int argc, char *argv[]) plugin_main(argv, init, PLUGIN_STATIC, true, &features, commands, ARRAY_SIZE(commands), NULL, 0, hooks, ARRAY_SIZE(hooks), - NULL, 0, NULL); + notification_topics, ARRAY_SIZE(notification_topics), NULL); } diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index d8193fbadaf8..7538022da014 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -1844,6 +1844,8 @@ static void payment_finished(struct payment *p) struct json_stream *ret; struct command *cmd = p->cmd; const char *msg; + struct json_stream *n; + struct payment *root = payment_root(p); /* Either none of the leaf attempts succeeded yet, or we have a * preimage. */ @@ -1885,6 +1887,12 @@ static void payment_finished(struct payment *p) json_add_string(ret, "status", "complete"); + n = plugin_notification_start(p->plugin, "pay_success"); + json_add_sha256(n, "payment_hash", p->payment_hash); + if (root->invstring != NULL) + json_add_string(n, "bolt11", root->invstring); + plugin_notification_end(p->plugin, n); + if (command_finished(cmd, ret)) {/* Ignore result. */} return; } else if (p->aborterror != NULL) { diff --git a/plugins/pay.c b/plugins/pay.c index f5ce5f4ee64a..c3007dc64715 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -2193,12 +2193,17 @@ static const struct plugin_command commands[] = { }, }; +static const char *notification_topics[] = { + "pay_success", + "pay_failure", +}; + int main(int argc, char *argv[]) { setup_locale(); plugin_main(argv, init, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), NULL, 0, NULL, 0, - NULL, 0, + notification_topics, ARRAY_SIZE(notification_topics), plugin_option("disable-mpp", "flag", "Disable multi-part payments.", flag_option, &disablempp), diff --git a/tests/plugins/custom_notifications.py b/tests/plugins/custom_notifications.py index 092f2d2c2479..a3fa9b9aa837 100755 --- a/tests/plugins/custom_notifications.py +++ b/tests/plugins/custom_notifications.py @@ -24,6 +24,16 @@ def faulty_emit(plugin): plugin.notify("ididntannouncethis", "Hello world") +@plugin.subscribe("pay_success") +def on_pay_success(origin, payload, **kwargs): + plugin.log( + "Got a pay_success notification from plugin {} for payment_hash {}".format( + origin, + payload['payment_hash'] + ) + ) + + @plugin.subscribe("ididntannouncethis") def on_faulty_emit(origin, payload, **kwargs): """We should never receive this as it gets dropped. diff --git a/tests/test_plugin.py b/tests/test_plugin.py index d7d91020f64e..7d9ab6e65e17 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2408,10 +2408,15 @@ def test_custom_notification_topics(node_factory): plugin = os.path.join( os.path.dirname(__file__), "plugins", "custom_notifications.py" ) - l1 = node_factory.get_node(options={'plugin': plugin}) + l1, l2 = node_factory.line_graph(2, opts=[{'plugin': plugin}, {}]) l1.rpc.emit() l1.daemon.wait_for_log(r'Got a custom notification Hello world') + inv = l2.rpc.invoice(42, "lbl", "desc")['bolt11'] + l1.rpc.pay(inv) + + l1.daemon.wait_for_log(r'Got a pay_success notification from plugin pay for payment_hash [0-9a-f]{64}') + # And now make sure that we drop unannounced notifications l1.rpc.faulty_emit() l1.daemon.wait_for_log( From 4e3c9d66ef8d1552ea8eed332fd553726b681a19 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 29 Apr 2021 10:16:47 +0200 Subject: [PATCH 16/17] pay: Add pay_failure notification --- plugins/keysend.c | 1 + plugins/libplugin-pay.c | 25 ++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/plugins/keysend.c b/plugins/keysend.c index 54f5ecfa14d5..97196a3a5c59 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -379,6 +379,7 @@ static const struct plugin_hook hooks[] = { static const char *notification_topics[] = { "pay_success", + "pay_failure", }; int main(int argc, char *argv[]) diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 7538022da014..75ebcee66365 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -1834,6 +1834,21 @@ static void payment_json_add_attempts(struct json_stream *s, json_array_end(s); } +static void payment_notify_failure(struct payment *p, const char *error_message) { + struct payment *root = payment_root(p); + struct json_stream *n; + n = plugin_notification_start(p->plugin, "pay_failure"); + json_add_sha256(n, "payment_hash", p->payment_hash); + if (root->invstring != NULL) + json_add_string(n, "bolt11", root->invstring); + + json_object_start(n, "error"); + json_add_string(n, "message", error_message); + json_object_end(n); /* .error */ + + plugin_notification_end(p->plugin, n); +} + /* This function is called whenever a payment ends up in a final state, or all * leafs in the subtree rooted in the payment are all in a final state. It is * called only once, and it is guaranteed to be called in post-order @@ -1901,6 +1916,9 @@ static void payment_finished(struct payment *p) ret = jsonrpc_stream_fail(cmd, PAY_STOPPED_RETRYING, p->aborterror); payment_json_add_attempts(ret, "attempts", p); + + payment_notify_failure(p, p->aborterror); + if (command_finished(cmd, ret)) {/* Ignore result. */} return; } else if (result.failure == NULL || result.failure->failcode < NODE) { @@ -1913,6 +1931,9 @@ static void payment_finished(struct payment *p) ret = jsonrpc_stream_fail(cmd, PAY_STOPPED_RETRYING, msg); payment_json_add_attempts(ret, "attempts", p); + + payment_notify_failure(p, msg); + if (command_finished(cmd, ret)) {/* Ignore result. */} return; @@ -1978,7 +1999,9 @@ static void payment_finished(struct payment *p) *failure->erring_direction); } - if (command_finished(cmd, ret)) {/* Ignore result. */} + payment_notify_failure(p, failure->message); + + if (command_finished(cmd, ret)) { /* Ignore result. */} return; } } else { From 9faa9071d91a50913d51ddc029956df579ff3db2 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 29 Apr 2021 11:02:47 +0200 Subject: [PATCH 17/17] docs: Document the custom plugin notifications --- doc/PLUGINS.md | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index 6f81c44105ce..d3f738e0d9df 100644 --- a/doc/PLUGINS.md +++ b/doc/PLUGINS.md @@ -102,6 +102,11 @@ example: "init": "0E000000", "invoice": "00AD0000" }, + "notifications": [ + { + "method": "mycustomnotification" + } + ], "dynamic": true } ``` @@ -147,6 +152,13 @@ position bits for experiments. If you'd like to standardize your extension please reach out to the [specification repository][spec] to get a featurebit assigned. +The `notifications` array allows plugins to announce which custom +notifications they intend to send to `lightningd`. These custom +notifications can then be subscribed to by other plugins, allowing +them to communicate with each other via the existing publish-subscribe +mechanism and react to events that happen in other plugins, or collect +information based on the notification topics. + Plugins are free to register any `name` for their `rpcmethod` as long as the name was not previously registered. This includes both built-in methods, such as `help` and `getinfo`, as well as methods registered @@ -206,6 +218,60 @@ Here's an example option set, as sent in response to `getmanifest` ], ``` +#### Custom notifications + +The plugins may emit custom notifications for topics they have +announced during startup. The list of notification topics declared +during startup must include all topics that may be emitted, in order +to verify that all topics plugins subscribe to are also emitted by +some other plugin, and warn if a plugin subscribes to a non-existent +topic. In case a plugin emits notifications it has not announced the +notification will be ignored and not forwarded to subscribers. + +When forwarding a custom notification `lightningd` will wrap the +payload of the notification in an object that contains metadata about +the notification. The following is an example of this +transformation. The first listing is the original notification emitted +by the `sender` plugin, while the second is the the notification as +received by the `receiver` plugin (both listings show the full +[JSON-RPC][jsonrpc-spec] notification to illustrate the wrapping). + +```json +{ + "jsonrpc": "2.0", + "method": "mycustomnotification", + "params": { + "key": "value", + "message": "Hello fellow plugin!" + } +} +``` + +is delivered as + +```json +{ + "jsonrpc": "2.0", + "method": "mycustomnotification", + "params": { + "origin": "sender", + "payload": { + "key": "value", + "message": "Hello fellow plugin!" + } + } +} + +``` + +The notification topic (`method` in the JSON-RPC message) must not +match one of the internal events in order to prevent breaking +subscribers that expect the existing notification format. Multiple +plugins are allowed to emit notifications for the same topics, +allowing things like metric aggregators where the aggregator +subscribes to a common topic and other plugins publish metrics as +notifications. + ### The `init` method The `init` method is required so that `lightningd` can pass back the