From c737fa6b918373577bfd99f5442191e7f017833b Mon Sep 17 00:00:00 2001
From: trueptolemy <trueptolemy@foxmail.com>
Date: Sat, 14 Sep 2019 16:48:01 +0800
Subject: [PATCH] pay: Fix logic of the intereface `find_worst_channel`

---
 plugins/pay.c     | 27 ++++++++++++++++++---------
 tests/test_pay.py | 12 ++++++++----
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/plugins/pay.c b/plugins/pay.c
index 2d23051a8..eb773c2ef 100644
--- a/plugins/pay.c
+++ b/plugins/pay.c
@@ -494,22 +494,31 @@ static struct command_result *sendpay_error(struct command *cmd,
 
 static const jsmntok_t *find_worst_channel(const char *buf,
 					   const jsmntok_t *route,
-					   const char *fieldname,
-					   u64 final)
+					   const char *fieldname)
 {
-	u64 prev = final, worstval = 0;
-	const jsmntok_t *worst = NULL, *t;
+	u64 prev, worstval = 0;
+	const jsmntok_t *worst = NULL, *t, *t_prev = NULL;
 	size_t i;
 
 	json_for_each_arr(i, t, route) {
 		u64 val;
 
 		json_to_u64(buf, json_get_member(buf, t, fieldname), &val);
-		if (worst == NULL || val - prev > worstval) {
-			worst = t;
-			worstval = val - prev;
+
+		/* For the first hop, now we can't know if it's the worst.
+		 * Just store the info and continue. */
+		if (!i) {
+			prev = val;
+			t_prev = t;
+			continue;
+		}
+
+		if (worst == NULL || prev - val > worstval) {
+			worst = t_prev;
+			worstval = prev - val;
 		}
 		prev = val;
+		t_prev = t;
 	}
 
 	return worst;
@@ -606,7 +615,7 @@ static struct command_result *getroute_done(struct command *cmd,
 
 		/* Try excluding most fee-charging channel (unless it's in
 		 * routeboost). */
-		charger = find_worst_channel(buf, t, "msatoshi", pc->msat.millisatoshis); /* Raw: shared function needs u64 */
+		charger = find_worst_channel(buf, t, "msatoshi");
 		if (maybe_exclude(pc, buf, charger)) {
 			return start_pay_attempt(cmd, pc,
 						 "Excluded expensive channel %s",
@@ -633,7 +642,7 @@ static struct command_result *getroute_done(struct command *cmd,
 		else
 			tal_free(failed);
 
-		delayer = find_worst_channel(buf, t, "delay", pc->final_cltv);
+		delayer = find_worst_channel(buf, t, "delay");
 
 		/* Try excluding most delaying channel (unless it's in
 		 * routeboost). */
diff --git a/tests/test_pay.py b/tests/test_pay.py
index 50b7b1256..d73eb19b4 100644
--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@@ -94,25 +94,29 @@ def test_pay_limits(node_factory):
 
     assert err.value.error['code'] == PAY_ROUTE_TOO_EXPENSIVE
 
-    # It should have retried (once without routehint, too)
+    # It should have retried two more times (one without routehint and one with routehint)
     status = l1.rpc.call('paystatus', {'bolt11': inv['bolt11']})['pay'][0]['attempts']
 
     # Excludes channel, then ignores routehint which includes that, then
     # it excludes other channel.
-    assert len(status) == 2
+    assert len(status) == 3
     assert status[0]['strategy'] == "Initial attempt"
+    # Exclude the channel l1->l2
     assert status[1]['strategy'].startswith("Excluded expensive channel ")
+    # With the routehint
+    assert status[2]['strategy'].startswith("Trying route hint")
 
     # Delay too high.
     with pytest.raises(RpcError, match=r'Route wanted delay of .* blocks') as err:
         l1.rpc.call('pay', {'bolt11': inv['bolt11'], 'msatoshi': 100000, 'maxdelay': 0})
 
     assert err.value.error['code'] == PAY_ROUTE_TOO_EXPENSIVE
-    # Should also have retried.
+    # Should also have retried two more times.
     status = l1.rpc.call('paystatus', {'bolt11': inv['bolt11']})['pay'][1]['attempts']
-    assert len(status) == 2
+    assert len(status) == 3
     assert status[0]['strategy'] == "Initial attempt"
     assert status[1]['strategy'].startswith("Excluded delaying channel ")
+    assert status[2]['strategy'].startswith("Trying route hint")
 
     # This works, because fee is less than exemptfee.
     l1.rpc.call('pay', {'bolt11': inv['bolt11'], 'msatoshi': 100000, 'maxfeepercent': 0.0001, 'exemptfee': 2000})