connectd: get addresses from lightningd, not gossipd.

It's weird to have connectd ask gossipd, when lightningd can just do it
and hand all the addresses together.

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

connectd/connectd.c

@@ -1669,37 +1669,19 @@ static void add_seed_addrs(struct wireaddr_internal **addrs,
 	}
 }
 
-static bool wireaddr_int_equals_wireaddr(struct wireaddr_internal *addr_a,
-					 struct wireaddr *addr_b)
+static bool wireaddr_int_equals_wireaddr(const struct wireaddr_internal *addr_a,
+					 const struct wireaddr *addr_b)
 {
 	if (!addr_a || !addr_b)
 		return false;
 	return wireaddr_eq(&addr_a->u.wireaddr, addr_b);
 }
 
-/*~ This asks gossipd for any addresses advertized by the node. */
+/*~ Orders the addresses which lightningd gave us. */
 static void add_gossip_addrs(struct wireaddr_internal **addrs,
-			     const struct node_id *id,
-			     struct wireaddr_internal *addrhint)
+			     const struct wireaddr *normal_addrs,
+			     const struct wireaddr_internal *addrhint)
 {
-	u8 *msg;
-	struct wireaddr *normal_addrs;
-
-	/* For simplicity, we do this synchronous. */
-	msg = towire_gossipd_get_addrs(NULL, id);
-	if (!wire_sync_write(GOSSIPCTL_FD, take(msg)))
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-			      "Failed writing to gossipctl: %s",
-			      strerror(errno));
-
-	/* This returns 'struct wireaddr's since that's what's supported by
-	 * the BOLT #7 protocol. */
-	msg = wire_sync_read(tmpctx, GOSSIPCTL_FD);
-	if (!fromwire_gossipd_get_addrs_reply(tmpctx, msg, &normal_addrs))
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-			      "Failed parsing get_addrs_reply gossipctl: %s",
-			      tal_hex(tmpctx, msg));
-
 	/* Wrap each one in a wireaddr_internal and add to addrs. */
 	for (size_t i = 0; i < tal_count(normal_addrs); i++) {
 		/* This is not supported, ignore. */
@@ -1736,6 +1718,7 @@ static void add_gossip_addrs(struct wireaddr_internal **addrs,
 static void try_connect_peer(struct daemon *daemon,
 			     const struct node_id *id,
 			     u32 seconds_waited,
+			     struct wireaddr *gossip_addrs,
 			     struct wireaddr_internal *addrhint STEALS)
 {
 	struct wireaddr_internal *addrs;
@@ -1767,7 +1750,7 @@ static void try_connect_peer(struct daemon *daemon,
 	if (addrhint)
 		tal_arr_expand(&addrs, *addrhint);
 
-	add_gossip_addrs(&addrs, id, addrhint);
+	add_gossip_addrs(&addrs, gossip_addrs, addrhint);
 
 	if (tal_count(addrs) == 0) {
 		/* Don't resolve via DNS seed if we're supposed to use proxy. */
@@ -1824,13 +1807,14 @@ static void connect_to_peer(struct daemon *daemon, const u8 *msg)
 	struct node_id id;
 	u32 seconds_waited;
 	struct wireaddr_internal *addrhint;
+	struct wireaddr *addrs;
 
 	if (!fromwire_connectd_connect_to_peer(tmpctx, msg,
-						 &id, &seconds_waited,
-						 &addrhint))
+					       &id, &seconds_waited,
+					       &addrs, &addrhint))
 		master_badmsg(WIRE_CONNECTD_CONNECT_TO_PEER, msg);
 
-	try_connect_peer(daemon, &id, seconds_waited, addrhint);
+	try_connect_peer(daemon, &id, seconds_waited, addrs, addrhint);
 }
 
 /* A peer is gone: clean things up. */

connectd/connectd_gossipd_wire.csv

@@ -11,11 +11,3 @@ msgdata,gossipd_new_peer,gossip_queries_feature,bool,
 # if success: + gossip fd
 msgtype,gossipd_new_peer_reply,4100
 msgdata,gossipd_new_peer_reply,success,bool,
-
-# Connectd asks gossipd for any known addresses for that node.
-msgtype,gossipd_get_addrs,4001
-msgdata,gossipd_get_addrs,id,node_id,
-
-msgtype,gossipd_get_addrs_reply,4101
-msgdata,gossipd_get_addrs_reply,num,u16,
-msgdata,gossipd_get_addrs_reply,addrs,wireaddr,num

connectd/connectd_wire.csv

@@ -47,6 +47,8 @@ msgdata,connectd_reconnected,id,node_id,
 msgtype,connectd_connect_to_peer,2001
 msgdata,connectd_connect_to_peer,id,node_id,
 msgdata,connectd_connect_to_peer,seconds_waited,u32,
+msgdata,connectd_connect_to_peer,len,u32,
+msgdata,connectd_connect_to_peer,addrs,wireaddr,len
 msgdata,connectd_connect_to_peer,addrhint,?wireaddr_internal,
 
 # Connectd->master: connect failed.

gossipd/gossipd.c

@@ -891,10 +891,10 @@ static struct io_plan *connectd_new_peer(struct io_conn *conn,
 	return daemon_conn_read_next(conn, daemon->connectd);
 }
 
-/*~ connectd can also ask us if we know any addresses for a given id. */
-static struct io_plan *connectd_get_address(struct io_conn *conn,
-					    struct daemon *daemon,
-					    const u8 *msg)
+/*~ lightningd asks us if we know any addresses for a given id. */
+static struct io_plan *handle_get_address(struct io_conn *conn,
+					  struct daemon *daemon,
+					  const u8 *msg)
 {
 	struct node_id id;
 	u8 rgb_color[3];
@@ -903,20 +903,17 @@ static struct io_plan *connectd_get_address(struct io_conn *conn,
 	struct wireaddr *addrs;
 	struct lease_rates *rates;
 
-	if (!fromwire_gossipd_get_addrs(msg, &id)) {
-		status_broken("Bad gossipd_get_addrs msg from connectd: %s",
-			      tal_hex(tmpctx, msg));
-		return io_close(conn);
-	}
+	if (!fromwire_gossipd_get_addrs(msg, &id))
+		master_badmsg(WIRE_GOSSIPD_GET_ADDRS, msg);
 
 	if (!get_node_announcement_by_id(tmpctx, daemon, &id,
 					 rgb_color, alias, &features, &addrs,
 					 &rates))
 		addrs = NULL;
 
-	daemon_conn_send(daemon->connectd,
+	daemon_conn_send(daemon->master,
 			 take(towire_gossipd_get_addrs_reply(NULL, addrs)));
-	return daemon_conn_read_next(conn, daemon->connectd);
+	return daemon_conn_read_next(conn, daemon->master);
 }
 
 /*~ connectd's input handler is very simple. */
@@ -930,12 +927,8 @@ static struct io_plan *connectd_req(struct io_conn *conn,
 	case WIRE_GOSSIPD_NEW_PEER:
 		return connectd_new_peer(conn, daemon, msg);
 
-	case WIRE_GOSSIPD_GET_ADDRS:
-		return connectd_get_address(conn, daemon, msg);
-
 	/* We send these, don't receive them. */
 	case WIRE_GOSSIPD_NEW_PEER_REPLY:
-	case WIRE_GOSSIPD_GET_ADDRS_REPLY:
 		break;
 	}
 
@@ -1417,6 +1410,9 @@ static struct io_plan *recv_req(struct io_conn *conn,
 		handle_new_lease_rates(daemon, msg);
 		goto done;
 
+	case WIRE_GOSSIPD_GET_ADDRS:
+		return handle_get_address(conn, daemon, msg);
+
 #if DEVELOPER
 	case WIRE_GOSSIPD_DEV_SET_MAX_SCIDS_ENCODE_SIZE:
 		dev_set_max_scids_encode_size(daemon, msg);
@@ -1455,6 +1451,7 @@ static struct io_plan *recv_req(struct io_conn *conn,
 	case WIRE_GOSSIPD_GOT_ONIONMSG_TO_US:
 	case WIRE_GOSSIPD_ADDGOSSIP_REPLY:
 	case WIRE_GOSSIPD_NEW_BLOCKHEIGHT_REPLY:
+	case WIRE_GOSSIPD_GET_ADDRS_REPLY:
 		break;
 	}
 

gossipd/gossipd_wire.csv

@@ -108,3 +108,11 @@ msgdata,gossipd_addgossip_reply,err,wirestring,
 # Updated lease rates available
 msgtype,gossipd_new_lease_rates,3046
 msgdata,gossipd_new_lease_rates,rates,lease_rates,
+
+# Lightningd asks gossipd for any known addresses for that node.
+msgtype,gossipd_get_addrs,3050
+msgdata,gossipd_get_addrs,id,node_id,
+
+msgtype,gossipd_get_addrs_reply,3150
+msgdata,gossipd_get_addrs_reply,num,u16,
+msgdata,gossipd_get_addrs_reply,addrs,wireaddr,num

lightningd/channel.c

@@ -874,10 +874,10 @@ static void err_and_reconnect(struct channel *channel,
 	channel_set_owner(channel, NULL);
 
 	/* Their address only useful if we connected to them */
-	delay_then_reconnect(channel, seconds_before_reconnect,
-			     channel->peer->connected_incoming
-			     ? NULL
-			     : &channel->peer->addr);
+	try_reconnect(channel, seconds_before_reconnect,
+		      channel->peer->connected_incoming
+		      ? NULL
+		      : &channel->peer->addr);
 }
 
 void channel_fail_reconnect_later(struct channel *channel, const char *fmt, ...)

lightningd/connect_control.c

@@ -10,6 +10,7 @@
 #include <common/timeout.h>
 #include <common/type_to_string.h>
 #include <connectd/connectd_wiregen.h>
+#include <gossipd/gossipd_wiregen.h>
 #include <hsmd/capabilities.h>
 #include <lightningd/channel.h>
 #include <lightningd/connect_control.h>
@@ -69,6 +70,14 @@ static struct command_result *connect_cmd_succeed(struct command *cmd,
 	return command_success(cmd, response);
 }
 
+/* FIXME: Reorder! */
+static void try_connect(const tal_t *ctx,
+			struct lightningd *ld,
+			const struct node_id *id,
+			struct channel *channel,
+			u32 seconds_delay,
+			const struct wireaddr_internal *addrhint);
+
 static struct command_result *json_connect(struct command *cmd,
 					   const char *buffer,
 					   const jsmntok_t *obj UNNEEDED,
@@ -82,7 +91,6 @@ static struct command_result *json_connect(struct command *cmd,
 	char *ataddr = NULL;
 	const char *name;
 	struct wireaddr_internal *addr;
-	u8 *msg;
 	const char *err_msg;
 	struct peer *peer;
 
@@ -165,8 +173,7 @@ static struct command_result *json_connect(struct command *cmd,
 	} else
 		addr = NULL;
 
-	msg = towire_connectd_connect_to_peer(NULL, &id, 0, addr);
-	subd_send_msg(cmd->ld->connectd, take(msg));
+	try_connect(cmd, cmd->ld, &id, NULL, 0, addr);
 
 	/* Leave this here for peer_connected or connect_failed. */
 	new_connect(cmd->ld, &id, cmd);
@@ -182,40 +189,80 @@ static const struct json_command connect_command = {
 };
 AUTODATA(json_command, &connect_command);
 
+/* We actually use this even if we don't need a delay, while we talk to
+ * gossipd to get the addresses. */
 struct delayed_reconnect {
+	struct lightningd *ld;
+	struct node_id id;
+	/* May be unset if there's no associated channel */
 	struct channel *channel;
 	u32 seconds_delayed;
 	struct wireaddr_internal *addrhint;
 };
 
-static void maybe_reconnect(struct delayed_reconnect *d)
+static void gossipd_got_addrs(struct subd *subd,
+			      const u8 *msg,
+			      const int *fds,
+			      struct delayed_reconnect *d)
 {
-	struct peer *peer = d->channel->peer;
-
-	/* Might have gone onchain since we started timer. */
-	if (channel_active(d->channel)) {
-		u8 *msg = towire_connectd_connect_to_peer(NULL, &peer->id,
-							    d->seconds_delayed,
-							    d->addrhint);
-		subd_send_msg(peer->ld->connectd, take(msg));
+	struct wireaddr *addrs;
+	u8 *connectmsg;
+
+	if (!fromwire_gossipd_get_addrs_reply(tmpctx, msg, &addrs))
+		fatal("Gossipd gave bad GOSSIPD_GET_ADDRS_REPLY %s",
+		      tal_hex(msg, msg));
+
+	/* Might have gone onchain (if it was actually freed, we were too). */
+	if (d->channel && !channel_active(d->channel)) {
+		tal_free(d);
+		return;
 	}
+
+	connectmsg = towire_connectd_connect_to_peer(NULL,
+						     &d->id,
+						     d->seconds_delayed,
+						     addrs,
+						     d->addrhint);
+	subd_send_msg(d->ld->connectd, take(connectmsg));
 	tal_free(d);
 }
 
-void delay_then_reconnect(struct channel *channel, u32 seconds_delay,
-			  const struct wireaddr_internal *addrhint)
+/* We might be off a delay timer.  Now ask gossipd about public addresses. */
+static void do_connect(struct delayed_reconnect *d)
 {
-	struct delayed_reconnect *d;
-	struct lightningd *ld = channel->peer->ld;
+	u8 *msg = towire_gossipd_get_addrs(NULL, &d->id);
 
-	if (!ld->reconnect)
-		return;
+	subd_req(d, d->ld->gossip, take(msg), -1, 0, gossipd_got_addrs, d);
+}
 
-	d = tal(channel, struct delayed_reconnect);
+/* channel may be NULL here */
+static void try_connect(const tal_t *ctx,
+			struct lightningd *ld,
+			const struct node_id *id,
+			struct channel *channel,
+			u32 seconds_delay,
+			const struct wireaddr_internal *addrhint)
+{
+	struct delayed_reconnect *d;
+
+	d = tal(ctx, struct delayed_reconnect);
+	d->ld = ld;
+	d->id = *id;
 	d->channel = channel;
 	d->seconds_delayed = seconds_delay;
 	d->addrhint = tal_dup_or_null(d, struct wireaddr_internal, addrhint);
 
+	if (!seconds_delay) {
+		do_connect(d);
+		return;
+	}
+
+	/* We never have a delay when connecting without a channel */
+	assert(channel);
+	channel_set_billboard(channel, false,
+			      tal_fmt(tmpctx,
+				      "Will attempt reconnect "
+				      "in %u seconds", seconds_delay));
 	log_debug(channel->log, "Will try reconnect in %u seconds",
 		  seconds_delay);
 
@@ -224,7 +271,22 @@ void delay_then_reconnect(struct channel *channel, u32 seconds_delay,
 	notleak(new_reltimer(ld->timers, d,
 			     timerel_add(time_from_sec(seconds_delay),
 					 time_from_usec(pseudorand(1000000))),
-			     maybe_reconnect, d));
+			     do_connect, d));
+}
+
+void try_reconnect(struct channel *channel,
+		   u32 seconds_delay,
+		   const struct wireaddr_internal *addrhint)
+{
+	if (!channel->peer->ld->reconnect)
+		return;
+
+	try_connect(channel,
+		    channel->peer->ld,
+		    &channel->peer->id,
+		    channel,
+		    seconds_delay,
+		    addrhint);
 }
 
 static void connect_failed(struct lightningd *ld, const u8 *msg)
@@ -251,7 +313,7 @@ static void connect_failed(struct lightningd *ld, const u8 *msg)
 	/* If we have an active channel, then reconnect. */
 	channel = active_channel_by_id(ld, &id, NULL);
 	if (channel)
-		delay_then_reconnect(channel, seconds_to_delay, addrhint);
+		try_reconnect(channel, seconds_to_delay, addrhint);
 }
 
 void connect_succeeded(struct lightningd *ld, const struct peer *peer,

lightningd/connect_control.h

@@ -10,8 +10,8 @@ struct wireaddr_internal;
 int connectd_init(struct lightningd *ld);
 void connectd_activate(struct lightningd *ld);
 
-void delay_then_reconnect(struct channel *channel, u32 seconds_delay,
-			  const struct wireaddr_internal *addrhint TAKES);
+void try_reconnect(struct channel *channel, u32 seconds_delay,
+		   const struct wireaddr_internal *addrhint TAKES);
 void connect_succeeded(struct lightningd *ld, const struct peer *peer,
 		       bool incoming,
 		       const struct wireaddr_internal *addr);

lightningd/gossip_control.c

@@ -127,13 +127,15 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 	case WIRE_GOSSIPD_NEW_BLOCKHEIGHT:
 	case WIRE_GOSSIPD_SEND_ONIONMSG:
 	case WIRE_GOSSIPD_ADDGOSSIP:
+	case WIRE_GOSSIPD_GET_ADDRS:
 	/* This is a reply, so never gets through to here. */
 	case WIRE_GOSSIPD_INIT_REPLY:
 	case WIRE_GOSSIPD_DEV_MEMLEAK_REPLY:
 	case WIRE_GOSSIPD_DEV_COMPACT_STORE_REPLY:
 	case WIRE_GOSSIPD_GET_STRIPPED_CUPDATE_REPLY:
 	case WIRE_GOSSIPD_ADDGOSSIP_REPLY:
 	case WIRE_GOSSIPD_NEW_BLOCKHEIGHT_REPLY:
+	case WIRE_GOSSIPD_GET_ADDRS_REPLY:
 		break;
 
 	case WIRE_GOSSIPD_GOT_ONIONMSG_TO_US: