Actually check, cleanup, notification schemas

cln-rpc/src/notifications.rs

@@ -190,14 +190,15 @@ impl ToString for ChannelStateChangedCause {
 pub struct ChannelStateChangedNotification {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub old_state: Option<ChannelState>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub short_channel_id: Option<ShortChannelId>,
     // Path `channel_state_changed.cause`
     pub cause: ChannelStateChangedCause,
     // Path `channel_state_changed.new_state`
     pub new_state: ChannelState,
     pub channel_id: Sha256,
     pub message: String,
     pub peer_id: PublicKey,
-    pub short_channel_id: ShortChannelId,
     pub timestamp: String,
 }
 

contrib/msggen/msggen/schema.json

@@ -35671,7 +35671,6 @@
         "required": [
           "peer_id",
           "channel_id",
-          "short_channel_id",
           "timestamp",
           "new_state",
           "cause",
@@ -35725,7 +35724,7 @@
               "DUALOPEND_OPEN_COMMIT_READY"
             ],
             "description": [
-              "The channel state, in particular \"CHANNELD_NORMAL\" means the channel can be used normally.",
+              "The channel state, in particular \"CHANNELD_NORMAL\" and \"CHANNELD_AWAITING_SPLICE\" mean the channel can be used normally.",
               "The deprecated value 'unknown' is also present for new channels: after v26.03 this field will be omitted instead."
             ],
             "added": "pre-v0.10.1"
@@ -35749,7 +35748,7 @@
               "DUALOPEND_OPEN_COMMIT_READY"
             ],
             "description": [
-              "The channel state, in particular \"CHANNELD_NORMAL\" means the channel can be used normally"
+              "The channel state, in particular \"CHANNELD_NORMAL\" and \"CHANNELD_AWAITING_SPLICE\" mean the channel can be used normally"
             ],
             "added": "pre-v0.10.1"
           },

contrib/pyln-testing/pyln/testing/fixtures.py

@@ -3,6 +3,7 @@
 from pyln.testing.utils import NodeFactory, BitcoinD, ElementsD, env, LightningNode, TEST_DEBUG, TEST_NETWORK
 from pyln.client import Millisatoshi
 from typing import Dict
+from pathlib import Path
 
 import json
 import jsonschema  # type: ignore
@@ -497,6 +498,7 @@ def map_node_error(nodes, f, msg):
     map_node_error(nf.nodes, checkBroken, "had BROKEN messages")
     map_node_error(nf.nodes, lambda n: not n.allow_warning and n.daemon.is_in_log(r' WARNING:'), "had warning messages")
     map_node_error(nf.nodes, checkReconnect, "had unexpected reconnections")
+    map_node_error(nf.nodes, checkPluginJSON, "had malformed hooks/notifications")
 
     # On any bad gossip complaints, print out every nodes' gossip_store
     if map_node_error(nf.nodes, checkBadGossip, "had bad gossip messages"):
@@ -618,6 +620,51 @@ def checkBroken(node):
     return 0
 
 
+def checkPluginJSON(node):
+    if node.bad_notifications:
+        return 0
+    notifications = {}
+    for fname in os.listdir('doc/schemas/notification'):
+        if fname.endswith('.json'):
+            base = fname.replace('.json', '')
+            # Request is 0 and Response is 1
+            notifications[base] = _load_schema(os.path.join('doc/schemas/notification', fname))
+
+    # FIXME: add doc/schemas/hook/
+    hooks = {}
+
+    for f in (Path(node.daemon.lightning_dir) / "plugin-io").iterdir():
+        # e.g. hook_in-peer_connected-124567-358
+        io = json.loads(f.read_text())
+        parts = f.name.split('-')
+        if parts[0] == 'hook_in':
+            schema = hooks.get(parts[1])
+            req = io['result']
+            direction = 1
+        elif parts[0] == 'hook_out':
+            schema = hooks.get(parts[1])
+            req = io['params']
+            direction = 0
+        else:
+            assert parts[0] == 'notification_out'
+            schema = notifications.get(parts[1])
+            # The notification is wrapped in an object of its own name.
+            req = io['params'][parts[1]]
+            direction = 1
+
+        # Until v26.09, with channel_state_changed.null_scid, that notification will be non-schema compliant.
+        if f.name.startswith('notification_out-channel_state_changed-') and node.daemon.opts.get('allow-deprecated-apis', True) is True:
+            continue
+
+        if schema is not None:
+            try:
+                schema[direction].validate(req)
+            except jsonschema.exceptions.ValidationError as e:
+                print(f"Failed validating {f}: {e}")
+                return 1
+    return 0
+
+
 def checkBadReestablish(node):
     if node.daemon.is_in_log('Bad reestablish'):
         return 1

