From 0279be1d1301b99adf9997f108b3186c45f04a75 Mon Sep 17 00:00:00 2001
From: ZmnSCPxj jxPCSnmZ <zmnscpxj@protonmail.com>
Date: Sun, 9 Aug 2020 15:19:21 +0800
Subject: [PATCH] plugins/libplugin-pay.c: Be less aggressive with advancing
 through routehints.

Only advance through routehints if no route was found at all, or if the
estimated capacity at the routehint is lower than the amount that we
have to send through the routehint.

Changelog-Fixed: pay: Be less aggressive with forgetting routehints.
---
 plugins/libplugin-pay.c | 56 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c
index 1d3814fea..c735fc8c0 100644
--- a/plugins/libplugin-pay.c
+++ b/plugins/libplugin-pay.c
@@ -384,6 +384,7 @@ static struct command_result *payment_getroute_result(struct command *cmd,
 	/* Ensure that our fee and CLTV budgets are respected. */
 	if (amount_msat_greater(fee, p->constraints.fee_budget)) {
 		payment_exclude_most_expensive(p);
+		p->route = tal_free(p->route);
 		payment_fail(
 		    p, "Fee exceeds our fee budget: %s > %s, discarding route",
 		    type_to_string(tmpctx, struct amount_msat, &fee),
@@ -393,9 +394,11 @@ static struct command_result *payment_getroute_result(struct command *cmd,
 	}
 
 	if (p->route[0].delay > p->constraints.cltv_budget) {
+		u32 delay = p->route[0].delay;
 		payment_exclude_longest_delay(p);
+		p->route = tal_free(p->route);
 		payment_fail(p, "CLTV delay exceeds our CLTV budget: %d > %d",
-			     p->route[0].delay, p->constraints.cltv_budget);
+			     delay, p->constraints.cltv_budget);
 		return command_still_pending(cmd);
 	}
 
@@ -1760,12 +1763,17 @@ static struct route_info **filter_routehints(struct routehints_data *d,
 	return tal_steal(d, hints);
 }
 
+static bool route_msatoshi(struct amount_msat *total,
+			   const struct amount_msat msat,
+			   const struct route_info *route, size_t num_route);
+
 static bool routehint_excluded(struct payment *p,
 			       const struct route_info *routehint)
 {
 	const struct node_id *nodes = payment_get_excluded_nodes(tmpctx, p);
 	const struct short_channel_id_dir *chans =
 	    payment_get_excluded_channels(tmpctx, p);
+	const struct channel_hint *hints = payment_root(p)->channel_hints;
 
 	/* Note that we ignore direction here: in theory, we could have
 	 * found that one direction of a channel is unavailable, but they
@@ -1779,6 +1787,41 @@ static bool routehint_excluded(struct payment *p,
 		for (size_t j = 0; j < tal_count(chans); j++)
 			if (short_channel_id_eq(&chans[j].scid, &r->short_channel_id))
 				return true;
+
+		/* Skip the capacity check if this is the last hop
+		 * in the routehint.
+		 * The last hop in the routehint delivers the exact
+		 * final amount to the destination, which
+		 * payment_get_excluded_channels uses for excluding
+		 * already.
+		 * Thus, the capacity check below only really matters
+		 * for multi-hop routehints.
+		 */
+		if (i == tal_count(routehint) - 1)
+			continue;
+
+		/* Check our capacity fits.  */
+		struct amount_msat needed_capacity;
+		if (!route_msatoshi(&needed_capacity, p->amount,
+				    r + 1, tal_count(routehint) - i - 1))
+			return true;
+		/* Why do we scan the hints again if
+		 * payment_get_excluded_channels already does?
+		 * Because payment_get_excluded_channels checks the
+		 * amount at destination, but we know that we are
+		 * a specific distance from the destination and we
+		 * know the exact capacity we need to send via this
+		 * channel, which is greater than the destination.
+		 */
+		for (size_t j = 0; j < tal_count(hints); j++) {
+			if (!short_channel_id_eq(&hints[j].scid.scid, &r->short_channel_id))
+				continue;
+			/* We exclude on equality because we set the estimate
+			 * to the smallest failed attempt.  */
+			if (amount_msat_greater_eq(needed_capacity,
+						   hints[j].estimated_capacity))
+				return true;
+		}
 	}
 	return false;
 }
@@ -2044,9 +2087,14 @@ static struct routehints_data *routehint_data_init(struct payment *p)
 		pd = payment_mod_routehints_get_data(payment_root(p));
 		d->destination_reachable = pd->destination_reachable;
 		d->routehints = pd->routehints;
-		if (p->parent->step == PAYMENT_STEP_RETRY)
-			d->offset = pd->offset + 1;
-		else
+		pd = payment_mod_routehints_get_data(p->parent);
+		if (p->parent->step == PAYMENT_STEP_RETRY) {
+			d->offset = pd->offset;
+			/* If the previous try failed to route, advance
+			 * to the next routehint.  */
+			if (!p->parent->route)
+				++d->offset;
+		} else
 			d->offset = 0;
 		return d;
 	} else {