lightningd: remove deserialize step for plugin hooks.

This seems like overkill, at least for now.  Handling the JSON
inline is clearer, for the existing examples at least.

We also remove the dummy hook, rather than fix it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/invoice.c

@@ -129,14 +129,25 @@ invoice_payment_serialize(struct invoice_payment_hook_payload *payload,
 	json_object_end(stream); /* .payment */
 }
 
-/* We cheat and return 0 (not a valid onion_type) for "OK" */
-static enum onion_type
-invoice_payment_deserialize(const tal_t *ctx, const char *buffer,
-			    const jsmntok_t *toks)
+/* Peer dies?  Remove hin ptr from payload so we know to ignore plugin return */
+static void invoice_payload_remove_hin(struct htlc_in *hin,
+				       struct invoice_payment_hook_payload *payload)
+{
+	assert(payload->hin == hin);
+	payload->hin = NULL;
+}
+
+static bool hook_gives_failcode(const char *buffer,
+				const jsmntok_t *toks,
+				enum onion_type *failcode)
 {
 	const jsmntok_t *resulttok, *t;
 	unsigned int val;
 
+	/* No plugin registered on hook at all? */
+	if (!buffer)
+		return false;
+
 	resulttok = json_get_member(buffer, toks, "result");
 	if (!resulttok)
 		fatal("Invalid invoice_payment_hook response: %.*s",
@@ -144,35 +155,30 @@ invoice_payment_deserialize(const tal_t *ctx, const char *buffer,
 
 	t = json_get_member(buffer, resulttok, "failure_code");
 	if (!t)
-		return 0;
+		return false;
+
 
 	if (!json_to_number(buffer, t, &val))
 		fatal("Invalid invoice_payment_hook failure_code: %.*s",
 		      toks[0].end - toks[1].start, buffer);
 
-	/* UPDATE isn't valid for final nodes to return, and I think we
-	 * assert elsewhere that we don't do this! */
+	/* UPDATE isn't valid for final nodes to return, and I think
+	 * we assert elsewhere that we don't do this! */
 	if (val & UPDATE)
 		fatal("Invalid invoice_payment_hook UPDATE failure_code: %.*s",
 		      toks[0].end - toks[1].start, buffer);
-
-	return val;
-}
-
-/* Peer dies?  Remove hin ptr from payload so we know to ignore plugin return */
-static void invoice_payload_remove_hin(struct htlc_in *hin,
-				       struct invoice_payment_hook_payload *payload)
-{
-	assert(payload->hin == hin);
-	payload->hin = NULL;
+	*failcode = val;
+	return true;
 }
 
 static void
 invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload,
-			enum onion_type failcode)
+			const char *buffer,
+			const jsmntok_t *toks)
 {
 	struct lightningd *ld = payload->ld;
 	struct invoice invoice;
+	enum onion_type failcode;
 
 	tal_del_destructor2(payload->hin, invoice_payload_remove_hin, payload);
 	/* We want to free this, whatever happens. */
@@ -193,7 +199,8 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload,
 		return;
 	}
 
-	if (failcode != 0) {
+	/* Did we have a hook result? */
+	if (hook_gives_failcode(buffer, toks, &failcode)) {
 		fail_htlc(payload->hin, failcode);
 		return;
 	}
@@ -209,9 +216,7 @@ REGISTER_PLUGIN_HOOK(invoice_payment,
 		     invoice_payment_hook_cb,
 		     struct invoice_payment_hook_payload *,
 		     invoice_payment_serialize,
-		     struct invoice_payment_hook_payload *,
-		     invoice_payment_deserialize,
-		     enum onion_type);
+		     struct invoice_payment_hook_payload *);
 
 void invoice_try_pay(struct lightningd *ld,
 		     struct htlc_in *hin,

lightningd/peer_control.c

@@ -418,12 +418,6 @@ void channel_errmsg(struct channel *channel,
 			       err_for_them ? "sent" : "received", desc);
 }
 