contrib/pyln-testing/pyln/testing/utils.py

@@ -789,11 +789,13 @@ def __init__(self, node_id, lightning_dir, bitcoind, executor, valgrind, may_fai
                  jsonschemas={},
                  valgrind_plugins=True,
                  executable=None,
+                 bad_notifications=False,
                  **kwargs):
         self.bitcoin = bitcoind
         self.executor = executor
         self.may_fail = may_fail
         self.may_reconnect = may_reconnect
+        self.bad_notifications = bad_notifications
         self.broken_log = broken_log
         self.allow_bad_gossip = allow_bad_gossip
         self.allow_warning = allow_warning
@@ -853,6 +855,10 @@ def __init__(self, node_id, lightning_dir, bitcoind, executor, valgrind, may_fai
         if self.cln_version >= "v24.11":
             self.daemon.opts.update({"autoconnect-seeker-peers": 0})
 
+        jsondir = Path(lightning_dir) / "plugin-io"
+        jsondir.mkdir()
+        self.daemon.opts['dev-save-plugin-io'] = jsondir
+
         if options is not None:
             self.daemon.opts.update(options)
         dsn = db.get_dsn()
@@ -1601,6 +1607,7 @@ def split_options(self, opts):
             'broken_log',
             'allow_warning',
             'may_reconnect',
+            'bad_notifications',
             'random_hsm',
             'feerates',
             'wait_for_bitcoind_sync',

doc/developers-guide/deprecations.md

@@ -18,6 +18,9 @@ hidden: false
 | listpeerchannels.max_total_htlc_in_msat | Field           | v25.02           | v26.03         | Use our_max_total_htlc_out_msat                                                                                                              |
 | wait.details                         | Field              | v25.05           | v26.06         | Use subsystem-specific object instead                                                                                                  |
 | channel_state_changed.old_state.unknown | Notification Field | v25.05        | v26.03         | Value "unknown" is deprecated: field will be omitted instead                                                                           |
+| channel_state_changed.null_scid         | Notification Field | v25.09        | v26.09         | In channel_state_changed notification, `short_channel_id` will be missing instead of `null`                                            |
+| notification.payload                    | Notification Field | v25.09        | v26.09         | Notifications from plugins used to have fields in `payload` sub-object, now they are not (just like normal notifications)              |
+| pay_notifications.raw_fields            | Field              | v25.09           | v26.09         | `channel_hint_update`, `pay_failure` and `pay_success` notifications now wrap members in an object of the same name                    |
 
 Inevitably there are features which need to change: either to be generalized, or removed when they can no longer be supported.
 

doc/schemas/notification/channel_state_changed.json

@@ -13,7 +13,6 @@
     "required": [
       "peer_id",
       "channel_id",
-      "short_channel_id",
       "timestamp",
       "new_state",
       "cause",
@@ -67,7 +66,7 @@
           "DUALOPEND_OPEN_COMMIT_READY"
         ],
         "description": [
-          "The channel state, in particular \"CHANNELD_NORMAL\" means the channel can be used normally.",
+          "The channel state, in particular \"CHANNELD_NORMAL\" and \"CHANNELD_AWAITING_SPLICE\" mean the channel can be used normally.",
           "The deprecated value 'unknown' is also present for new channels: after v26.03 this field will be omitted instead."
         ],
         "added": "pre-v0.10.1"
@@ -91,7 +90,7 @@
           "DUALOPEND_OPEN_COMMIT_READY"
         ],
         "description": [
-          "The channel state, in particular \"CHANNELD_NORMAL\" means the channel can be used normally"
+            "The channel state, in particular \"CHANNELD_NORMAL\" and \"CHANNELD_AWAITING_SPLICE\" mean the channel can be used normally"
         ],
         "added": "pre-v0.10.1"
       },

