lightningd/: Hooks now support a consistent interface for 'no operation'.

Changelog-Changed: The hooks `db_write`, `invoice_payment`, and `rpc_command` now accept `{ "result": "continue" }` to mean "do default action", in addition to `true` (`db_write`), `{}` (`invoice_payment`), and `{"continue": true}` (`rpc_command`). The older "default" indicators are now deprecated and are now recognized only if `--deprecated-apis` is set.

doc/PLUGINS.md

@@ -473,6 +473,10 @@ Hooks are considered to be an advanced feature due to the fact that
 carefully, and make sure your plugins always return a valid response
 to any hook invocation.
 
+As a convention, for all hooks, returning the object
+`{ "result" : "continue" }` results in `lightningd` behaving exactly as if
+no plugin is registered on the hook.
+
 ### Hook Types
 
 #### `peer_connected`
@@ -571,7 +575,8 @@ and:
 The "rolling up" of the database could be done periodically as well
 if the log of SQL statements has grown large.
 
-Any response but "true" will cause lightningd to error without
+Any response other than `{"result": "continue"}` will cause lightningd
+to error without
 committing to the database!
 This is the expected way to halt and catch fire.
 
@@ -592,8 +597,9 @@ This hook is called whenever a valid payment for an unpaid invoice has arrived.
 The hook is sparse on purpose, since the plugin can use the JSON-RPC
 `listinvoices` command to get additional details about this invoice.
 It can return a non-zero `failure_code` field as defined for final
-nodes in [BOLT 4][bolt4-failure-codes], or otherwise an empty object
-to accept the payment.
+nodes in [BOLT 4][bolt4-failure-codes], a `result` field with the string
+`reject` to fail it with `incorrect_or_unknown_payment_details`, or a
+`result` field with the string `continue` to accept the payment.
 
 
 #### `openchannel`
@@ -769,7 +775,7 @@ Let `lightningd` execute the command with
 
 ```json
 {
-    "continue": true
+    "result" : "continue"
 }
 ```
 Replace the request made to `lightningd`:
@@ -839,7 +845,8 @@ details). The plugin must implement the parsing of the message, including the
 type prefix, since c-lightning does not know how to parse the message.
 
 The result for this hook is currently being discarded. For future uses of the
-result we suggest just returning a `null`. This will ensure backward
+result we suggest just returning `{'result': 'continue'}`.
+This will ensure backward
 compatibility should the semantics be changed in future.
 
 

lightningd/invoice.c

@@ -9,6 +9,7 @@
 #include <common/amount.h>
 #include <common/bech32.h>
 #include <common/bolt11.h>
+#include <common/configdir.h>
 #include <common/features.h>
 #include <common/json_command.h>
 #include <common/json_helpers.h>
@@ -161,10 +162,12 @@ static void invoice_payload_remove_set(struct htlc_set *set,
 	payload->set = NULL;
 }
 
