routing: return entire routing_channel array from find_route.

Returning the separate first routing_channel was a weird API: just
return the entire array.  Sure, we have to treat the first node a bit
differently (because we don't charge ourselves fees), but it's still
simpler.

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

gossipd/routing.c

@@ -365,16 +365,16 @@ static bool nc_is_routable(const struct node_connection *nc, time_t now)
 }
 
 /* riskfactor is already scaled to per-block amount */
-static struct routing_channel *
+static struct routing_channel **
 find_route(const tal_t *ctx, struct routing_state *rstate,
 	   const struct pubkey *from, const struct pubkey *to, u64 msatoshi,
 	   double riskfactor,
 	   double fuzz, const struct siphash_seed *base_seed,
-	   u64 *fee, struct routing_channel ***route)
+	   u64 *fee)
 {
+	struct routing_channel **route;
 	struct node *n, *src, *dst;
 	struct node_map_iter it;
-	struct routing_channel *first_chan;
 	int runs, i, best;
 	/* Call time_now() once at the start, so that our tight loop
 	 * does not keep calling into operating system for the
@@ -455,23 +455,20 @@ find_route(const tal_t *ctx, struct routing_state *rstate,
 		return NULL;
 	}
 
-	/* Save route from *next* hop (we return first hop as peer).
+	/* We (dst) don't charge ourselves fees, so skip first hop */
-	 * Note that we take our own fees into account for routing, even
+	n = other_node(dst, dst->bfg[best].prev);
-	 * though we don't pay them: it presumably effects preference. */
+	*fee = n->bfg[best-1].total - msatoshi;
-	first_chan = dst->bfg[best].prev;
-	dst = other_node(dst, dst->bfg[best].prev);
-	best--;
 
-	*fee = dst->bfg[best].total - msatoshi;
+	/* Lay out route */
-	*route = tal_arr(ctx, struct routing_channel *, best);
+	route = tal_arr(ctx, struct routing_channel *, best);
 	for (i = 0, n = dst;
 	     i < best;
 	     n = other_node(n, n->bfg[best-i].prev), i++) {
-		(*route)[i] = n->bfg[best-i].prev;
+		route[i] = n->bfg[best-i].prev;
 	}
 	assert(n == src);
 
-	return first_chan;
+	return route;
 }
 
 /* Verify the signature of a channel_update message */
