gossipd: don't hang if we try to connect to already-connected peer.

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

gossipd/gossip.c

@@ -167,7 +167,7 @@ static struct io_plan *peer_start_gossip(struct io_conn *conn,
 					 struct peer *peer);
 static bool send_peer_with_fds(struct peer *peer, const u8 *msg);
 static void wake_pkt_out(struct peer *peer);
-static void try_reach_peer(struct daemon *daemon, const struct pubkey *id);
+static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id);
 
 static void destroy_peer(struct peer *peer)
 {
@@ -1474,16 +1474,17 @@ static void try_connect(struct reaching *reach)
 	io_new_conn(reach, fd, conn_init, reach);
 }
 
-static void try_reach_peer(struct daemon *daemon, const struct pubkey *id)
+/* Returns true if we're already connected. */
+static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id)
 {
 	struct reaching *reach;
 	struct peer *peer;
 
 	if (find_reaching(daemon, id)) {
 		/* FIXME: Perhaps kick timer in this case? */
-		status_trace("try_reach_peer: already reaching %s",
+		status_trace("try_reach_peer: already trying to reach %s",
 			     type_to_string(trc, struct pubkey, id));
-		return;
+		return false;
 	}
 
 	/* Master might find out before we do that a peer is dead; if we
@@ -1493,7 +1494,7 @@ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id)
 		status_trace("reach_peer: have %s, will retry if it dies",
 			     type_to_string(trc, struct pubkey, id));
 		peer->reach_again = true;
-		return;
+		return true;
 	}
 
 	reach = tal(daemon, struct reaching);
@@ -1504,6 +1505,7 @@ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id)
 	tal_add_destructor(reach, destroy_reaching);
 
 	try_connect(reach);
+	return false;
 }
 
 /* This catches all kinds of failures, like network errors. */
@@ -1515,7 +1517,12 @@ static struct io_plan *reach_peer(struct io_conn *conn,
 	if (!fromwire_gossipctl_reach_peer(msg, NULL, &id))
 		master_badmsg(WIRE_GOSSIPCTL_REACH_PEER, msg);
 
-	try_reach_peer(daemon, &id);
+	/* Master can't check this itself, because that's racy. */
+	if (try_reach_peer(daemon, &id)) {
+		daemon_conn_send(&daemon->master,
+				 take(towire_gossip_peer_already_connected(conn,
+									  &id)));
+	}
 
 	return daemon_conn_read_next(conn, &daemon->master);
 }
@@ -1632,6 +1639,7 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master
 	case WIRE_GOSSIP_PING_REPLY:
 	case WIRE_GOSSIP_RESOLVE_CHANNEL_REPLY:
 	case WIRE_GOSSIP_PEER_CONNECTED:
+	case WIRE_GOSSIP_PEER_ALREADY_CONNECTED:
 	case WIRE_GOSSIP_PEER_NONGOSSIP:
 	case WIRE_GOSSIP_GET_UPDATE:
 	case WIRE_GOSSIP_GET_UPDATE_REPLY:

gossipd/gossip_wire.csv

@@ -22,7 +22,8 @@ gossipctl_peer_addrhint,3014
 gossipctl_peer_addrhint,,id,struct pubkey
 gossipctl_peer_addrhint,,addr,struct wireaddr
 
-# Master -> gossipd: connect to a peer.  We may get a peer_connected.
+# Master -> gossipd: connect to a peer.  We may get a peer_connected or
+# peer_already_connected
 gossipctl_reach_peer,3001
 gossipctl_reach_peer,,id,struct pubkey
 
@@ -37,6 +38,10 @@ gossip_peer_connected,,gfeatures,gflen*u8
 gossip_peer_connected,,lflen,u16
 gossip_peer_connected,,lfeatures,lflen*u8
 
+# Gossipd -> master: you asked to reach a peer, we already had.
+gossip_peer_already_connected,3015
+gossip_peer_already_connected,,id,struct pubkey
+
 # Gossipd -> master: peer sent non-gossip packet. Two fds: peer and gossip
 gossip_peer_nongossip,3003
 gossip_peer_nongossip,,id,struct pubkey

lightningd/gossip_control.c

@@ -120,6 +120,9 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 			return 2;
 		peer_connected(gossip->ld, msg, fds[0], fds[1]);
 		break;
+	case WIRE_GOSSIP_PEER_ALREADY_CONNECTED:
+		peer_already_connected(gossip->ld, msg);
+		break;
 	case WIRE_GOSSIP_PEER_NONGOSSIP:
 		if (tal_count(fds) != 2)
 			return 2;

lightningd/peer_control.c

@@ -667,6 +667,19 @@ send_error:
 	subd_send_fd(ld->gossip, gossip_fd);
 }
 
+/* Gossipd tells us peer was already connected. */
+void peer_already_connected(struct lightningd *ld, const u8 *msg)
+{
+	struct pubkey id;
+
+	if (!fromwire_gossip_peer_already_connected(msg, NULL, &id))
+		fatal("Gossip gave bad GOSSIP_PEER_ALREADY_CONNECTED message %s",
+		      tal_hex(msg, msg));
+
+	/* If we were waiting for connection, we succeeded. */
+	connect_succeeded(ld, &id);
+}
+
 void peer_sent_nongossip(struct lightningd *ld,
 			 const struct pubkey *id,
 			 const struct wireaddr *addr,

lightningd/peer_control.h

@@ -173,6 +173,9 @@ void peer_last_tx(struct peer *peer, struct bitcoin_tx *tx,
 void peer_connected(struct lightningd *ld, const u8 *msg,
 		    int peer_fd, int gossip_fd);
 
+/* This simply means we asked to reach a peer, but we already have it */
+void peer_already_connected(struct lightningd *ld, const u8 *msg);
+
 void peer_sent_nongossip(struct lightningd *ld,
 			 const struct pubkey *id,
 			 const struct wireaddr *addr,