doc/schemas/notification/connect.json

@@ -53,7 +53,8 @@
               "ipv4",
               "ipv6",
               "torv2",
-              "torv3"
+              "torv3",
+              "websocket"
             ],
             "description": [
               "Type of connection (*torv2*/*torv3* only if **direction** is *out*)"

lightningd/jsonrpc.c

@@ -1562,14 +1562,21 @@ static struct command_result *param_command(struct command *cmd,
 			    tok->end - tok->start, buffer + tok->start);
 }
 
-struct jsonrpc_notification *jsonrpc_notification_start(const tal_t *ctx, const char *method)
+struct jsonrpc_notification *jsonrpc_notification_start_noparams(const tal_t *ctx, const char *method)
 {
 	struct jsonrpc_notification *n = tal(ctx, struct jsonrpc_notification);
 	n->method = tal_strdup(n, method);
 	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", method);
+
+	return n;
+}
+
+struct jsonrpc_notification *jsonrpc_notification_start(const tal_t *ctx, const char *method)
+{
+	struct jsonrpc_notification *n = jsonrpc_notification_start_noparams(ctx, method);
 	json_object_start(n->stream, "params");
 
 	return n;
@@ -1578,6 +1585,11 @@ struct jsonrpc_notification *jsonrpc_notification_start(const tal_t *ctx, const
 void jsonrpc_notification_end(struct jsonrpc_notification *n)
 {
 	json_object_end(n->stream); /* closes '.params' */
+	jsonrpc_notification_end_noparams(n);
+}
+
+void jsonrpc_notification_end_noparams(struct jsonrpc_notification *n)
+{
 	json_object_end(n->stream); /* closes '.' */
 
 	/* We guarantee to have \n\n at end of each response. */

lightningd/jsonrpc.h

@@ -226,11 +226,23 @@ bool jsonrpc_command_add(struct jsonrpc *rpc, struct json_command *command,
  */
 struct jsonrpc_notification *jsonrpc_notification_start(const tal_t *ctx, const char *topic);
 
+/**
+ * Begin a JSON-RPC notification with the specified topic.
+ *
+ * Does *not* start a "params" object.
+ */
+struct jsonrpc_notification *jsonrpc_notification_start_noparams(const tal_t *ctx, const char *topic);
+
 /**
  * Counterpart to jsonrpc_notification_start.
  */
 void jsonrpc_notification_end(struct jsonrpc_notification *n);
 
+/**
+ * Counterpart to jsonrpc_notification_start_noparams.
+ */
+void jsonrpc_notification_end_noparams(struct jsonrpc_notification *n);
+
 /**
  * start a JSONRPC request; id_prefix is non-NULL if this was triggered by
  * another JSONRPC request.

lightningd/notification.c

@@ -290,7 +290,10 @@ static void channel_state_changed_notification_serialize(struct json_stream *str
 	json_add_channel_id(stream, "channel_id", cid);
 	if (scid)
 		json_add_short_channel_id(stream, "short_channel_id", *scid);
-	else
+	else if (lightningd_deprecated_out_ok(ld, ld->deprecated_ok,
+					      "channel_state_changed",
+					      "null_scid",
+					      "v25.09", "v26.09"))
 		json_add_null(stream, "short_channel_id");
 	json_add_timeiso(stream, "timestamp", timestamp);
 	if (old_state != 0 || lightningd_deprecated_out_ok(ld, ld->deprecated_ok,

lightningd/options.c

@@ -931,6 +931,10 @@ static void dev_register_opts(struct lightningd *ld)
 		       opt_set_bool,
 		       &ld->dev_hsmd_warn_on_overgrind,
 		       "Warn if we create signatures that are not exactly 71 bytes.");
+	clnopt_witharg("--dev-save-plugin-io", OPT_DEV,
+		       opt_set_charp, opt_show_charp,
+		       &ld->plugins->dev_save_io,
+		       "Directory to place all plugin notifications/hooks JSON into.");
 	/* This is handled directly in daemon_developer_mode(), so we ignore it here */
 	clnopt_noarg("--dev-debug-self", OPT_DEV,
 		     opt_ignore,

lightningd/plugin.c

@@ -4,9 +4,11 @@
 #include <ccan/crc32c/crc32c.h>
 #include <ccan/io/io.h>
 #include <ccan/json_escape/json_escape.h>
+#include <ccan/json_out/json_out.h>
 #include <ccan/mem/mem.h>
 #include <ccan/opt/opt.h>
 #include <ccan/pipecmd/pipecmd.h>
+#include <ccan/read_write_all/read_write_all.h>
 #include <ccan/tal/path/path.h>
 #include <ccan/tal/str/str.h>
 #include <ccan/utf8/utf8.h>
@@ -23,6 +25,7 @@
 #include <db/exec.h>
 #include <dirent.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <lightningd/io_loop_with_timers.h>
 #include <lightningd/notification.h>
 #include <lightningd/plugin.h>
@@ -76,6 +79,7 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book,
 	p->plugin_idx = 0;
 	p->dev_builtin_plugins_unimportant = false;
 	p->want_db_transaction = true;
+	p->dev_save_io = NULL;
 	p->subscriptions = tal(p, struct plugin_subscription_htable);
 	plugin_subscription_htable_init(p->subscriptions);
 
@@ -625,10 +629,10 @@ static const char *plugin_notification_handle(struct plugin *plugin,
 			    "forwarding to subscribers.",
 			    methname);
 	} else if (notifications_have_topic(plugin->plugins, methname)) {
-		n = jsonrpc_notification_start(NULL, methname);
+		n = jsonrpc_notification_start_noparams(NULL, methname);
 		json_add_string(n->stream, "origin", plugin->shortname);
-		json_add_tok(n->stream, "payload", paramstok, plugin->buffer);
-		jsonrpc_notification_end(n);
+		json_add_tok(n->stream, "params", paramstok, plugin->buffer);
+		jsonrpc_notification_end_noparams(n);
 
 		plugins_notify(plugin->plugins, take(n));
 	}
@@ -2535,6 +2539,10 @@ void plugins_notify(struct plugins *plugins,
 	if (!plugins)
 		return;
 
+	dev_save_plugin_io_out(plugins,
+			       "notification_out",
+			       n->method, n->stream);
+
 	for (struct plugin_subscription *sub
 		     = plugin_subscription_htable_getfirst(plugins->subscriptions,
 							   n->method, &it);
@@ -2674,3 +2682,49 @@ void shutdown_plugins(struct lightningd *ld)
 		}
 	}
 }
+
+static void dev_save_plugin_io(struct plugins *plugins,
+ 			       const char *type,
+			       const char *name,
+			       const char *buf, size_t len)
+{
+	static size_t counter;
+	const char *file;
+	int fd;
+
+	if (!plugins->dev_save_io)
+		return;
+
+	file = path_join(tmpctx, plugins->dev_save_io,
+			 take(tal_fmt(NULL, "%s-%s-%u-%zu",
+				      type, name,
+				      (unsigned int)getpid(),
+				      counter++)));
+	fd = open(file, O_CREAT|O_EXCL|O_WRONLY, 0600);
+	if (fd < 0 || !write_all(fd, buf, len))
+		fatal("Writing --dev-save-plugin-io %s: %s",
+		      file, strerror(errno));
+	close(fd);
+}
+
+void dev_save_plugin_io_in(struct plugins *plugins,
+			   const char *type,
+			   const char *name,
+			   const char *buffer,
+			   const jsmntok_t *tok)
+{
+	dev_save_plugin_io(plugins, type, name,
+			   buffer + tok->start, tok->end - tok->start);
+}
+
+void dev_save_plugin_io_out(struct plugins *plugins,
+			    const char *type,
+			    const char *name,
+			    const struct json_stream *stream)
+{
+	size_t len;
+	const char *buf;
+
+	buf = json_out_contents(stream->jout, &len);
+	dev_save_plugin_io(plugins, type, name, buf, len);
+}