dijkstra: fix heap ordering.

We were always ordering heap by distance, not score (which are different
if we are routing by cheapest, not shortest!).

This simplifies our callbacks, too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/dijkstra.c

@@ -22,6 +22,9 @@ struct dijkstra {
 	/* NULL means it's been visited already. */
 	const struct gossmap_node **heapptr;
 
+	/* How we decide "best" */
+	u64 score;
+
 	/* We could re-evaluate to determine this, but keeps it simple */
 	struct gossmap_chan *best_chan;
 };
@@ -67,9 +70,9 @@ static int less_comparer(const void *const ctx,
 			 const void *const b)
 {
 	return get_dijkstra(global_dijkstra, global_map,
-			    *(struct gossmap_node **)a)->distance
+			    *(struct gossmap_node **)a)->score
 		> get_dijkstra(global_dijkstra, global_map,
-			       *(struct gossmap_node **)b)->distance;
+			       *(struct gossmap_node **)b)->score;
 }
 
 static void item_mover(void *const dst, const void *const src)
@@ -101,6 +104,7 @@ static const struct gossmap_node **mkheap(const tal_t *ctx,
 			d->distance = 0;
 			d->total_delay = 0;
 			d->cost = sent;
+			d->score = 0;
 			i--;
 		} else {
 			heap[i] = n;
@@ -108,6 +112,7 @@ static const struct gossmap_node **mkheap(const tal_t *ctx,
 			d->distance = UINT_MAX;
 			d->cost = AMOUNT_MSAT(-1ULL);
 			d->total_delay = 0;
+			d->score = -1ULL;
 		}
 	}
 	assert(i == tal_count(heap));
@@ -142,13 +147,9 @@ dijkstra_(const tal_t *ctx,
 			     int dir,
 			     struct amount_msat amount,
 			     void *arg),
