diff --git a/gossipd/routing.c b/gossipd/routing.c index 24b2f81f0..4965a3ea8 100644 --- a/gossipd/routing.c +++ b/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). - * Note that we take our own fees into account for routing, even - * though we don't pay them: it presumably effects preference. */ - first_chan = dst->bfg[best].prev; - dst = other_node(dst, dst->bfg[best].prev); - best--; + /* We (dst) don't charge ourselves fees, so skip first hop */ + n = other_node(dst, dst->bfg[best].prev); + *fee = n->bfg[best-1].total - msatoshi; - *fee = dst->bfg[best].total - msatoshi; - *route = tal_arr(ctx, struct routing_channel *, best); + /* Lay out route */ + 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! */ - first_chan = find_route(ctx, rstate, source, destination, msatoshi, - riskfactor / BLOCKS_PER_YEAR / 10000, - fuzz, base_seed, - &fee, &route); + route = find_route(ctx, rstate, source, destination, msatoshi, + riskfactor / BLOCKS_PER_YEAR / 10000, + fuzz, base_seed, &fee); - 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 + 1].nodeid = n->id; - hops[i + 1].amount = total_amount; + hops[i].channel_id = route[i]->scid; + hops[i].nodeid = n->id; + 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 */ - 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; + assert(pubkey_eq(&n->id, source)); /* FIXME: Shadow route! */ return hops; diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index 84fb70840..7957e92c2 100644 --- a/gossipd/test/run-bench-find_route.c +++ b/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); - num_success += (chan != NULL); + &fee); + num_success += (route != NULL); tal_free(route); } end = time_mono(); diff --git a/gossipd/test/run-find_route-specific.c b/gossipd/test/run-find_route-specific.c index 1c7be3614..8255d313b 100644 --- a/gossipd/test/run-find_route-specific.c +++ b/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); - assert(first); - assert(tal_count(route) == 1); - assert(pubkey_eq(&first->nodes[0]->id, &a) - || pubkey_eq(&first->nodes[1]->id, &a)); - 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)); + route = find_route(ctx, rstate, &a, &c, 100000, riskfactor, 0.0, NULL, &fee); + assert(route); + assert(tal_count(route) == 2); + assert(channel_is_between(route[0], &a, &b)); + assert(channel_is_between(route[1], &b, &c)); tal_free(ctx); secp256k1_context_destroy(secp256k1_ctx); diff --git a/gossipd/test/run-find_route.c b/gossipd/test/run-find_route.c index 0e7806453..d48c1fe78 100644 --- a/gossipd/test/run-find_route.c +++ b/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); - assert(first); - assert(tal_count(route) == 0); + route = find_route(ctx, rstate, &a, &b, 1000, riskfactor, 0.0, NULL, &fee); + assert(tal_count(route) == 1); 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); - assert(first); - assert(tal_count(route) == 1); + route = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee); + assert(tal_count(route) == 2); 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); - assert(first); - assert(tal_count(route) == 1); - assert(channel_is_between(route[0], &d, &c)); + route = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee); + assert(tal_count(route) == 2); + assert(channel_is_between(route[0], &a, &d)); + 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); - assert(first); - assert(tal_count(route) == 1); - assert(channel_is_between(route[0], &b, &c)); + route = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee); + assert(tal_count(route) == 2); + assert(channel_is_between(route[0], &a, &b)); + 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); - assert(first); - assert(tal_count(route) == 1); - assert(channel_is_between(route[0], &d, &c)); + route = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee); + assert(tal_count(route) == 2); + assert(channel_is_between(route[0], &a, &d)); + assert(channel_is_between(route[1], &d, &c)); assert(fee == 0 + 6); tal_free(ctx);