diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 637513b9c..cf4199218 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -278,11 +278,10 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload STEALS, htlc_set_fulfill(payload->set, &payload->preimage); } -REGISTER_PLUGIN_HOOK(invoice_payment, - PLUGIN_HOOK_SINGLE, - invoice_payment_hook_cb, - invoice_payment_serialize, - struct invoice_payment_hook_payload *); +REGISTER_SINGLE_PLUGIN_HOOK(invoice_payment, + invoice_payment_hook_cb, + invoice_payment_serialize, + struct invoice_payment_hook_payload *); const struct invoice_details * invoice_check_payment(const tal_t *ctx, diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 092621a72..5ce063409 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -769,10 +769,10 @@ rpc_command_hook_callback(struct rpc_command_hook_payload *p STEALS, "Bad response to 'rpc_command' hook.")); } -REGISTER_PLUGIN_HOOK(rpc_command, PLUGIN_HOOK_SINGLE, - rpc_command_hook_callback, - rpc_command_hook_serialize, - struct rpc_command_hook_payload *); +REGISTER_SINGLE_PLUGIN_HOOK(rpc_command, + rpc_command_hook_callback, + rpc_command_hook_serialize, + struct rpc_command_hook_payload *); static void call_rpc_command_hook(struct rpc_command_hook_payload *p) { diff --git a/lightningd/onion_message.c b/lightningd/onion_message.c index 28ffb9c58..3de82b5eb 100644 --- a/lightningd/onion_message.c +++ b/lightningd/onion_message.c @@ -41,17 +41,15 @@ onion_message_serialize(struct onion_message_hook_payload *payload, } static void -onion_message_hook_cb(struct onion_message_hook_payload *payload STEALS, - const char *buffer, - const jsmntok_t *toks) +onion_message_hook_cb(struct onion_message_hook_payload *payload STEALS) { - /* The core infra checks the "result"; anything other than continue + /* plugin_hook_continue checks the "result"; anything other than continue * just stops. */ tal_free(payload); } REGISTER_PLUGIN_HOOK(onion_message, - PLUGIN_HOOK_CHAIN, + plugin_hook_continue, onion_message_hook_cb, onion_message_serialize, struct onion_message_hook_payload *); diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 69d216bc7..9224c3efb 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -826,11 +826,10 @@ static void openchannel_hook_cb(struct openchannel_hook_payload *payload STEALS, our_upfront_shutdown_script))); } -REGISTER_PLUGIN_HOOK(openchannel, - PLUGIN_HOOK_SINGLE, - openchannel_hook_cb, - openchannel_hook_serialize, - struct openchannel_hook_payload *); +REGISTER_SINGLE_PLUGIN_HOOK(openchannel, + openchannel_hook_cb, + openchannel_hook_serialize, + struct openchannel_hook_payload *); static void opening_got_offer(struct subd *openingd, const u8 *msg, diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index bd82ece86..b3e0db5b7 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -972,10 +972,10 @@ send_error: tal_free(payload); } -REGISTER_PLUGIN_HOOK(peer_connected, PLUGIN_HOOK_SINGLE, - peer_connected_hook_cb, - peer_connected_serialize, - struct peer_connected_hook_payload *); +REGISTER_SINGLE_PLUGIN_HOOK(peer_connected, + peer_connected_hook_cb, + peer_connected_serialize, + 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. */ @@ -2309,10 +2309,10 @@ 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, - custommsg_payload_serialize, - struct custommsg_payload *); +REGISTER_SINGLE_PLUGIN_HOOK(custommsg, + custommsg_callback, + custommsg_payload_serialize, + struct custommsg_payload *); void handle_custommsg_in(struct lightningd *ld, const struct node_id *peer_id, const u8 *msg) diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 8be0f3865..da4c69b4d 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -795,14 +795,6 @@ struct htlc_accepted_hook_payload { size_t failtlvpos; }; -/* The possible return value types that a plugin may return for the - * `htlc_accepted` hook. */ -enum htlc_accepted_result { - htlc_accepted_continue, - htlc_accepted_fail, - htlc_accepted_resolve, -}; - /* We only handle the simplest cases here */ static u8 *convert_failcode(const tal_t *ctx, struct lightningd *ld, @@ -835,22 +827,19 @@ static u8 *convert_failcode(const tal_t *ctx, } /** - * Parses the JSON-RPC response into a struct understood by the callback. + * Callback when a plugin answers to the htlc_accepted hook */ -static enum htlc_accepted_result -htlc_accepted_hook_deserialize(const tal_t *ctx, - struct lightningd *ld, - const char *buffer, const jsmntok_t *toks, - /* If accepted */ - struct preimage *payment_preimage, - /* If rejected (tallocated off ctx) */ - const u8 **failmsg) +static bool htlc_accepted_hook_deserialize(struct htlc_accepted_hook_payload *request, + const char *buffer, + const jsmntok_t *toks) { + struct htlc_in *hin = request->hin; + struct lightningd *ld = request->ld; + struct preimage payment_preimage; const jsmntok_t *resulttok, *paykeytok; - enum htlc_accepted_result result; if (!toks || !buffer) - return htlc_accepted_continue; + return true; resulttok = json_get_member(buffer, toks, "result"); @@ -862,18 +851,18 @@ htlc_accepted_hook_deserialize(const tal_t *ctx, } if (json_tok_streq(buffer, resulttok, "continue")) { - return htlc_accepted_continue; + return true; } if (json_tok_streq(buffer, resulttok, "fail")) { + u8 *failmsg; const jsmntok_t *failmsgtok, *failcodetok; - result = htlc_accepted_fail; failmsgtok = json_get_member(buffer, toks, "failure_message"); if (failmsgtok) { - *failmsg = json_tok_bin_from_hex(ctx, buffer, - failmsgtok); - if (!*failmsg) + failmsg = json_tok_bin_from_hex(NULL, buffer, + failmsgtok); + if (!failmsg) fatal("Bad failure_message for htlc_accepted" " hook: %.*s", failmsgtok->end - failmsgtok->start, @@ -888,11 +877,12 @@ htlc_accepted_hook_deserialize(const tal_t *ctx, failcodetok->end - failcodetok->start, buffer + failcodetok->start); - *failmsg = convert_failcode(ctx, ld, failcode); + failmsg = convert_failcode(NULL, ld, failcode); } else - *failmsg = towire_temporary_node_failure(ctx); + failmsg = towire_temporary_node_failure(NULL); + local_fail_in_htlc(hin, take(failmsg)); + return false; } else if (json_tok_streq(buffer, resulttok, "resolve")) { - result = htlc_accepted_resolve; paykeytok = json_get_member(buffer, toks, "payment_key"); if (!paykeytok) fatal( @@ -900,18 +890,16 @@ htlc_accepted_hook_deserialize(const tal_t *ctx, "value to the htlc_accepted hook: %s", json_strdup(tmpctx, buffer, resulttok)); - if (!json_to_preimage(buffer, paykeytok, - payment_preimage)) + if (!json_to_preimage(buffer, paykeytok, &payment_preimage)) fatal("Plugin specified an invalid 'payment_key': %s", json_tok_full(buffer, resulttok)); + fulfill_htlc(hin, &payment_preimage); + return false; } else { fatal("Plugin responded with an unknown result to the " "htlc_accepted hook: %s", json_strdup(tmpctx, buffer, resulttok)); } - - /* cppcheck-suppress uninitvar - false positive on fatal() above */ - return result; } static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p, @@ -976,57 +964,40 @@ 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 STEALS, - const char *buffer, const jsmntok_t *toks) +htlc_accepted_hook_final(struct htlc_accepted_hook_payload *request STEALS) { struct route_step *rs = request->route_step; struct htlc_in *hin = request->hin; struct channel *channel = request->channel; - struct lightningd *ld = request->ld; - struct preimage payment_preimage; - enum htlc_accepted_result result; - const u8 *failmsg; - result = htlc_accepted_hook_deserialize(request, ld, buffer, toks, &payment_preimage, &failmsg); - - switch (result) { - case htlc_accepted_continue: - /* *Now* we barf if it failed to decode */ - if (!request->payload) { - log_debug(channel->log, - "Failing HTLC because of an invalid payload"); - local_fail_in_htlc(hin, - take(towire_invalid_onion_payload( - NULL, request->failtlvtype, - request->failtlvpos))); - } else if (rs->nextcase == ONION_FORWARD) { - forward_htlc(hin, hin->cltv_expiry, - request->payload->amt_to_forward, - request->payload->outgoing_cltv, - request->payload->forward_channel, - serialize_onionpacket(tmpctx, rs->next), - request->next_blinding); - } else - handle_localpay(hin, - request->payload->amt_to_forward, - request->payload->outgoing_cltv, - *request->payload->total_msat, - request->payload->payment_secret); - break; - case htlc_accepted_fail: + + /* *Now* we barf if it failed to decode */ + if (!request->payload) { log_debug(channel->log, - "Failing incoming HTLC as instructed by plugin hook"); - local_fail_in_htlc(hin, take(failmsg)); - break; - case htlc_accepted_resolve: - fulfill_htlc(hin, &payment_preimage); - break; - } + "Failing HTLC because of an invalid payload"); + local_fail_in_htlc(hin, + take(towire_invalid_onion_payload( + NULL, request->failtlvtype, + request->failtlvpos))); + } else if (rs->nextcase == ONION_FORWARD) { + forward_htlc(hin, hin->cltv_expiry, + request->payload->amt_to_forward, + request->payload->outgoing_cltv, + request->payload->forward_channel, + serialize_onionpacket(tmpctx, rs->next), + request->next_blinding); + } else + handle_localpay(hin, + request->payload->amt_to_forward, + request->payload->outgoing_cltv, + *request->payload->total_msat, + request->payload->payment_secret); tal_free(request); } -REGISTER_PLUGIN_HOOK(htlc_accepted, PLUGIN_HOOK_CHAIN, - htlc_accepted_hook_callback, +REGISTER_PLUGIN_HOOK(htlc_accepted, + htlc_accepted_hook_deserialize, + htlc_accepted_hook_final, htlc_accepted_hook_serialize, struct htlc_accepted_hook_payload *); diff --git a/lightningd/plugin_hook.c b/lightningd/plugin_hook.c index 2b7593369..97b8fc248 100644 --- a/lightningd/plugin_hook.c +++ b/lightningd/plugin_hook.c @@ -139,6 +139,12 @@ static void plugin_hook_killed(struct plugin_hook_call_link *link) } } +bool plugin_hook_continue(void *unused, const char *buffer, const jsmntok_t *toks) +{ + const jsmntok_t *resrestok = json_get_member(buffer, toks, "result"); + return resrestok && json_tok_streq(buffer, resrestok, "continue"); +} + /** * Callback to be passed to the jsonrpc_request. * @@ -149,10 +155,10 @@ static void plugin_hook_callback(const char *buffer, const jsmntok_t *toks, const jsmntok_t *idtok, struct plugin_hook_request *r) { - const jsmntok_t *resulttok, *resrestok; + const jsmntok_t *resulttok; struct db *db = r->db; - bool more_plugins, cont; struct plugin_hook_call_link *last, *it; + bool in_transaction = false; if (r->ld->state == LD_STATE_SHUTDOWN) { log_debug(r->ld->log, @@ -173,40 +179,47 @@ static void plugin_hook_callback(const char *buffer, const jsmntok_t *toks, r->hook->name, toks->end - toks->start, buffer + toks->start); - resrestok = json_get_member(buffer, resulttok, "result"); + if (r->hook->type == PLUGIN_HOOK_CHAIN) { + db_begin_transaction(db); + if (!r->hook->deserialize_cb(r->cb_arg, buffer, + resulttok)) { + tal_free(r->cb_arg); + db_commit_transaction(db); + goto cleanup; + } + in_transaction = true; + } } else { - /* Buffer and / or resulttok could be used by the reponse_cb - * to identify no-result responses. So make sure both are - * set */ + /* plugin died */ resulttok = NULL; - /* cppcheck isn't smart enough to notice that `resrestok` - * doesn't need to be initialized in the expression - * initializing `cont`, so set it to NULL to shut it up. */ - resrestok = NULL; } - /* If this is a hook response containing a `continue` and we have more - * plugins queue the next call. In that case we discard the remainder - * of the result, and let the next plugin decide. */ - cont = buffer == NULL || (resrestok && json_tok_streq(buffer, resrestok, "continue")); - more_plugins = !list_empty(&r->call_chain); - if (cont && more_plugins) { + if (!list_empty(&r->call_chain)) { + if (in_transaction) + db_commit_transaction(db); plugin_hook_call_next(r); - } else { - db_begin_transaction(db); - r->hook->response_cb(r->cb_arg, buffer, resulttok); - db_commit_transaction(db); - - /* We need to remove the destructors from the remaining - * call-chain, otherwise they'd still be called when the - * plugin dies or we shut down. */ - list_for_each(&r->call_chain, it, list) { - tal_del_destructor(it, plugin_hook_killed); - tal_steal(r, it); - } + return; + } - tal_free(r); + /* We optimize for the case where we already called deserialize_cb */ + if (!in_transaction) + db_begin_transaction(db); + if (r->hook->type == PLUGIN_HOOK_CHAIN) + r->hook->final_cb(r->cb_arg); + else + r->hook->single_response_cb(r->cb_arg, buffer, resulttok); + db_commit_transaction(db); + +cleanup: + /* We need to remove the destructors from the remaining + * call-chain, otherwise they'd still be called when the + * plugin dies or we shut down. */ + list_for_each(&r->call_chain, it, list) { + tal_del_destructor(it, plugin_hook_killed); + tal_steal(r, it); } + + tal_free(r); } static void plugin_hook_call_next(struct plugin_hook_request *ph_req) @@ -258,7 +271,10 @@ 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, NULL); + if (hook->type == PLUGIN_HOOK_CHAIN) + hook->final_cb(cb_arg); + else + hook->single_response_cb(cb_arg, NULL, NULL); } } diff --git a/lightningd/plugin_hook.h b/lightningd/plugin_hook.h index 231bd4fd5..e2be6277c 100644 --- a/lightningd/plugin_hook.h +++ b/lightningd/plugin_hook.h @@ -31,15 +31,20 @@ * - `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 + * For single-plugin hooks: + * - `single_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. It must free * or otherwise take ownership of the cb_arg_type argument. * + * For chained-plugin hooks: + * - `deserialize_cb` is called for each plugin, if it returns true the + * next one is called, otherwise the cb_arg_type argument is free. + * - If all `deserialize_cb` return true, `final_cb` is called. It must free + * or otherwise take ownership of the cb_arg_type argument. * - * To make hook invocations easier, each hook registered with - * `REGISTER_PLUGIN_HOOK` provides a `plugin_hook_call_hookname` + * To make hook invocations easier, each hook provides a `plugin_hook_call_hookname` * function that performs typechecking at compile time, and makes sure * that all the provided functions for serialization, deserialization * and callback have the correct type. @@ -57,7 +62,17 @@ struct plugin_hook { * register this hook, and how the hooks are called. */ enum plugin_hook_type type; - void (*response_cb)(void *arg, const char *buffer, const jsmntok_t *toks); + /* For PLUGIN_HOOK_SINGLE hooks */ + void (*single_response_cb)(void *arg, + const char *buffer, const jsmntok_t *toks); + + /* For PLUGIN_HOOK_CHAIN hooks: */ + /* Returns false if we should stop iterating (and free arg). */ + bool (*deserialize_cb)(void *arg, + const char *buffer, const jsmntok_t *toks); + void (*final_cb)(void *arg); + + /* To send the payload to the plugin */ void (*serialize_payload)(void *src, struct json_stream *dest); /* Which plugins have registered this hook? This is a `tal_arr` @@ -72,6 +87,8 @@ AUTODATA_TYPE(hooks, struct plugin_hook); void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook, tal_t *cb_arg STEALS); +/* Generic deserialize_cb: returns true iff 'result': 'continue' */ +bool plugin_hook_continue(void *arg, const char *buffer, const jsmntok_t *toks); /* Create a small facade in from of `plugin_hook_call_` to make sure * arguments are of the correct type before downcasting them to `void @@ -94,15 +111,39 @@ 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, \ - serialize_payload, cb_arg_type) \ +#define REGISTER_SINGLE_PLUGIN_HOOK(name, response_cb, \ + serialize_payload, cb_arg_type) \ struct plugin_hook name##_hook_gen = { \ stringify(name), \ - type, \ + PLUGIN_HOOK_SINGLE, \ typesafe_cb_cast( \ void (*)(void *STEALS, const char *, const jsmntok_t *), \ void (*)(cb_arg_type STEALS, const char *, const jsmntok_t *), \ response_cb), \ + NULL, NULL, \ + typesafe_cb_cast(void (*)(void *, struct json_stream *), \ + void (*)(cb_arg_type, struct json_stream *), \ + serialize_payload), \ + NULL, /* .plugins */ \ + }; \ + AUTODATA(hooks, &name##_hook_gen); \ + PLUGIN_HOOK_CALL_DEF(name, cb_arg_type) + + +#define REGISTER_PLUGIN_HOOK(name, deserialize_cb, final_cb, \ + serialize_payload, cb_arg_type) \ + struct plugin_hook name##_hook_gen = { \ + stringify(name), \ + PLUGIN_HOOK_CHAIN, \ + NULL, \ + typesafe_cb_cast( \ + bool (*)(void *, const char *, const jsmntok_t *), \ + bool (*)(cb_arg_type, const char *, const jsmntok_t *), \ + deserialize_cb), \ + typesafe_cb_cast( \ + void (*)(void *STEALS), \ + void (*)(cb_arg_type STEALS), \ + final_cb), \ typesafe_cb_cast(void (*)(void *, struct json_stream *), \ void (*)(cb_arg_type, struct json_stream *), \ serialize_payload), \ diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 87ac163d7..40d25a6b1 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -377,6 +377,9 @@ void per_peer_state_set_fds(struct per_peer_state *pps UNNEEDED, void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED, tal_t *cb_arg UNNEEDED) { fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); } +/* Generated stub for plugin_hook_continue */ +bool plugin_hook_continue(void *arg UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *toks UNNEEDED) +{ fprintf(stderr, "plugin_hook_continue called!\n"); abort(); } /* Generated stub for subd_release_channel */ void subd_release_channel(struct subd *owner UNNEEDED, void *channel UNNEEDED) { fprintf(stderr, "subd_release_channel called!\n"); abort(); } diff --git a/lightningd/test/run-jsonrpc.c b/lightningd/test/run-jsonrpc.c index 1d7e6720f..1d62de49c 100644 --- a/lightningd/test/run-jsonrpc.c +++ b/lightningd/test/run-jsonrpc.c @@ -98,6 +98,9 @@ struct command_result *param_tok(struct command *cmd UNNEEDED, const char *name void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED, tal_t *cb_arg UNNEEDED) { fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); } +/* Generated stub for plugin_hook_continue */ +bool plugin_hook_continue(void *arg UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *toks UNNEEDED) +{ fprintf(stderr, "plugin_hook_continue called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ bool deprecated_apis; diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 6ae3f8a9c..3a12cda37 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -549,6 +549,9 @@ void per_peer_state_set_fds(struct per_peer_state *pps UNNEEDED, void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED, tal_t *cb_arg UNNEEDED) { fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); } +/* Generated stub for plugin_hook_continue */ +bool plugin_hook_continue(void *arg UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *toks UNNEEDED) +{ fprintf(stderr, "plugin_hook_continue called!\n"); abort(); } /* Generated stub for process_onionpacket */ struct route_step *process_onionpacket( const tal_t * ctx UNNEEDED,