From 9b900138d06b8fc1dc689096eaa57cb021be2e68 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 2 Mar 2018 19:29:16 +1030 Subject: [PATCH] gossip: put 'routing_channel' in charge of 'node_connection'. This makes 'routing_channel' the primary object in the system; it can have one or two 'node_connection's attached, and points to two nodes. The nodes are freed when no more routing_channel refer to them. The routing_channel are freed when they contain no more 'node_connection'. This fixes #1072 which I surmise was caused by a dangling routing_channel after pruning. Each node contains a single array of 'routing_channel's, not one for each direction. The 'routing_channel' itself orders nodes in key order (conveniently the index is equal to the direction flag we use), and 'node_connection' with source in the same order. There are helpers to assist with common questions like "which 'node_connection' leads out of this node?". There are now two ways to find a channel: 1. Direct scid lookup via rstate->channels map. 2. Node key lookup, followed by channel traversal. Several FIXMEs are inserted for where we can now do things more optimally. Fixes: #1072 Signed-off-by: Rusty Russell --- gossipd/gossip.c | 71 ++--- gossipd/routing.c | 382 +++++++++++++------------ gossipd/routing.h | 49 +++- gossipd/test/run-bench-find_route.c | 8 +- gossipd/test/run-find_route-specific.c | 23 +- gossipd/test/run-find_route.c | 8 +- 6 files changed, 313 insertions(+), 228 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index c42fc21d7..be3a0943e 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -721,6 +721,8 @@ static void handle_get_update(struct peer *peer, const u8 *msg) return; } + /* FIXME: Do direct scid lookup to get channel */ + /* We want update than comes from our end. */ us = get_node(peer->daemon->rstate, &peer->daemon->id); if (!us) { @@ -732,11 +734,16 @@ static void handle_get_update(struct peer *peer, const u8 *msg) goto reply; } - for (i = 0; i < tal_count(us->out); i++) { - if (!structeq(&us->out[i]->short_channel_id, &schanid)) + for (i = 0; i < tal_count(us->channels); i++) { + struct node_connection *c; + if (!structeq(&us->channels[i]->scid, &schanid)) continue; - update = us->out[i]->channel_update; + c = connection_from(us, us->channels[i]); + if (!c) + update = NULL; + else + update = c->channel_update; status_trace("peer %s schanid %s: %s update", type_to_string(trc, struct pubkey, &peer->id), type_to_string(trc, struct short_channel_id, @@ -761,7 +768,6 @@ static void handle_local_add_channel(struct peer *peer, u8 *msg) u32 fee_base_msat, fee_proportional_millionths; u64 htlc_minimum_msat; struct node_connection *c; - struct routing_channel *chan; if (!fromwire_gossip_local_add_channel( msg, &scid, &chain_hash, &remote_node_id, &flags, @@ -778,19 +784,19 @@ static void handle_local_add_channel(struct peer *peer, u8 *msg) return; } + /* FIXME: use uintmap_get */ if (get_connection_by_scid(rstate, &scid, 0) || get_connection_by_scid(rstate, &scid, 1)) { status_trace("Attempted to local_add_channel a know channel"); return; } - chan = routing_channel_new(rstate, &scid); - chan->public = false; - uintmap_add(&rstate->channels, scid.u64, chan); + /* FIXME: unused */ + (void)flags; direction = get_channel_direction(&rstate->local_id, &remote_node_id); - c = half_add_connection(rstate, &rstate->local_id, &remote_node_id, &scid, direction); - channel_add_connection(rstate, chan, c); - + c = half_add_connection(rstate, &peer->daemon->id, &remote_node_id, + &scid); + /* FIXME: Deduplicate with code in routing.c */ c->active = true; c->last_timestamp = 0; c->delay = cltv_expiry_delta; @@ -1079,6 +1085,9 @@ static void append_half_channel(struct gossip_getchannels_entry **entries, struct gossip_getchannels_entry *e; size_t n; + if (!c) + return; + n = tal_count(*entries); tal_resize(entries, n+1); e = &(*entries)[n]; @@ -1113,12 +1122,14 @@ static struct io_plan *getchannels_req(struct io_conn *conn, struct daemon *daem entries = tal_arr(tmpctx, struct gossip_getchannels_entry, 0); n = node_map_first(daemon->rstate->nodes, &i); while (n != NULL) { - for (j=0; jout); j++){ - if (scid && - !structeq(scid, &n->out[j]->short_channel_id)) { + for (j=0; jchannels); j++){ + struct routing_channel *chan = n->channels[j]; + if (scid && !structeq(scid, &chan->scid)) { continue; } - append_half_channel(&entries, n->out[j]); + /* FIXME: this avoids printing twice, but better + * to iterate over channels directly */ + append_half_channel(&entries, connection_from(n, chan)); } n = node_map_next(daemon->rstate->nodes, &i); } @@ -1329,10 +1340,12 @@ static void gossip_prune_network(struct daemon *daemon) if (n) { /* Iterate through all outgoing connection and check whether * it's time to re-announce */ - for (size_t i = 0; i < tal_count(n->out); i++) { - struct node_connection *nc = n->out[i]; + for (size_t i = 0; i < tal_count(n->channels); i++) { + struct node_connection *nc; + + nc = connection_from(n, n->channels[i]); - if (!nc->channel_update) { + if (!nc || !nc->channel_update) { /* Connection is not public yet, so don't even * try to re-announce it */ continue; @@ -1355,10 +1368,14 @@ static void gossip_prune_network(struct daemon *daemon) /* Now iterate through all channels and see if it is still alive */ for (n = node_map_first(daemon->rstate->nodes, &it); n; n = node_map_next(daemon->rstate->nodes, &it)) { - for (size_t i = 0; i < tal_count(n->out); i++) { - struct node_connection *nc = n->out[i]; + /* Since we cover all nodes, we would consider each channel twice + * so we only look (arbitrarily) at outgoing */ + for (size_t i = 0; i < tal_count(n->channels); i++) { + struct node_connection *nc; + + nc = connection_from(n, n->channels[i]); - if (!nc->channel_update) { + if (!nc || !nc->channel_update) { /* Not even announced yet */ continue; } @@ -1374,18 +1391,8 @@ static void gossip_prune_network(struct daemon *daemon) &nc->short_channel_id), get_channel_direction(&nc->src->id, &nc->dst->id), now - nc->last_timestamp); - /* Calls remove_conn_from_array internally, removes on - * both src and dst side. */ - tal_free(nc); - } - } - - /* Finally remove all nodes that do not have any edges - * anymore. Reparent onto pruned, then free them all. */ - for (n = node_map_first(daemon->rstate->nodes, &it); n; - n = node_map_next(daemon->rstate->nodes, &it)) { - if (tal_count(n->in) == 0 && tal_count(n->out) == 0) { - tal_steal(pruned, n); + /* This may free nodes, so do outside loop. */ + tal_steal(pruned, nc); } } diff --git a/gossipd/routing.c b/gossipd/routing.c index c26ac68cd..8424dafc7 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -131,10 +131,8 @@ static void destroy_node(struct node *node, struct routing_state *rstate) node_map_del(rstate->nodes, node); /* These remove themselves from the array. */ - while (tal_count(node->in)) - tal_free(node->in[0]); - while (tal_count(node->out)) - tal_free(node->out[0]); + while (tal_count(node->channels)) + tal_free(node->channels[0]); } struct node *get_node(struct routing_state *rstate, const struct pubkey *id) @@ -151,8 +149,7 @@ static struct node *new_node(struct routing_state *rstate, n = tal(rstate, struct node); n->id = *id; - n->in = tal_arr(n, struct node_connection *, 0); - n->out = tal_arr(n, struct node_connection *, 0); + n->channels = tal_arr(n, struct routing_channel *, 0); n->alias = NULL; n->node_announcement = NULL; n->announcement_idx = 0; @@ -164,50 +161,78 @@ static struct node *new_node(struct routing_state *rstate, return n; } -static bool remove_conn_from_array(struct node_connection ***conns, - struct node_connection *nc) +static bool remove_channel_from_array(struct routing_channel ***chans, + struct routing_channel *c) { size_t i, n; - n = tal_count(*conns); + n = tal_count(*chans); for (i = 0; i < n; i++) { - if ((*conns)[i] != nc) + if ((*chans)[i] != c) continue; n--; - memmove(*conns + i, *conns + i + 1, sizeof(**conns) * (n - i)); - tal_resize(conns, n); + memmove(*chans + i, *chans + i + 1, sizeof(**chans) * (n - i)); + tal_resize(chans, n); return true; } return false; } -static void destroy_connection(struct node_connection *nc) +static void destroy_routing_channel(struct routing_channel *chan, + struct routing_state *rstate) { - if (!remove_conn_from_array(&nc->dst->in, nc) - || !remove_conn_from_array(&nc->src->out, nc)) + if (!remove_channel_from_array(&chan->nodes[0]->channels, chan) + || !remove_channel_from_array(&chan->nodes[1]->channels, chan)) /* FIXME! */ abort(); + + uintmap_del(&rstate->channels, chan->scid.u64); + + if (tal_count(chan->nodes[0]->channels) == 0) + tal_free(chan->nodes[0]); + if (tal_count(chan->nodes[1]->channels) == 0) + tal_free(chan->nodes[1]); } -static struct node_connection * get_connection(struct routing_state *rstate, +/* Returns routing_channel connecting from and to: *idx set to refer + * to connection with src=from, dst=to */ +static struct routing_channel *find_channel(struct routing_state *rstate, + const struct node *from, + const struct node *to, + int *idx) +{ + int i, n; + + *idx = pubkey_idx(&from->id, &to->id); + + n = tal_count(to->channels); + for (i = 0; i < n; i++) { + if (to->channels[i]->nodes[*idx] == from) + return to->channels[i]; + } + return NULL; +} + +static struct node_connection *get_connection(struct routing_state *rstate, const struct pubkey *from_id, const struct pubkey *to_id) { - int i, n; + int idx; struct node *from, *to; + struct routing_channel *c; + from = get_node(rstate, from_id); to = get_node(rstate, to_id); if (!from || ! to) return NULL; - n = tal_count(to->in); - for (i = 0; i < n; i++) { - if (to->in[i]->src == from) - return to->in[i]; - } - return NULL; + c = find_channel(rstate, from, to, &idx); + if (!c) + return NULL; + return c->connections[idx]; } +/* FIXME: All users of this are confused. */ struct node_connection *get_connection_by_scid(const struct routing_state *rstate, const struct short_channel_id *scid, const u8 direction) @@ -220,80 +245,121 @@ struct node_connection *get_connection_by_scid(const struct routing_state *rstat return chan->connections[direction]; } -static struct node_connection * -get_or_make_connection(struct routing_state *rstate, - const struct pubkey *from_id, - const struct pubkey *to_id) +static struct routing_channel *new_routing_channel(struct routing_state *rstate, + const struct short_channel_id *scid, + struct node *n1, + struct node *n2) { - size_t i, n; - struct node *from, *to; - struct node_connection *nc; + struct routing_channel *chan = tal(rstate, struct routing_channel); + int n1idx = pubkey_idx(&n1->id, &n2->id); + size_t n; - from = get_node(rstate, from_id); - if (!from) - from = new_node(rstate, from_id); - to = get_node(rstate, to_id); - if (!to) - to = new_node(rstate, to_id); - - n = tal_count(to->in); - for (i = 0; i < n; i++) { - if (to->in[i]->src == from) { - status_trace("Updating existing route from %s to %s", - type_to_string(trc, struct pubkey, - &from->id), - type_to_string(trc, struct pubkey, - &to->id)); - return to->in[i]; - } - } + chan->scid = *scid; + chan->connections[0] = chan->connections[1] = NULL; + chan->nodes[n1idx] = n1; + chan->nodes[!n1idx] = n2; + chan->txout_script = NULL; + chan->public = false; + memset(&chan->msg_indexes, 0, sizeof(chan->msg_indexes)); - SUPERVERBOSE("Creating new route from %s to %s", - type_to_string(trc, struct pubkey, &from->id), - type_to_string(trc, struct pubkey, &to->id)); + n = tal_count(n2->channels); + tal_resize(&n2->channels, n+1); + n2->channels[n] = chan; + n = tal_count(n1->channels); + tal_resize(&n1->channels, n+1); + n1->channels[n] = chan; - nc = tal(rstate, struct node_connection); - nc->src = from; - nc->dst = to; - nc->channel_announcement = NULL; - nc->channel_update = NULL; - nc->unroutable_until = 0; + uintmap_add(&rstate->channels, scid->u64, chan); - /* Hook it into in/out arrays. */ - i = tal_count(to->in); - tal_resize(&to->in, i+1); - to->in[i] = nc; - i = tal_count(from->out); - tal_resize(&from->out, i+1); - from->out[i] = nc; - - tal_add_destructor(nc, destroy_connection); - return nc; + tal_add_destructor2(chan, destroy_routing_channel, rstate); + return chan; } -static void delete_connection(const struct node_connection *connection) +static void destroy_node_connection(struct node_connection *nc, + struct routing_channel *chan) { - tal_free(connection); + int dir = nc->flags & 0x1; + struct node_connection *c = chan->connections[dir]; + + assert(nc == c); + chan->connections[dir] = NULL; + + /* Both sides deleted? Free channel */ + if (!chan->connections[!dir]) + tal_free(chan); } -struct node_connection *half_add_connection( - struct routing_state *rstate, - const struct pubkey *from, - const struct pubkey *to, - const struct short_channel_id *schanid, - const u16 flags - ) +static struct node_connection *new_node_connection(struct routing_state *rstate, + struct routing_channel *chan, + struct node *from, + struct node *to, + int idx) { - struct node_connection *nc; - nc = get_or_make_connection(rstate, from, to); - nc->short_channel_id = *schanid; - nc->active = false; - nc->flags = flags; - nc->last_timestamp = -1; - return nc; + struct node_connection *c; + + /* We are going to put this in the right way? */ + assert(idx == pubkey_idx(&from->id, &to->id)); + assert(from == chan->nodes[idx]); + assert(to == chan->nodes[!idx]); + + c = tal(rstate, struct node_connection); + c->src = from; + c->dst = to; + c->short_channel_id = chan->scid; + c->channel_announcement = NULL; + c->channel_update = NULL; + c->unroutable_until = 0; + c->active = false; + c->flags = idx; + c->last_timestamp = -1; + + /* Hook it into in/out arrays. */ + assert(!chan->connections[idx]); + chan->connections[idx] = c; + + tal_add_destructor2(c, destroy_node_connection, chan); + return c; } +struct node_connection *half_add_connection(struct routing_state *rstate, + const struct pubkey *from_id, + const struct pubkey *to_id, + const struct short_channel_id *scid) +{ + int idx; + struct node *from, *to; + struct routing_channel *chan; + struct node_connection *c; + + from = get_node(rstate, from_id); + if (!from) + from = new_node(rstate, from_id); + to = get_node(rstate, to_id); + if (!to) + to = new_node(rstate, to_id); + + /* FIXME: callers all have no channel? */ + chan = find_channel(rstate, from, to, &idx); + if (!chan) + chan = new_routing_channel(rstate, scid, from, to); + c = chan->connections[idx]; + if (!c) { + SUPERVERBOSE("Creating new route from %s to %s", + type_to_string(trc, struct pubkey, &from->id), + type_to_string(trc, struct pubkey, &to->id)); + c = chan->connections[idx] + = new_node_connection(rstate, chan, from, to, idx); + } else { + status_trace("Updating existing route from %s to %s", + type_to_string(trc, struct pubkey, &from->id), + type_to_string(trc, struct pubkey, &to->id)); + } + assert(c->src == from); + assert(c->dst == to); + + return c; +} /* Too big to reach, but don't overflow if added. */ #define INFINITE 0x3FFFFFFFFFFFFFFFULL @@ -333,10 +399,10 @@ 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, size_t edgenum, double riskfactor, +static void bfg_one_edge(struct node *node, + struct node_connection *c, double riskfactor, double fuzz, const struct siphash_seed *base_seed) { - struct node_connection *c = node->in[edgenum]; size_t h; double fee_scale = 1.0; @@ -390,7 +456,7 @@ static void bfg_one_edge(struct node *node, size_t edgenum, double riskfactor, /* Determine if the given node_connection is routable */ static bool nc_is_routable(const struct node_connection *nc, time_t now) { - return nc->active && nc->unroutable_until < now; + return nc && nc->active && nc->unroutable_until < now; } /* riskfactor is already scaled to per-block amount */ @@ -449,18 +515,21 @@ find_route(const tal_t *ctx, struct routing_state *rstate, for (n = node_map_first(rstate->nodes, &it); n; n = node_map_next(rstate->nodes, &it)) { - size_t num_edges = tal_count(n->in); + size_t num_edges = tal_count(n->channels); for (i = 0; i < num_edges; i++) { + struct node_connection *c; SUPERVERBOSE("Node %s edge %i/%zu", type_to_string(trc, struct pubkey, &n->id), i, num_edges); - if (!nc_is_routable(n->in[i], now)) { + + c = connection_to(n, n->channels[i]); + if (!nc_is_routable(c, now)) { SUPERVERBOSE("...unroutable"); continue; } - bfg_one_edge(n, i, riskfactor, - fuzz, base_seed); + bfg_one_edge(n, c, + riskfactor, fuzz, base_seed); SUPERVERBOSE("...done"); } } @@ -539,7 +608,7 @@ add_channel_direction(struct routing_state *rstate, const struct pubkey *from, c = c1; } else { /* We don't know this channel at all, create it */ - c = half_add_connection(rstate, from, to, short_channel_id, direction); + c = half_add_connection(rstate, from, to, short_channel_id); } /* Remember the announcement so we can forward it to new peers */ @@ -585,41 +654,6 @@ static bool check_channel_announcement( check_signed_hash(&hash, bitcoin2_sig, bitcoin2_key); } -struct routing_channel *routing_channel_new(const tal_t *ctx, - struct short_channel_id *scid) -{ - struct routing_channel *chan = tal(ctx, struct routing_channel); - chan->scid = *scid; - chan->connections[0] = chan->connections[1] = NULL; - chan->nodes[0] = chan->nodes[1] = NULL; - chan->txout_script = NULL; - chan->public = false; - memset(&chan->msg_indexes, 0, sizeof(chan->msg_indexes)); - return chan; -} - -static void destroy_node_connection(struct node_connection *nc, - struct routing_state *rstate) -{ - struct routing_channel *chan = get_channel(rstate,&nc->short_channel_id); - struct node_connection *c = chan->connections[nc->flags & 0x1]; - if (c == NULL) - return; - /* If we found a channel it should be the same */ - assert(nc == c); - chan->connections[nc->flags & 0x1] = NULL; -} - -void channel_add_connection(struct routing_state *rstate, - struct routing_channel *chan, - struct node_connection *nc) -{ - int direction = get_channel_direction(&nc->src->id, &nc->dst->id); - assert(chan != NULL); - chan->connections[direction] = nc; - tal_add_destructor2(nc, destroy_node_connection, rstate); -} - static void add_pending_node_announcement(struct routing_state *rstate, struct pubkey *nodeid) { struct pending_node_announce *pna = tal(rstate, struct pending_node_announce); @@ -834,20 +868,6 @@ bool handle_pending_cannouncement(struct routing_state *rstate, return false; } - chan = get_channel(rstate, &pending->short_channel_id); - - /* So you're new in town, ey? Let's find you a room in the Inn. */ - if (!chan) { - chan = routing_channel_new(rstate, &pending->short_channel_id); - uintmap_add(&rstate->channels, pending->short_channel_id.u64, chan); - } else { - /* See handle_channel_announcement */ - assert(!chan->public); - } - - /* Channel is now public. */ - chan->public = true; - /* Is this a new connection? It is if we don't know the * channel yet, or do not have a matching announcement in the * case of side-loaded channels*/ @@ -867,8 +887,9 @@ bool handle_pending_cannouncement(struct routing_state *rstate, c1 = add_channel_direction(rstate, &pending->node_id_2, &pending->node_id_1, &pending->short_channel_id, pending->announce); - channel_add_connection(rstate, chan, c0); - channel_add_connection(rstate, chan, c1); + /* Channel is now public. */ + chan = get_channel(rstate, scid); + chan->public = true; if (forward) { if (replace_broadcast(rstate->broadcasts, @@ -1234,30 +1255,20 @@ struct route_hop *get_route(tal_t *ctx, struct routing_state *rstate, return hops; } -/* Get the struct node_connection matching the short_channel_id, - * which must be an out connection of the given node. */ -static struct node_connection * -get_out_node_connection_of(const struct node *node, - const struct short_channel_id *short_channel_id) -{ - int i; - - for (i = 0; i < tal_count(node->out); ++i) { - if (structeq(&node->out[i]->short_channel_id, short_channel_id)) - return node->out[i]; - } - - return NULL; -} - /** - * routing_failure_on_nc - Handle routing failure on a specific - * node_connection. + * routing_failure_channel_out - Handle routing failure on a specific channel */ -static void routing_failure_on_nc(enum onion_type failcode, - struct node_connection *nc, - time_t now) +static void routing_failure_channel_out(struct node *node, + enum onion_type failcode, + struct routing_channel *chan, + time_t now) { + struct node_connection *nc; + + nc = connection_from(node, chan); + if (!nc) + return; + /* BOLT #4: * * - if the PERM bit is NOT set: @@ -1267,7 +1278,7 @@ static void routing_failure_on_nc(enum onion_type failcode, /* Prevent it for 20 seconds. */ nc->unroutable_until = now + 20; else - delete_connection(nc); + tal_free(nc); } void routing_failure(struct routing_state *rstate, @@ -1278,7 +1289,6 @@ void routing_failure(struct routing_state *rstate, { const tal_t *tmpctx = tal_tmpctx(rstate); struct node *node; - struct node_connection *nc; int i; enum wire_type t; time_t now = time_now().ts.tv_sec; @@ -1308,23 +1318,35 @@ void routing_failure(struct routing_state *rstate, * */ if (failcode & NODE) { - for (i = 0; i < tal_count(node->in); ++i) - routing_failure_on_nc(failcode, node->in[i], now); - for (i = 0; i < tal_count(node->out); ++i) - routing_failure_on_nc(failcode, node->out[i], now); + /* If permanent, we forget entire node and all its channels. + * If we did this in a loop, we might use-after-free. */ + if (failcode & PERM) { + tal_free(node); + } else { + for (i = 0; i < tal_count(node->channels); ++i) + routing_failure_channel_out(node, failcode, + node->channels[i], + now); + } } else { - nc = get_out_node_connection_of(node, scid); - if (nc) - routing_failure_on_nc(failcode, nc, now); + struct routing_channel *chan = get_channel(rstate, scid); + + if (!chan) + status_unusual("routing_failure: " + "Channel %s unknown", + type_to_string(tmpctx, + struct short_channel_id, + scid)); + else if (chan->nodes[0] != node && chan->nodes[1] != node) + status_unusual("routing_failure: " + "Channel %s does not connect to %s", + type_to_string(tmpctx, + struct short_channel_id, + scid), + type_to_string(tmpctx, struct pubkey, + erring_node_pubkey)); else - status_trace("UNUSUAL routing_failure: " - "Channel %s not an out channel " - "of node %s", - type_to_string(tmpctx, - struct short_channel_id, - scid), - type_to_string(tmpctx, struct pubkey, - erring_node_pubkey)); + routing_failure_channel_out(node, failcode, chan, now); } /* Update the channel if UPDATE failcode. Do diff --git a/gossipd/routing.h b/gossipd/routing.h index 32f1ee50f..733ab5d27 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -13,6 +13,7 @@ #define ROUTING_FLAGS_DISABLED 2 struct node_connection { + /* FIXME: Remove */ struct node *src, *dst; /* millisatoshi. */ u32 base_fee; @@ -31,6 +32,7 @@ struct node_connection { u32 htlc_minimum_msat; /* The channel ID, as determined by the anchor transaction */ + /* FIXME: Remove */ struct short_channel_id short_channel_id; /* Flags as specified by the `channel_update`s, among other @@ -38,6 +40,7 @@ struct node_connection { u16 flags; /* Cached `channel_announcement` and `channel_update` we might forward to new peers*/ + /* FIXME: Remove */ u8 *channel_announcement; u8 *channel_update; @@ -55,8 +58,8 @@ struct node { /* IP/Hostname and port of this node (may be NULL) */ struct wireaddr *addresses; - /* Routes connecting to us, from us. */ - struct node_connection **in, **out; + /* Channels connecting us to other nodes */ + struct routing_channel **channels; /* Temporary data for routefinding. */ struct { @@ -93,15 +96,48 @@ struct routing_channel { struct short_channel_id scid; u8 *txout_script; + /* One of these might be NULL. + * connections[0]->src == nodes[0] connections[0]->dst == nodes[1] + * connections[1]->src == nodes[1] connections[1]->dst == nodes[0] + */ struct node_connection *connections[2]; + /* nodes[0].id < nodes[1].id */ struct node *nodes[2]; + /* FIXME: Move msg_index[MSG_INDEX_CUPDATE*] into connections[] */ u64 msg_indexes[3]; /* Is this a public channel, or was it only added locally? */ bool public; }; +/* If the two nodes[] are id1 and id2, which index would id1 be? */ +static inline int pubkey_idx(const struct pubkey *id1, const struct pubkey *id2) +{ + return pubkey_cmp(id1, id2) > 0; +} + +/* 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) +{ + int idx = (chan->nodes[1] == n); + + assert(!chan->connections[idx] || chan->connections[idx]->src == n); + assert(!chan->connections[!idx] || chan->connections[!idx]->dst == n); + return chan->connections[idx]; +} + +static inline struct node_connection *connection_to(const struct node *n, + struct routing_channel *chan) +{ + int idx = (chan->nodes[1] == n); + + assert(!chan->connections[idx] || chan->connections[idx]->src == n); + assert(!chan->connections[!idx] || chan->connections[!idx]->dst == n); + return chan->connections[!idx]; +} + struct routing_state { /* All known nodes. */ struct node_map *nodes; @@ -148,8 +184,7 @@ struct routing_state *new_routing_state(const tal_t *ctx, struct node_connection *half_add_connection(struct routing_state *rstate, const struct pubkey *from, const struct pubkey *to, - const struct short_channel_id *schanid, - const u16 flags); + const struct short_channel_id *scid); /* Given a short_channel_id, retrieve the matching connection, or NULL if it is * unknown. */ @@ -203,15 +238,13 @@ void routing_failure(struct routing_state *rstate, void mark_channel_unroutable(struct routing_state *rstate, const struct short_channel_id *channel); -/* routing_channel constructor */ -struct routing_channel *routing_channel_new(const tal_t *ctx, - struct short_channel_id *scid); - /* Add the connection to the channel */ void channel_add_connection(struct routing_state *rstate, struct routing_channel *chan, struct node_connection *nc); +void delete_connection(struct routing_channel *chan, int idx); + /* Utility function that, given a source and a destination, gives us * the direction bit the matching channel should get */ #define get_channel_direction(from, to) (pubkey_cmp(from, to) > 0) diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index c091e7eff..aa0237265 100644 --- a/gossipd/test/run-bench-find_route.c +++ b/gossipd/test/run-bench-find_route.c @@ -108,12 +108,16 @@ static struct node_connection *add_connection(struct routing_state *rstate, u32 base_fee, s32 proportional_fee, u32 delay) { - struct node_connection *c = get_or_make_connection(rstate, from, to); + struct short_channel_id scid; + struct node_connection *c; + + memset(&scid, 0, sizeof(scid)); + c = half_add_connection(rstate, from, to, &scid); c->base_fee = base_fee; c->proportional_fee = proportional_fee; c->delay = delay; c->active = true; - memset(&c->short_channel_id, 0, sizeof(c->short_channel_id)); + c->short_channel_id = scid; c->flags = get_channel_direction(from, to); return c; } diff --git a/gossipd/test/run-find_route-specific.c b/gossipd/test/run-find_route-specific.c index ae638bc07..6362c9f67 100644 --- a/gossipd/test/run-find_route-specific.c +++ b/gossipd/test/run-find_route-specific.c @@ -65,6 +65,21 @@ void towire_u16(u8 **pptr UNNEEDED, u16 v UNNEEDED) const void *trc; +bool short_channel_id_from_str(const char *str, size_t strlen, + struct short_channel_id *dst); + +static struct node_connection * +get_or_make_connection(struct routing_state *rstate, + const struct pubkey *from_id, + const struct pubkey *to_id, + const char *shortid) +{ + struct short_channel_id scid; + if (!short_channel_id_from_str(shortid, strlen(shortid), &scid)) + abort(); + return half_add_connection(rstate, from_id, to_id, &scid); +} + int main(void) { static const struct bitcoin_blkid zerohash; @@ -92,7 +107,7 @@ int main(void) rstate = new_routing_state(ctx, &zerohash, &a); /* [{'active': True, 'short_id': '6990:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'last_update': 1504064344}, */ - nc = get_or_make_connection(rstate, &c, &b); + nc = get_or_make_connection(rstate, &c, &b, "6990:2:1"); nc->active = true; nc->base_fee = 0; nc->proportional_fee = 10; @@ -101,7 +116,7 @@ int main(void) nc->last_timestamp = 1504064344; /* {'active': True, 'short_id': '6989:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, */ - nc = get_or_make_connection(rstate, &b, &a); + nc = get_or_make_connection(rstate, &b, &a, "6989:2:1"); nc->active = true; nc->base_fee = 0; nc->proportional_fee = 10; @@ -110,7 +125,7 @@ int main(void) nc->last_timestamp = 1504064344; /* {'active': True, 'short_id': '6990:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, */ - nc = get_or_make_connection(rstate, &b, &c); + nc = get_or_make_connection(rstate, &b, &c, "6990:2:1"); nc->active = true; nc->base_fee = 0; nc->proportional_fee = 10; @@ -119,7 +134,7 @@ int main(void) nc->last_timestamp = 1504064344; /* {'active': True, 'short_id': '6989:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'last_update': 1504064344}]} */ - nc = get_or_make_connection(rstate, &a, &b); + nc = get_or_make_connection(rstate, &a, &b, "6989:2:1"); nc->active = true; nc->base_fee = 0; nc->proportional_fee = 10; diff --git a/gossipd/test/run-find_route.c b/gossipd/test/run-find_route.c index e25bd48bd..6643c7b18 100644 --- a/gossipd/test/run-find_route.c +++ b/gossipd/test/run-find_route.c @@ -70,12 +70,16 @@ static struct node_connection *add_connection(struct routing_state *rstate, u32 base_fee, s32 proportional_fee, u32 delay) { - struct node_connection *c = get_or_make_connection(rstate, from, to); + struct short_channel_id scid; + struct node_connection *c; + + memset(&scid, 0, sizeof(scid)); + c = half_add_connection(rstate, from, to, &scid); c->base_fee = base_fee; c->proportional_fee = proportional_fee; c->delay = delay; c->active = true; - memset(&c->short_channel_id, 0, sizeof(c->short_channel_id)); + c->short_channel_id = scid; c->flags = get_channel_direction(from, to); return c; }