-static bool hook_gives_failcode(const char *buffer,
+static bool hook_gives_failcode(struct log *log,
+				const char *buffer,
 				const jsmntok_t *toks,
 				enum onion_type *failcode)
 {
+	const jsmntok_t *resulttok;
 	const jsmntok_t *t;
 	unsigned int val;
 
@@ -172,9 +175,39 @@ static bool hook_gives_failcode(const char *buffer,
 	if (!buffer)
 		return false;
 
+	resulttok = json_get_member(buffer, toks, "result");
+	if (resulttok) {
+		if (json_tok_streq(buffer, resulttok, "continue")) {
+			return false;
+		} else if (json_tok_streq(buffer, resulttok, "reject")) {
+			*failcode = WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS;
+			return true;
+		} else
+			fatal("Invalid invoice_payment hook result: %.*s",
+			      toks[0].end - toks[0].start, buffer);
+	}
+
 	t = json_get_member(buffer, toks, "failure_code");
-	if (!t)
+#ifdef COMPAT_V080
+	if (!t && deprecated_apis) {
+		static bool warned = false;
+		if (!warned) {
+			warned = true;
+			log_unusual(log,
+				    "Plugin did not return object with "
+				    "'result' or 'failure_code' fields.  "
+				    "This is now deprecated and you should "
+				    "return {'result': 'continue' } or "
+				    "{'result': 'reject'} or "
+				    "{'failure_code': 42} instead.");
+		}
 		return false;
+	}
+#endif /* defined(COMPAT_V080) */
+	if (!t)
+		fatal("Invalid invoice_payment_hook response, expecting "
+		      "'result' or 'failure_code' field: %.*s",
+		      toks[0].end - toks[0].start, buffer);
 
 	if (!json_to_number(buffer, t, &val))
 		fatal("Invalid invoice_payment_hook failure_code: %.*s",
@@ -222,7 +255,7 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload,
 	}
 
 	/* Did we have a hook result? */
-	if (hook_gives_failcode(buffer, toks, &failcode)) {
+	if (hook_gives_failcode(ld->log, buffer, toks, &failcode)) {
 		htlc_set_fail(payload->set, failcode);
 		return;
 	}

lightningd/jsonrpc.c

@@ -667,9 +667,9 @@ static void
 rpc_command_hook_callback(struct rpc_command_hook_payload *p,
                           const char *buffer, const jsmntok_t *resulttok)
 {
-	const jsmntok_t *tok, *params, *custom_return, *tok_continue;
+	const jsmntok_t *tok, *params, *custom_return;
+	const jsmntok_t *innerresulttok;
 	struct json_stream *response;
-	bool exec;
 
 	params = json_get_member(p->buffer, p->request, "params");
 
@@ -678,12 +678,38 @@ rpc_command_hook_callback(struct rpc_command_hook_payload *p,
 	if (buffer == NULL)
 	    return was_pending(command_exec(p->cmd->jcon, p->cmd, p->buffer,
 		                                p->request, params));
-	else {
+
+#ifdef COMPAT_V080
+	if (deprecated_apis) {
+		const jsmntok_t *tok_continue;
+		bool exec;
 		tok_continue = json_get_member(buffer, resulttok, "continue");
-		if (tok_continue && json_to_bool(buffer, tok_continue, &exec) && exec)
+		if (tok_continue && json_to_bool(buffer, tok_continue, &exec) && exec) {
+			static bool warned = false;
+			if (!warned) {
+				warned = true;
+				log_unusual(p->cmd->ld->log,
+					    "Plugin returned 'continue' : true "
+					    "to rpc_command hook.  "
+					    "This is now deprecated and "
+					    "you should return with "
+					    "{'result': 'continue'} instead.");
+			}
 			return was_pending(command_exec(p->cmd->jcon, p->cmd, p->buffer,
 			                                p->request, params));
 		}
+	}
+#endif /* defined(COMPAT_V080) */
+
+	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));
+		}
+		return was_pending(command_fail(p->cmd, JSONRPC2_INVALID_REQUEST,
+						"Bad 'result' to 'rpc_command' hook."));
+	}
 
 	/* If the registered plugin did not respond with continue,
 	 * it wants either to replace the request... */

lightningd/plugin_hook.c

@@ -1,8 +1,10 @@
 #include <ccan/io/io.h>
+#include <common/configdir.h>
 #include <common/memleak.h>
 #include <lightningd/jsonrpc.h>
 #include <lightningd/plugin_hook.h>
 #include <wallet/db.h>
+#include <wallet/db_common.h>
 
 /* Struct containing all the information needed to deserialize and
  * dispatch an eventual plugin_hook response. */
@@ -136,21 +138,49 @@ static void db_hook_response(const char *buffer, const jsmntok_t *toks,
 			     struct plugin_hook_request *ph_req)
 {
 	const jsmntok_t *resulttok;
-	bool resp;
 
 	resulttok = json_get_member(buffer, toks, "result");
 	if (!resulttok)
 		fatal("Plugin returned an invalid response to the db_write "
 		      "hook: %s", buffer);
 
-	/* We expect result: True.  Anything else we abort. */
-	if (!json_to_bool(buffer, resulttok, &resp))
-		fatal("Plugin returned an invalid result to the db_write "
-		      "hook: %s", buffer);
-
+#ifdef COMPAT_V080
+	/* For back-compatibility we allow to return a simple Boolean true.  */
+	if (deprecated_apis) {
+		bool resp;
+		if (json_to_bool(buffer, resulttok, &resp)) {
+			static bool warned = false;
 			/* If it fails, we must not commit to our db. */
 			if (!resp)
+				fatal("Plugin returned failed db_write: %s.",
+				      buffer);
+			if (!warned) {
+				warned = true;
+				log_unusual(ph_req->db->log,
+					    "Plugin returned 'true' to "
+					    "'db_hook'.  "
+					    "This is now deprecated and "
+					    "you should return "
+					    "{'result': 'continue'} "
+					    "instead.");
+			}
+			/* Resume.  */
+			io_break(ph_req);
+			return;
+		}
+	}
+#endif /* defined(COMPAT_V080) */
+
+	/* We expect result: { 'result' : 'continue' }.
+	 * Anything else we abort.
+	 */
+	resulttok = json_get_member(buffer, resulttok, "result");
+	if (resulttok) {
+		if (!json_tok_streq(buffer, resulttok, "continue"))
 			fatal("Plugin returned failed db_write: %s.", buffer);
+	} else
+		fatal("Plugin returned an invalid result to the db_write "
+		      "hook: %s", buffer);
 
 	/* We're done, exit exclusive loop. */
 	io_break(ph_req);