feat: chain hook rpc command

common/json_stream.c

@@ -194,8 +194,9 @@ void json_add_jsonstr(struct json_stream *js,
 	size_t len = strlen(jsonstr);
 
 	p = json_member_direct(js, fieldname, len);
-
-	memcpy(p, jsonstr, len);
+	/* Could be OOM! */
+	if (p)
+		memcpy(p, jsonstr, len);
 }
 
 /* This is where we read the json_stream and write it to conn */

doc/PLUGINS.md

@@ -1198,6 +1198,9 @@ Return a custom error to the request sender:
 }
 ```
 
+Note: The `rpc_command` hook is chainable. If two or more plugins try to
+replace/result/error the same `method`, only the first plugin in the chain
+will be respected. Others will be ignored and a warning will be logged.
 
 ### `custommsg`
 

lightningd/jsonrpc.c

@@ -665,6 +665,12 @@ struct rpc_command_hook_payload {
 	struct command *cmd;
 	const char *buffer;
 	const jsmntok_t *request;
+
+	/* custom response/replace/error options plugins can have */
+	const char *custom_result;
+	const char *custom_error;
+	const jsmntok_t *custom_replace;
+	const char *custom_buffer;
 };
 
 static void rpc_command_hook_serialize(struct rpc_command_hook_payload *p,
@@ -734,50 +740,89 @@ static void replace_command(struct rpc_command_hook_payload *p,
 				 "Bad response to 'rpc_command' hook: %s", bad));
 }
 
-static void
-rpc_command_hook_callback(struct rpc_command_hook_payload *p STEALS,
-                          const char *buffer, const jsmntok_t *resulttok)
+static void rpc_command_hook_final(struct rpc_command_hook_payload *p STEALS)
 {
-	const jsmntok_t *tok, *params, *custom_return;
-	const jsmntok_t *innerresulttok;
-	struct json_stream *response;
+	const jsmntok_t *params;
 
 	/* Free payload with cmd */
 	tal_steal(p->cmd, p);
 
+	if (p->custom_result != NULL) {
+		struct json_stream *s = json_start(p->cmd);
+		json_add_jsonstr(s, "result", p->custom_result);
+		json_object_compat_end(s);
+		return was_pending(command_raw_complete(p->cmd, s));
+	}
+	if (p->custom_error != NULL) {
+		struct json_stream *s = json_start(p->cmd);
+		json_add_jsonstr(s, "error", p->custom_error);
+		json_object_compat_end(s);
+		return was_pending(command_raw_complete(p->cmd, s));
+	}
+	if (p->custom_replace != NULL)
+		return replace_command(p, p->custom_buffer, p->custom_replace);
+
+	/* If no plugin requested a change, just continue command execution. */
 	params = json_get_member(p->buffer, p->request, "params");
+	return was_pending(command_exec(p->cmd->jcon,
+					p->cmd,
+					p->buffer,
+					p->request,
+					params));
+}
+
+static bool
+rpc_command_hook_callback(struct rpc_command_hook_payload *p,
+			  const char *buffer, const jsmntok_t *resulttok)
+{
+	const struct lightningd *ld = p->cmd->ld;
+	const jsmntok_t *tok, *custom_return;
+	static char *error = "";
+	char *method;
+
+	if (!resulttok || !buffer)
+		return true;
 
-	/* If no plugin registered, just continue command execution. Same if
-	 * the registered plugin tells us to do so. */
-	if (buffer == NULL)
-	    return was_pending(command_exec(p->cmd->jcon, p->cmd, p->buffer,
-		                                p->request, params));
-
-	innerresulttok = json_get_member(buffer, resulttok, "result");
-	if (innerresulttok) {
-		if (json_tok_streq(buffer, innerresulttok, "continue")) {
-			return was_pending(command_exec(p->cmd->jcon, p->cmd, p->buffer,
-							p->request, params));
+	tok = json_get_member(buffer, resulttok, "result");
+	if (tok) {
+		if (!json_tok_streq(buffer, tok, "continue")) {
+			error = "'result' should only be 'continue'.";
+			goto log_error_and_skip;
 		}
-		return was_pending(command_fail(p->cmd, JSONRPC2_INVALID_REQUEST,
-						"Bad 'result' to 'rpc_command' hook."));
+		/* plugin tells us to do nothing. just pass. */
+		return true;
+	}
+
+	/* didn't just continue but hook was already modified by prior plugin */
+	if (p->custom_result != NULL ||
+	    p->custom_error != NULL ||
+	    p->custom_replace != NULL) {
+		/* get method name and log error (only the first time). */
+		tok = json_get_member(p->buffer, p->request, "method");
+		method = tal_strndup(p, p->buffer + tok->start, tok->end - tok->start);
+		log_unusual(ld->log, "rpc_command hook '%s' already modified, ignoring.", method );
+		rpc_command_hook_final(p);
+		return false;
 	}
 
 	/* If the registered plugin did not respond with continue,
 	 * it wants either to replace the request... */
 	tok = json_get_member(buffer, resulttok, "replace");
-	if (tok)
-		return replace_command(p, buffer, tok);
+	if (tok) {
+		/* We need to make copies here, as buffer and tokens
+		 * can be reused. */
+		p->custom_replace = json_tok_copy(p, tok);
+		p->custom_buffer = tal_dup_talarr(p, char, buffer);
+		return true;
+	}
 
 	/* ...or return a custom JSONRPC response. */
 	tok = json_get_member(buffer, resulttok, "return");
 	if (tok) {
 		custom_return = json_get_member(buffer, tok, "result");
 		if (custom_return) {
-			response = json_start(p->cmd);
-			json_add_tok(response, "result", custom_return, buffer);
-			json_object_compat_end(response);
-			return was_pending(command_raw_complete(p->cmd, response));
+			p->custom_result = json_strdup(p, buffer, custom_return);
+			return true;
 		}
 
 		custom_return = json_get_member(buffer, tok, "error");
@@ -786,29 +831,32 @@ rpc_command_hook_callback(struct rpc_command_hook_payload *p STEALS,
 			const char *errmsg;
 			if (!json_to_errcode(buffer,
 					     json_get_member(buffer, custom_return, "code"),
-					     &code))
-				return was_pending(command_fail(p->cmd, JSONRPC2_INVALID_REQUEST,
-				                                "Bad response to 'rpc_command' hook: "
-				                                "'error' object does not contain a code."));
+					     &code)) {
+				error = "'error' object does not contain a code.";
+				goto log_error_and_skip;
+			}
 			errmsg = json_strdup(tmpctx, buffer,
 			                     json_get_member(buffer, custom_return, "message"));
-			if (!errmsg)
-				return was_pending(command_fail(p->cmd, JSONRPC2_INVALID_REQUEST,
-				                                "Bad response to 'rpc_command' hook: "
-				                                "'error' object does not contain a message."));
-			response = json_stream_fail_nodata(p->cmd, code, errmsg);
-			return was_pending(command_failed(p->cmd, response));
+			if (!errmsg) {
+				error = "'error' object does not contain a message.";
+				goto log_error_and_skip;
+			}
+			p->custom_error = json_strdup(p, buffer, custom_return);
+			return true;
 		}
 	}
 
-	was_pending(command_fail(p->cmd, JSONRPC2_INVALID_REQUEST,
-	                         "Bad response to 'rpc_command' hook."));
+log_error_and_skip:
+	/* Just log BROKEN errors. Give other plugins a chance. */
+	log_broken(ld->log, "Bad response to 'rpc_command' hook. %s", error);
+	return true;
 }
 
-REGISTER_SINGLE_PLUGIN_HOOK(rpc_command,
-			    rpc_command_hook_callback,
-			    rpc_command_hook_serialize,
-			    struct rpc_command_hook_payload *);
+REGISTER_PLUGIN_HOOK(rpc_command,
+		     rpc_command_hook_callback,
+		     rpc_command_hook_final,
+		     rpc_command_hook_serialize,
+		     struct rpc_command_hook_payload *);
 
 /* We return struct command_result so command_fail return value has a natural
  * sink; we don't actually use the result. */
@@ -884,6 +932,12 @@ parse_request(struct json_connection *jcon, const jsmntok_t tok[])
 	rpc_hook->buffer = tal_dup_talarr(rpc_hook, char, jcon->buffer);
 	rpc_hook->request = tal_dup_talarr(rpc_hook, jsmntok_t, tok);
 
+	/* NULL the custom_ values for the hooks */
+	rpc_hook->custom_result = NULL;
+	rpc_hook->custom_error = NULL;
+	rpc_hook->custom_replace = NULL;
+	rpc_hook->custom_buffer = NULL;
+
 	db_begin_transaction(jcon->ld->wallet->db);
 	completed = plugin_hook_call_rpc_command(jcon->ld, rpc_hook);
 	db_commit_transaction(jcon->ld->wallet->db);

tests/plugins/rpc_command.py → tests/plugins/rpc_command_1.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 """
-This plugin is used to test the `rpc_command` hook.
+This plugin is used to test the chained `rpc_command` hook.
 """
 from pyln.client import Plugin
 
@@ -12,15 +12,11 @@ def on_rpc_command(plugin, rpc_command, **kwargs):
     request = rpc_command
     if request["method"] == "invoice":
         # Replace part of this command
-        request["params"]["description"] = "A plugin modified this description"
+        request["params"]["description"] = "rpc_command_1 modified this description"
         return {"replace": request}
     elif request["method"] == "listfunds":
         # Return a custom result to the command
-        return {"return": {"result": ["Custom result"]}}
-    elif request["method"] == "sendpay":
-        # Don't allow this command to be executed
-        return {"return": {"error": {"code": -1,
-                                     "message": "You cannot do this"}}}
+        return {"return": {"result": ["Custom rpc_command_1 result"]}}
     elif request["method"] == "help":
         request["method"] = "autocleaninvoice"
         return {"replace": request}

tests/plugins/rpc_command_2.py

@@ -0,0 +1,24 @@
+#!/usr/bin/env python3
+"""
+This plugin is used to test the chained `rpc_command` hook.
+"""
+from pyln.client import Plugin
+
+plugin = Plugin()
+
+
+@plugin.hook("rpc_command")
+def on_rpc_command(plugin, rpc_command, **kwargs):
+    request = rpc_command
+    if request["method"] == "invoice":
+        # Replace part of this command
+        request["params"]["description"] = "rpc_command_2 modified this description"
+        return {"replace": request}
+    elif request["method"] == "sendpay":
+        # Don't allow this command to be executed
+        return {"return": {"error": {"code": -1,
+                                     "message": "rpc_command_2 cannot do this"}}}
+    return {"result": "continue"}
+
+
+plugin.run()

tests/test_plugin.py

@@ -16,6 +16,7 @@
 import json
 import os
 import pytest
+import random
 import re
 import signal
 import sqlite3
@@ -1339,28 +1340,42 @@ def test_sendpay_notifications_nowaiter(node_factory):
 
 
 def test_rpc_command_hook(node_factory):
-    """Test the `sensitive_command` hook"""
-    plugin = os.path.join(os.getcwd(), "tests/plugins/rpc_command.py")
+    """Test the `rpc_command` hook chain"""
+    plugin = [
+        os.path.join(os.getcwd(), "tests/plugins/rpc_command_1.py"),
+        os.path.join(os.getcwd(), "tests/plugins/rpc_command_2.py")
+    ]
     l1 = node_factory.get_node(options={"plugin": plugin})
 
-    # Usage of "sendpay" has been restricted by the plugin
-    with pytest.raises(RpcError, match=r"You cannot do this"):
+    # rpc_command_2 plugin restricts using "sendpay"
+    with pytest.raises(RpcError, match=r"rpc_command_2 cannot do this"):
         l1.rpc.call("sendpay")
 
-    # The plugin replaces a call made for the "invoice" command
+    # Both plugins will replace calls made for the "invoice" command
+    # The first will win, for the second a warning should be logged
     invoice = l1.rpc.invoice(10**6, "test_side", "test_input")
     decoded = l1.rpc.decodepay(invoice["bolt11"])
-    assert decoded["description"] == "A plugin modified this description"
+    assert decoded["description"] == "rpc_command_1 modified this description"
+    l1.daemon.wait_for_log("rpc_command hook 'invoice' already modified, ignoring.")
 
-    # The plugin sends a custom response to "listfunds"
+    # rpc_command_1 plugin sends a custom response to "listfunds"
     funds = l1.rpc.listfunds()
-    assert funds[0] == "Custom result"
+    assert funds[0] == "Custom rpc_command_1 result"
 
     # Test command redirection to a plugin
     l1.rpc.call('help', [0])
 
-    # Test command which removes plugin itself!
-    l1.rpc.plugin_stop('rpc_command.py')
+    # Check the 'already modified' warning is not logged on just 'continue'
+    assert not l1.daemon.is_in_log("rpc_command hook 'listfunds' already modified, ignoring.")
+
+    # Tests removing a chained hook in random order.
+    # Note: This will get flaky by design if theres a problem.
+    if bool(random.getrandbits(1)):
+        l1.rpc.plugin_stop('rpc_command_2.py')
+        l1.rpc.plugin_stop('rpc_command_1.py')
+    else:
+        l1.rpc.plugin_stop('rpc_command_1.py')
+        l1.rpc.plugin_stop('rpc_command_2.py')
 
 
 def test_libplugin(node_factory):