@@ -1077,21 +1074,18 @@ struct route_hop *get_route(tal_t *ctx, struct routing_state *rstate,
 	u64 fee;
 	struct route_hop *hops;
 	int i;
-	struct routing_channel *first_chan;
 	struct node *n;
 
-	/* FIXME: make find_route simply return entire route array! */
+	route = find_route(ctx, rstate, source, destination, msatoshi,
-	first_chan = find_route(ctx, rstate, source, destination, msatoshi,
+			   riskfactor / BLOCKS_PER_YEAR / 10000,
-				riskfactor / BLOCKS_PER_YEAR / 10000,
+			   fuzz, base_seed, &fee);
-				fuzz, base_seed,
-				&fee, &route);
 
-	if (!first_chan) {
+	if (!route) {
 		return NULL;
 	}
 
 	/* Fees, delays need to be calculated backwards along route. */
-	hops = tal_arr(ctx, struct route_hop, tal_count(route) + 1);
+	hops = tal_arr(ctx, struct route_hop, tal_count(route));
 	total_amount = msatoshi;
 	total_delay = final_cltv;
 
@@ -1101,21 +1095,15 @@ struct route_hop *get_route(tal_t *ctx, struct routing_state *rstate,
 		const struct node_connection *c;
 		int idx = connection_to(n, route[i]);
 		c = &route[i]->connections[idx];
-		hops[i + 1].channel_id = route[i]->scid;
+		hops[i].channel_id = route[i]->scid;
-		hops[i + 1].nodeid = n->id;
+		hops[i].nodeid = n->id;
-		hops[i + 1].amount = total_amount;
+		hops[i].amount = total_amount;
+		hops[i].delay = total_delay;
 		total_amount += connection_fee(c, total_amount);
-
-		hops[i + 1].delay = total_delay;
 		total_delay += c->delay;
-		n = route[i]->nodes[!idx];
+		n = other_node(n, route[i]);
 	}
-	/* Backfill the first hop manually */
+	assert(pubkey_eq(&n->id, source));
-	hops[0].channel_id = first_chan->scid;
-	hops[0].nodeid = n->id;
-	/* We don't charge ourselves any fees, nor require delay */
-	hops[0].amount = total_amount;
-	hops[0].delay = total_delay;
 
 	/* FIXME: Shadow route! */
 	return hops;

gossipd/test/run-bench-find_route.c

@@ -218,14 +218,14 @@ int main(int argc, char *argv[])
 		struct pubkey from = nodeid(pseudorand(num_nodes));
 		struct pubkey to = nodeid(pseudorand(num_nodes));
 		u64 fee;
-		struct routing_channel **route = NULL, *chan;
+		struct routing_channel **route;
 
-		chan = find_route(ctx, rstate, &from, &to,
+		route = find_route(ctx, rstate, &from, &to,
 				  pseudorand(100000),
 				  riskfactor,
 				  0.75, &base_seed,
-				  &fee, &route);
+				  &fee);
-		num_success += (chan != NULL);
+		num_success += (route != NULL);
 		tal_free(route);
 	}
 	end = time_mono();

gossipd/test/run-find_route-specific.c

@@ -86,6 +86,20 @@ get_or_make_connection(struct routing_state *rstate,
 	return &chan->connections[pubkey_idx(from_id, to_id)];
 }
 
+static bool channel_is_between(const struct routing_channel *chan,
+			       const struct pubkey *a, const struct pubkey *b)
+{
+	if (pubkey_eq(&chan->nodes[0]->id, a)
+	    && pubkey_eq(&chan->nodes[1]->id, b))
+		return true;
+
+	if (pubkey_eq(&chan->nodes[0]->id, b)
+	    && pubkey_eq(&chan->nodes[1]->id, a))
+		return true;
+
+	return false;
+}
+
 int main(void)
 {
 	static const struct bitcoin_blkid zerohash;
@@ -95,7 +109,6 @@ int main(void)
 	struct pubkey a, b, c;
 	u64 fee;
 	struct routing_channel **route;
-	struct routing_channel *first;
 	const double riskfactor = 1.0 / BLOCKS_PER_YEAR / 10000;
 
 	secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY
@@ -149,17 +162,11 @@ int main(void)
 	nc->flags = 1;
 	nc->last_timestamp = 1504064344;
 
-	first = find_route(ctx, rstate, &a, &c, 100000, riskfactor, 0.0, NULL, &fee, &route);
+	route = find_route(ctx, rstate, &a, &c, 100000, riskfactor, 0.0, NULL, &fee);
-	assert(first);
+	assert(route);
-	assert(tal_count(route) == 1);
+	assert(tal_count(route) == 2);
-	assert(pubkey_eq(&first->nodes[0]->id, &a)
+	assert(channel_is_between(route[0], &a, &b));
-	       || pubkey_eq(&first->nodes[1]->id, &a));
+	assert(channel_is_between(route[1], &b, &c));
-	assert(pubkey_eq(&first->nodes[0]->id, &b)
-	       || pubkey_eq(&first->nodes[1]->id, &b));
-	assert(pubkey_eq(&route[0]->nodes[0]->id, &b)
-	       || pubkey_eq(&route[0]->nodes[1]->id, &b));
-	assert(pubkey_eq(&route[0]->nodes[0]->id, &c)
-	       || pubkey_eq(&route[0]->nodes[1]->id, &c));
 
 	tal_free(ctx);
 	secp256k1_context_destroy(secp256k1_ctx);

gossipd/test/run-find_route.c

@@ -153,7 +153,6 @@ int main(void)
 	struct privkey tmp;
 	u64 fee;
 	struct routing_channel **route;
-	struct routing_channel *first;
 	const double riskfactor = 1.0 / BLOCKS_PER_YEAR / 10000;
 
 	secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY
@@ -172,9 +171,8 @@ int main(void)
 	/* A<->B */
 	add_connection(rstate, &a, &b, 1, 1, 1);
 
-	first = find_route(ctx, rstate, &a, &b, 1000, riskfactor, 0.0, NULL, &fee, &route);
+	route = find_route(ctx, rstate, &a, &b, 1000, riskfactor, 0.0, NULL, &fee);
-	assert(first);
+	assert(tal_count(route) == 1);
-	assert(tal_count(route) == 0);
 	assert(fee == 0);
 
 	/* A<->B<->C */
@@ -187,9 +185,8 @@ int main(void)
 	status_trace("C = %s", type_to_string(trc, struct pubkey, &c));
 	add_connection(rstate, &b, &c, 1, 1, 1);
 
-	first = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee, &route);
+	route = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee);
-	assert(first);
+	assert(tal_count(route) == 2);
-	assert(tal_count(route) == 1);
 	assert(fee == 1);
 
 	/* A<->D<->C: Lower base, higher percentage. */
@@ -202,25 +199,25 @@ int main(void)
 	add_connection(rstate, &d, &c, 0, 2, 1);
 
 	/* Will go via D for small amounts. */
-	first = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee, &route);
+	route = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee);
-	assert(first);
+	assert(tal_count(route) == 2);
-	assert(tal_count(route) == 1);
+	assert(channel_is_between(route[0], &a, &d));
-	assert(channel_is_between(route[0], &d, &c));
+	assert(channel_is_between(route[1], &d, &c));
 	assert(fee == 0);
 
 	/* Will go via B for large amounts. */
-	first = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee, &route);
+	route = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee);
-	assert(first);
+	assert(tal_count(route) == 2);
-	assert(tal_count(route) == 1);
+	assert(channel_is_between(route[0], &a, &b));
-	assert(channel_is_between(route[0], &b, &c));
+	assert(channel_is_between(route[1], &b, &c));
 	assert(fee == 1 + 3);
 
 	/* Make B->C inactive, force it back via D */
 	get_connection(rstate, &b, &c)->active = false;
-	first = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee, &route);
+	route = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee);
-	assert(first);
+	assert(tal_count(route) == 2);
-	assert(tal_count(route) == 1);
+	assert(channel_is_between(route[0], &a, &d));
-	assert(channel_is_between(route[0], &d, &c));
+	assert(channel_is_between(route[1], &d, &c));
 	assert(fee == 0 + 6);
 
 	tal_free(ctx);