renepay: disabled channels in a map

Expecting to have more than just a bunch of disabled channels if we
prune the lightning network heavily I am changing the internal data
structure of disabledmap from a simple array to a hashtable.

Have a finer control over disabled channels by targeting
short_channel_id_dir instead of short_channel_id,
ie. we can disable a single direction of a channel.

Signed-off-by: Lagrang3 <[email protected]>

Author: Lagrang3

plugins/renepay/disabledmap.c

@@ -7,35 +7,71 @@ struct disabledmap *disabledmap_new(const tal_t *ctx)
 	if (!obj)
 		return NULL;
 
-	obj->disabled_scids = tal_arr(obj, struct short_channel_id, 0);
-	obj->warned_scids = tal_arr(obj, struct short_channel_id, 0);
+	obj->disabled_map = tal(obj, struct scidd_map);
+	obj->warned_map = tal(obj, struct scidd_map);
+	obj->disabled_ctx = tal(obj, tal_t);
+	obj->warned_ctx = tal(obj, tal_t);
 	obj->disabled_nodes = tal_arr(obj, struct node_id, 0);
 
-	if (!obj->disabled_scids || !obj->warned_scids || !obj->disabled_nodes)
+	if (!obj->disabled_map || !obj->warned_map || !obj->disabled_nodes ||
+	    !obj->disabled_ctx || !obj->warned_ctx)
 		return tal_free(obj);
+
+	scidd_map_init(obj->disabled_map);
+	scidd_map_init(obj->warned_map);
 	return obj;
 }
 
 // FIXME: check success
 void disabledmap_reset(struct disabledmap *p)
 {
-	tal_resize(&p->disabled_scids, 0);
-	tal_resize(&p->warned_scids, 0);
+	/* This will remove and free every element in the maps. */
+	p->warned_ctx = tal_free(p->warned_ctx);
+	p->disabled_ctx = tal_free(p->disabled_ctx);
+
 	tal_resize(&p->disabled_nodes, 0);
+
+	p->disabled_ctx = tal(p, tal_t);
+	p->warned_ctx = tal(p, tal_t);
+}
+
+static void remove_scidd(struct short_channel_id_dir *scidd,
+			 struct scidd_map *map)
+{
+	scidd_map_del(map, scidd);
 }
 
 // FIXME: check success
 void disabledmap_add_channel(struct disabledmap *p,
-			     struct short_channel_id scid)
+			     struct short_channel_id_dir scidd)
 {
-	tal_arr_expand(&p->disabled_scids, scid);
+	struct short_channel_id_dir *ptr_scidd =
+	    scidd_map_get(p->disabled_map, scidd);
+	if (ptr_scidd) {
+		/* htable allows for duplicates, but we don't want duplicates.
+		 */
+		return;
+	}
+	ptr_scidd =
+	    tal_dup(p->disabled_ctx, struct short_channel_id_dir, &scidd);
+	scidd_map_add(p->disabled_map, ptr_scidd);
+	tal_add_destructor2(ptr_scidd, remove_scidd, p->disabled_map);
 }
 
 // FIXME: check success
 void disabledmap_warn_channel(struct disabledmap *p,
-			      struct short_channel_id scid)
+			      struct short_channel_id_dir scidd)
 {
-	tal_arr_expand(&p->warned_scids, scid);
+	struct short_channel_id_dir *ptr_scidd =
+	    scidd_map_get(p->warned_map, scidd);
+	if (ptr_scidd) {
+		/* htable allows for duplicates, but we don't want duplicates.
+		 */
+		return;
+	}
+	ptr_scidd = tal_dup(p->warned_ctx, struct short_channel_id_dir, &scidd);
+	scidd_map_add(p->warned_map, ptr_scidd);
+	tal_add_destructor2(ptr_scidd, remove_scidd, p->warned_map);
 }
 
 // FIXME: check success
