From 56349ab008e4ad2186f4f73cd867a38045faeaa4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 4 Mar 2018 12:53:32 +1030 Subject: [PATCH] routing: work with struct routing_channel not struct node_connection. To remove the redundant fields in `struct node_connection` (ie. 'src' and 'dst' pointers) we need to deal with `struct routing_channel`. This means we get a series of channels, from which the direction is implied, so it's a bit more complex to decode. We add a helper `other_node` to help with this, and since we're the only user of `connection_to` we change that function to return the index. Signed-off-by: Rusty Russell --- gossipd/routing.c | 93 ++++++++++++-------------- gossipd/routing.h | 16 +++-- gossipd/test/run-bench-find_route.c | 14 ++-- gossipd/test/run-find_route-specific.c | 17 +++-- gossipd/test/run-find_route.c | 44 +++++++----- 5 files changed, 104 insertions(+), 80 deletions(-) diff --git a/gossipd/routing.c b/gossipd/routing.c index 54d6ea5b6..24b2f81f0 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -303,16 +303,16 @@ static u64 risk_fee(u64 amount, u32 delay, double riskfactor) /* We track totals, rather than costs. That's because the fee depends * on the current amount passing through. */ static void bfg_one_edge(struct node *node, - struct node_connection *c, double riskfactor, + struct routing_channel *chan, int idx, + double riskfactor, double fuzz, const struct siphash_seed *base_seed) { size_t h; double fee_scale = 1.0; + const struct node_connection *c = &chan->connections[idx]; if (fuzz != 0.0) { - u64 h = siphash24(base_seed, - &c->short_channel_id, - sizeof(c->short_channel_id)); + u64 h = siphash24(base_seed, &chan->scid, sizeof(chan->scid)); /* Scale fees for this channel */ /* rand = (h / UINT64_MAX) random number between 0.0 -> 1.0 @@ -322,8 +322,8 @@ static void bfg_one_edge(struct node *node, fee_scale = 1.0 + (2.0 * fuzz * h / UINT64_MAX) - fuzz; } - assert(c->dst == node); for (h = 0; h < ROUTING_MAX_HOPS; h++) { + struct node *src; /* FIXME: Bias against smaller channels. */ u64 fee; u64 risk; @@ -343,15 +343,17 @@ static void bfg_one_edge(struct node *node, continue; } + /* nodes[0] is src for connections[0] */ + src = chan->nodes[idx]; if (node->bfg[h].total + fee + risk - < c->src->bfg[h+1].total + c->src->bfg[h+1].risk) { + < src->bfg[h+1].total + src->bfg[h+1].risk) { SUPERVERBOSE("...%s can reach here in hoplen %zu total %"PRIu64, type_to_string(trc, struct pubkey, - &c->src->id), + &src->id), h, node->bfg[h].total + fee); - c->src->bfg[h+1].total = node->bfg[h].total + fee; - c->src->bfg[h+1].risk = risk; - c->src->bfg[h+1].prev = c; + src->bfg[h+1].total = node->bfg[h].total + fee; + src->bfg[h+1].risk = risk; + src->bfg[h+1].prev = chan; } } } @@ -363,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 node_connection * +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 node_connection ***route) + u64 *fee, struct routing_channel ***route) { struct node *n, *src, *dst; struct node_map_iter it; - struct node_connection *first_conn; + 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 @@ -420,18 +422,20 @@ find_route(const tal_t *ctx, struct routing_state *rstate, n = node_map_next(rstate->nodes, &it)) { size_t num_edges = tal_count(n->channels); for (i = 0; i < num_edges; i++) { - struct node_connection *c; + struct routing_channel *chan = n->channels[i]; + int idx = connection_to(n, chan); + SUPERVERBOSE("Node %s edge %i/%zu", type_to_string(trc, struct pubkey, &n->id), i, num_edges); - c = connection_to(n, n->channels[i]); - if (!nc_is_routable(c, now)) { + if (!nc_is_routable(&chan->connections[idx], + now)) { SUPERVERBOSE("...unroutable"); continue; } - bfg_one_edge(n, c, + bfg_one_edge(n, chan, idx, riskfactor, fuzz, base_seed); SUPERVERBOSE("...done"); } @@ -454,37 +458,20 @@ find_route(const tal_t *ctx, struct routing_state *rstate, /* 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_conn = dst->bfg[best].prev; - dst = dst->bfg[best].prev->dst; + first_chan = dst->bfg[best].prev; + dst = other_node(dst, dst->bfg[best].prev); best--; *fee = dst->bfg[best].total - msatoshi; - *route = tal_arr(ctx, struct node_connection *, best); + *route = tal_arr(ctx, struct routing_channel *, best); for (i = 0, n = dst; i < best; - n = n->bfg[best-i].prev->dst, i++) { + n = other_node(n, n->bfg[best-i].prev), i++) { (*route)[i] = n->bfg[best-i].prev; } assert(n == src); - msatoshi += *fee; - status_trace("find_route: via %s", - type_to_string(trc, struct pubkey, &first_conn->dst->id)); - /* If there are intermediaries, dump them, and total fees. */ - if (best != 0) { - for (i = 0; i < best; i++) { - status_trace(" %s (%i+%i=%"PRIu64")", - type_to_string(trc, struct pubkey, - &(*route)[i]->dst->id), - (*route)[i]->base_fee, - (*route)[i]->proportional_fee, - connection_fee((*route)[i], msatoshi)); - msatoshi -= connection_fee((*route)[i], msatoshi); - } - status_trace(" =%"PRIi64"(%+"PRIi64")", - (*route)[best-1]->dst->bfg[best-1].total, *fee); - } - return first_conn; + return first_chan; } /* Verify the signature of a channel_update message */ @@ -1084,20 +1071,22 @@ struct route_hop *get_route(tal_t *ctx, struct routing_state *rstate, u32 final_cltv, double fuzz, const struct siphash_seed *base_seed) { - struct node_connection **route; + struct routing_channel **route; u64 total_amount; unsigned int total_delay; u64 fee; struct route_hop *hops; int i; - struct node_connection *first_conn; + struct routing_channel *first_chan; + struct node *n; - first_conn = find_route(ctx, rstate, source, destination, msatoshi, + /* 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); - if (!first_conn) { + if (!first_chan) { return NULL; } @@ -1106,18 +1095,24 @@ struct route_hop *get_route(tal_t *ctx, struct routing_state *rstate, total_amount = msatoshi; total_delay = final_cltv; + /* Start at destination node. */ + n = get_node(rstate, destination); for (i = tal_count(route) - 1; i >= 0; i--) { - hops[i + 1].channel_id = route[i]->short_channel_id; - hops[i + 1].nodeid = route[i]->dst->id; + 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; - total_amount += connection_fee(route[i], total_amount); + total_amount += connection_fee(c, total_amount); hops[i + 1].delay = total_delay; - total_delay += route[i]->delay; + total_delay += c->delay; + n = route[i]->nodes[!idx]; } /* Backfill the first hop manually */ - hops[0].channel_id = first_conn->short_channel_id; - hops[0].nodeid = first_conn->dst->id; + 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; diff --git a/gossipd/routing.h b/gossipd/routing.h index 82da1421a..b415d69a0 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -66,7 +66,7 @@ struct node { /* Total risk premium of this route. */ u64 risk; /* Where that came from. */ - struct node_connection *prev; + struct routing_channel *prev; } bfg[ROUTING_MAX_HOPS+1]; /* UTF-8 encoded alias as tal_arr, not zero terminated */ @@ -118,6 +118,14 @@ static inline int pubkey_idx(const struct pubkey *id1, const struct pubkey *id2) return pubkey_cmp(id1, id2) > 0; } +static inline struct node *other_node(const struct node *n, + struct routing_channel *chan) +{ + int idx = (chan->nodes[1] == n); + + return chan->nodes[!idx]; +} + /* FIXME: We could avoid these by having two channels arrays */ static inline struct node_connection *connection_from(const struct node *n, struct routing_channel *chan) @@ -129,14 +137,14 @@ static inline struct node_connection *connection_from(const struct node *n, return &chan->connections[idx]; } -static inline struct node_connection *connection_to(const struct node *n, - struct routing_channel *chan) +static inline int connection_to(const struct node *n, + struct routing_channel *chan) { int idx = (chan->nodes[1] == n); assert(chan->connections[idx].src == n); assert(chan->connections[!idx].dst == n); - return &chan->connections[!idx]; + return !idx; } struct routing_state { diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index eb8746251..84fb70840 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 node_connection **route = NULL, *nc; + struct routing_channel **route = NULL, *chan; - nc = find_route(ctx, rstate, &from, &to, - pseudorand(100000), - riskfactor, - 0.75, &base_seed, - &fee, &route); - num_success += (nc != NULL); + chan = find_route(ctx, rstate, &from, &to, + pseudorand(100000), + riskfactor, + 0.75, &base_seed, + &fee, &route); + num_success += (chan != 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 b26e4f980..1c7be3614 100644 --- a/gossipd/test/run-find_route-specific.c +++ b/gossipd/test/run-find_route-specific.c @@ -94,7 +94,8 @@ int main(void) struct routing_state *rstate; struct pubkey a, b, c; u64 fee; - struct node_connection **route; + 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 @@ -148,11 +149,17 @@ int main(void) nc->flags = 1; nc->last_timestamp = 1504064344; - nc = find_route(ctx, rstate, &a, &c, 100000, riskfactor, 0.0, NULL, &fee, &route); - assert(nc); + first = find_route(ctx, rstate, &a, &c, 100000, riskfactor, 0.0, NULL, &fee, &route); + assert(first); assert(tal_count(route) == 1); - assert(pubkey_eq(&route[0]->src->id, &b)); - assert(pubkey_eq(&route[0]->dst->id, &c)); + 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)); 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 baa342315..0e7806453 100644 --- a/gossipd/test/run-find_route.c +++ b/gossipd/test/run-find_route.c @@ -130,16 +130,30 @@ static struct node_connection *get_connection(struct routing_state *rstate, return &c->connections[idx]; } +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; const tal_t *ctx = trc = tal_tmpctx(NULL); - struct node_connection *nc; struct routing_state *rstate; struct pubkey a, b, c, d; struct privkey tmp; u64 fee; - struct node_connection **route; + 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 @@ -158,8 +172,8 @@ int main(void) /* A<->B */ add_connection(rstate, &a, &b, 1, 1, 1); - nc = find_route(ctx, rstate, &a, &b, 1000, riskfactor, 0.0, NULL, &fee, &route); - assert(nc); + first = find_route(ctx, rstate, &a, &b, 1000, riskfactor, 0.0, NULL, &fee, &route); + assert(first); assert(tal_count(route) == 0); assert(fee == 0); @@ -173,8 +187,8 @@ int main(void) status_trace("C = %s", type_to_string(trc, struct pubkey, &c)); add_connection(rstate, &b, &c, 1, 1, 1); - nc = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee, &route); - assert(nc); + first = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee, &route); + assert(first); assert(tal_count(route) == 1); assert(fee == 1); @@ -188,25 +202,25 @@ int main(void) add_connection(rstate, &d, &c, 0, 2, 1); /* Will go via D for small amounts. */ - nc = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee, &route); - assert(nc); + first = find_route(ctx, rstate, &a, &c, 1000, riskfactor, 0.0, NULL, &fee, &route); + assert(first); assert(tal_count(route) == 1); - assert(pubkey_eq(&route[0]->src->id, &d)); + assert(channel_is_between(route[0], &d, &c)); assert(fee == 0); /* Will go via B for large amounts. */ - nc = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee, &route); - assert(nc); + first = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee, &route); + assert(first); assert(tal_count(route) == 1); - assert(pubkey_eq(&route[0]->src->id, &b)); + assert(channel_is_between(route[0], &b, &c)); assert(fee == 1 + 3); /* Make B->C inactive, force it back via D */ get_connection(rstate, &b, &c)->active = false; - nc = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee, &route); - assert(nc); + first = find_route(ctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee, &route); + assert(first); assert(tal_count(route) == 1); - assert(pubkey_eq(&route[0]->src->id, &d)); + assert(channel_is_between(route[0], &d, &c)); assert(fee == 0 + 6); tal_free(ctx);