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

@@ -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; i<tal_count(root->channel_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; i<tal_count(peer->channels); 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);

plugins/libplugin-pay.h

@@ -6,21 +6,6 @@
 #include <plugins/libplugin.h>
 #include <wire/gen_onion_wire.h>
 
-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

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;
+}

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 */

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,
 };

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)