From ff0bfba9af48a8bf9d78ae3f55087e802eacf9f3 Mon Sep 17 00:00:00 2001 From: darosior Date: Tue, 28 Jan 2020 12:28:25 +0100 Subject: [PATCH 01/10] libplugin: expose helpers to start and end a jsonrpc response The usual json_stream starters and command_result enders. --- plugins/libplugin.c | 40 ++++++++++++++++++++++++++++++++++++++++ plugins/libplugin.h | 19 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 6b8aacfd55e6..0a88babe1a9c 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -150,6 +150,37 @@ static struct json_stream *jsonrpc_stream_start(struct command *cmd) return js; } +struct json_stream *jsonrpc_stream_success(struct command *cmd) +{ + struct json_stream *js = jsonrpc_stream_start(cmd); + + json_object_start(js, "result"); + return js; +} + +struct json_stream *jsonrpc_stream_fail(struct command *cmd, + int code, + const char *err) +{ + struct json_stream *js = jsonrpc_stream_start(cmd); + + json_object_start(js, "error"); + json_add_member(js, "code", false, "%d", code); + json_add_string(js, "message", err); + + return js; +} + +struct json_stream *jsonrpc_stream_fail_data(struct command *cmd, + int code, + const char *err) +{ + struct json_stream *js = jsonrpc_stream_fail(cmd, code, err); + + json_object_start(js, "data"); + return js; +} + static struct command_result *command_complete(struct command *cmd, struct json_stream *result) { @@ -162,6 +193,15 @@ static struct command_result *command_complete(struct command *cmd, return &complete; } +struct command_result *WARN_UNUSED_RESULT +command_finished(struct command *cmd, struct json_stream *response) +{ + /* "result" or "error" object */ + json_object_end(response); + + return command_complete(cmd, response); +} + struct json_out *json_out_obj(const tal_t *ctx, const char *fieldname, const char *str) diff --git a/plugins/libplugin.h b/plugins/libplugin.h index 0518cc3e4f1a..4bc5f682af9b 100644 --- a/plugins/libplugin.h +++ b/plugins/libplugin.h @@ -70,6 +70,25 @@ struct plugin_hook { const jsmntok_t *params); }; +/* Helper to create a JSONRPC2 response stream with a "result" object. */ +struct json_stream *jsonrpc_stream_success(struct command *cmd); + +/* Helper to create a JSONRPC2 response stream with an "error" object. */ +struct json_stream *jsonrpc_stream_fail(struct command *cmd, + int code, + const char *err); + +/* Helper to create a JSONRPC2 response stream with an "error" object, + * to which will be added a "data" object. */ +struct json_stream *jsonrpc_stream_fail_data(struct command *cmd, + int code, + const char *err); + +/* This command is finished, here's the response (the content of the + * "result" or "error" field) */ +struct command_result *WARN_UNUSED_RESULT +command_finished(struct command *cmd, struct json_stream *response); + /* Helper to create a zero or single-value JSON object; if @str is NULL, * object is empty. */ struct json_out *json_out_obj(const tal_t *ctx, From 60e3c7779a6d71d4bd04879372b975683e0611ee Mon Sep 17 00:00:00 2001 From: darosior Date: Tue, 28 Jan 2020 18:30:27 +0100 Subject: [PATCH 02/10] libplugin: use json_stream helpers for handle_getmanifest --- plugins/libplugin.c | 51 +++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 0a88babe1a9c..7354b31f14e5 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -568,50 +568,47 @@ send_outreq_(struct plugin *plugin, static struct command_result * handle_getmanifest(struct command *getmanifest_cmd) { - struct json_out *params = json_out_new(tmpctx); + struct json_stream *params = jsonrpc_stream_success(getmanifest_cmd); struct plugin *p = getmanifest_cmd->plugin; - json_out_start(params, NULL, '{'); - json_out_start(params, "options", '['); + json_array_start(params, "options"); for (size_t i = 0; i < tal_count(p->opts); i++) { - json_out_start(params, NULL, '{'); - json_out_addstr(params, "name", p->opts[i].name); - json_out_addstr(params, "type", p->opts[i].type); - json_out_addstr(params, "description", p->opts[i].description); - json_out_end(params, '}'); + json_object_start(params, NULL); + json_add_string(params, "name", p->opts[i].name); + json_add_string(params, "type", p->opts[i].type); + json_add_string(params, "description", p->opts[i].description); + json_object_end(params); } - json_out_end(params, ']'); + json_array_end(params); - json_out_start(params, "rpcmethods", '['); + json_object_start(params, "rpcmethods"); for (size_t i = 0; i < p->num_commands; i++) { - json_out_start(params, NULL, '{'); - json_out_addstr(params, "name", p->commands[i].name); - json_out_addstr(params, "usage", + json_object_start(params, NULL); + json_add_string(params, "name", p->commands[i].name); + json_add_string(params, "usage", strmap_get(&p->usagemap, p->commands[i].name)); - json_out_addstr(params, "description", p->commands[i].description); + json_add_string(params, "description", p->commands[i].description); if (p->commands[i].long_description) - json_out_addstr(params, "long_description", + json_add_string(params, "long_description", p->commands[i].long_description); - json_out_end(params, '}'); + json_object_end(params); } - json_out_end(params, ']'); + json_array_end(params); - json_out_start(params, "subscriptions", '['); + json_array_start(params, "subscriptions"); for (size_t i = 0; i < p->num_notif_subs; i++) - json_out_addstr(params, NULL, p->notif_subs[i].name); - json_out_end(params, ']'); + json_add_string(params, NULL, p->notif_subs[i].name); + json_array_end(params); - json_out_start(params, "hooks", '['); + json_array_start(params, "hooks"); for (size_t i = 0; i < p->num_hook_subs; i++) - json_out_addstr(params, NULL, p->hook_subs[i].name); - json_out_end(params, ']'); + json_add_string(params, NULL, p->hook_subs[i].name); + json_array_end(params); - json_out_addstr(params, "dynamic", + json_add_string(params, "dynamic", p->restartability == PLUGIN_RESTARTABLE ? "true" : "false"); - json_out_end(params, '}'); - json_out_finished(params); - return command_success(getmanifest_cmd, params); + return command_finished(getmanifest_cmd, params); } static void rpc_conn_finished(struct io_conn *conn, From 15a298639ad7006bb414710d9cbf7af41c5e66ae Mon Sep 17 00:00:00 2001 From: darosior Date: Thu, 30 Jan 2020 00:55:55 +0100 Subject: [PATCH 03/10] libplugin: use json_stream helpers for RPC calls This adds helpers to start and send a jsonrpc request using json_stream in order to benefit from the helpers. This then simplifies existing plugins RPC requests by using json_stream helpers. --- plugins/autoclean.c | 13 ++- plugins/fundchannel.c | 192 ++++++++++++++++-------------------------- plugins/libplugin.c | 94 +++++++++------------ plugins/libplugin.h | 82 +++++++++++------- plugins/pay.c | 126 +++++++++++++-------------- 5 files changed, 226 insertions(+), 281 deletions(-) diff --git a/plugins/autoclean.c b/plugins/autoclean.c index 0a605e8f2871..fff1b41e805a 100644 --- a/plugins/autoclean.c +++ b/plugins/autoclean.c @@ -23,16 +23,13 @@ static struct command_result *ignore(struct command *timer, static struct command_result *do_clean(struct plugin *p) { - struct json_out *params = json_out_new(NULL); - json_out_start(params, NULL, '{'); - json_out_add(params, "maxexpirytime", false, "%"PRIu64, + /* FIXME: delexpiredinvoice should be in our plugin too! */ + struct out_req *req = jsonrpc_request_start(p, NULL, "delexpiredinvoice", + ignore, ignore, p); + json_add_u64(req->js, "maxexpirytime", time_now().ts.tv_sec - expired_by); - json_out_end(params, '}'); - json_out_finished(params); - /* FIXME: delexpiredinvoice should be in our plugin too! */ - return send_outreq(p, NULL, "delexpiredinvoice", ignore, ignore, p, - take(params)); + return send_outreq(p, req); } static struct command_result *json_autocleaninvoice(struct command *cmd, diff --git a/plugins/fundchannel.c b/plugins/fundchannel.c index 68700ee5afa6..f16be1c11555 100644 --- a/plugins/fundchannel.c +++ b/plugins/fundchannel.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -50,32 +51,6 @@ static void json_out_add_raw_len(struct json_out *jout, memcpy(p, jsonstr, len); } -/* Helper to add a boolean to a json_out */ -static void json_out_addbool(struct json_out *jout, - const char *fieldname, - const bool val) -{ - if (val) - json_out_add(jout, fieldname, false, "true"); - else - json_out_add(jout, fieldname, false, "false"); -} - -/* Copy field and member to output, if it exists: return member */ -static const jsmntok_t *copy_member(struct json_out *ret, - const char *buf, const jsmntok_t *obj, - const char *membername) -{ - const jsmntok_t *m = json_get_member(buf, obj, membername); - if (!m) - return NULL; - - /* Literal copy: it's already JSON escaped, and may be a string. */ - json_out_add_raw_len(ret, membername, - json_tok_full(buf, m), json_tok_full_len(m)); - return m; -} - static struct command_result *send_prior(struct command *cmd, const char *buf, const jsmntok_t *error, @@ -89,23 +64,20 @@ static struct command_result *tx_abort(struct command *cmd, const jsmntok_t *error, struct funding_req *fr) { - struct json_out *ret; + struct out_req *req; /* We stash the error so we can return it after we've cleaned up */ fr->error = json_strdup(fr, buf, error); - ret = json_out_new(NULL); - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "txid", + req = jsonrpc_request_start(cmd->plugin, cmd, "txdiscard", + send_prior, send_prior, fr); + json_add_string(req->js, "txid", type_to_string(tmpctx, struct bitcoin_txid, &fr->tx_id)); - json_out_end(ret, '}'); /* We need to call txdiscard, and forward the actual cause for the * error after we've cleaned up. We swallow any errors returned by * this call, as we don't really care if it succeeds or not */ - return send_outreq(cmd->plugin, cmd, "txdiscard", - send_prior, send_prior, - fr, take(ret)); + return send_outreq(cmd->plugin, req); } /* We're basically done, we just need to format the output to match @@ -135,7 +107,7 @@ static struct command_result *send_tx(struct command *cmd, struct funding_req *fr) { - struct json_out *ret; + struct out_req *req; const jsmntok_t *tok; bool commitments_secured; @@ -149,15 +121,12 @@ static struct command_result *send_tx(struct command *cmd, tok = json_get_member(buf, result, "channel_id"); fr->chanstr = json_strdup(fr, buf, tok); - ret = json_out_new(NULL); - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "txid", + req = jsonrpc_request_start(cmd->plugin, cmd, "txsend", + finish, tx_abort, fr); + json_add_string(req->js, "txid", type_to_string(tmpctx, struct bitcoin_txid, &fr->tx_id)); - json_out_end(ret, '}'); - return send_outreq(cmd->plugin, cmd, "txsend", - finish, tx_abort, - fr, take(ret)); + return send_outreq(cmd->plugin, req); } static struct command_result *tx_prepare_done(struct command *cmd, @@ -167,7 +136,7 @@ static struct command_result *tx_prepare_done(struct command *cmd, { const jsmntok_t *txid_tok; const jsmntok_t *tx_tok; - struct json_out *ret; + struct out_req *req; const struct bitcoin_tx *tx; const char *hex; u32 outnum; @@ -206,17 +175,14 @@ static struct command_result *tx_prepare_done(struct command *cmd, if (!bitcoin_txid_from_hex(hex, strlen(hex), &fr->tx_id)) plugin_err(cmd->plugin, "Unable to parse txid %s", hex); - ret = json_out_new(NULL); - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "id", node_id_to_hexstr(tmpctx, fr->id)); + req = jsonrpc_request_start(cmd->plugin, cmd, "fundchannel_complete", + send_tx, tx_abort, fr); + json_add_string(req->js, "id", node_id_to_hexstr(tmpctx, fr->id)); /* Note that hex is reused from above */ - json_out_addstr(ret, "txid", hex); - json_out_add(ret, "txout", false, "%u", outnum); - json_out_end(ret, '}'); + json_add_string(req->js, "txid", hex); + json_add_u32(req->js, "txout", outnum); - return send_outreq(cmd->plugin, cmd, "fundchannel_complete", - send_tx, tx_abort, - fr, take(ret)); + return send_outreq(cmd->plugin, req); } static struct command_result *cancel_start(struct command *cmd, @@ -224,59 +190,51 @@ static struct command_result *cancel_start(struct command *cmd, const jsmntok_t *error, struct funding_req *fr) { - struct json_out *ret; + struct out_req *req; /* We stash the error so we can return it after we've cleaned up */ fr->error = json_strdup(fr, buf, error); - ret = json_out_new(NULL); - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "id", node_id_to_hexstr(tmpctx, fr->id)); - json_out_end(ret, '}'); + req = jsonrpc_request_start(cmd->plugin, cmd, "fundchannel_cancel", + send_prior, send_prior, fr); + json_add_string(req->js, "id", node_id_to_hexstr(tmpctx, fr->id)); - return send_outreq(cmd->plugin, cmd, "fundchannel_cancel", - send_prior, send_prior, - fr, take(ret)); + return send_outreq(cmd->plugin, req); } -static struct json_out *txprepare(struct command *cmd, - struct funding_req *fr, - const char *destination) +static void txprepare(struct json_stream *js, + struct funding_req *fr, + const char *destination) { - struct json_out *ret; - ret = json_out_new(NULL); - json_out_start(ret, NULL, '{'); - /* Add the 'outputs' */ - json_out_start(ret, "outputs", '['); - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, destination, fr->funding_str); - json_out_end(ret, '}'); - json_out_end(ret, ']'); + json_array_start(js, "outputs"); + json_object_start(js, NULL); + json_add_string(js, destination, fr->funding_str); + json_object_end(js); + json_array_end(js); if (fr->feerate_str) - json_out_addstr(ret, "feerate", fr->feerate_str); + json_add_string(js, "feerate", fr->feerate_str); if (fr->minconf) - json_out_add(ret, "minconf", false, "%u", *fr->minconf); + json_add_u32(js, "minconf", *fr->minconf); if (fr->utxo_str) - json_out_add_raw_len(ret, "utxos", fr->utxo_str, strlen(fr->utxo_str)); - json_out_end(ret, '}'); - - return ret; + json_out_add_raw_len(js->jout, "utxos", fr->utxo_str, + strlen(fr->utxo_str)); } static struct command_result *prepare_actual(struct command *cmd, - const char *buf, - const jsmntok_t *result, - struct funding_req *fr) + const char *buf, + const jsmntok_t *result, + struct funding_req *fr) { - struct json_out *ret; + struct out_req *req; - ret = txprepare(cmd, fr, fr->funding_addr); + req = jsonrpc_request_start(cmd->plugin, cmd, "txprepare", + tx_prepare_done, cancel_start, + fr); + txprepare(req->js, fr, fr->funding_addr); - return send_outreq(cmd->plugin, cmd, "txprepare", - tx_prepare_done, cancel_start, - fr, take(ret)); + return send_outreq(cmd->plugin, req); } static struct command_result *fundchannel_start_done(struct command *cmd, @@ -284,7 +242,7 @@ static struct command_result *fundchannel_start_done(struct command *cmd, const jsmntok_t *result, struct funding_req *fr) { - struct json_out *ret; + struct out_req *req; /* Save the outscript so we can fund the outnum later */ fr->out_script = json_tok_bin_from_hex(fr, buf, @@ -295,44 +253,39 @@ static struct command_result *fundchannel_start_done(struct command *cmd, json_get_member(buf, result, "funding_address")); /* Now that we're ready to go, cancel the reserved tx */ - ret = json_out_new(NULL); - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "txid", + req = jsonrpc_request_start(cmd->plugin, cmd, "txdiscard", + prepare_actual, cancel_start, + fr); + json_add_string(req->js, "txid", type_to_string(tmpctx, struct bitcoin_txid, &fr->tx_id)); - json_out_end(ret, '}'); - return send_outreq(cmd->plugin, cmd, "txdiscard", - prepare_actual, cancel_start, - fr, take(ret)); + return send_outreq(cmd->plugin, req); } static struct command_result *fundchannel_start(struct command *cmd, struct funding_req *fr) { - struct json_out *ret = json_out_new(NULL); + struct out_req *req = jsonrpc_request_start(cmd->plugin, cmd, + "fundchannel_start", + fundchannel_start_done, + tx_abort, fr); - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "id", node_id_to_hexstr(tmpctx, fr->id)); + json_add_string(req->js, "id", node_id_to_hexstr(tmpctx, fr->id)); if (deprecated_apis) - json_out_addstr(ret, "satoshi", fr->funding_str); + json_add_string(req->js, "satoshi", fr->funding_str); else - json_out_addstr(ret, "amount", fr->funding_str); + json_add_string(req->js, "amount", fr->funding_str); if (fr->feerate_str) - json_out_addstr(ret, "feerate", fr->feerate_str); + json_add_string(req->js, "feerate", fr->feerate_str); if (fr->announce_channel) - json_out_addbool(ret, "announce", *fr->announce_channel); + json_add_bool(req->js, "announce", *fr->announce_channel); if (fr->push_msat) - json_out_addstr(ret, "push_msat", + json_add_string(req->js, "push_msat", type_to_string(tmpctx, struct amount_msat, fr->push_msat)); - json_out_end(ret, '}'); - json_out_finished(ret); - - return send_outreq(cmd->plugin, cmd, "fundchannel_start", - fundchannel_start_done, tx_abort, - fr, take(ret)); + return send_outreq(cmd->plugin, req); } static struct command_result *post_dryrun(struct command *cmd, @@ -391,31 +344,28 @@ static struct command_result *exec_dryrun(struct command *cmd, const jsmntok_t *result, struct funding_req *fr) { - struct json_out *ret; + struct out_req *req = jsonrpc_request_start(cmd->plugin, cmd, "txprepare", + post_dryrun, forward_error, + fr); /* Now that we've tried connecting, we do a 'dry-run' of txprepare, * so we can get an accurate idea of the funding amount */ - ret = txprepare(cmd, fr, placeholder_funding_addr); + txprepare(req->js, fr, placeholder_funding_addr); - return send_outreq(cmd->plugin, cmd, "txprepare", - post_dryrun, forward_error, - fr, take(ret)); + return send_outreq(cmd->plugin, req); } static struct command_result *connect_to_peer(struct command *cmd, struct funding_req *fr) { - struct json_out *ret = json_out_new(NULL); + struct out_req *req = jsonrpc_request_start(cmd->plugin, cmd, "connect", + exec_dryrun, forward_error, + fr); - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "id", node_id_to_hexstr(tmpctx, fr->id)); - json_out_end(ret, '}'); - json_out_finished(ret); + json_add_string(req->js, "id", node_id_to_hexstr(tmpctx, fr->id)); - return send_outreq(cmd->plugin, cmd, "connect", - exec_dryrun, forward_error, - fr, take(ret)); + return send_outreq(cmd->plugin, req); } /* We will use 'id' and 'amount' to build a output: {id: amount}. diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 7354b31f14e5..bdb8781719cb 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -28,24 +28,6 @@ struct plugin_timer { struct command_result *(*cb)(struct plugin *p); }; -struct out_req { - /* The unique id of this request. */ - u64 id; - /* The command which is why we're calling this rpc. */ - struct command *cmd; - /* The callback when we get a response. */ - struct command_result *(*cb)(struct command *command, - const char *buf, - const jsmntok_t *result, - void *arg); - /* The callback when we get an error. */ - struct command_result *(*errcb)(struct command *command, - const char *buf, - const jsmntok_t *error, - void *arg); - void *arg; -}; - struct rpc_conn { int fd; MEMBUF(char) mb; @@ -132,6 +114,39 @@ static void ld_rpc_send(struct plugin *plugin, struct json_stream *stream) /* FIXME: Move lightningd/jsonrpc to common/ ? */ +struct out_req * +jsonrpc_request_start_(struct plugin *plugin, struct command *cmd, + const char *method, + struct command_result *(*cb)(struct command *command, + const char *buf, + const jsmntok_t *result, + void *arg), + struct command_result *(*errcb)(struct command *command, + const char *buf, + const jsmntok_t *result, + void *arg), + void *arg) +{ + struct out_req *out; + + out = tal(plugin, struct out_req); + out->id = plugin->next_outreq_id++; + out->cmd = cmd; + out->cb = cb; + out->errcb = errcb; + out->arg = arg; + uintmap_add(&plugin->out_reqs, out->id, out); + + out->js = new_json_stream(NULL, cmd, NULL); + json_object_start(out->js, NULL); + json_add_string(out->js, "jsonrpc", "2.0"); + json_add_u64(out->js, "id", out->id); + json_add_string(out->js, "method", method); + json_object_start(out->js, "params"); + + return out; +} + static void jsonrpc_finish_and_send(struct plugin *p, struct json_stream *js) { json_object_compat_end(js); @@ -524,43 +539,14 @@ static void handle_rpc_reply(struct plugin *plugin, const jsmntok_t *toks) } struct command_result * -send_outreq_(struct plugin *plugin, - struct command *cmd, - const char *method, - struct command_result *(*cb)(struct command *command, - const char *buf, - const jsmntok_t *result, - void *arg), - struct command_result *(*errcb)(struct command *command, - const char *buf, - const jsmntok_t *result, - void *arg), - void *arg, - const struct json_out *params TAKES) +send_outreq(struct plugin *plugin, const struct out_req *req) { - struct json_stream *js; - struct out_req *out; + /* The "param" object. */ + json_object_end(req->js); + json_object_compat_end(req->js); + json_stream_close(req->js, req->cmd); - out = tal(plugin, struct out_req); - out->id = plugin->next_outreq_id++; - out->cmd = cmd; - out->cb = cb; - out->errcb = errcb; - out->arg = arg; - uintmap_add(&plugin->out_reqs, out->id, out); - - js = new_json_stream(NULL, NULL, NULL); - json_object_start(js, NULL); - json_add_string(js, "jsonrpc", "2.0"); - json_add_u64(js, "id", out->id); - json_add_string(js, "method", method); - json_out_add_splice(js->jout, "params", params); - json_object_compat_end(js); - json_stream_close(js, NULL); - ld_rpc_send(plugin, js); - - if (taken(params)) - tal_free(params); + ld_rpc_send(plugin, req->js); return &pending; } @@ -581,7 +567,7 @@ handle_getmanifest(struct command *getmanifest_cmd) } json_array_end(params); - json_object_start(params, "rpcmethods"); + json_array_start(params, "rpcmethods"); for (size_t i = 0; i < p->num_commands; i++) { json_object_start(params, NULL); json_add_string(params, "name", p->commands[i].name); diff --git a/plugins/libplugin.h b/plugins/libplugin.h index 4bc5f682af9b..829a6ab8e6a8 100644 --- a/plugins/libplugin.h +++ b/plugins/libplugin.h @@ -27,6 +27,26 @@ enum plugin_restartability { PLUGIN_RESTARTABLE }; +struct out_req { + /* The unique id of this request. */ + u64 id; + /* The command which is why we're calling this rpc. */ + struct command *cmd; + /* The request stream. */ + struct json_stream *js; + /* The callback when we get a response. */ + struct command_result *(*cb)(struct command *command, + const char *buf, + const jsmntok_t *result, + void *arg); + /* The callback when we get an error. */ + struct command_result *(*errcb)(struct command *command, + const char *buf, + const jsmntok_t *error, + void *arg); + void *arg; +}; + struct command { u64 *id; const char *methodname; @@ -70,6 +90,35 @@ struct plugin_hook { const jsmntok_t *params); }; +/* Helper to create a JSONRPC2 request stream. Send it with `send_outreq`. */ +struct out_req * +jsonrpc_request_start_(struct plugin *plugin, struct command *cmd, + const char *method, + struct command_result *(*cb)(struct command *command, + const char *buf, + const jsmntok_t *result, + void *arg), + struct command_result *(*errcb)(struct command *command, + const char *buf, + const jsmntok_t *result, + void *arg), + void *arg); + +#define jsonrpc_request_start(plugin, cmd, method, cb, errcb, arg) \ + jsonrpc_request_start_((plugin), (cmd), (method), \ + typesafe_cb_preargs(struct command_result *, void *, \ + (cb), (arg), \ + struct command *command, \ + const char *buf, \ + const jsmntok_t *result), \ + typesafe_cb_preargs(struct command_result *, void *, \ + (errcb), (arg), \ + struct command *command, \ + const char *buf, \ + const jsmntok_t *result), \ + (arg)) + + /* Helper to create a JSONRPC2 response stream with a "result" object. */ struct json_stream *jsonrpc_stream_success(struct command *cmd); @@ -131,38 +180,9 @@ const char *rpc_delve(const tal_t *ctx, const struct json_out *params TAKES, const char *guide); -/* Async rpc request. - * @cmd can be NULL if we're coming from a timer callback. - * @params can be NULL, otherwise it's an array or object. - */ +/* Send an async rpc request to lightningd. */ struct command_result * -send_outreq_(struct plugin *plugin, - struct command *cmd, - const char *method, - struct command_result *(*cb)(struct command *command, - const char *buf, - const jsmntok_t *result, - void *arg), - struct command_result *(*errcb)(struct command *command, - const char *buf, - const jsmntok_t *result, - void *arg), - void *arg, - const struct json_out *params TAKES); - -#define send_outreq(plugin, cmd, method, cb, errcb, arg, params) \ - send_outreq_((plugin), (cmd), (method), \ - typesafe_cb_preargs(struct command_result *, void *, \ - (cb), (arg), \ - struct command *command, \ - const char *buf, \ - const jsmntok_t *result), \ - typesafe_cb_preargs(struct command_result *, void *, \ - (errcb), (arg), \ - struct command *command, \ - const char *buf, \ - const jsmntok_t *result), \ - (arg), (params)) +send_outreq(struct plugin *plugin, const struct out_req *req); /* Callback to just forward error and close request; @cmd cannot be NULL */ struct command_result *forward_error(struct command *cmd, diff --git a/plugins/pay.c b/plugins/pay.c index 48632d4c9e39..1ec8790a3b53 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -197,14 +198,6 @@ static void attempt_failed_tok(struct pay_command *pc, const char *method, failed_end(failed); } -/* Helper to add a u32. */ -static void json_out_add_u32(struct json_out *jout, - const char *fieldname, - u32 val) -{ - json_out_add(jout, fieldname, false, "%"PRIu32, val); -} - /* Helper to add a u64. */ static void json_out_add_u64(struct json_out *jout, const char *fieldname, @@ -360,7 +353,7 @@ execute_waitblockheight(struct command *cmd, u32 blockheight, struct pay_command *pc) { - struct json_out *params; + struct out_req *req; struct timeabs now = time_now(); struct timerel remaining; @@ -369,18 +362,14 @@ execute_waitblockheight(struct command *cmd, remaining = time_between(pc->stoptime, now); - params = json_out_new(tmpctx); - json_out_start(params, NULL, '{'); - json_out_add_u32(params, "blockheight", blockheight); - json_out_add_u64(params, "timeout", time_to_sec(remaining)); - json_out_end(params, '}'); - json_out_finished(params); - - return send_outreq(cmd->plugin, cmd, "waitblockheight", - &waitblockheight_done, - &waitblockheight_error, - pc, - params); + req = jsonrpc_request_start(cmd->plugin, cmd, "waitblockheight", + &waitblockheight_done, + &waitblockheight_error, + pc); + json_add_u32(req->js, "blockheight", blockheight); + json_add_u32(req->js, "timeout", time_to_sec(remaining)); + + return send_outreq(cmd->plugin, req); } /* Gets the remote height from a @@ -581,10 +570,13 @@ static struct command_result *sendpay_done(struct command *cmd, const jsmntok_t *result, struct pay_command *pc) { - return send_outreq(cmd->plugin, cmd, "waitsendpay", - waitsendpay_done, waitsendpay_error, pc, - take(json_out_obj(NULL, "payment_hash", - pc->payment_hash))); + struct out_req *req = jsonrpc_request_start(cmd->plugin, cmd, + "waitsendpay", + waitsendpay_done, + waitsendpay_error, pc); + json_add_string(req->js, "payment_hash", pc->payment_hash); + + return send_outreq(cmd->plugin, req); } /* Calculate how many millisatoshi we need at the start of this route @@ -737,7 +729,7 @@ static struct command_result *getroute_done(struct command *cmd, struct amount_msat fee; u32 delay; double feepercent; - struct json_out *params; + struct out_req *req; if (!t) plugin_err(cmd->plugin, "getroute gave no 'route'? '%.*s'", @@ -837,21 +829,17 @@ static struct command_result *getroute_done(struct command *cmd, } attempt->sendpay = true; - params = json_out_new(NULL); - json_out_start(params, NULL, '{'); - json_out_add_raw(params, "route", attempt->route); - json_out_add(params, "payment_hash", true, "%s", pc->payment_hash); - json_out_add(params, "bolt11", true, "%s", pc->ps->bolt11); + req = jsonrpc_request_start(cmd->plugin, cmd, "sendpay", + sendpay_done, sendpay_error, pc); + json_out_add_raw(req->js->jout, "route", attempt->route); + json_add_string(req->js, "payment_hash", pc->payment_hash); + json_add_string(req->js, "bolt11", pc->ps->bolt11); if (pc->label) - json_out_add(params, "label", true, "%s", pc->label); + json_add_string(req->js, "label", pc->label); if (pc->payment_secret) - json_out_add(params, "payment_secret", true, "%s", - pc->payment_secret); - json_out_end(params, '}'); - - return send_outreq(cmd->plugin, cmd, "sendpay", sendpay_done, sendpay_error, pc, - take(params)); + json_add_string(req->js, "payment_secret", pc->payment_secret); + return send_outreq(cmd->plugin, req); } static struct command_result *getroute_error(struct command *cmd, @@ -896,7 +884,7 @@ static struct command_result *execute_getroute(struct command *cmd, struct amount_msat msat; const char *dest; u32 cltv; - struct json_out *params; + struct out_req *req; /* routehint set below. */ @@ -929,24 +917,22 @@ static struct command_result *execute_getroute(struct command *cmd, } /* OK, ask for route to destination */ - params = json_out_new(NULL); - json_out_start(params, NULL, '{'); - json_out_addstr(params, "id", dest); - json_out_addstr(params, "msatoshi", + req = jsonrpc_request_start(cmd->plugin, cmd, "getroute", + getroute_done, getroute_error, pc); + json_add_string(req->js, "id", dest); + json_add_string(req->js, "msatoshi", type_to_string(tmpctx, struct amount_msat, &msat)); - json_out_add_u32(params, "cltv", cltv); - json_out_add_u32(params, "maxhops", max_hops); - json_out_add(params, "riskfactor", false, "%f", pc->riskfactor); + json_add_u32(req->js, "cltv", cltv); + json_add_u32(req->js, "maxhops", max_hops); + json_add_member(req->js, "riskfactor", false, "%f", pc->riskfactor); if (tal_count(pc->excludes) != 0) { - json_out_start(params, "exclude", '['); + json_array_start(req->js, "exclude"); for (size_t i = 0; i < tal_count(pc->excludes); i++) - json_out_addstr(params, NULL, pc->excludes[i]); - json_out_end(params, ']'); + json_add_string(req->js, NULL, pc->excludes[i]); + json_array_end(req->js); } - json_out_end(params, '}'); - return send_outreq(cmd->plugin, cmd, "getroute", getroute_done, getroute_error, pc, - take(params)); + return send_outreq(cmd->plugin, req); } static struct command_result * @@ -989,11 +975,11 @@ static struct command_result * execute_getstartblockheight(struct command *cmd, struct pay_command *pc) { - return send_outreq(cmd->plugin, cmd, "getinfo", - &getstartblockheight_done, - &getstartblockheight_error, - pc, - take(json_out_obj(NULL, NULL, NULL))); + struct out_req *req = jsonrpc_request_start(cmd->plugin, cmd, "getinfo", + &getstartblockheight_done, + &getstartblockheight_error, + pc); + return send_outreq(cmd->plugin, req); } static struct command_result *start_pay_attempt(struct command *cmd, @@ -1106,6 +1092,8 @@ static struct command_result *add_shadow_route(struct command *cmd, static struct command_result *shadow_route(struct command *cmd, struct pay_command *pc) { + struct out_req *req; + #if DEVELOPER if (!pc->use_shadow) return start_pay_attempt(cmd, pc, "Initial attempt"); @@ -1113,9 +1101,9 @@ static struct command_result *shadow_route(struct command *cmd, if (pseudorand(2) == 0) return start_pay_attempt(cmd, pc, "Initial attempt"); - return send_outreq(cmd->plugin, cmd, "listchannels", - add_shadow_route, forward_error, pc, - take(json_out_obj(NULL, "source", pc->shadow_dest))); + req = jsonrpc_request_start(cmd->plugin, cmd, "listchannels", + add_shadow_route, forward_error, pc); + return send_outreq(cmd->plugin, req); } /* gossipd doesn't know much about the current state of channels; here we @@ -1271,6 +1259,7 @@ static struct command_result *json_pay(struct command *cmd, double *maxfeepercent; unsigned int *maxdelay; struct amount_msat *exemptfee; + struct out_req *req; #if DEVELOPER bool *use_shadow; #endif @@ -1357,8 +1346,9 @@ static struct command_result *json_pay(struct command *cmd, #endif /* Get capacities of local channels (no parameters) */ - return send_outreq(cmd->plugin, cmd, "listpeers", listpeers_done, forward_error, pc, - take(json_out_obj(NULL, NULL, NULL))); + req = jsonrpc_request_start(cmd->plugin, cmd, "listpeers", + listpeers_done, forward_error, pc); + return send_outreq(cmd->plugin, req); } /* FIXME: Add this to ccan/time? */ @@ -1663,6 +1653,7 @@ static struct command_result *json_listpays(struct command *cmd, const jsmntok_t *params) { const char *b11str; + struct out_req *req; /* FIXME: would be nice to parse as a bolt11 so check worked in future */ if (!param(cmd, buf, params, @@ -1670,11 +1661,12 @@ static struct command_result *json_listpays(struct command *cmd, NULL)) return command_param_failed(); - return send_outreq(cmd->plugin, cmd, "listsendpays", - listsendpays_done, forward_error, - cast_const(char *, b11str), - /* Neatly returns empty object if b11str is NULL */ - take(json_out_obj(NULL, "bolt11", b11str))); + req = jsonrpc_request_start(cmd->plugin, cmd, "listsendpays", + listsendpays_done, forward_error, + cast_const(char *, b11str)); + if (b11str) + json_add_string(req->js, "bolt11", b11str); + return send_outreq(cmd->plugin, req); } static void init(struct plugin *p, From 920576436557b6cbd396ae0ddda081bf9c976269 Mon Sep 17 00:00:00 2001 From: darosior Date: Fri, 31 Jan 2020 01:12:07 +0100 Subject: [PATCH 04/10] libplugin: use json_stream for all plugins' commands --- plugins/fundchannel.c | 14 ++-- plugins/pay.c | 162 +++++++++++++++++++----------------------- 2 files changed, 79 insertions(+), 97 deletions(-) diff --git a/plugins/fundchannel.c b/plugins/fundchannel.c index f16be1c11555..1faa4a47116e 100644 --- a/plugins/fundchannel.c +++ b/plugins/fundchannel.c @@ -87,17 +87,15 @@ static struct command_result *finish(struct command *cmd, const jsmntok_t *result, struct funding_req *fr) { - struct json_out *out; + struct json_stream *out; - out = json_out_new(NULL); - json_out_start(out, NULL, '{'); - copy_member(out, buf, result, "tx"); - json_out_addstr(out, "txid", + out = jsonrpc_stream_success(cmd); + json_add_tok(out, "tx", json_get_member(buf, result, "tx"), buf); + json_add_string(out, "txid", type_to_string(tmpctx, struct bitcoin_txid, &fr->tx_id)); - json_out_addstr(out, "channel_id", fr->chanstr); - json_out_end(out, '}'); + json_add_string(out, "channel_id", fr->chanstr); - return command_success(cmd, out); + return command_finished(cmd, out); } /* We're ready to broadcast the transaction */ diff --git a/plugins/pay.c b/plugins/pay.c index 1ec8790a3b53..94e8402f8995 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -198,14 +198,6 @@ static void attempt_failed_tok(struct pay_command *pc, const char *method, failed_end(failed); } -/* Helper to add a u64. */ -static void json_out_add_u64(struct json_out *jout, - const char *fieldname, - u64 val) -{ - json_out_add(jout, fieldname, false, "%"PRIu64, val); -} - static struct command_result *start_pay_attempt(struct command *cmd, struct pay_command *pc, const char *fmt, ...); @@ -255,26 +247,26 @@ static struct command_result *waitsendpay_expired(struct command *cmd, struct pay_command *pc) { char *errmsg; - struct json_out *data; + struct json_stream *data; size_t num_attempts = count_sendpays(pc->ps->attempts); errmsg = tal_fmt(pc, "Gave up after %zu attempt%s: see paystatus", num_attempts, num_attempts == 1 ? "" : "s"); - data = json_out_new(NULL); - json_out_start(data, NULL, '{'); - json_out_start(data, "attempts", '['); + data = jsonrpc_stream_fail(cmd, PAY_STOPPED_RETRYING, errmsg); + json_object_start(data, "data"); + json_array_start(data, "attempts"); for (size_t i = 0; i < tal_count(pc->ps->attempts); i++) { - json_out_start(data, NULL, '{'); + json_object_start(data, NULL); if (pc->ps->attempts[i].route) - json_out_add_raw(data, "route", + json_add_member(data, "route", false, "%s", pc->ps->attempts[i].route); - json_out_add_splice(data, "failure", + json_out_add_splice(data->jout, "failure", pc->ps->attempts[i].failure); - json_out_end(data, '}'); + json_object_end(data); } - json_out_end(data, ']'); - json_out_end(data, '}'); - return command_done_err(cmd, PAY_STOPPED_RETRYING, errmsg, data); + json_array_end(data); + json_object_end(data); + return command_finished(cmd, data); } static bool routehint_excluded(struct plugin *plugin, @@ -1363,7 +1355,7 @@ static void utc_timestring(const struct timeabs *time, char str[UTC_TIMELEN]) (int) time->ts.tv_nsec / 1000000); } -static void add_attempt(struct json_out *ret, +static void add_attempt(struct json_stream *ret, const struct pay_status *ps, const struct pay_attempt *attempt) { @@ -1371,56 +1363,56 @@ static void add_attempt(struct json_out *ret, utc_timestring(&attempt->start, timestr); - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "strategy", attempt->why); - json_out_addstr(ret, "start_time", timestr); - json_out_add_u64(ret, "age_in_seconds", + json_object_start(ret, NULL); + json_add_string(ret, "strategy", attempt->why); + json_add_string(ret, "start_time", timestr); + json_add_u64(ret, "age_in_seconds", time_to_sec(time_between(time_now(), attempt->start))); if (attempt->result || attempt->failure) { utc_timestring(&attempt->end, timestr); - json_out_addstr(ret, "end_time", timestr); - json_out_add_u64(ret, "duration_in_seconds", - time_to_sec(time_between(attempt->end, - attempt->start))); + json_add_string(ret, "end_time", timestr); + json_add_u64(ret, "duration_in_seconds", + time_to_sec(time_between(attempt->end, + attempt->start))); } if (tal_count(attempt->routehint)) { - json_out_start(ret, "routehint", '['); + json_array_start(ret, "routehint"); for (size_t i = 0; i < tal_count(attempt->routehint); i++) { - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "id", + json_object_start(ret, NULL); + json_add_string(ret, "id", type_to_string(tmpctx, struct node_id, &attempt->routehint[i].pubkey)); - json_out_addstr(ret, "channel", + json_add_string(ret, "channel", type_to_string(tmpctx, struct short_channel_id, &attempt->routehint[i].short_channel_id)); - json_out_add_u64(ret, "fee_base_msat", - attempt->routehint[i].fee_base_msat); - json_out_add_u64(ret, "fee_proportional_millionths", - attempt->routehint[i].fee_proportional_millionths); - json_out_add_u64(ret, "cltv_expiry_delta", - attempt->routehint[i].cltv_expiry_delta); - json_out_end(ret, '}'); + json_add_u64(ret, "fee_base_msat", + attempt->routehint[i].fee_base_msat); + json_add_u64(ret, "fee_proportional_millionths", + attempt->routehint[i].fee_proportional_millionths); + json_add_u64(ret, "cltv_expiry_delta", + attempt->routehint[i].cltv_expiry_delta); + json_object_end(ret); } - json_out_end(ret, ']'); + json_array_end(ret); } if (tal_count(attempt->excludes)) { - json_out_start(ret, "excluded_nodes_or_channels", '['); + json_array_start(ret, "excluded_nodes_or_channels"); for (size_t i = 0; i < tal_count(attempt->excludes); i++) - json_out_addstr(ret, NULL, attempt->excludes[i]); - json_out_end(ret, ']'); + json_add_string(ret, NULL, attempt->excludes[i]); + json_array_end(ret); } if (attempt->route) - json_out_add_raw(ret, "route", attempt->route); + json_add_member(ret, "route", true, "%s", attempt->route); if (attempt->failure) - json_out_add_splice(ret, "failure", attempt->failure); + json_out_add_splice(ret->jout, "failure", attempt->failure); if (attempt->result) - json_out_add_raw(ret, "success", attempt->result); + json_add_member(ret, "success", true, "%s", attempt->result); - json_out_end(ret, '}'); + json_object_end(ret); } static struct command_result *json_paystatus(struct command *cmd, @@ -1429,51 +1421,49 @@ static struct command_result *json_paystatus(struct command *cmd, { struct pay_status *ps; const char *b11str; - struct json_out *ret; + struct json_stream *ret; if (!param(cmd, buf, params, p_opt("bolt11", param_string, &b11str), NULL)) return command_param_failed(); - ret = json_out_new(NULL); - json_out_start(ret, NULL, '{'); - json_out_start(ret, "pay", '['); + ret = jsonrpc_stream_success(cmd); + json_array_start(ret, "pay"); /* FIXME: Index by bolt11 string! */ list_for_each(&pay_status, ps, list) { if (b11str && !streq(b11str, ps->bolt11)) continue; - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "bolt11", ps->bolt11); - json_out_add_u64(ret, "msatoshi", + json_object_start(ret, NULL); + json_add_string(ret, "bolt11", ps->bolt11); + json_add_u64(ret, "msatoshi", ps->msat.millisatoshis); /* Raw: JSON */ - json_out_addstr(ret, "amount_msat", - type_to_string(tmpctx, struct amount_msat, - &ps->msat)); - json_out_addstr(ret, "destination", ps->dest); + json_add_string(ret, "amount_msat", + type_to_string(tmpctx, struct amount_msat, + &ps->msat)); + json_add_string(ret, "destination", ps->dest); if (ps->label) - json_out_addstr(ret, "label", ps->label); + json_add_string(ret, "label", ps->label); if (ps->routehint_modifications) - json_out_addstr(ret, "routehint_modifications", + json_add_string(ret, "routehint_modifications", ps->routehint_modifications); if (ps->shadow && !streq(ps->shadow, "")) - json_out_addstr(ret, "shadow", ps->shadow); + json_add_string(ret, "shadow", ps->shadow); if (ps->exclusions) - json_out_addstr(ret, "local_exclusions", ps->exclusions); + json_add_string(ret, "local_exclusions", ps->exclusions); assert(tal_count(ps->attempts)); - json_out_start(ret, "attempts", '['); + json_array_start(ret, "attempts"); for (size_t i = 0; i < tal_count(ps->attempts); i++) add_attempt(ret, ps, &ps->attempts[i]); - json_out_end(ret, ']'); - json_out_end(ret, '}'); + json_array_end(ret); + json_object_end(ret); } - json_out_end(ret, ']'); - json_out_end(ret, '}'); + json_array_end(ret); - return command_success(cmd, ret); + return command_finished(cmd, ret); } static bool attempt_ongoing(const char *b11) @@ -1540,28 +1530,24 @@ static void add_amount_sent(struct plugin *p, type_to_string(tmpctx, struct amount_msat, &sent)); } -static void add_new_entry(struct json_out *ret, +static void add_new_entry(struct json_stream *ret, const char *buf, const struct pay_mpp *pm) { - json_out_start(ret, NULL, '{'); - json_out_addstr(ret, "bolt11", pm->b11); - json_out_addstr(ret, "status", pm->status); + json_object_start(ret, NULL); + json_add_string(ret, "bolt11", pm->b11); + json_add_string(ret, "status", pm->status); if (pm->label) - json_out_add_raw_len(ret, "label", - json_tok_full(buf, pm->label), - json_tok_full_len(pm->label)); + json_add_tok(ret, "label", pm->label, buf); if (pm->preimage) - json_out_add_raw_len(ret, "preimage", - json_tok_full(buf, pm->preimage), - json_tok_full_len(pm->preimage)); - json_out_addstr(ret, "amount_sent_msat", + json_add_tok(ret, "preimage", pm->preimage, buf); + json_add_string(ret, "amount_sent_msat", fmt_amount_msat(tmpctx, &pm->amount_sent)); if (pm->num_nonfailed_parts > 1) - json_out_add_u64(ret, "number_of_parts", - pm->num_nonfailed_parts); - json_out_end(ret, '}'); + json_add_u64(ret, "number_of_parts", + pm->num_nonfailed_parts); + json_object_end(ret); } static struct command_result *listsendpays_done(struct command *cmd, @@ -1571,7 +1557,7 @@ static struct command_result *listsendpays_done(struct command *cmd, { size_t i; const jsmntok_t *t, *arr; - struct json_out *ret; + struct json_stream *ret; struct pay_map pay_map; struct pay_map_iter it; struct pay_mpp *pm; @@ -1583,9 +1569,8 @@ static struct command_result *listsendpays_done(struct command *cmd, return command_fail(cmd, LIGHTNINGD, "Unexpected non-array result from listsendpays"); - ret = json_out_new(NULL); - json_out_start(ret, NULL, '{'); - json_out_start(ret, "pays", '['); + ret = jsonrpc_stream_success(cmd); + json_array_start(ret, "pays"); json_for_each_arr(i, t, arr) { const jsmntok_t *status, *b11tok; const char *b11; @@ -1643,9 +1628,8 @@ static struct command_result *listsendpays_done(struct command *cmd, } pay_map_clear(&pay_map); - json_out_end(ret, ']'); - json_out_end(ret, '}'); - return command_success(cmd, ret); + json_array_end(ret); + return command_finished(cmd, ret); } static struct command_result *json_listpays(struct command *cmd, From bd18dd17ea3bdbfa752dd31d59a8812c8a886a3e Mon Sep 17 00:00:00 2001 From: darosior Date: Fri, 31 Jan 2020 18:36:26 +0100 Subject: [PATCH 05/10] pytest: add a C testing plugin to test libplugin We mark the test as xfail() as it exposes that libplugin's PLUGIN_RESTARTABLE was not taken into account ! --- Makefile | 1 + tests/plugins/Makefile | 18 ++++++++++++ tests/plugins/test_libplugin.c | 52 ++++++++++++++++++++++++++++++++++ tests/test_plugin.py | 25 ++++++++++++++++ 4 files changed, 96 insertions(+) create mode 100644 tests/plugins/Makefile create mode 100644 tests/plugins/test_libplugin.c diff --git a/Makefile b/Makefile index 24d39063a04e..35b91d5539fb 100644 --- a/Makefile +++ b/Makefile @@ -237,6 +237,7 @@ include doc/Makefile include devtools/Makefile include tools/Makefile include plugins/Makefile +include tests/plugins/Makefile # Git doesn't maintain timestamps, so we only regen if git says we should. CHANGED_FROM_GIT = [ x"`git log $@ | head -n1`" != x"`git log $< | head -n1`" -o x"`git diff $<`" != x"" ] diff --git a/tests/plugins/Makefile b/tests/plugins/Makefile new file mode 100644 index 000000000000..0b0eee0b4e1d --- /dev/null +++ b/tests/plugins/Makefile @@ -0,0 +1,18 @@ +PLUGIN_TESTLIBPLUGIN_SRC := tests/plugins/test_libplugin.c +PLUGIN_TESTLIBPLUGIN_OBJS := $(PLUGIN_TESTLIBPLUGIN_SRC:.c=.o) + +tests/plugins/test_libplugin: bitcoin/chainparams.o $(PLUGIN_TESTLIBPLUGIN_OBJS) $(PLUGIN_LIB_OBJS) $(PLUGIN_COMMON_OBJS) $(JSMN_OBJS) $(CCAN_OBJS) + +$(PLUGIN_TESTLIBPLUGIN_OBJS): $(PLUGIN_LIB_HEADER) + +# Make sure these depend on everything. +ALL_PROGRAMS += tests/plugins/test_libplugin +ALL_OBJS += $(PLUGIN_TESTLIBPLUGIN_OBJS) + +check-source: $(PLUGIN_TESTLIBPLUGIN_SRC:%=check-src-include-order/%) +check-whitespace: $(PLUGIN_TESTLIBPLUGIN_SRC:%=check-whitespace/%) + +clean: test-plugin-clean + +test-plugin-clean: + $(RM) $(PLUGIN_TESTLIBPLUGIN_OBJS) diff --git a/tests/plugins/test_libplugin.c b/tests/plugins/test_libplugin.c new file mode 100644 index 000000000000..9630d10be0dc --- /dev/null +++ b/tests/plugins/test_libplugin.c @@ -0,0 +1,52 @@ +#include +#include + + +const char *name_option; + + +static struct command_result *json_helloworld(struct command *cmd, + const char *buf, + const jsmntok_t *params) +{ + const char *name; + + if (!param(cmd, buf, params, + p_opt("name", param_string, &name), + NULL)) + return command_param_failed(); + + if (!name) + name = name_option ? name_option : tal_strdup(tmpctx, "world"); + + return command_success_str(cmd, tal_fmt(tmpctx, "hello %s", name)); +} + +static void init(struct plugin *p, + const char *buf UNUSED, const jsmntok_t *config UNUSED) +{ + plugin_log(p, LOG_DBG, "test_libplugin initialised!"); +} + +static const struct plugin_command commands[] = { { + "helloworld", + "utils", + "Say hello to the world.", + "Returns 'hello world' by default, 'hello {name}' if the name" + " option was set, and 'hello {name}' if the name parameter " + "was passed (takes over the option)", + json_helloworld, + } +}; + +int main(int argc, char *argv[]) +{ + setup_locale(); + plugin_main(argv, init, PLUGIN_RESTARTABLE, commands, ARRAY_SIZE(commands), + NULL, 0, NULL, 0, + plugin_option("name", + "string", + "Who to say hello to.", + charp_option, &name_option), + NULL); +} diff --git a/tests/test_plugin.py b/tests/test_plugin.py index c4bb19cd97c7..55dd82624902 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -789,3 +789,28 @@ def test_rpc_command_hook(node_factory): # Test command which removes plugin itself! l1.rpc.plugin_stop('rpc_command.py') + + +@pytest.mark.xfail(strict=True) +def test_libplugin(node_factory): + """Sanity checks for plugins made with libplugin""" + plugin = os.path.join(os.getcwd(), "tests/plugins/test_libplugin") + l1 = node_factory.get_node(options={"plugin": plugin}) + + # Test startup + assert l1.daemon.is_in_log("test_libplugin initialised!") + # Test dynamic startup + l1.rpc.plugin_stop(plugin) + l1.rpc.plugin_start(plugin) + l1.rpc.check("helloworld") + + # Test commands + assert l1.rpc.call("helloworld") == "hello world" + assert l1.rpc.call("helloworld", {"name": "test"}) == "hello test" + l1.stop() + l1.daemon.opts["plugin"] = plugin + l1.daemon.opts["name"] = "test_opt" + l1.start() + assert l1.rpc.call("helloworld") == "hello test_opt" + # But param takes over! + assert l1.rpc.call("helloworld", {"name": "test"}) == "hello test" From f9c198a3fc34131d965bfb131784ef5a3b464992 Mon Sep 17 00:00:00 2001 From: darosior Date: Fri, 31 Jan 2020 19:13:59 +0100 Subject: [PATCH 06/10] libplugin: fix 'dynamic' field in getmanifest As a separated commit because it was pre-existent (changelog + xfail test). This also fix a logical problem in lightningd/plugin_control: we were assuming a plugin started with 'plugin start' but which did not comport a 'dynamic' entry in its manifest to be dynamic, though it should have been treated as static. Changelog-fixed: plugins: Dynamic C plugins can now be managed when lightningd is up --- lightningd/plugin.c | 9 +++++---- lightningd/plugin_control.c | 2 +- plugins/libplugin.c | 3 +-- tests/test_plugin.py | 1 - 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index c82f8d710399..6d0a36fc64fb 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -73,6 +73,7 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES) p->js_arr = tal_arr(p, struct json_stream *, 0); p->used = 0; p->subscriptions = NULL; + p->dynamic = false; p->log = new_log(p, plugins->log_book, NULL, "plugin-%s", path_basename(tmpctx, p->cmd)); @@ -810,22 +811,22 @@ static void plugin_manifest_timeout(struct plugin *plugin) fatal("Can't recover from plugin failure, terminating."); } - bool plugin_parse_getmanifest_response(const char *buffer, const jsmntok_t *toks, const jsmntok_t *idtok, struct plugin *plugin) { const jsmntok_t *resulttok, *dynamictok; - bool dynamic_plugin; resulttok = json_get_member(buffer, toks, "result"); if (!resulttok || resulttok->type != JSMN_OBJECT) return false; dynamictok = json_get_member(buffer, resulttok, "dynamic"); - if (dynamictok && json_to_bool(buffer, dynamictok, &dynamic_plugin)) - plugin->dynamic = dynamic_plugin; + if (dynamictok && !json_to_bool(buffer, dynamictok, &plugin->dynamic)) + plugin_kill(plugin, "Bad 'dynamic' field ('%.*s')", + json_tok_full_len(dynamictok), + json_tok_full(buffer, dynamictok)); if (!plugin_opts_add(plugin, buffer, resulttok) || !plugin_rpcmethods_add(plugin, buffer, resulttok) || diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 3e6b0833a1c7..18d4f2611f05 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -115,7 +115,7 @@ static struct command_result *plugin_start(struct dynamic_plugin *dp) struct jsonrpc_request *req; struct plugin *p = dp->plugin; - p->dynamic = true; + p->dynamic = false; p_cmd = tal_arrz(NULL, char *, 2); p_cmd[0] = p->cmd; /* In case the plugin create files, this is a better default. */ diff --git a/plugins/libplugin.c b/plugins/libplugin.c index bdb8781719cb..99c38707c458 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -591,8 +591,7 @@ handle_getmanifest(struct command *getmanifest_cmd) json_add_string(params, NULL, p->hook_subs[i].name); json_array_end(params); - json_add_string(params, "dynamic", - p->restartability == PLUGIN_RESTARTABLE ? "true" : "false"); + json_add_bool(params, "dynamic", p->restartability == PLUGIN_RESTARTABLE); return command_finished(getmanifest_cmd, params); } diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 55dd82624902..e69ba0890fd5 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -791,7 +791,6 @@ def test_rpc_command_hook(node_factory): l1.rpc.plugin_stop('rpc_command.py') -@pytest.mark.xfail(strict=True) def test_libplugin(node_factory): """Sanity checks for plugins made with libplugin""" plugin = os.path.join(os.getcwd(), "tests/plugins/test_libplugin") From 25521eafc248219ffc43a28476716479df4f01d7 Mon Sep 17 00:00:00 2001 From: darosior Date: Sat, 1 Feb 2020 16:00:32 +0100 Subject: [PATCH 07/10] pytest: test hooks and notifications with libplugin --- tests/plugins/test_libplugin.c | 45 +++++++++++++++++++++++++++++++++- tests/test_plugin.py | 6 +++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/tests/plugins/test_libplugin.c b/tests/plugins/test_libplugin.c index 9630d10be0dc..a432fae9bc96 100644 --- a/tests/plugins/test_libplugin.c +++ b/tests/plugins/test_libplugin.c @@ -22,6 +22,37 @@ static struct command_result *json_helloworld(struct command *cmd, return command_success_str(cmd, tal_fmt(tmpctx, "hello %s", name)); } +static struct command_result * +json_peer_connected(struct command *cmd, + const char *buf, + const jsmntok_t *params) +{ + const jsmntok_t *peertok, *idtok; + struct json_stream *response; + + peertok = json_get_member(buf, params, "peer"); + assert(peertok); + idtok = json_get_member(buf, peertok, "id"); + assert(idtok); + plugin_log(cmd->plugin, LOG_INFORM, "%s peer_connected", + json_strdup(tmpctx, buf, idtok)); + + response = jsonrpc_stream_success(cmd); + json_add_string(response, "result", "continue"); + + return command_finished(cmd, response); +} + +static void json_connected(struct command *cmd, + const char *buf, + const jsmntok_t *params) +{ + const jsmntok_t *idtok = json_get_member(buf, params, "id"); + assert(idtok); + plugin_log(cmd->plugin, LOG_INFORM, "%s connected", + json_strdup(tmpctx, buf, idtok)); +} + static void init(struct plugin *p, const char *buf UNUSED, const jsmntok_t *config UNUSED) { @@ -39,11 +70,23 @@ static const struct plugin_command commands[] = { { } }; +static const struct plugin_hook hooks[] = { { + "peer_connected", + json_peer_connected, + } +}; + +static const struct plugin_notification notifs[] = { { + "connect", + json_connected, + } +}; + int main(int argc, char *argv[]) { setup_locale(); plugin_main(argv, init, PLUGIN_RESTARTABLE, commands, ARRAY_SIZE(commands), - NULL, 0, NULL, 0, + notifs, ARRAY_SIZE(notifs), hooks, ARRAY_SIZE(hooks), plugin_option("name", "string", "Who to say hello to.", diff --git a/tests/test_plugin.py b/tests/test_plugin.py index e69ba0890fd5..832940c6641e 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -813,3 +813,9 @@ def test_libplugin(node_factory): assert l1.rpc.call("helloworld") == "hello test_opt" # But param takes over! assert l1.rpc.call("helloworld", {"name": "test"}) == "hello test" + + # Test hooks and notifications + l2 = node_factory.get_node() + l2.connect(l1) + assert l1.daemon.is_in_log("{} peer_connected".format(l2.info["id"])) + l1.daemon.wait_for_log("{} connected".format(l2.info["id"])) From b86c84d6a3c47cc1f1427ded5f0a87fa85b8c4f5 Mon Sep 17 00:00:00 2001 From: darosior Date: Sat, 1 Feb 2020 18:25:49 +0100 Subject: [PATCH 08/10] pytest: test libplugin's send_outreq --- tests/plugins/test_libplugin.c | 38 ++++++++++++++++++++++++++++++++++ tests/test_plugin.py | 3 +++ 2 files changed, 41 insertions(+) diff --git a/tests/plugins/test_libplugin.c b/tests/plugins/test_libplugin.c index a432fae9bc96..2cc9c2a3ac10 100644 --- a/tests/plugins/test_libplugin.c +++ b/tests/plugins/test_libplugin.c @@ -1,4 +1,5 @@ #include +#include #include @@ -53,6 +54,36 @@ static void json_connected(struct command *cmd, json_strdup(tmpctx, buf, idtok)); } +static struct command_result *testrpc_cb(struct command *cmd, + const char *buf, + const jsmntok_t *params, + void *cb_arg UNUSED) +{ + int i = 0; + const jsmntok_t *t; + struct json_stream *response; + + response = jsonrpc_stream_success(cmd); + json_for_each_obj(i, t, params) + json_add_tok(response, json_strdup(tmpctx, buf, t), t+1, buf); + + return command_finished(cmd, response); +} + +static struct command_result *json_testrpc(struct command *cmd, + const char *buf, + const jsmntok_t *params) +{ + struct out_req *req; + + if (!param(cmd, buf, params, NULL)) + return command_param_failed(); + + req = jsonrpc_request_start(cmd->plugin, cmd, "getinfo", testrpc_cb, + testrpc_cb, NULL); + return send_outreq(cmd->plugin, req); +} + static void init(struct plugin *p, const char *buf UNUSED, const jsmntok_t *config UNUSED) { @@ -67,6 +98,13 @@ static const struct plugin_command commands[] = { { " option was set, and 'hello {name}' if the name parameter " "was passed (takes over the option)", json_helloworld, + }, + { + "testrpc", + "utils", + "Makes a simple getinfo call, to test rpc socket.", + "", + json_testrpc, } }; diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 832940c6641e..7c1850f4d783 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -819,3 +819,6 @@ def test_libplugin(node_factory): l2.connect(l1) assert l1.daemon.is_in_log("{} peer_connected".format(l2.info["id"])) l1.daemon.wait_for_log("{} connected".format(l2.info["id"])) + + # Test RPC calls FIXME: test concurrent ones ? + assert l1.rpc.call("testrpc") == l1.rpc.getinfo() From 3692159c4155e2798ec9a3544e6190c2b4855c9e Mon Sep 17 00:00:00 2001 From: darosior Date: Tue, 4 Feb 2020 22:00:22 +0100 Subject: [PATCH 09/10] ccan: retrieve last updates to opt/ Co-authored-by: Rusty Russell --- ccan/ccan/opt/opt.c | 17 +++++++++++++++++ ccan/ccan/opt/opt.h | 9 +++++++++ 2 files changed, 26 insertions(+) diff --git a/ccan/ccan/opt/opt.c b/ccan/ccan/opt/opt.c index 0514dc8702df..d4601dfbb230 100644 --- a/ccan/ccan/opt/opt.c +++ b/ccan/ccan/opt/opt.c @@ -176,6 +176,23 @@ void _opt_register(const char *names, enum opt_type type, add_opt(&opt); } +bool opt_unregister(const char *names) +{ + int found = -1, i; + + for (i = 0; i < opt_count; i++) { + if (opt_table[i].type == OPT_SUBTABLE) + continue; + if (strcmp(opt_table[i].names, names) == 0) + found = i; + } + if (found == -1) + return false; + opt_count--; + memmove(&opt_table[found], &opt_table[found+1], opt_count - found); + return true; +} + void opt_register_table(const struct opt_table entry[], const char *desc) { unsigned int i, start = opt_count; diff --git a/ccan/ccan/opt/opt.h b/ccan/ccan/opt/opt.h index c642ec6fafc8..6f4b9dda8c85 100644 --- a/ccan/ccan/opt/opt.h +++ b/ccan/ccan/opt/opt.h @@ -228,6 +228,15 @@ void opt_register_table(const struct opt_table *table, const char *desc); _opt_register((names), OPT_CB_ARG((cb), OPT_EARLY, (show),(arg)), \ (arg), (desc)) +/** + * opt_unregister - unregister an option. + * @names: the names it was registered with. + * + * This undoes opt_register[_early]_[no]arg. Returns true if the option was + * found, otherwise false. + */ +bool opt_unregister(const char *names); + /** * opt_parse - parse arguments. * @argc: pointer to argc From 77e7f0f56faff23467733a96d05c1111f5631b62 Mon Sep 17 00:00:00 2001 From: darosior Date: Mon, 3 Feb 2020 12:25:06 +0100 Subject: [PATCH 10/10] lightningd/plugin: unregister a plugin's options when stopping it This also remove the now duplicate plugin_hook_unregister_all(), added in the tal destructor of the struct plugin. --- lightningd/plugin.c | 2 +- lightningd/plugin_control.c | 17 ++++++++++++++--- tests/test_plugin.py | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 6d0a36fc64fb..aefa5a8f283b 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -487,7 +487,7 @@ static bool plugin_opt_add(struct plugin *plugin, const char *buffer, popt = tal(plugin, struct plugin_opt); popt->value = talz(popt, struct plugin_opt_value); - popt->name = tal_fmt(plugin, "--%.*s", nametok->end - nametok->start, + popt->name = tal_fmt(popt, "--%.*s", nametok->end - nametok->start, buffer + nametok->start); if (json_tok_streq(buffer, typetok, "string")) { popt->type = "string"; diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 18d4f2611f05..a5b83c8e96fa 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -196,6 +197,18 @@ plugin_dynamic_startdir(struct command *cmd, const char *dir_path) return command_still_pending(cmd); } +static void clear_plugin(struct plugin *p, const char *name) +{ + struct plugin_opt *opt; + + list_for_each(&p->plugin_opts, opt, list) + if (!opt_unregister(opt->name)) + fatal("Could not unregister %s from plugin %s", + opt->name, name); + plugin_kill(p, "%s stopped by lightningd via RPC", name); + tal_free(p); +} + static struct command_result * plugin_dynamic_stop(struct command *cmd, const char *plugin_name) { @@ -209,9 +222,7 @@ plugin_dynamic_stop(struct command *cmd, const char *plugin_name) "%s cannot be managed when " "lightningd is up", plugin_name); - plugin_hook_unregister_all(p); - plugin_kill(p, "%s stopped by lightningd via RPC", plugin_name); - tal_free(p); + clear_plugin(p, plugin_name); response = json_stream_success(cmd); if (deprecated_apis) json_add_string(response, "", diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 7c1850f4d783..53a4eb233007 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -791,6 +791,7 @@ def test_rpc_command_hook(node_factory): l1.rpc.plugin_stop('rpc_command.py') +@flaky def test_libplugin(node_factory): """Sanity checks for plugins made with libplugin""" plugin = os.path.join(os.getcwd(), "tests/plugins/test_libplugin")