lightningd/: Hooks now support a consistent interface for 'no operati…

…on'.

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`). Eventually the older "default" indicators will be deprecated, but for now they will be supported, but logged as `UNUSUAL` level.

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, returning the object `{ "result" : "continue" }`
+results in `lightningd` behaving exactly as if no plugin is
+registered on the hook.
+
 ### Hook Types
 
 #### `peer_connected`
@@ -522,7 +526,8 @@ It is currently extremely restricted:
 }
 ```
 
-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!
 
 #### `invoice_payment`
@@ -542,8 +547,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`
@@ -719,7 +725,7 @@ Let `lightningd` execute the command with
 
 ```json
 {
-    "continue": true
+    "result" : "continue"
 }
 ```
 Replace the request made to `lightningd`:
@@ -789,7 +795,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

@@ -162,20 +162,47 @@ 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;
 
 	/* No plugin registered on hook at all? */
 	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)
+	if (!t) {
+		/* FIXME: In the future, fatal if not 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;
+	}
 
 	if (!json_to_number(buffer, t, &val))
 		fatal("Invalid invoice_payment_hook failure_code: %.*s",
@@ -223,7 +250,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

@@ -668,6 +668,7 @@ 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 *innerresulttok;
 	struct json_stream *response;
 	bool exec;
 
@@ -679,10 +680,26 @@ rpc_command_hook_callback(struct rpc_command_hook_payload *p,
 	    return was_pending(command_exec(p->cmd->jcon, p->cmd, p->buffer,
 		                                p->request, params));
 	else {
+		/* FIXME: In the future disable this path if !deprected_apis.  */
 		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));
+		}
+		innerresulttok = json_get_member(buffer, resulttok, "result");
+		if (innerresulttok &&
+		    json_tok_streq(buffer, innerresulttok, "continue")) {
+		}
 	}
 
 	/* If the registered plugin did not respond with continue,

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. */
@@ -143,15 +145,29 @@ static void db_hook_response(const char *buffer, const jsmntok_t *toks,
 		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))
+	/* We expect result: True or result: { result: 'continue' }.
+	 * Anything else we abort.
+	 * FIXME: In the future, disable this path with deprecated_apis.
+	 */
+	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.");
+		}
+	} else if ((resulttok = json_get_member(buffer, resulttok, "result"))) {
+		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);
 
-	/* If it fails, we must not commit to our db. */
-	if (!resp)
-		fatal("Plugin returned failed db_write: %s.", buffer);
-
 	/* We're done, exit exclusive loop. */
 	io_break(ph_req);
 }