routing: clean up channel public/active states.

1. If we have a channel_announcement, the channel is public, otherwise
   it's not.  Not all channels are public, as they can be local: those
   have a NULL channel_announcement.

2. If we don't have a channel_update, we know nothing about that half
   of the channel, and no other fields are valid.

3. We can tell if a half channel is disabled by the flags field directly.

Note that we never send halfchannels without an update over
gossip_getchannels_reply so that marshalling/unmarshalling can be
vastly simplified.

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

gossipd/gossip.c

@@ -1246,11 +1246,7 @@ static void append_half_channel(struct gossip_getchannels_entry **entries,
 	struct gossip_getchannels_entry *e;
 	size_t n;
 
-	if (!c)
-		return;
-
-	/* Don't mention inactive or unannounced channels. */
-	if (!c->active && !c->channel_update)
+	if (!is_halfchan_defined(c))
 		return;
 
 	n = tal_count(*entries);
@@ -1260,16 +1256,13 @@ static void append_half_channel(struct gossip_getchannels_entry **entries,
 	e->source = chan->nodes[idx]->id;
 	e->destination = chan->nodes[!idx]->id;
 	e->satoshis = chan->satoshis;
-	e->active = c->active;
 	e->flags = c->flags;
-	e->public = chan->public && (c->channel_update != NULL);
+	e->public = is_chan_public(chan);
 	e->short_channel_id = chan->scid;
-	e->last_update_timestamp = c->channel_update ? c->last_timestamp : -1;
-	if (e->last_update_timestamp >= 0) {
-		e->base_fee_msat = c->base_fee;
-		e->fee_per_millionth = c->proportional_fee;
-		e->delay = c->delay;
-	}
+	e->last_update_timestamp = c->last_timestamp;
+	e->base_fee_msat = c->base_fee;
+	e->fee_per_millionth = c->proportional_fee;
+	e->delay = c->delay;
 }
 
 static void append_channel(struct gossip_getchannels_entry **entries,
@@ -1516,7 +1509,7 @@ static void gossip_refresh_network(struct daemon *daemon)
 		for (size_t i = 0; i < tal_count(n->chans); i++) {
 			struct half_chan *hc = half_chan_from(n, n->chans[i]);
 
-			if (!hc->channel_update) {
+			if (!is_halfchan_defined(hc)) {
 				/* Connection is not announced yet, so don't even
 				 * try to re-announce it */
 				continue;
@@ -1527,7 +1520,7 @@ static void gossip_refresh_network(struct daemon *daemon)
 				continue;
 			}
 
-			if (!hc->active) {
+			if (!is_halfchan_enabled(hc)) {
 				/* Only send keepalives for active connections */
 				continue;
 			}
@@ -2339,11 +2332,9 @@ static struct io_plan *handle_disable_channel(struct io_conn *conn,
 
 	status_trace("Disabling channel %s/%d, active %d -> %d",
 		     type_to_string(msg, struct short_channel_id, &scid),
-		     direction, hc->active, active);
-
-	hc->active = active;
+		     direction, is_halfchan_enabled(hc), active);
 
-	if (!hc->channel_update) {
+	if (!is_halfchan_defined(hc)) {
 		status_trace(
 		    "Channel %s/%d doesn't have a channel_update yet, can't "
 		    "disable",

gossipd/routing.c

@@ -195,7 +195,6 @@ static void init_half_chan(struct routing_state *rstate,
 
 	c->channel_update = NULL;
 	c->unroutable_until = 0;
-	c->active = false;
 	c->flags = idx;
 	/* We haven't seen channel_update: make it halfway to prune time,
 	 * which should be older than any update we'd see. */
@@ -228,7 +227,6 @@ struct chan *new_chan(struct routing_state *rstate,
 	chan->nodes[!n1idx] = n2;
 	chan->txout_script = NULL;
 	chan->channel_announce = NULL;
-	chan->public = false;
 	chan->satoshis = 0;
 
 	n = tal_count(n2->chans);
@@ -345,7 +343,7 @@ static void bfg_one_edge(struct node *node,
 /* Determine if the given half_chan is routable */
 static bool hc_is_routable(const struct half_chan *hc, time_t now)
 {
-	return hc->active && hc->unroutable_until < now;
+	return is_halfchan_enabled(hc) && hc->unroutable_until < now;
 }
 
 /* riskfactor is already scaled to per-block amount */
@@ -622,10 +620,8 @@ bool routing_add_channel_announcement(struct routing_state *rstate,
 	if (!chan)
 		chan = new_chan(rstate, &scid, &node_id_1, &node_id_2);
 
-	/* Channel is now public. */
-	chan->public = true;
 	chan->satoshis = satoshis;
-
+	/* Channel is now public. */
 	chan->channel_announce = tal_dup_arr(chan, u8, msg, tal_len(msg), 0);
 
 	/* Now we can broadcast channel announce */
@@ -633,7 +629,7 @@ bool routing_add_channel_announcement(struct routing_state *rstate,
 
 	/* If we had private updates for channels, we can broadcast them too. */
 	for (size_t i = 0; i < ARRAY_SIZE(chan->half); i++) {
-		if (!chan->half[i].channel_update)
+		if (!is_halfchan_defined(&chan->half[i]))
 			continue;
 		insert_broadcast(rstate->broadcasts,
 				 chan->half[i].channel_update);
@@ -681,7 +677,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
 	/* Check if we know the channel already (no matter in what
 	 * state, we stop here if yes). */
 	chan = get_channel(rstate, &pending->short_channel_id);
-	if (chan != NULL && chan->public) {
+	if (chan != NULL && is_chan_public(chan)) {
 		SUPERVERBOSE("%s: %s already has public channel",
 			     __func__,
 			     type_to_string(tmpctx, struct short_channel_id,
@@ -884,7 +880,7 @@ static void set_connection_values(struct chan *chan,
 				  u32 base_fee,
 				  u32 proportional_fee,
 				  u32 delay,
-				  bool active,
+				  u16 flags,
 				  u64 timestamp,
 				  u32 htlc_minimum_msat)
 {
@@ -894,9 +890,9 @@ static void set_connection_values(struct chan *chan,
 	c->htlc_minimum_msat = htlc_minimum_msat;
 	c->base_fee = base_fee;
 	c->proportional_fee = proportional_fee;
-	c->active = active;
+	c->flags = flags;
 	c->last_timestamp = timestamp;
-	assert((c->flags & 0x1) == idx);
+	assert((c->flags & ROUTING_FLAGS_DIRECTION) == idx);
 
 	/* If it was temporarily unroutable, re-enable */
 	c->unroutable_until = 0;
@@ -912,7 +908,7 @@ static void set_connection_values(struct chan *chan,
 					    &chan->scid),
 			     idx,
 			     c->proportional_fee);
-		c->active = false;
+		c->flags |= ROUTING_FLAGS_DISABLED;
 	}
 }
 
@@ -943,8 +939,7 @@ bool routing_add_channel_update(struct routing_state *rstate,
 	direction = flags & 0x1;
 	set_connection_values(chan, direction, fee_base_msat,
 			      fee_proportional_millionths, expiry,
-			      (flags & ROUTING_FLAGS_DISABLED) == 0, timestamp,
-			      htlc_minimum_msat);
+			      flags, timestamp, htlc_minimum_msat);
 
 	/* Replace any old one. */
 	tal_free(chan->half[direction].channel_update);
@@ -1008,7 +1003,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update,
 	chan = get_channel(rstate, &short_channel_id);
 
 	/* Optimization: only check for pending if not public */
-	if (!chan || !chan->public) {
+	if (!chan || !is_chan_public(chan)) {
 		struct pending_cannouncement *pending;
 
 		pending = find_pending_cannouncement(rstate, &short_channel_id);
@@ -1028,7 +1023,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update,
 
 	c = &chan->half[direction];
 
-	if (c->last_timestamp >= timestamp) {
+	if (is_halfchan_defined(c) && c->last_timestamp >= timestamp) {
 		SUPERVERBOSE("Ignoring outdated update.");
 		return NULL;
 	}
@@ -1141,7 +1136,7 @@ bool routing_add_node_announcement(struct routing_state *rstate, const u8 *msg T
 static bool node_has_public_channels(struct node *node)
 {
 	for (size_t i = 0; i < tal_count(node->chans); i++)
-		if (node->chans[i]->public)
+		if (is_chan_public(node->chans[i]))
 			return true;
 	return false;
 }
@@ -1485,7 +1480,7 @@ void route_prune(struct routing_state *rstate)
 	     chan;
 	     chan = uintmap_after(&rstate->chanmap, &idx)) {
 		/* Local-only?  Don't prune. */
-		if (!chan->public)
+		if (!is_chan_public(chan))
 			continue;
 
 		if (chan->half[0].last_timestamp < highwater

gossipd/routing.h

@@ -11,9 +11,20 @@
 #include <wire/wire.h>
 
 #define ROUTING_MAX_HOPS 20
-#define ROUTING_FLAGS_DISABLED 2
+/* BOLT #7:
+ *
+ * The `flags` bitfield...individual bits:
+ *...
+ * | 0             | `direction` | Direction this update refers to. |
+ * | 1             | `disable`   | Disable the channel.             |
+ */
+#define ROUTING_FLAGS_DIRECTION (1 << 0)
+#define ROUTING_FLAGS_DISABLED  (1 << 1)
 
 struct half_chan {
+	/* Cached `channel_update` which initialized below (or NULL) */
+	const u8 *channel_update;
+
 	/* millisatoshi. */
 	u32 base_fee;
 	/* millionths */
@@ -22,9 +33,7 @@ struct half_chan {
 	/* Delay for HTLC in blocks.*/
 	u32 delay;
 
-	/* Is this connection active? */
-	bool active;
-
+	/* -1 if channel_update is NULL */
 	s64 last_timestamp;
 
 	/* Minimum number of msatoshi in an HTLC */
@@ -34,9 +43,6 @@ struct half_chan {
 	 * things indicated direction wrt the `channel_id` */
 	u16 flags;
 
-	/* Cached `channel_update` we might forward to new peers (or NULL) */
-	const u8 *channel_update;
-
 	/* If greater than current time, this connection should not
 	 * be used for routing. */
 	time_t unroutable_until;
@@ -54,15 +60,27 @@ struct chan {
 	/* node[0].id < node[1].id */
 	struct node *nodes[2];
 
-	/* NULL if not announced yet */
+	/* NULL if not announced yet (ie. not public). */
 	const u8 *channel_announce;
 
-	/* Is this a public channel, or was it only added locally? */
-	bool public;
-
 	u64 satoshis;
 };
 
+static inline bool is_chan_public(const struct chan *chan)
+{
+	return chan->channel_announce != NULL;
+}
+
+static inline bool is_halfchan_defined(const struct half_chan *hc)
+{
+	return hc->channel_update != NULL;
+}
+
+static inline bool is_halfchan_enabled(const struct half_chan *hc)
+{
+	return is_halfchan_defined(hc) && !(hc->flags & ROUTING_FLAGS_DISABLED);
+}
+
 struct node {
 	struct pubkey id;
 

gossipd/test/run-bench-find_route.c

@@ -145,7 +145,6 @@ static struct half_chan *add_connection(struct routing_state *rstate,
 	c->base_fee = base_fee;
 	c->proportional_fee = proportional_fee;
 	c->delay = delay;
-	c->active = true;
 	c->flags = get_channel_direction(from, to);
 	return c;
 }

gossipd/test/run-find_route-specific.c

@@ -106,6 +106,8 @@ get_or_make_connection(struct routing_state *rstate,
 	if (!chan)
 		chan = new_chan(rstate, &scid, from_id, to_id);
 
+	/* Make sure it's seen as initialized (update non-NULL). */
+	chan->half[pubkey_idx(from_id, to_id)].channel_update = (void *)chan;
 	return &chan->half[pubkey_idx(from_id, to_id)];
 }
 
@@ -153,7 +155,6 @@ int main(void)
 
 	/* [{'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, "6990:2:1");
-	nc->active = true;
 	nc->base_fee = 0;
 	nc->proportional_fee = 10;
 	nc->delay = 5;
@@ -162,7 +163,6 @@ int main(void)
 
 	/* {'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, "6989:2:1");
-	nc->active = true;
 	nc->base_fee = 0;
 	nc->proportional_fee = 10;
 	nc->delay = 5;
@@ -171,7 +171,6 @@ int main(void)
 
 	/* {'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, "6990:2:1");
-	nc->active = true;
 	nc->base_fee = 0;
 	nc->proportional_fee = 10;
 	nc->delay = 5;
@@ -180,7 +179,6 @@ int main(void)
 
 	/* {'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, "6989:2:1");
-	nc->active = true;
 	nc->base_fee = 0;
 	nc->proportional_fee = 10;
 	nc->delay = 5;

gossipd/test/run-find_route.c

@@ -107,10 +107,11 @@ static struct half_chan *add_connection(struct routing_state *rstate,
 		chan = new_chan(rstate, &scid, from, to);
 
 	c = &chan->half[pubkey_idx(from, to)];
+	/* Make sure it's seen as initialized (update non-NULL). */
+	c->channel_update = (void *)c;
 	c->base_fee = base_fee;
 	c->proportional_fee = proportional_fee;
 	c->delay = delay;
-	c->active = true;
 	c->flags = get_channel_direction(from, to);
 	return c;
 }
@@ -242,7 +243,7 @@ int main(void)
 	assert(fee == 1 + 3);
 
 	/* Make B->C inactive, force it back via D */
-	get_connection(rstate, &b, &c)->active = false;
+	get_connection(rstate, &b, &c)->flags |= ROUTING_FLAGS_DISABLED;
 	route = find_route(tmpctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee);
 	assert(route);
 	assert(tal_count(route) == 2);

lightningd/gossip_control.c

@@ -485,19 +485,18 @@ static void json_listchannels_reply(struct subd *gossip UNUSED, const u8 *reply,
 		json_add_string(response, "short_channel_id",
 				type_to_string(reply, struct short_channel_id,
 					       &entries[i].short_channel_id));
-		json_add_num(response, "flags", entries[i].flags);
-		json_add_bool(response, "active", entries[i].active);
 		json_add_bool(response, "public", entries[i].public);
 		json_add_u64(response, "satoshis", entries[i].satoshis);
-		if (entries[i].last_update_timestamp >= 0) {
-			json_add_num(response, "last_update",
-				     entries[i].last_update_timestamp);
-			json_add_num(response, "base_fee_millisatoshi",
-				     entries[i].base_fee_msat);
-			json_add_num(response, "fee_per_millionth",
-				     entries[i].fee_per_millionth);
-			json_add_num(response, "delay", entries[i].delay);
-		}
+		json_add_num(response, "flags", entries[i].flags);
+		json_add_bool(response, "active",
+			      !(entries[i].flags & ROUTING_FLAGS_DISABLED));
+		json_add_num(response, "last_update",
+			     entries[i].last_update_timestamp);
+		json_add_num(response, "base_fee_millisatoshi",
+			     entries[i].base_fee_msat);
+		json_add_num(response, "fee_per_millionth",
+			     entries[i].fee_per_millionth);
+		json_add_num(response, "delay", entries[i].delay);
 		json_object_end(response);
 	}
 	json_array_end(response);

lightningd/gossip_msg.c

@@ -74,15 +74,12 @@ void fromwire_gossip_getchannels_entry(const u8 **pptr, size_t *max,
 	fromwire_pubkey(pptr, max, &entry->source);
 	fromwire_pubkey(pptr, max, &entry->destination);
 	entry->satoshis = fromwire_u64(pptr, max);
-	entry->active = fromwire_bool(pptr, max);
 	entry->flags = fromwire_u16(pptr, max);
 	entry->public = fromwire_bool(pptr, max);
-	entry->last_update_timestamp = fromwire_u64(pptr, max);
-	if (entry->last_update_timestamp >= 0) {
-		entry->base_fee_msat = fromwire_u32(pptr, max);
-		entry->fee_per_millionth = fromwire_u32(pptr, max);
-		entry->delay = fromwire_u32(pptr, max);
-	}
+	entry->last_update_timestamp = fromwire_u32(pptr, max);
+	entry->base_fee_msat = fromwire_u32(pptr, max);
+	entry->fee_per_millionth = fromwire_u32(pptr, max);
+	entry->delay = fromwire_u32(pptr, max);
 }
 
 void towire_gossip_getchannels_entry(u8 **pptr,
@@ -92,13 +89,10 @@ void towire_gossip_getchannels_entry(u8 **pptr,
 	towire_pubkey(pptr, &entry->source);
 	towire_pubkey(pptr, &entry->destination);
 	towire_u64(pptr, entry->satoshis);
-	towire_bool(pptr, entry->active);
 	towire_u16(pptr, entry->flags);
 	towire_bool(pptr, entry->public);
-	towire_u64(pptr, entry->last_update_timestamp);
-	if (entry->last_update_timestamp >= 0) {
-		towire_u32(pptr, entry->base_fee_msat);
-		towire_u32(pptr, entry->fee_per_millionth);
-		towire_u32(pptr, entry->delay);
-	}
+	towire_u32(pptr, entry->last_update_timestamp);
+	towire_u32(pptr, entry->base_fee_msat);
+	towire_u32(pptr, entry->fee_per_millionth);
+	towire_u32(pptr, entry->delay);
 }

lightningd/gossip_msg.h

@@ -15,12 +15,10 @@ struct gossip_getnodes_entry {
 struct gossip_getchannels_entry {
 	struct pubkey source, destination;
 	u64 satoshis;
-	bool active;
 	struct short_channel_id short_channel_id;
 	u16 flags;
 	bool public;
-	s64 last_update_timestamp; /* -1 means never */
-	/* These are only set if last_update_timestamp >= 0 */
+	u32 last_update_timestamp;
 	u32 delay;
 	u32 base_fee_msat;
 	u32 fee_per_millionth;