-/* Possible outcomes returned by the `connected` hook. */
-enum peer_connected_hook_result {
-	PEER_CONNECTED_CONTINUE,
-	PEER_CONNECTED_DISCONNECT,
-};
-
 struct peer_connected_hook_payload {
 	struct lightningd *ld;
 	struct crypto_state crypto_state;
@@ -434,10 +428,6 @@ struct peer_connected_hook_payload {
 	int gossip_fd;
 };
 
-struct peer_connected_hook_response {
-	enum peer_connected_hook_result result;
-};
-
 static void json_add_htlcs(struct lightningd *ld,
 			   struct json_stream *response,
 			   const struct channel *channel)
@@ -691,32 +681,10 @@ peer_connected_serialize(struct peer_connected_hook_payload *payload,
 	json_object_end(stream); /* .peer */
 }
 
-static struct peer_connected_hook_response *
-peer_connected_deserialize(const tal_t *ctx, const char *buffer,
-			   const jsmntok_t *toks)
-{
-	const jsmntok_t *resulttok;
-	struct peer_connected_hook_response *resp;
-	resulttok = json_get_member(buffer, toks, "result");
-	if (!resulttok) {
-		return NULL;
-	}
-
-	resp = tal(ctx, struct peer_connected_hook_response);
-	if (json_tok_streq(buffer, resulttok, "continue"))
-		resp->result = PEER_CONNECTED_CONTINUE;
-	else if (json_tok_streq(buffer, resulttok, "disconnect"))
-		resp->result = PEER_CONNECTED_DISCONNECT;
-	else
-		fatal("Plugin returned an invalid response to the connected "
-		      "hook: %s", buffer);
-
-	return resp;
-}
-
 static void
 peer_connected_hook_cb(struct peer_connected_hook_payload *payload,
-		       struct peer_connected_hook_response *response)
+		       const char *buffer,
+		       const jsmntok_t *toks)
 {
 	struct lightningd *ld = payload->ld;
 	struct crypto_state *cs = &payload->crypto_state;
@@ -727,10 +695,23 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload,
 	int peer_fd = payload->peer_fd;
 	u8 *error;
 
-	if (response && response->result == PEER_CONNECTED_DISCONNECT) {
-		close(peer_fd);
-		tal_free(payload);
-		return;
+	/* If we had a hook, interpret result. */
+	if (buffer) {
+		const jsmntok_t *resulttok;
+
+		resulttok = json_get_member(buffer, toks, "result");
+		if (!resulttok) {
+			fatal("Plugin returned an invalid response to the connected "
+			      "hook: %s", buffer);
+		}
+
+		if (json_tok_streq(buffer, resulttok, "disconnect")) {
+			close(peer_fd);
+			tal_free(payload);
+			return;
+		} else if (!json_tok_streq(buffer, resulttok, "continue"))
+			fatal("Plugin returned an invalid response to the connected "
+			      "hook: %s", buffer);
 	}
 
 	if (channel) {
@@ -810,9 +791,7 @@ send_error:
 REGISTER_PLUGIN_HOOK(peer_connected, peer_connected_hook_cb,
 		     struct peer_connected_hook_payload *,
 		     peer_connected_serialize,
-		     struct peer_connected_hook_payload *,
-		     peer_connected_deserialize,
-		     struct peer_connected_hook_response *);
+		     struct peer_connected_hook_payload *);
 
 /* Connectd tells us a peer has connected: it never hands us duplicates, since
  * it holds them until we say peer_died. */

lightningd/plugin_hook.c

@@ -39,9 +39,6 @@ bool plugin_hook_register(struct plugin *plugin, const char *method)
 	return true;
 }
 
-/* FIXME(cdecker): Remove dummy hook, once we have a real one */
-REGISTER_PLUGIN_HOOK(hello, NULL, void *, NULL, void *, NULL, void *);
-
 /**
  * Callback to be passed to the jsonrpc_request.
  *
@@ -53,16 +50,14 @@ static void plugin_hook_callback(const char *buffer, const jsmntok_t *toks,
 				 struct plugin_hook_request *r)
 {
 	const jsmntok_t *resulttok = json_get_member(buffer, toks, "result");
-	void *response;
 
 	if (!resulttok)
 		fatal("Plugin for %s returned non-result response %.*s",
 		      r->hook->name,
 		      toks->end - toks->start, buffer + toks->end);
 
-	response = r->hook->deserialize_response(r, buffer, resulttok);
 	db_begin_transaction(r->db);
-	r->hook->response_cb(r->cb_arg, response);
+	r->hook->response_cb(r->cb_arg, buffer, resulttok);
 	db_commit_transaction(r->db);
 	tal_free(r);
 }
@@ -94,7 +89,7 @@ void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
 		 * roundtrip to the serializer and deserializer. If we
 		 * were expecting a default response it should have
 		 * been part of the `cb_arg`. */
-		hook->response_cb(cb_arg, NULL);
+		hook->response_cb(cb_arg, NULL, NULL);
 	}
 }
 
