diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index 25fc3ad42..af0a37769 100644 --- a/doc/PLUGINS.md +++ b/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. diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 2c03f12aa..dde292477 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -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; } diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index bde211189..d2f896904 100644 --- a/lightningd/jsonrpc.c +++ b/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,11 +678,37 @@ 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, diff --git a/lightningd/plugin_hook.c b/lightningd/plugin_hook.c index 271389c49..064735e28 100644 --- a/lightningd/plugin_hook.c +++ b/lightningd/plugin_hook.c @@ -1,8 +1,10 @@ #include +#include #include #include #include #include +#include /* Struct containing all the information needed to deserialize and * dispatch an eventual plugin_hook response. */ @@ -136,22 +138,50 @@ 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)) +#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); - /* 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); }