gossipd: fix json_listpeers printing node information.

json_listpeers returns an array of peers, and an array of nodes: the latter
is a subset of the former, and is used for printing alias/color information.

This changes it so there is a 1:1 correspondance between the peer information
and nodes, meaning no more O(n^2) search.

If there is no node_announce for a peer, we use a negative timestamp
(already used to indicate that the rest of the gossip_getnodes_entry
is not valid).

Other fixes:
1. Use get_node instead of iterating through the node map.
2. A node without addresses is perfectly valid: we have to use the timestamp
   to see if the alias/color are set.  Previously we wouldn't print that
   if it didn't also advertize an address.

Signed-off-by: Rusty Russell <[email protected]>

gossipd/gossip.c

@@ -2103,17 +2103,20 @@ static struct io_plan *getchannels_req(struct io_conn *conn, struct daemon *daem
 }
 
 static void append_node(const struct gossip_getnodes_entry ***nodes,
+			const struct pubkey *nodeid,
+			/* If non-NULL, contains more information */
 			const struct node *n)
 {
 	struct gossip_getnodes_entry *new;
 	size_t num_nodes = tal_count(*nodes);
 
 	new = tal(*nodes, struct gossip_getnodes_entry);
-	new->nodeid = n->id;
-	new->last_timestamp = n->last_timestamp;
-	if (n->last_timestamp < 0) {
+	new->nodeid = *nodeid;
+	if (!n || n->last_timestamp < 0) {
+		new->last_timestamp = -1;
 		new->addresses = NULL;
 	} else {
+		new->last_timestamp = n->last_timestamp;
 		new->addresses = n->addresses;
 		new->alias = n->alias;
 		memcpy(new->color, n->rgb_color, 3);
@@ -2137,13 +2140,13 @@ static struct io_plan *getnodes(struct io_conn *conn, struct daemon *daemon,
 		for (size_t i = 0; i < tal_count(ids); i++) {
 			n = get_node(daemon->rstate, &ids[i]);
 			if (n)
-				append_node(&nodes, n);
+				append_node(&nodes, &ids[i], n);
 		}
 	} else {
 		struct node_map_iter i;
 		n = node_map_first(daemon->rstate->nodes, &i);
 		while (n != NULL) {
-			append_node(&nodes, n);
+			append_node(&nodes, &n->id, n);
 			n = node_map_next(daemon->rstate->nodes, &i);
 		}
 	}
@@ -3430,14 +3433,12 @@ static struct io_plan *get_peers(struct io_conn *conn,
 	size_t n = 0;
 	struct pubkey *id = tal_arr(conn, struct pubkey, n);
 	struct wireaddr_internal *wireaddr = tal_arr(conn, struct wireaddr_internal, n);
-	const struct gossip_getnodes_entry **nodes;
+	const struct gossip_getnodes_entry **nodes = tal_arr(conn, const struct gossip_getnodes_entry *, 0);
 	struct pubkey *specific_id = NULL;
-	struct node_map_iter it;
 
 	if (!fromwire_gossip_getpeers_request(msg, msg, &specific_id))
 		master_badmsg(WIRE_GOSSIPCTL_PEER_ADDRHINT, msg);
 
-	nodes = tal_arr(conn, const struct gossip_getnodes_entry*, 0);
 	list_for_each(&daemon->peers, peer, list) {
 		if (specific_id && !pubkey_eq(specific_id, &peer->id))
 			continue;
@@ -3446,14 +3447,8 @@ static struct io_plan *get_peers(struct io_conn *conn,
 
 		id[n] = peer->id;
 		wireaddr[n] = peer->addr;
-
-		struct node* nd = NULL;
-		for (nd = node_map_first(daemon->rstate->nodes, &it); nd; nd = node_map_next(daemon->rstate->nodes, &it)) {
-			if (pubkey_eq(&nd->id, &peer->id)) {
-				append_node(&nodes, nd);
-				break;
-			}
-		}
+		append_node(&nodes, &peer->id,
+			    get_node(daemon->rstate, &peer->id));
 		n++;
 	}
 

gossipd/gossip_wire.csv

@@ -209,8 +209,7 @@ gossip_getpeers_reply,3111
 gossip_getpeers_reply,,num,u16
 gossip_getpeers_reply,,id,num*struct pubkey
 gossip_getpeers_reply,,addr,num*struct wireaddr_internal
-gossip_getpeers_reply,,numnodes,u16
-gossip_getpeers_reply,,nodes,numnodes*struct gossip_getnodes_entry
+gossip_getpeers_reply,,nodes,num*struct gossip_getnodes_entry
 
 # Channel daemon can ask for updates for a specific channel, for sending
 # errors.  Must be distinct from WIRE_CHANNEL_ANNOUNCEMENT etc. gossip msgs!

lightningd/peer_control.c

@@ -704,27 +704,18 @@ struct getpeers_args {
 };
 
 static void json_add_node_decoration(struct json_result *response,
-				     struct gossip_getnodes_entry **nodes,
-				     const struct pubkey *id)
+				     struct gossip_getnodes_entry *node)
 {
-	for (size_t i = 0; i < tal_count(nodes); i++) {
-		struct json_escaped *esc;
+	struct json_escaped *esc;
 
-		/* If no addresses, then this node announcement hasn't been
-		 * received yet So no alias information either.
-		 */
-		if (nodes[i]->addresses == NULL)
-			continue;
-
-		if (!pubkey_eq(&nodes[i]->nodeid, id))
-			continue;
+	/* If node announcement hasn't been received yet, no alias information.
+	 */
+	if (node->last_timestamp < 0)
+		return;
 
-		esc = json_escape(NULL, (const char *)nodes[i]->alias);
-		json_add_escaped_string(response, "alias", take(esc));
-		json_add_hex(response, "color",
-			     nodes[i]->color, ARRAY_SIZE(nodes[i]->color));
-		break;
-	}
+	esc = json_escape(NULL, (const char *)node->alias);
+	json_add_escaped_string(response, "alias", take(esc));
+	json_add_hex(response, "color", node->color, ARRAY_SIZE(node->color));
 }
 
 static void gossipd_getpeers_complete(struct subd *gossip, const u8 *msg,
@@ -778,7 +769,14 @@ static void gossipd_getpeers_complete(struct subd *gossip, const u8 *msg,
 			json_array_end(response);
 		}
 
-		json_add_node_decoration(response, nodes, &p->id);
+		/* Search gossip reply for this ID, to add extra info. */
+		for (size_t i = 0; i < tal_len(nodes); i++) {
+			if (pubkey_eq(&nodes[i]->nodeid, &p->id)) {
+				json_add_node_decoration(response, nodes[i]);
+				break;
+			}
+		}
+
 		json_array_start(response, "channels");
 		json_add_uncommitted_channel(response, p->uncommitted_channel);
 
@@ -909,7 +907,7 @@ static void gossipd_getpeers_complete(struct subd *gossip, const u8 *msg,
 		/* Fake state. */
 		json_add_string(response, "state", "GOSSIPING");
 		json_add_pubkey(response, "id", ids+i);
-		json_add_node_decoration(response, nodes, ids+i);
+		json_add_node_decoration(response, nodes[i]);
 		json_array_start(response, "netaddr");
 		if (addrs[i].itype != ADDR_INTERNAL_WIREADDR
 		    || addrs[i].u.wireaddr.type != ADDR_TYPE_PADDING)