Skip to content

Commit

Permalink
plugin: simplify hooks calling methods, and make lifetime requirement…
Browse files Browse the repository at this point in the history
…s explicit.

They callback must take ownership of the payload (almost all do, but
now it's explicit).

And since the payload and cb_arg arguments to plugin_hook_call_() are
always identical, make them a single parameter.

Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
rustyrussell committed Apr 16, 2020
1 parent 1e34d89 commit 9aedb0c
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 49 deletions.
7 changes: 3 additions & 4 deletions lightningd/invoice.c
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ static const u8 *hook_gives_failmsg(const tal_t *ctx,
}

static void
invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload,
invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload STEALS,
const char *buffer,
const jsmntok_t *toks)
{
Expand Down Expand Up @@ -281,7 +281,6 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload,
REGISTER_PLUGIN_HOOK(invoice_payment,
PLUGIN_HOOK_SINGLE,
invoice_payment_hook_cb,
struct invoice_payment_hook_payload *,
invoice_payment_serialize,
struct invoice_payment_hook_payload *);

Expand Down Expand Up @@ -360,15 +359,15 @@ void invoice_try_pay(struct lightningd *ld,
{
struct invoice_payment_hook_payload *payload;

payload = tal(ld, struct invoice_payment_hook_payload);
payload = tal(NULL, struct invoice_payment_hook_payload);
payload->ld = ld;
payload->label = tal_steal(payload, details->label);
payload->msat = set->so_far;
payload->preimage = details->r;
payload->set = set;
tal_add_destructor2(set, invoice_payload_remove_set, payload);

plugin_hook_call_invoice_payment(ld, payload, payload);
plugin_hook_call_invoice_payment(ld, payload);
}

static bool hsm_sign_b11(const u5 *u5bytes,
Expand Down
11 changes: 7 additions & 4 deletions lightningd/jsonrpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <common/memleak.h>
#include <common/param.h>
#include <common/timeout.h>
#include <common/utils.h>
#include <common/version.h>
#include <common/wireaddr.h>
#include <errno.h>
Expand Down Expand Up @@ -676,13 +677,16 @@ static void replace_command(struct rpc_command_hook_payload *p,
}

static void
rpc_command_hook_callback(struct rpc_command_hook_payload *p,
rpc_command_hook_callback(struct rpc_command_hook_payload *p STEALS,
const char *buffer, const jsmntok_t *resulttok)
{
const jsmntok_t *tok, *params, *custom_return;
const jsmntok_t *innerresulttok;
struct json_stream *response;

/* Free payload with cmd */
tal_steal(p->cmd, p);

params = json_get_member(p->buffer, p->request, "params");

/* If no plugin registered, just continue command execution. Same if
Expand Down Expand Up @@ -767,13 +771,12 @@ rpc_command_hook_callback(struct rpc_command_hook_payload *p,

REGISTER_PLUGIN_HOOK(rpc_command, PLUGIN_HOOK_SINGLE,
rpc_command_hook_callback,
struct rpc_command_hook_payload *,
rpc_command_hook_serialize,
struct rpc_command_hook_payload *);

static void call_rpc_command_hook(struct rpc_command_hook_payload *p)
{
plugin_hook_call_rpc_command(p->cmd->ld, p, p);
plugin_hook_call_rpc_command(p->cmd->ld, p);
}

/* We return struct command_result so command_fail return value has a natural
Expand Down Expand Up @@ -842,7 +845,7 @@ parse_request(struct json_connection *jcon, const jsmntok_t tok[])
jcon->buffer + method->start);
}

rpc_hook = tal(c, struct rpc_command_hook_payload);
rpc_hook = tal(NULL, struct rpc_command_hook_payload);
rpc_hook->cmd = c;
/* Duplicate since we might outlive the connection */
rpc_hook->buffer = tal_dup_talarr(rpc_hook, char, jcon->buffer);
Expand Down
5 changes: 2 additions & 3 deletions lightningd/onion_message.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ onion_message_serialize(struct onion_message_hook_payload *payload,
}

static void
onion_message_hook_cb(struct onion_message_hook_payload *payload,
onion_message_hook_cb(struct onion_message_hook_payload *payload STEALS,
const char *buffer,
const jsmntok_t *toks)
{
Expand All @@ -53,7 +53,6 @@ onion_message_hook_cb(struct onion_message_hook_payload *payload,
REGISTER_PLUGIN_HOOK(onion_message,
PLUGIN_HOOK_CHAIN,
onion_message_hook_cb,
struct onion_message_hook_payload *,
onion_message_serialize,
struct onion_message_hook_payload *);

Expand Down Expand Up @@ -112,7 +111,7 @@ void handle_onionmsg_to_us(struct channel *channel, const u8 *msg)
log_debug(channel->log, "Got onionmsg%s%s",
payload->reply_blinding ? " reply_blinding": "",
payload->reply_path ? " reply_path": "");
plugin_hook_call_onion_message(ld, payload, payload);
plugin_hook_call_onion_message(ld, payload);
}

void handle_onionmsg_forward(struct channel *channel, const u8 *msg)
Expand Down
7 changes: 3 additions & 4 deletions lightningd/opening_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ static void openchannel_payload_remove_openingd(struct subd *openingd,
payload->openingd = NULL;
}

static void openchannel_hook_cb(struct openchannel_hook_payload *payload,
static void openchannel_hook_cb(struct openchannel_hook_payload *payload STEALS,
const char *buffer,
const jsmntok_t *toks)
{
Expand Down Expand Up @@ -829,7 +829,6 @@ static void openchannel_hook_cb(struct openchannel_hook_payload *payload,
REGISTER_PLUGIN_HOOK(openchannel,
PLUGIN_HOOK_SINGLE,
openchannel_hook_cb,
struct openchannel_hook_payload *,
openchannel_hook_serialize,
struct openchannel_hook_payload *);

Expand All @@ -847,7 +846,7 @@ static void opening_got_offer(struct subd *openingd,
return;
}

payload = tal(openingd->ld, struct openchannel_hook_payload);
payload = tal(openingd, struct openchannel_hook_payload);
payload->openingd = openingd;
if (!fromwire_opening_got_offer(payload, msg,
&payload->funding_satoshis,
Expand All @@ -868,7 +867,7 @@ static void opening_got_offer(struct subd *openingd,
}

tal_add_destructor2(openingd, openchannel_payload_remove_openingd, payload);
plugin_hook_call_openchannel(openingd->ld, payload, payload);
plugin_hook_call_openchannel(openingd->ld, payload);
}

static unsigned int openingd_msg(struct subd *openingd,
Expand Down
14 changes: 7 additions & 7 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ peer_connected_serialize(struct peer_connected_hook_payload *payload,
}

static void
peer_connected_hook_cb(struct peer_connected_hook_payload *payload,
peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
const char *buffer,
const jsmntok_t *toks)
{
Expand Down Expand Up @@ -974,7 +974,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload,

REGISTER_PLUGIN_HOOK(peer_connected, PLUGIN_HOOK_SINGLE,
peer_connected_hook_cb,
struct peer_connected_hook_payload *,
peer_connected_serialize,
struct peer_connected_hook_payload *);

Expand Down Expand Up @@ -1018,7 +1017,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg,
assert(!peer->uncommitted_channel);
hook_payload->channel = peer_active_channel(peer);

plugin_hook_call_peer_connected(ld, hook_payload, hook_payload);
plugin_hook_call_peer_connected(ld, hook_payload);
}

/* FIXME: Unify our watch code so we get notified by txout, instead, like
Expand Down Expand Up @@ -2297,7 +2296,7 @@ struct custommsg_payload {
const u8 *msg;
};

static void custommsg_callback(struct custommsg_payload *payload,
static void custommsg_callback(struct custommsg_payload *payload STEALS,
const char *buffer, const jsmntok_t *toks)
{
tal_free(payload);
Expand All @@ -2310,8 +2309,9 @@ static void custommsg_payload_serialize(struct custommsg_payload *payload,
json_add_node_id(stream, "peer_id", &payload->peer_id);
}

REGISTER_PLUGIN_HOOK(custommsg, PLUGIN_HOOK_SINGLE, custommsg_callback,
struct custommsg_payload *, custommsg_payload_serialize,
REGISTER_PLUGIN_HOOK(custommsg, PLUGIN_HOOK_SINGLE,
custommsg_callback,
custommsg_payload_serialize,
struct custommsg_payload *);

void handle_custommsg_in(struct lightningd *ld, const struct node_id *peer_id,
Expand All @@ -2329,7 +2329,7 @@ void handle_custommsg_in(struct lightningd *ld, const struct node_id *peer_id,

p->peer_id = *peer_id;
p->msg = tal_steal(p, custommsg);
plugin_hook_call_custommsg(ld, p, p);
plugin_hook_call_custommsg(ld, p);
}

static struct command_result *json_sendcustommsg(struct command *cmd,
Expand Down
7 changes: 3 additions & 4 deletions lightningd/peer_htlcs.c
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,7 @@ static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p,
* Callback when a plugin answers to the htlc_accepted hook
*/
static void
htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request,
htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request STEALS,
const char *buffer, const jsmntok_t *toks)
{
struct route_step *rs = request->route_step;
Expand Down Expand Up @@ -1027,7 +1027,6 @@ htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request,

REGISTER_PLUGIN_HOOK(htlc_accepted, PLUGIN_HOOK_CHAIN,
htlc_accepted_hook_callback,
struct htlc_accepted_hook_payload *,
htlc_accepted_hook_serialize,
struct htlc_accepted_hook_payload *);

Expand Down Expand Up @@ -1160,7 +1159,7 @@ static bool peer_accepted_htlc(const tal_t *ctx,
goto fail;
}

hook_payload = tal(hin, struct htlc_accepted_hook_payload);
hook_payload = tal(NULL, struct htlc_accepted_hook_payload);

hook_payload->route_step = tal_steal(hook_payload, rs);
hook_payload->payload = onion_decode(hook_payload, rs,
Expand All @@ -1186,7 +1185,7 @@ static bool peer_accepted_htlc(const tal_t *ctx,
#endif
hook_payload->next_blinding = NULL;

plugin_hook_call_htlc_accepted(ld, hook_payload, hook_payload);
plugin_hook_call_htlc_accepted(ld, hook_payload);

/* Falling through here is ok, after all the HTLC locked */
return true;
Expand Down
8 changes: 3 additions & 5 deletions lightningd/plugin_hook.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ struct plugin_hook_request {
struct plugin *plugin;
const struct plugin_hook *hook;
void *cb_arg;
void *payload;
struct db *db;
struct lightningd *ld;
};
Expand Down Expand Up @@ -221,13 +220,13 @@ static void plugin_hook_call_next(struct plugin_hook_request *ph_req)
plugin_get_log(ph_req->plugin),
plugin_hook_callback, ph_req);

hook->serialize_payload(ph_req->payload, req->stream);
hook->serialize_payload(ph_req->cb_arg, req->stream);
jsonrpc_request_end(req);
plugin_request_send(ph_req->plugin, req);
}

void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
void *payload, void *cb_arg)
tal_t *cb_arg STEALS)
{
struct plugin_hook_request *ph_req;
struct plugin_hook_call_link *link;
Expand All @@ -239,9 +238,8 @@ void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
* to eventually to inspect in-flight requests. */
ph_req = notleak(tal(hook->plugins, struct plugin_hook_request));
ph_req->hook = hook;
ph_req->cb_arg = cb_arg;
ph_req->cb_arg = tal_steal(ph_req, cb_arg);
ph_req->db = ld->wallet->db;
ph_req->payload = tal_steal(ph_req, payload);
ph_req->ld = ld;

list_head_init(&ph_req->call_chain);
Expand Down
28 changes: 13 additions & 15 deletions lightningd/plugin_hook.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@
* delivery to the plugin, and from a JSON-object to the internal
* representation:
*
* - `serialize_payload` which takes a payload of type `payload_type`
* - `serialize_payload` which takes a payload of type `cb_arg_type`
* and serializes it into the given `json_stream`. `
*
* - `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.
* that may contain any additional context necessary. It must free
* or otherwise take ownership of the cb_arg_type argument.
*
*
* To make hook invocations easier, each hook registered with
Expand Down Expand Up @@ -69,7 +70,7 @@ AUTODATA_TYPE(hooks, struct plugin_hook);
* wrappers generated by the `PLUGIN_HOOK_REGISTER` macro.
*/
void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
void *payload, void *cb_arg);
tal_t *cb_arg STEALS);


/* Create a small facade in from of `plugin_hook_call_` to make sure
Expand All @@ -78,13 +79,11 @@ void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
* the method-name is correct for the call.
*/
/* FIXME: Find a way to avoid back-to-back declaration and definition */
#define PLUGIN_HOOK_CALL_DEF(name, payload_type, response_cb_arg_type) \
#define PLUGIN_HOOK_CALL_DEF(name, cb_arg_type) \
UNNEEDED static inline void plugin_hook_call_##name( \
struct lightningd *ld, payload_type payload, \
response_cb_arg_type cb_arg) \
struct lightningd *ld, cb_arg_type cb_arg STEALS) \
{ \
plugin_hook_call_(ld, &name##_hook_gen, (void *)payload, \
(void *)cb_arg); \
plugin_hook_call_(ld, &name##_hook_gen, cb_arg); \
}

/* Typechecked registration of a plugin hook. We check that the
Expand All @@ -95,23 +94,22 @@ void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
* response_cb function accepts the deserialized response format and
* an arbitrary extra argument used to maintain context.
*/
#define REGISTER_PLUGIN_HOOK(name, type, response_cb, response_cb_arg_type, \
serialize_payload, payload_type) \
#define REGISTER_PLUGIN_HOOK(name, type, response_cb, \
serialize_payload, cb_arg_type) \
struct plugin_hook name##_hook_gen = { \
stringify(name), \
type, \
typesafe_cb_cast( \
void (*)(void *, const char *, const jsmntok_t *), \
void (*)(response_cb_arg_type, const char *, \
const jsmntok_t *), \
void (*)(void *STEALS, const char *, const jsmntok_t *), \
void (*)(cb_arg_type STEALS, const char *, const jsmntok_t *), \
response_cb), \
typesafe_cb_cast(void (*)(void *, struct json_stream *), \
void (*)(payload_type, struct json_stream *), \
void (*)(cb_arg_type, struct json_stream *), \
serialize_payload), \
NULL, /* .plugins */ \
}; \
AUTODATA(hooks, &name##_hook_gen); \
PLUGIN_HOOK_CALL_DEF(name, payload_type, response_cb_arg_type);
PLUGIN_HOOK_CALL_DEF(name, cb_arg_type)

bool plugin_hook_register(struct plugin *plugin, const char *method);

Expand Down
2 changes: 1 addition & 1 deletion lightningd/test/run-invoice-select-inchan.c
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ void per_peer_state_set_fds(struct per_peer_state *pps UNNEEDED,
{ fprintf(stderr, "per_peer_state_set_fds called!\n"); abort(); }
/* Generated stub for plugin_hook_call_ */
void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
void *payload UNNEEDED, void *cb_arg UNNEEDED)
tal_t *cb_arg UNNEEDED)
{ fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); }
/* Generated stub for subd_release_channel */
void subd_release_channel(struct subd *owner UNNEEDED, void *channel UNNEEDED)
Expand Down
2 changes: 1 addition & 1 deletion lightningd/test/run-jsonrpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ struct command_result *param_tok(struct command *cmd UNNEEDED, const char *name
{ fprintf(stderr, "param_tok called!\n"); abort(); }
/* Generated stub for plugin_hook_call_ */
void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
void *payload UNNEEDED, void *cb_arg UNNEEDED)
tal_t *cb_arg UNNEEDED)
{ fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); }
/* AUTOGENERATED MOCKS END */

Expand Down
2 changes: 1 addition & 1 deletion wallet/test/run-wallet.c
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ void per_peer_state_set_fds(struct per_peer_state *pps UNNEEDED,
{ fprintf(stderr, "per_peer_state_set_fds called!\n"); abort(); }
/* Generated stub for plugin_hook_call_ */
void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
void *payload UNNEEDED, void *cb_arg UNNEEDED)
tal_t *cb_arg UNNEEDED)
{ fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); }
/* Generated stub for process_onionpacket */
struct route_step *process_onionpacket(
Expand Down

0 comments on commit 9aedb0c

Please sign in to comment.