@@ -45,29 +81,31 @@ void disabledmap_add_node(struct disabledmap *p, struct node_id node)
 }
 
 bool disabledmap_channel_is_warned(struct disabledmap *p,
-				   struct short_channel_id scid)
+				   struct short_channel_id_dir scidd)
 {
-	for (size_t i = 0; i < tal_count(p->warned_scids); i++) {
-		if (short_channel_id_eq(scid, p->warned_scids[i]))
-			return true;
-	}
-	return false;
+	return scidd_map_get(p->warned_map, scidd) != NULL;
 }
 
 bitmap *tal_disabledmap_get_bitmap(const tal_t *ctx, struct disabledmap *p,
 				   const struct gossmap *gossmap)
 {
-	bitmap *disabled =
-	    tal_arrz(ctx, bitmap, BITMAP_NWORDS(gossmap_max_chan_idx(gossmap)));
+	bitmap *disabled = tal_arrz(
+	    ctx, bitmap, 2 * BITMAP_NWORDS(gossmap_max_chan_idx(gossmap)));
 	if (!disabled)
 		return NULL;
 
 	/* Disable every channel in the list of disabled scids. */
-	for (size_t i = 0; i < tal_count(p->disabled_scids); i++) {
+	struct scidd_map_iter it;
+	for(struct short_channel_id_dir *scidd = scidd_map_first(p->disabled_map,&it);
+		scidd;
+		scidd = scidd_map_next(p->disabled_map, &it)){
+
 		struct gossmap_chan *c =
-		    gossmap_find_chan(gossmap, &p->disabled_scids[i]);
+		    gossmap_find_chan(gossmap, &scidd->scid);
 		if (c)
-			bitmap_set_bit(disabled, gossmap_chan_idx(gossmap, c));
+			bitmap_set_bit(disabled,
+				       gossmap_chan_idx(gossmap, c) * 2 +
+					   scidd->dir);
 	}
 
 	/* Disable all channels that lead to a disabled node. */
@@ -79,7 +117,8 @@ bitmap *tal_disabledmap_get_bitmap(const tal_t *ctx, struct disabledmap *p,
 			int half;
 			const struct gossmap_chan *c =
 			    gossmap_nth_chan(gossmap, node, j, &half);
-			bitmap_set_bit(disabled, gossmap_chan_idx(gossmap, c));
+			bitmap_set_bit(disabled,
+				       gossmap_chan_idx(gossmap, c) * 2 + half);
 		}
 	}
 	return disabled;

plugins/renepay/disabledmap.h

@@ -4,33 +4,60 @@
 #include "config.h"
 #include <bitcoin/short_channel_id.h>
 #include <ccan/bitmap/bitmap.h>
+#include <ccan/htable/htable_type.h>
 #include <common/gossmap.h>
 #include <common/node_id.h>
 
+static inline size_t hash_scidd(const struct short_channel_id_dir scidd)
+{
+	/* scids cost money to generate, so simple hash works here. Letting same
+	 * scid with two directions collide. */
+	return (scidd.scid.u64 >> 32) ^ (scidd.scid.u64 >> 16) ^ scidd.scid.u64;
+}
+
+static inline struct short_channel_id_dir
+self_scidd(const struct short_channel_id_dir *self)
+{
+	return *self;
+}
+
+static inline bool
+my_short_channel_id_dir_eq(const struct short_channel_id_dir *scidd_a,
+			   const struct short_channel_id_dir scidd_b)
+{
+	return short_channel_id_eq(scidd_a->scid, scidd_b.scid) &&
+	       scidd_a->dir == scidd_b.dir;
+}
+
+/* A htable for short_channel_id_dir, the structure itself is the element key.
+ */
+HTABLE_DEFINE_TYPE(struct short_channel_id_dir, self_scidd, hash_scidd,
+		   my_short_channel_id_dir_eq, scidd_map);
+
 struct disabledmap {
 	/* Channels we decided to disable for various reasons. */
-	/* FIXME: disabled_scids should be a set rather than an array, so that
-	 * we don't have to worry about disabling the same channel multiple
-	 * times. */
-	struct short_channel_id *disabled_scids;
+	struct scidd_map *disabled_map;
+	tal_t *disabled_ctx;
 
 	/* Channels that we flagged for failures. If warned two times we will
 	 * disable it. */
-	struct short_channel_id *warned_scids;
+	struct scidd_map *warned_map;
+	tal_t *warned_ctx;
 
 	/* nodes we disable */
+	// FIXME: use a map also for nodes
 	struct node_id *disabled_nodes;
 };
 
 void disabledmap_reset(struct disabledmap *p);
 struct disabledmap *disabledmap_new(const tal_t *ctx);
 void disabledmap_add_channel(struct disabledmap *p,
-			     struct short_channel_id scid);
+			     struct short_channel_id_dir scidd);
 void disabledmap_warn_channel(struct disabledmap *p,
-			      struct short_channel_id scid);
+			      struct short_channel_id_dir scidd);
 void disabledmap_add_node(struct disabledmap *p, struct node_id node);
 bool disabledmap_channel_is_warned(struct disabledmap *p,
-				   struct short_channel_id scid);
+				   struct short_channel_id_dir scidd);
 bitmap *tal_disabledmap_get_bitmap(const tal_t *ctx, struct disabledmap *p,
 				   const struct gossmap *gossmap);
 

plugins/renepay/mcf.c