-	  bool (*path_better)(u32 old_distance,
+	  u64 (*path_score)(u32 distance,
-			      u32 new_distance,
+			    struct amount_msat cost,
-			      struct amount_msat old_cost,
+			    struct amount_msat risk),
-			      struct amount_msat new_cost,
-			      struct amount_msat old_risk,
-			      struct amount_msat new_risk,
-			      void *arg),
 	  void *arg)
 {
 	struct dijkstra *dij;
@@ -229,7 +230,8 @@ dijkstra_(const tal_t *ctx,
 			int which_half;
 			struct gossmap_chan *c;
 			struct dijkstra *d;
-			struct amount_msat cost, new_risk, old_risk;
+			struct amount_msat cost, risk;
+			u64 score;
 
 			c = gossmap_nth_chan(map, cur, i, &which_half);
 			neighbor = gossmap_nth_node(map, c, !which_half);
@@ -251,20 +253,11 @@ dijkstra_(const tal_t *ctx,
 				continue;
 
 			/* cltv_delay can't overflow: only 20 bits per hop. */
-			new_risk = risk_price(cost, riskfactor,
+			risk = risk_price(cost, riskfactor,
-					      cur_d->total_delay
+					  cur_d->total_delay
-					      + c->half[!which_half].delay);
+					  + c->half[!which_half].delay);
-
+			score = path_score(cur_d->distance + 1, cost, risk);
-			old_risk = risk_price(d->cost, riskfactor,
+			if (score >= d->score)
-					      d->total_delay);
-
-			if (!path_better(d->distance,
-					 cur_d->distance + 1,
-					 d->cost,
-					 cost,
-					 old_risk,
-					 new_risk,
-					 arg))
 				continue;
 
 			d->distance = cur_d->distance + 1;
@@ -272,6 +265,7 @@ dijkstra_(const tal_t *ctx,
 				+ c->half[!which_half].delay;
 			d->cost = cost;
 			d->best_chan = c;
+			d->score = score;
 			gheap_restore_heap_after_item_increase(&gheap_ctx,
 							       heap, heapsize,
 							       d->heapptr - heap);

common/dijkstra.h

@@ -21,28 +21,19 @@ dijkstra_(const tal_t *ctx,
 			     int dir,
 			     struct amount_msat amount,
 			     void *arg),
-	  bool (*path_better)(u32 old_distance,
+	  u64 (*path_score)(u32 distance,
-			      u32 new_distance,
+			    struct amount_msat cost,
-			      struct amount_msat old_cost,
+			    struct amount_msat risk),
-			      struct amount_msat new_cost,
-			      struct amount_msat old_risk,
-			      struct amount_msat new_risk,
-			      void *arg),
 	  void *arg);
 
 #define dijkstra(ctx, map, start, amount, riskfactor, channel_ok,	\
-		 path_better, arg)					\
+		 path_score, arg)					\
 	dijkstra_((ctx), (map), (start), (amount), (riskfactor),	\
 		  typesafe_cb_preargs(bool, void *, (channel_ok), (arg), \
 				      const struct gossmap *,		\
 				      const struct gossmap_chan *,	\
 				      int, struct amount_msat),		\
-		  typesafe_cb_preargs(bool, void *, (path_better), (arg), \
+		  (path_score),						\
-				      u32, u32,				\
-				      struct amount_msat,		\
-				      struct amount_msat,		\
-				      struct amount_msat,		\
-				      struct amount_msat),		\
 		  (arg))
 
 /* Returns UINT_MAX if unreachable. */

common/route.c

@@ -37,43 +37,28 @@ bool route_can_carry(const struct gossmap *map,
 	return route_can_carry_even_disabled(map, c, dir, amount, arg);
 }
 
-bool route_path_shorter(u32 old_distance, u32 new_distance,
+/* Prioritize distance over costs */
-			struct amount_msat old_cost,
+u64 route_score_shorter(u32 distance,
-			struct amount_msat new_cost,
+			struct amount_msat cost,
-			struct amount_msat old_risk,
+			struct amount_msat risk)
-			struct amount_msat new_risk,
-			void *unused)
 {
-	if (new_distance > old_distance)
+	u64 costs = cost.millisatoshis + risk.millisatoshis; /* Raw: score */
-		return false;
+	if (costs > 0xFFFFFFFF)
-	if (new_distance < old_distance)
+		costs = 0xFFFFFFFF;
-		return true;
 
-	/* Tiebreak by cost */
+	return costs + ((u64)distance << 32);
-	if (!amount_msat_add(&old_cost, old_cost, old_risk)
-	    || !amount_msat_add(&new_cost, new_cost, new_risk))
-		return false;
-	return amount_msat_less(new_cost, old_cost);
 }
 
-bool route_path_cheaper(u32 old_distance, u32 new_distance,
+/* Prioritize costs over distance */
-			struct amount_msat old_cost,
+u64 route_score_cheaper(u32 distance,
-			struct amount_msat new_cost,
+			struct amount_msat cost,
-			struct amount_msat old_risk,
+			struct amount_msat risk)
-			struct amount_msat new_risk,
-			void *unused)
 {
-	if (!amount_msat_add(&old_cost, old_cost, old_risk)
+	u64 costs = cost.millisatoshis + risk.millisatoshis; /* Raw: score */
-	    || !amount_msat_add(&new_cost, new_cost, new_risk))
+	if (costs > 0xFFFFFFFF)
-		return false;
+		costs = 0xFFFFFFFF;
-
-	if (amount_msat_greater(new_cost, old_cost))
-		return false;
-	if (amount_msat_less(new_cost, old_cost))
-		return true;
 
-	/* Tiebreak by distance */
+	return (costs << 32) + distance;
-	return new_distance < old_distance;
 }
 
 struct route **route_from_dijkstra(const struct gossmap *map,

common/route.h

@@ -27,20 +27,14 @@ bool route_can_carry_even_disabled(const struct gossmap *map,
 				   void *unused);
 
 /* Shortest path, with lower amount tiebreak */
-bool route_path_shorter(u32 old_distance, u32 new_distance,
+u64 route_score_shorter(u32 distance,
-			struct amount_msat old_cost,
+			struct amount_msat cost,
-			struct amount_msat new_cost,
+			struct amount_msat risk);
-			struct amount_msat old_risk,
-			struct amount_msat new_risk,
-			void *unused);
 
 /* Cheapest path, with shorter path tiebreak */
-bool route_path_cheaper(u32 old_distance, u32 new_distance,
+u64 route_score_cheaper(u32 distance,
-			struct amount_msat old_cost,
+			struct amount_msat cost,
-			struct amount_msat new_cost,
+			struct amount_msat risk);
-			struct amount_msat old_risk,
-			struct amount_msat new_risk,
-			void *unused);
 
 /* Extract route tal_arr from completed dijkstra */
 struct route **route_from_dijkstra(const struct gossmap *map,

devtools/route.c

@@ -52,7 +52,7 @@ static struct route **least_cost(struct gossmap *map,
 	tstart = time_mono();
 	dij = dijkstra(tmpctx, map, dst,
 		       sent, riskfactor, route_can_carry,
-		       route_path_cheaper, NULL);
+		       route_score_cheaper, NULL);
 	tstop = time_mono();
 
 	printf("# Time to find route: %"PRIu64" usec\n",

devtools/topology.c

@@ -55,7 +55,7 @@ static size_t count_possible_sources(const struct gossmap *map,
 	size_t distance_budget, num;
 
 	dij = dijkstra(tmpctx, map, n, AMOUNT_MSAT(0), 0,
-		       channel_usable_to_excl, route_path_shorter, exclude);
+		       channel_usable_to_excl, route_score_shorter, exclude);
 
 	if (is_last_node)
 		distance_budget = ROUTING_MAX_HOPS - 1;
@@ -132,7 +132,7 @@ static size_t count_possible_destinations(const struct gossmap *map,
 	size_t distance_budget, num;
 
 	dij = dijkstra(tmpctx, map, start, AMOUNT_MSAT(0), 0,
-		       channel_usable_from_excl, route_path_shorter, exclude);
+		       channel_usable_from_excl, route_score_shorter, exclude);
 
 	if (is_first_node)
 		distance_budget = ROUTING_MAX_HOPS - 1;
@@ -191,7 +191,7 @@ static bool measure_least_cost(struct gossmap *map,
 	tstart = time_mono();
 	dij = dijkstra(tmpctx, map, dst,
 		       sent, riskfactor, channel_usable,
-		       route_path_cheaper, NULL);
+		       route_score_cheaper, NULL);
 	tstop = time_mono();
 
 	printf("# Time to find path: %"PRIu64" usec\n",