From f5579555154cdb04173ae467fe43a3eb9838c478 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 9 Jun 2020 18:30:14 +0200 Subject: [PATCH] paymod: Consolidate parsing RPC results in libplugin We handle these in a number of different ways, and regularly get the parsing and logic for optional fields wrong, so let's consolidate them here. --- plugins/libplugin-pay.c | 185 +++++++++++++++++++++++---------------- plugins/libplugin-pay.h | 27 ++---- plugins/libplugin.c | 189 ++++++++++++++++++++++++++++++++++++++++ plugins/libplugin.h | 55 ++++++++++++ plugins/pay.c | 3 +- tests/test_pay.py | 52 ++++++++++- 6 files changed, 414 insertions(+), 97 deletions(-) diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 5275df90e..179fa63b6 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -176,48 +176,6 @@ void payment_start(struct payment *p) payment_rpc_failure, p)); } -static bool route_hop_from_json(struct route_hop *dst, const char *buffer, - const jsmntok_t *toks) -{ - const jsmntok_t *idtok = json_get_member(buffer, toks, "id"); - const jsmntok_t *channeltok = json_get_member(buffer, toks, "channel"); - const jsmntok_t *directiontok = json_get_member(buffer, toks, "direction"); - const jsmntok_t *amounttok = json_get_member(buffer, toks, "amount_msat"); - const jsmntok_t *delaytok = json_get_member(buffer, toks, "delay"); - const jsmntok_t *styletok = json_get_member(buffer, toks, "style"); - - if (idtok == NULL || channeltok == NULL || directiontok == NULL || - amounttok == NULL || delaytok == NULL || styletok == NULL) - return false; - - json_to_node_id(buffer, idtok, &dst->nodeid); - json_to_short_channel_id(buffer, channeltok, &dst->channel_id); - json_to_int(buffer, directiontok, &dst->direction); - json_to_msat(buffer, amounttok, &dst->amount); - json_to_number(buffer, delaytok, &dst->delay); - dst->style = json_tok_streq(buffer, styletok, "legacy") - ? ROUTE_HOP_LEGACY - : ROUTE_HOP_TLV; - return true; -} - -static struct route_hop * -tal_route_from_json(const tal_t *ctx, const char *buffer, const jsmntok_t *toks) -{ - size_t num = toks->size, i; - struct route_hop *hops; - const jsmntok_t *rtok; - if (toks->type != JSMN_ARRAY) - return NULL; - - hops = tal_arr(ctx, struct route_hop, num); - json_for_each_arr(i, rtok, toks) { - if (!route_hop_from_json(&hops[i], buffer, rtok)) - return tal_free(hops); - } - return hops; -} - static void payment_exclude_most_expensive(struct payment *p) { struct payment *root = payment_root(p); @@ -357,7 +315,7 @@ static struct command_result *payment_getroute_result(struct command *cmd, const jsmntok_t *rtok = json_get_member(buffer, toks, "route"); struct amount_msat fee; assert(rtok != NULL); - p->route = tal_route_from_json(p, buffer, rtok); + p->route = json_to_route(p, buffer, rtok); p->step = PAYMENT_STEP_GOT_ROUTE; fee = payment_route_fee(p); @@ -495,37 +453,6 @@ static u8 *tal_towire_legacy_payload(const tal_t *ctx, const struct legacy_paylo return buf; } -static struct createonion_response * -tal_createonion_response_from_json(const tal_t *ctx, const char *buffer, - const jsmntok_t *toks) -{ - size_t i; - struct createonion_response *resp; - const jsmntok_t *oniontok = json_get_member(buffer, toks, "onion"); - const jsmntok_t *secretstok = json_get_member(buffer, toks, "shared_secrets"); - const jsmntok_t *cursectok; - - if (oniontok == NULL || secretstok == NULL) - return NULL; - resp = tal(ctx, struct createonion_response); - - if (oniontok->type != JSMN_STRING) - goto fail; - - resp->onion = json_tok_bin_from_hex(resp, buffer, oniontok); - resp->shared_secrets = tal_arr(resp, struct secret, secretstok->size); - - json_for_each_arr(i, cursectok, secretstok) { - if (cursectok->type != JSMN_STRING) - goto fail; - json_to_secret(buffer, cursectok, &resp->shared_secrets[i]); - } - return resp; - -fail: - return tal_free(resp); -} - static struct payment_result *tal_sendpay_result_from_json(const tal_t *ctx, const char *buffer, const jsmntok_t *toks) @@ -840,7 +767,7 @@ static struct command_result *payment_createonion_success(struct command *cmd, payment_chanhints_apply_route(p, false); - p->createonion_response = tal_createonion_response_from_json(p, buffer, toks); + p->createonion_response = json_to_createonion_response(p, buffer, toks); req = jsonrpc_request_start(p->plugin, NULL, "sendonion", payment_sendonion_success, @@ -1893,3 +1820,111 @@ static void shadow_route_cb(struct shadow_route_data *d, REGISTER_PAYMENT_MODIFIER(shadowroute, struct shadow_route_data *, shadow_route_init, shadow_route_cb); + +static void direct_pay_override(struct payment *p) { + + /* The root has performed the search for a direct channel. */ + struct payment *root = payment_root(p); + struct direct_pay_data *d; + struct channel_hint *hint = NULL; + + /* If we were unable to find a direct channel we don't need to do + * anything. */ + d = payment_mod_directpay_get_data(root); + + if (d->chan == NULL) + return payment_continue(p); + + /* If we have a channel we need to make sure that it still has + * sufficient capacity. Look it up in the channel_hints. */ + for (size_t i=0; ichannel_hints); i++) { + struct short_channel_id_dir *cur = &root->channel_hints[i].scid; + if (short_channel_id_eq(&cur->scid, &d->chan->scid) && + cur->dir == d->chan->dir) { + hint = &root->channel_hints[i]; + break; + } + } + + if (hint && hint->enabled && + amount_msat_greater(hint->estimated_capacity, p->amount)) { + /* Now build a route that consists only of this single hop */ + p->route = tal_arr(p, struct route_hop, 1); + p->route[0].amount = p->amount; + p->route[0].delay = p->getroute->cltv; + p->route[0].channel_id = hint->scid.scid; + p->route[0].direction = hint->scid.dir; + p->route[0].nodeid = *p->destination; + p->route[0].style = ROUTE_HOP_TLV; + plugin_log(p->plugin, LOG_DBG, + "Found a direct channel (%s) with sufficient " + "capacity, skipping route computation.", + type_to_string(tmpctx, struct short_channel_id_dir, + &hint->scid)); + + payment_set_step(p, PAYMENT_STEP_GOT_ROUTE); + } + + + payment_continue(p); +} + +/* Now that we have the listpeers result for the root payment, let's search + * for a direct channel that is a) connected and b) in state normal. We will + * check the capacity based on the channel_hints in the override. */ +static struct command_result *direct_pay_listpeers(struct command *cmd, + const char *buffer, + const jsmntok_t *toks, + struct payment *p) +{ + struct listpeers_result *r = + json_to_listpeers_result(tmpctx, buffer, toks); + struct direct_pay_data *d = payment_mod_directpay_get_data(p); + + if (tal_count(r->peers) == 1) { + struct listpeers_peer *peer = r->peers[0]; + if (!peer->connected) + goto cont; + + for (size_t i=0; ichannels); i++) { + struct listpeers_channel *chan = r->peers[0]->channels[i]; + if (!streq(chan->state, "CHANNELD_NORMAL")) + continue; + + d->chan = tal(d, struct short_channel_id_dir); + d->chan->scid = *chan->scid; + d->chan->dir = *chan->direction; + } + } +cont: + direct_pay_override(p); + return command_still_pending(cmd); + +} + +static void direct_pay_cb(struct direct_pay_data *d, struct payment *p) +{ + struct out_req *req; + +/* Look up the direct channel only on root. */ + if (p->step != PAYMENT_STEP_INITIALIZED) + return payment_continue(p); + + + + req = jsonrpc_request_start(p->plugin, NULL, "listpeers", + direct_pay_listpeers, direct_pay_listpeers, + p); + json_add_node_id(req->js, "id", p->destination); + send_outreq(p->plugin, req); +} + +static struct direct_pay_data *direct_pay_init(struct payment *p) +{ + struct direct_pay_data *d = tal(p, struct direct_pay_data); + d->chan = NULL; + return d; +} + +REGISTER_PAYMENT_MODIFIER(directpay, struct direct_pay_data *, direct_pay_init, + direct_pay_cb); diff --git a/plugins/libplugin-pay.h b/plugins/libplugin-pay.h index 8089f605f..a6945a6ff 100644 --- a/plugins/libplugin-pay.h +++ b/plugins/libplugin-pay.h @@ -6,21 +6,6 @@ #include #include -enum route_hop_style { - ROUTE_HOP_LEGACY = 1, - ROUTE_HOP_TLV = 2, -}; - -struct route_hop { - struct short_channel_id channel_id; - int direction; - struct node_id nodeid; - struct amount_msat amount; - u32 delay; - struct pubkey *blinding; - enum route_hop_style style; -}; - struct legacy_payload { struct short_channel_id scid; struct amount_msat forward_amt; @@ -42,11 +27,6 @@ struct createonion_request { struct secret *session_key; }; -struct createonion_response { - u8 *onion; - struct secret *shared_secrets; -}; - /* States returned by listsendpays, waitsendpay, etc. */ enum payment_result_state { PAYMENT_PENDING, @@ -325,11 +305,18 @@ struct shadow_route_data { struct node_id destination; struct route_hop *route; }; + +struct direct_pay_data { + /* If we have a direct channel remember it, so we can check each + * attempt against the channel hints. */ + struct short_channel_id_dir *chan; +}; /* List of globally available payment modifiers. */ REGISTER_PAYMENT_MODIFIER_HEADER(retry, struct retry_mod_data); REGISTER_PAYMENT_MODIFIER_HEADER(routehints, struct routehints_data); REGISTER_PAYMENT_MODIFIER_HEADER(exemptfee, struct exemptfee_data); REGISTER_PAYMENT_MODIFIER_HEADER(shadowroute, struct shadow_route_data); +REGISTER_PAYMENT_MODIFIER_HEADER(directpay, struct direct_pay_data); /* For the root payment we can seed the channel_hints with the result from * `listpeers`, hence avoid channels that we know have insufficient capacity diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 7a1f80ab2..b5c8b0bf1 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -1266,3 +1266,192 @@ void plugin_main(char *argv[], tal_free(plugin); } + +static struct listpeers_channel *json_to_listpeers_channel(const tal_t *ctx, + const char *buffer, + const jsmntok_t *tok) +{ + struct listpeers_channel *chan; + const jsmntok_t *privtok = json_get_member(buffer, tok, "private"), + *statetok = json_get_member(buffer, tok, "state"), + *ftxidtok = + json_get_member(buffer, tok, "funding_txid"), + *scidtok = + json_get_member(buffer, tok, "short_channel_id"), + *dirtok = json_get_member(buffer, tok, "direction"), + *tmsattok = + json_get_member(buffer, tok, "total_msat"), + *smsattok = + json_get_member(buffer, tok, "spendable_msat"); + + if (privtok == NULL || privtok->type != JSMN_PRIMITIVE || + statetok == NULL || statetok->type != JSMN_STRING || + ftxidtok == NULL || ftxidtok->type != JSMN_STRING || + (scidtok != NULL && scidtok->type != JSMN_STRING) || + (dirtok != NULL && dirtok->type != JSMN_PRIMITIVE) || + tmsattok == NULL || tmsattok->type != JSMN_STRING || + smsattok == NULL || smsattok->type != JSMN_STRING) + return NULL; + + chan = tal(ctx, struct listpeers_channel); + + json_to_bool(buffer, privtok, &chan->private); + chan->state = json_strdup(chan, buffer, statetok); + json_to_txid(buffer, ftxidtok, &chan->funding_txid); + if (scidtok != NULL) { + assert(dirtok != NULL); + chan->scid = tal(chan, struct short_channel_id); + chan->direction = tal(chan, int); + json_to_short_channel_id(buffer, scidtok, chan->scid); + json_to_int(buffer, dirtok, chan->direction); + }else { + assert(dirtok == NULL); + chan->scid = NULL; + chan->direction = NULL; + } + + json_to_msat(buffer, tmsattok, &chan->total_msat); + json_to_msat(buffer, smsattok, &chan->spendable_msat); + + return chan; +} + +static struct listpeers_peer *json_to_listpeers_peer(const tal_t *ctx, + const char *buffer, + const jsmntok_t *tok) +{ + struct listpeers_peer *res; + size_t i; + const jsmntok_t *iter; + const jsmntok_t *idtok = json_get_member(buffer, tok, "id"), + *conntok = json_get_member(buffer, tok, "connected"), + *netaddrtok = json_get_member(buffer, tok, "netaddr"), + *channelstok = json_get_member(buffer, tok, "channels"); + + /* Preliminary sanity checks. */ + if (idtok == NULL || idtok->type != JSMN_STRING || conntok == NULL || + conntok->type != JSMN_PRIMITIVE || + (netaddrtok != NULL && netaddrtok->type != JSMN_ARRAY) || + channelstok == NULL || channelstok->type != JSMN_ARRAY) + return NULL; + + res = tal(ctx, struct listpeers_peer); + json_to_node_id(buffer, idtok, &res->id); + json_to_bool(buffer, conntok, &res->connected); + + res->netaddr = tal_arr(res, const char *, 0); + if (netaddrtok != NULL) { + json_for_each_arr(i, iter, netaddrtok) { + tal_arr_expand(&res->netaddr, + json_strdup(res, buffer, iter)); + } + } + + res->channels = tal_arr(res, struct listpeers_channel *, 0); + json_for_each_arr(i, iter, channelstok) { + struct listpeers_channel *chan = json_to_listpeers_channel(res, buffer, iter); + assert(chan != NULL); + tal_arr_expand(&res->channels, chan); + } + + return res; +} + +struct listpeers_result *json_to_listpeers_result(const tal_t *ctx, + const char *buffer, + const jsmntok_t *toks) +{ + size_t i; + const jsmntok_t *iter; + struct listpeers_result *res; + const jsmntok_t *peerstok = json_get_member(buffer, toks, "peers"); + + if (peerstok == NULL || peerstok->type != JSMN_ARRAY) + return NULL; + + res = tal(ctx, struct listpeers_result); + res->peers = tal_arr(res, struct listpeers_peer *, 0); + + json_for_each_obj(i, iter, peerstok) { + struct listpeers_peer *p = + json_to_listpeers_peer(res, buffer, iter); + if (p == NULL) + return tal_free(res); + tal_arr_expand(&res->peers, p); + } + return res; +} + +struct createonion_response *json_to_createonion_response(const tal_t *ctx, + const char *buffer, + const jsmntok_t *toks) +{ + size_t i; + struct createonion_response *resp; + const jsmntok_t *oniontok = json_get_member(buffer, toks, "onion"); + const jsmntok_t *secretstok = json_get_member(buffer, toks, "shared_secrets"); + const jsmntok_t *cursectok; + + if (oniontok == NULL || secretstok == NULL) + return NULL; + + resp = tal(ctx, struct createonion_response); + + if (oniontok->type != JSMN_STRING) + goto fail; + + resp->onion = json_tok_bin_from_hex(resp, buffer, oniontok); + resp->shared_secrets = tal_arr(resp, struct secret, secretstok->size); + + json_for_each_arr(i, cursectok, secretstok) { + if (cursectok->type != JSMN_STRING) + goto fail; + json_to_secret(buffer, cursectok, &resp->shared_secrets[i]); + } + return resp; + +fail: + return tal_free(resp); +} + +static bool json_to_route_hop_inplace(struct route_hop *dst, const char *buffer, + const jsmntok_t *toks) +{ + const jsmntok_t *idtok = json_get_member(buffer, toks, "id"); + const jsmntok_t *channeltok = json_get_member(buffer, toks, "channel"); + const jsmntok_t *directiontok = json_get_member(buffer, toks, "direction"); + const jsmntok_t *amounttok = json_get_member(buffer, toks, "amount_msat"); + const jsmntok_t *delaytok = json_get_member(buffer, toks, "delay"); + const jsmntok_t *styletok = json_get_member(buffer, toks, "style"); + + if (idtok == NULL || channeltok == NULL || directiontok == NULL || + amounttok == NULL || delaytok == NULL || styletok == NULL) + return false; + + json_to_node_id(buffer, idtok, &dst->nodeid); + json_to_short_channel_id(buffer, channeltok, &dst->channel_id); + json_to_int(buffer, directiontok, &dst->direction); + json_to_msat(buffer, amounttok, &dst->amount); + json_to_number(buffer, delaytok, &dst->delay); + dst->style = json_tok_streq(buffer, styletok, "legacy") + ? ROUTE_HOP_LEGACY + : ROUTE_HOP_TLV; + return true; +} + +struct route_hop *json_to_route(const tal_t *ctx, const char *buffer, + const jsmntok_t *toks) +{ + size_t num = toks->size, i; + struct route_hop *hops; + const jsmntok_t *rtok; + if (toks->type != JSMN_ARRAY) + return NULL; + + hops = tal_arr(ctx, struct route_hop, num); + json_for_each_arr(i, rtok, toks) { + if (!json_to_route_hop_inplace(&hops[i], buffer, rtok)) + return tal_free(hops); + } + return hops; +} diff --git a/plugins/libplugin.h b/plugins/libplugin.h index 939122cce..85711bda9 100644 --- a/plugins/libplugin.h +++ b/plugins/libplugin.h @@ -254,4 +254,59 @@ void NORETURN LAST_ARG_NULL plugin_main(char *argv[], const struct plugin_hook *hook_subs, size_t num_hook_subs, ...); + +struct listpeers_channel { + bool private; + struct bitcoin_txid funding_txid; + const char *state; + struct short_channel_id *scid; + int *direction; + struct amount_msat total_msat; + struct amount_msat spendable_msat; + /* TODO Add fields as we need them. */ +}; + +struct listpeers_peer { + struct node_id id; + bool connected; + const char **netaddr; + struct feature_set *features; + struct listpeers_channel **channels; +}; + +struct listpeers_result { + struct listpeers_peer **peers; +}; + +struct listpeers_result *json_to_listpeers_result(const tal_t *ctx, + const char *buffer, + const jsmntok_t *tok); + +struct createonion_response { + u8 *onion; + struct secret *shared_secrets; +}; + +struct createonion_response *json_to_createonion_response(const tal_t *ctx, + const char *buffer, + const jsmntok_t *toks); + +enum route_hop_style { + ROUTE_HOP_LEGACY = 1, + ROUTE_HOP_TLV = 2, +}; + +struct route_hop { + struct short_channel_id channel_id; + int direction; + struct node_id nodeid; + struct amount_msat amount; + u32 delay; + struct pubkey *blinding; + enum route_hop_style style; +}; + +struct route_hop *json_to_route(const tal_t *ctx, const char *buffer, + const jsmntok_t *toks); + #endif /* LIGHTNING_PLUGINS_LIBPLUGIN_H */ diff --git a/plugins/pay.c b/plugins/pay.c index 6772bfc53..63fef4f48 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -1837,10 +1837,11 @@ static void init(struct plugin *p, } struct payment_modifier *paymod_mods[] = { + &local_channel_hints_pay_mod, + &directpay_pay_mod, &shadowroute_pay_mod, &exemptfee_pay_mod, &routehints_pay_mod, - &local_channel_hints_pay_mod, &retry_pay_mod, NULL, }; diff --git a/tests/test_pay.py b/tests/test_pay.py index 1aa210035..b7cb37aa4 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3120,4 +3120,54 @@ def test_pay_exemptfee(node_factory, compat): with pytest.raises(RpcError, match=err): l1.rpc.dev_pay(l3.rpc.invoice(threshold - 1, "lbl3", "desc")['bolt11'], use_shadow=False) - l1.rpc.pay(inv, exemptfee=5001) + # While this'll work just fine + l1.rpc.dev_pay(l3.rpc.invoice(int(5001 * 200), "lbl4", "desc")['bolt11'], use_shadow=False) + + +@unittest.skipIf(is_compat('090'), "Modifier only available with paymod") +@unittest.skipIf(not DEVELOPER, "Requires use_shadow flag") +def test_pay_peer(node_factory): + """If we have a direct channel to the destination we should use that. + + This is complicated a bit by not having sufficient capacity, but the + channel_hints can help us there. + + l1 -> l2 + | ^ + v / + l3 + """ + l1, l2 = node_factory.line_graph(2, fundamount=10**6) + l3 = node_factory.get_node() + + l1.openchannel(l3, 10**6, wait_for_announce=False) + l3.openchannel(l2, 10**6, wait_for_announce=True) + + wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 6) + + def spendable(n1, n2): + peer = n1.rpc.listpeers(n2.info['id'])['peers'][0] + chan = peer['channels'][0] + avail = chan['spendable_msat'] + return avail + + amt = Millisatoshi(10**8) + # How many payments do we expect to go through directly? + direct = spendable(l1, l2).millisatoshis // amt.millisatoshis + + # Remember the l1 -> l3 capacity, it should not change until we run out of + # direct capacity. + l1l3cap = spendable(l1, l3) + + for i in range(0, direct): + inv = l2.rpc.invoice(amt.millisatoshis, "lbl{}".format(i), + "desc{}".format(i))['bolt11'] + l1.rpc.dev_pay(inv, use_shadow=False) + + # We should not have more than amt in the direct channel anymore + assert(spendable(l1, l2) < amt) + assert(spendable(l1, l3) == l1l3cap) + + # Next one should take the alternative, but it should still work + inv = l2.rpc.invoice(amt.millisatoshis, "final", "final")['bolt11'] + l1.rpc.dev_pay(inv, use_shadow=False)