@@ -449,8 +449,8 @@ static bool channel_is_available(const struct gossmap_chan *c, int dir,
 	if (!gossmap_chan_set(c, dir))
 		return false;
 
-	const u32 chan_id = gossmap_chan_idx(gossmap, c);
-	return !bitmap_test_bit(disabled, chan_id);
+	const u32 chan_idx = gossmap_chan_idx(gossmap, c);
+	return !bitmap_test_bit(disabled, chan_idx * 2 + dir);
 }
 
 // TODO(eduardo): unit test this
@@ -1563,13 +1563,17 @@ static bool check_disabled(const bitmap *disabled,
 	assert(gossmap);
 	assert(chan_extra_map);
 
-	if(tal_bytelen(disabled) != bitmap_sizeof(gossmap_max_chan_idx(gossmap)))
+	if (tal_bytelen(disabled) !=
+	    2 * bitmap_sizeof(gossmap_max_chan_idx(gossmap)))
 		return false;
 
 	for (struct gossmap_chan *chan = gossmap_first_chan(gossmap); chan;
 	     chan = gossmap_next_chan(gossmap, chan)) {
-		const u32 chan_id = gossmap_chan_idx(gossmap, chan);
-		if (bitmap_test_bit(disabled, chan_id))
+		const u32 chan_idx = gossmap_chan_idx(gossmap, chan);
+		/* If both directions are disabled anyways, there is no need to
+		 * fetch their information in chan_extra. */
+		if (bitmap_test_bit(disabled, chan_idx * 2 + 0) &&
+		    bitmap_test_bit(disabled, chan_idx * 2 + 1))
 			continue;
 
 		struct short_channel_id scid = gossmap_chan_scid(gossmap, chan);

plugins/renepay/mods.c

@@ -408,7 +408,7 @@ static void gossmod_cb(struct gossmap_localmods *mods,
 
 	/* Is it disabled? */
 	if (!enabled)
-		payment_disable_chan(payment, scidd->scid, LOG_DBG,
+		payment_disable_chan(payment, *scidd, LOG_DBG,
 				     "listpeerchannels says not enabled");
 
 	/* Also update the uncertainty network by fixing the liquidity of the

plugins/renepay/payment.c

@@ -366,7 +366,7 @@ static struct command_result *payment_finish(struct payment *p)
 	return my_command_finish(p, cmd);
 }
 
-void payment_disable_chan(struct payment *p, struct short_channel_id scid,
+void payment_disable_chan(struct payment *p, struct short_channel_id_dir scidd,
 			  enum log_level lvl, const char *fmt, ...)
 {
 	assert(p);
@@ -378,12 +378,12 @@ void payment_disable_chan(struct payment *p, struct short_channel_id scid,
 	str = tal_vfmt(tmpctx, fmt, ap);
 	va_end(ap);
 	payment_note(p, lvl, "disabling %s: %s",
-		     fmt_short_channel_id(tmpctx, scid),
+		     fmt_short_channel_id_dir(tmpctx, &scidd),
 		     str);
-	disabledmap_add_channel(p->disabledmap, scid);
+	disabledmap_add_channel(p->disabledmap, scidd);
 }
 
-void payment_warn_chan(struct payment *p, struct short_channel_id scid,
+void payment_warn_chan(struct payment *p, struct short_channel_id_dir scidd,
 		       enum log_level lvl, const char *fmt, ...)
 {
 	assert(p);
@@ -395,16 +395,16 @@ void payment_warn_chan(struct payment *p, struct short_channel_id scid,
 	str = tal_vfmt(tmpctx, fmt, ap);
 	va_end(ap);
 
-	if (disabledmap_channel_is_warned(p->disabledmap, scid)) {
-		payment_disable_chan(p, scid, lvl, "%s, channel warned twice",
+	if (disabledmap_channel_is_warned(p->disabledmap, scidd)) {
+		payment_disable_chan(p, scidd, lvl, "%s, channel warned twice",
 				     str);
 		return;
 	}
 
 	payment_note(
 	    p, lvl, "flagged for warning %s: %s, next time it will be disabled",
-	    fmt_short_channel_id(tmpctx, scid), str);
-	disabledmap_warn_channel(p->disabledmap, scid);
+	    fmt_short_channel_id_dir(tmpctx, &scidd), str);
+	disabledmap_warn_channel(p->disabledmap, scidd);
 }
 
 void payment_disable_node(struct payment *p, struct node_id node,

plugins/renepay/payment.h

@@ -167,10 +167,10 @@ struct command_result *payment_fail(struct payment *payment,
 /* These log at LOG_DBG, append to notes, and send command notification */
 void payment_note(struct payment *p, enum log_level lvl, const char *fmt, ...);
 
-void payment_disable_chan(struct payment *p, struct short_channel_id scid,
+void payment_disable_chan(struct payment *p, struct short_channel_id_dir scidd,
 			  enum log_level lvl, const char *fmt, ...);
 
-void payment_warn_chan(struct payment *p, struct short_channel_id scid,
+void payment_warn_chan(struct payment *p, struct short_channel_id_dir scidd,
 		       enum log_level lvl, const char *fmt, ...);
 
 void payment_disable_node(struct payment *p, struct node_id node,

plugins/renepay/routebuilder.c

@@ -46,9 +46,11 @@ flow_adjust_htlcmax_constraints(struct flow *flow, struct gossmap *gossmap,
 
 	if (errorcode == RENEPAY_BAD_CHANNEL) {
 		// this is a channel that we can disable
-		// FIXME: log this error?
+		// FIXME: log this error? disabling both directions?
 		bitmap_set_bit(disabled_bitmap,
-			       gossmap_chan_idx(gossmap, bad_channel));
+			       gossmap_chan_idx(gossmap, bad_channel) * 2 + 0);
+		bitmap_set_bit(disabled_bitmap,
+			       gossmap_chan_idx(gossmap, bad_channel) * 2 + 1);
 	}
 
 	// we had an unexpected error
@@ -83,7 +85,8 @@ route_check_constraints(struct route *route, struct gossmap *gossmap,
 		    amount_msat_less(hop->amount,
 				     gossmap_chan_htlc_min(chan, dir))) {
 			bitmap_set_bit(disabled_bitmap,
-				       gossmap_chan_idx(gossmap, chan));
+				       gossmap_chan_idx(gossmap, chan) * 2 +
+					   dir);
 			return RENEPAY_BAD_CHANNEL;
 		}
 
@@ -164,15 +167,21 @@ struct route **get_routes(const tal_t *ctx,
 	}
 
 	/* Also disable every channel that we don't have in the chan_extra_map.
+	 * We might have channels in the gossmap that are not usable for
+	 * probability computations for example if we don't know their capacity.
+	 * We can tell the solver to ignore those channels by disabling them
+	 * here.
 	 */
 	for (struct gossmap_chan *chan = gossmap_first_chan(gossmap); chan;
 	     chan = gossmap_next_chan(gossmap, chan)) {
 		const u32 chan_id = gossmap_chan_idx(gossmap, chan);
 		struct short_channel_id scid = gossmap_chan_scid(gossmap, chan);
 		struct chan_extra *ce =
 		    chan_extra_map_get(uncertainty->chan_extra_map, scid);
-		if (!ce)
-			bitmap_set_bit(disabled_bitmap, chan_id);
+		if (!ce) {
+			bitmap_set_bit(disabled_bitmap, chan_id * 2 + 0);
+			bitmap_set_bit(disabled_bitmap, chan_id * 2 + 1);
+		}
 	}
 
 	const struct gossmap_node *src, *dst;

plugins/renepay/routefail.c

@@ -129,9 +129,11 @@ static struct command_result *update_gossip_failure(struct command *cmd UNUSED,
 	 * always be present here, but at least the documentation for
 	 * waitsendpay says it is present in the case of error. */
 	assert(r->route->result->erring_channel);
-
+	struct short_channel_id_dir scidd = {
+	    .scid = *r->route->result->erring_channel,
+	    .dir = *r->route->result->erring_direction};
 	payment_disable_chan(
-	    r->payment, *r->route->result->erring_channel, LOG_INFORM,
+	    r->payment, scidd, LOG_INFORM,
 	    "addgossip failed (%.*s)", json_tok_full_len(result),
 	    json_tok_full(buf, result));
 	return update_gossip_done(cmd, buf, result, r);
@@ -346,8 +348,11 @@ static struct command_result *handle_failure(struct routefail *r)
 
 		} else {
 			assert(result->erring_channel);
+			struct short_channel_id_dir scidd = {
+			    .scid = *result->erring_channel,
+			    .dir = *result->erring_direction};
 			payment_disable_chan(
-			    payment, *result->erring_channel, LOG_INFORM,
+			    payment, scidd, LOG_INFORM,
 			    "%s", onion_wire_name(result->failcode));
 		}
 		break;
@@ -398,8 +403,11 @@ static struct command_result *handle_failure(struct routefail *r)
 			 * information and try again. To avoid hitting this
 			 * error again with the same channel we flag it. */
 			assert(result->erring_channel);
+			struct short_channel_id_dir scidd = {
+			    .scid = *result->erring_channel,
+			    .dir = *result->erring_direction};
 			payment_warn_chan(payment,
-					  *result->erring_channel, LOG_INFORM,
+					  scidd, LOG_INFORM,
 					  "received error %s",
 					  onion_wire_name(result->failcode));
 		}