@@ -102,7 +97,7 @@ void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
  * annoying, and to make it clear that it's totally synchronous. */
 
 /* Special synchronous hook for db */
-static struct plugin_hook db_write_hook = { "db_write", NULL, NULL, NULL, NULL };
+static struct plugin_hook db_write_hook = { "db_write", NULL, NULL, NULL };
 AUTODATA(hooks, &db_write_hook);
 
 static void db_hook_response(const char *buffer, const jsmntok_t *toks,

lightningd/plugin_hook.h

@@ -31,13 +31,10 @@
  * - `serialize_payload` which takes a payload of type `payload_type`
  *   and serializes it into the given `json_stream`. `
  *
- * - `deserialize_response` takes a `json_stream` and parses it into a
- *   new struct of type `response_type`,
- *
- * - `response_cb` is called once the plugin has responded and the
- *   response has been parsed by `deserialize_response`. In addition
- *   an arbitrary additional argument of type `cb_arg_type` can be
- *   passed along that may contain any additional context necessary.
+ * - `response_cb` is called once the plugin has responded (or with
+ *   buffer == NULL if there's no plugin).  In addition an arbitrary
+ *   additional argument of type `cb_arg_type` can be passed along
+ *   that may contain any additional context necessary.
  *
  *
  * To make hook invocations easier, each hook registered with
@@ -49,10 +46,8 @@
 
 struct plugin_hook {
 	const char *name;
-	void (*response_cb)(void *arg, void *response);
+	void (*response_cb)(void *arg, const char *buffer, const jsmntok_t *toks);
 	void (*serialize_payload)(void *src, struct json_stream *dest);
-	void *(*deserialize_response)(const tal_t *, const char *buffer,
-				      const jsmntok_t *toks);
 
 	/* Which plugin has registered this hook? */
 	struct plugin *plugin;
@@ -90,21 +85,16 @@ void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
  * an arbitrary extra argument used to maintain context.
  */
 #define REGISTER_PLUGIN_HOOK(name, response_cb, response_cb_arg_type,          \
-			     serialize_payload, payload_type,                  \
-			     deserialize_response, response_type)              \
+			     serialize_payload, payload_type)                  \
 	struct plugin_hook name##_hook_gen = {                                 \
 	    stringify(name),                                                   \
-	    typesafe_cb_cast(void (*)(void *, void *),                         \
-			     void (*)(response_cb_arg_type, response_type),    \
+	    typesafe_cb_cast(void (*)(void *, const char *, const jsmntok_t *),\
+			     void (*)(response_cb_arg_type,		       \
+				      const char *, const jsmntok_t *),	       \
 			     response_cb),                                     \
 	    typesafe_cb_cast(void (*)(void *, struct json_stream *),           \
 			     void (*)(payload_type, struct json_stream *),     \
 			     serialize_payload),                               \
-	    typesafe_cb_cast(                                                  \
-		void *(*)(const tal_t *, const char *, const jsmntok_t *),     \
-		response_type (*)(const tal_t *, const char *,                 \
-				  const jsmntok_t *),                          \
-		deserialize_response),                                         \
 	    NULL, /* .plugin */                                                \
 	};                                                                     \
 	AUTODATA(hooks, &name##_hook_gen);                                     \