common: cleanups suggested by Christian Decker's review.

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

common/dijkstra.c

@@ -22,7 +22,7 @@ struct dijkstra {
 	/* NULL means it's been visited already. */
 	const struct gossmap_node **heapptr;
 
-	/* How we decide "best" */
+	/* How we decide "best", lower is better */
 	u64 score;
 
 	/* We could re-evaluate to determine this, but keeps it simple */

common/gossmap.c

@@ -130,16 +130,25 @@ static void map_nodeid(const struct gossmap *map, size_t offset,
 	map_copy(map, offset, id, sizeof(*id));
 }
 
-static bool map_feature_set(const struct gossmap *map, int bit,
+/* Returns optional or compulsory feature if set, otherwise -1 */
+static int map_feature_test(const struct gossmap *map,
+			    int compulsory_bit,
 			    size_t offset, size_t len)
 {
-	size_t bytenum = bit / 8;
+	size_t bytenum = compulsory_bit / 8;
+	u8 bits;
 
+	assert(COMPULSORY_FEATURE(compulsory_bit) == compulsory_bit);
 	if (bytenum >= len)
 		return false;
 
 	/* Note reversed! */
-	return map_u8(map, offset + len - 1 - bytenum) & (1 << (bit % 8));
+	bits = map_u8(map, offset + len - 1 - bytenum);
+	if (bits & (1 << (compulsory_bit % 8)))
+		return compulsory_bit;
+	if (bits & (1 << (OPTIONAL_FEATURE(compulsory_bit) % 8)))
+		return OPTIONAL_FEATURE(compulsory_bit);
+	return -1;
 }
 
 /* These values can change across calls to gossmap_check. */
@@ -877,13 +886,8 @@ int gossmap_chan_has_feature(const struct gossmap *map,
 
 	feature_len = map_be16(map, c->cann_off + feature_len_off);
 
-	if (map_feature_set(map, OPTIONAL_FEATURE(fbit),
+	return map_feature_test(map, COMPULSORY_FEATURE(fbit),
-			    c->cann_off + feature_len_off + 2, feature_len))
+				c->cann_off + feature_len_off + 2, feature_len);
-		return OPTIONAL_FEATURE(fbit);
-	if (map_feature_set(map, COMPULSORY_FEATURE(fbit),
-			    c->cann_off + feature_len_off + 2, feature_len))
-		return COMPULSORY_FEATURE(fbit);
-	return -1;
 }
 
 /* BOLT #7:
@@ -911,11 +915,6 @@ int gossmap_node_has_feature(const struct gossmap *map,
 
 	feature_len = map_be16(map, n->nann_off + feature_len_off);
 
-	if (map_feature_set(map, OPTIONAL_FEATURE(fbit),
+	return map_feature_test(map, COMPULSORY_FEATURE(fbit),
-			    n->nann_off + feature_len_off + 2, feature_len))
+				n->nann_off + feature_len_off + 2, feature_len);
-		return OPTIONAL_FEATURE(fbit);
-	if (map_feature_set(map, COMPULSORY_FEATURE(fbit),
-			    n->nann_off + feature_len_off + 2, feature_len))
-		return COMPULSORY_FEATURE(fbit);
-	return -1;
 }

common/route.c

@@ -37,16 +37,22 @@ bool route_can_carry(const struct gossmap *map,
 	return route_can_carry_even_disabled(map, c, dir, amount, arg);
 }
 
-/* Prioritize distance over costs */
+/* Squeeze total costs into a u32 */
-u64 route_score_shorter(u32 distance,
+static u32 costs_to_score(struct amount_msat cost,
-			struct amount_msat cost,
+			  struct amount_msat risk)
-			struct amount_msat risk)
 {
 	u64 costs = cost.millisatoshis + risk.millisatoshis; /* Raw: score */
 	if (costs > 0xFFFFFFFF)
 		costs = 0xFFFFFFFF;
+	return costs;
+}
 
-	return costs + ((u64)distance << 32);
+/* Prioritize distance over costs */
+u64 route_score_shorter(u32 distance,
+			struct amount_msat cost,
+			struct amount_msat risk)
+{
+	return costs_to_score(cost, risk) + ((u64)distance << 32);
 }
 
 /* Prioritize costs over distance */
@@ -54,11 +60,7 @@ u64 route_score_cheaper(u32 distance,
 			struct amount_msat cost,
 			struct amount_msat risk)
 {
-	u64 costs = cost.millisatoshis + risk.millisatoshis; /* Raw: score */
+	return ((u64)costs_to_score(cost, risk) << 32) + distance;
-	if (costs > 0xFFFFFFFF)
-		costs = 0xFFFFFFFF;
-
-	return (costs << 32) + distance;
 }
 
 struct route **route_from_dijkstra(const tal_t *ctx,