From 2556df5f7cbb0a5379b7d456ab15a6bd2e65dcef Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu, 23 Jul 2020 14:20:10 +0930
Subject: [PATCH] plugins/pay: Exclude the entrypoint to a routehint to avoid
 cycles

This uses @cdecker's idea of excluding the routehinted channel from the route,
and also consumes the route hints as it goes so that it makes progress.

I don't know if this is correct, but it reliably passes tests/test_pay.py::test_tlv_or_legacy
now.
---
 plugins/libplugin-pay.c | 22 ++++++++++++++++++++++
 plugins/libplugin-pay.h |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c
index 0fa21d339..fa477eb44 100644
--- a/plugins/libplugin-pay.c
+++ b/plugins/libplugin-pay.c
@@ -26,6 +26,7 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd,
 	p->getroute->riskfactorppm = 10000000;
 	p->abort = false;
 	p->route = NULL;
+	p->temp_exclusion = NULL;
 
 	/* Copy over the relevant pieces of information. */
 	if (parent != NULL) {
@@ -482,6 +483,16 @@ static void payment_getroute_add_excludes(struct payment *p,
 	for (size_t i=0; i<tal_count(nodes); i++)
 		json_add_node_id(js, NULL, &nodes[i]);
 
+	/* And make sure we don't route in a circle via the routehint! */
+	if (p->temp_exclusion) {
+		struct short_channel_id_dir scidd;
+		scidd.scid = *p->temp_exclusion;
+		for (size_t dir = 0; dir < 2; dir++) {
+			scidd.dir = dir;
+			json_add_short_channel_id_dir(js, NULL, &scidd);
+		}
+	}
+
 	json_array_end(js);
 }
 
@@ -1776,6 +1787,12 @@ static struct route_info *next_routehint(struct routehints_data *d,
 	if (d->routehints == NULL || numhints == 0)
 		return NULL;
 
+	/* BOLT #11:
+	 *
+	 *   - if a writer offers more than one of any field type, it:
+	 *     - MUST specify the most-preferred field first, followed
+	 *       by less-preferred fields, in order.
+	 */
 	for (; d->offset <numhints; d->offset++) {
 		curr = d->routehints[d->offset];
 		if (curr == NULL || !routehint_excluded(p, curr))
@@ -1840,8 +1857,13 @@ static void routehint_pre_getroute(struct routehints_data *d, struct payment *p)
 		    type_to_string(tmpctx, struct short_channel_id,
 				   &d->current_routehint->short_channel_id),
 		    d->current_routehint->cltv_expiry_delta);
+
+		/* Exclude the entrypoint to the routehint, so we don't end up
+		 * going through the destination to the entrypoint. */
+		p->temp_exclusion = &d->current_routehint[0].short_channel_id;
 	} else {
 		plugin_log(p->plugin, LOG_DBG, "Not using a routehint");
+		p->temp_exclusion = NULL;
 	}
 }
 
diff --git a/plugins/libplugin-pay.h b/plugins/libplugin-pay.h
index c48c61f1e..1bcb6db54 100644
--- a/plugins/libplugin-pay.h
+++ b/plugins/libplugin-pay.h
@@ -232,6 +232,9 @@ struct payment {
 	struct channel_hint *channel_hints;
 	struct node_id *excluded_nodes;
 
+	/* Optional temporarily excluded channel (i.e. this routehint) */
+	struct short_channel_id *temp_exclusion;
+
 	struct payment_result *result;
 
 	/* Did something happen that will cause all future attempts to fail?