From 2549dd6ea58ade5249c168e1bdc08a39e0914372 Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Sun, 23 Jul 2023 16:49:00 +0200
Subject: [PATCH] pay: Annotate both local alias and real scid with channel
 hints

Debugging a number of payments showed that we sometimes waste a number
of attempts routing through a channel via its alias, rather than its
scid. This is because while we annotate the scid when it has been set,
we do not do so for the alias. The alias then is picked for routing
despite not having enough capacity, failing the attempt locally.

It can also happen that we alternate between scid and alias, doubling
the number of failed attempts before we can make progress.

This patch sets the hint for the alias to a capacity of 0 and disables
it as if the peer were offline. This means when available we'll always
use the scid, which is also far easier to read in the logs.

Changelog-Fixed: pay: We now track spendable amounts when routing on both the local alias as well as the short channel ID
---
 plugins/libplugin-pay.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c
index eb46dbabf..86a662b6d 100644
--- a/plugins/libplugin-pay.c
+++ b/plugins/libplugin-pay.c
@@ -2360,7 +2360,6 @@ local_channel_hints_listpeerchannels(struct command *cmd, const char *buffer,
 	chans = json_to_listpeers_channels(tmpctx, buffer, toks);
 
 	for (size_t i = 0; i < tal_count(chans); i++) {
-		struct short_channel_id scid;
 		bool enabled;
 		u16 htlc_budget;
 
@@ -2369,11 +2368,6 @@ local_channel_hints_listpeerchannels(struct command *cmd, const char *buffer,
 		 * state. */
 		enabled = chans[i]->connected && streq(chans[i]->state, "CHANNELD_NORMAL");
 
-		if (chans[i]->scid != NULL)
-			scid = *chans[i]->scid;
-		else
-			scid = *chans[i]->alias[LOCAL];
-
 		/* Take the configured number of max_htlcs and
 		 * subtract any HTLCs that might already be added to
 		 * the channel. This is a best effort estimate and
@@ -2384,8 +2378,28 @@ local_channel_hints_listpeerchannels(struct command *cmd, const char *buffer,
 		else
 			htlc_budget = chans[i]->max_accepted_htlcs - chans[i]->num_htlcs;
 
-		channel_hints_update(p, scid, chans[i]->direction, enabled, true,
-				     &chans[i]->spendable_msat, &htlc_budget);
+		/* If we have both a scid and a local alias we want to
+		 * use the scid, and mark the alias as
+		 * unusable. Otherwise `getroute` might return the
+		 * alias, which we resolve correctly, but our
+		 * channel_hints would be off after updates, since
+		 * we'd only ever update one of the aliases. Causing
+		 * the other to be considered usable.
+		 */
+		if (chans[i]->scid != NULL) {
+			channel_hints_update(
+			    p, *chans[i]->scid, chans[i]->direction, enabled,
+			    true, &chans[i]->spendable_msat, &htlc_budget);
+			channel_hints_update(p, *chans[i]->alias[LOCAL],
+					     chans[i]->direction,
+					     false /* not enabled */, true,
+					     &AMOUNT_MSAT(0), &htlc_budget);
+		} else {
+			channel_hints_update(p, *chans[i]->alias[LOCAL],
+					     chans[i]->direction, enabled, true,
+					     &chans[i]->spendable_msat,
+					     &htlc_budget);
+		}
 	}
 
 	payment_continue(p);