lightningd/gossip: don't hand client fd until release.

The gossip subdaemon previously passed the fd after init: this is
unnecessary for peers which simply want to gossip (and not establish
channels).

Now we hand the gossip fd back with the peer fd.  This adds another
error message for when we fail to create the gossip fds.

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

lightningd/gossip/gossip.c

@@ -106,6 +106,47 @@ static struct peer *setup_new_peer(struct daemon *daemon, const u8 *msg)
 	return peer;
 }
 
+static struct io_plan *owner_msg_in(struct io_conn *conn,
+				    struct daemon_conn *dc);
+static struct io_plan *nonlocal_dump_gossip(struct io_conn *conn,
+					    struct daemon_conn *dc);
+
+/* When a peer is to be owned by another daemon, we create a socket
+ * pair to send/receive gossip from it */
+static void send_peer_with_fds(struct peer *peer, const u8 *msg)
+{
+	int fds[2];
+	u8 *out;
+
+	if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
+		out = towire_gossipstatus_peer_failed(msg,
+				peer->unique_id,
+				(u8 *)tal_fmt(msg,
+					      "Failed to create socketpair: %s",
+					      strerror(errno)));
+		daemon_conn_send(&peer->daemon->master, take(out));
+
+		/* FIXME: Send error to peer? */
+		/* Peer will be freed when caller closes conn. */
+		return;
+	}
+
+	/* Now we talk to socket to get to peer's owner daemon. */
+	peer->local = false;
+	daemon_conn_init(peer, &peer->owner_conn, fds[0], owner_msg_in);
+	peer->owner_conn.msg_queue_cleared_cb = nonlocal_dump_gossip;
+
+	/* Peer stays around, even though we're going to free conn. */
+	tal_steal(peer->daemon, peer);
+
+	daemon_conn_send(&peer->daemon->master, msg);
+	daemon_conn_send_fd(&peer->daemon->master, peer->fd);
+	daemon_conn_send_fd(&peer->daemon->master, fds[1]);
+
+	/* Don't get confused: we can't use this any more. */
+	peer->fd = -1;
+}
+
 static void handle_gossip_msg(struct routing_state *rstate, u8 *msg)
 {
 	int t = fromwire_peektype(msg);
@@ -164,9 +205,7 @@ static struct io_plan *peer_msgin(struct io_conn *conn,
 		/* Not our place to handle this, so we punt */
 		s = towire_gossipstatus_peer_nongossip(msg, peer->unique_id,
 						       &peer->pcs.cs, msg);
-		peer->local = false;
+		send_peer_with_fds(peer, take(s));
-		daemon_conn_send(&peer->daemon->master, take(s));
-		daemon_conn_send_fd(&peer->daemon->master, io_conn_fd(conn));
 		return io_close_taken_fd(conn);
 	}
 
@@ -304,22 +343,10 @@ static struct io_plan *nonlocal_dump_gossip(struct io_conn *conn, struct daemon_
 	}
 }
 
-static int peer_create_owner_conn(struct peer *peer)
-{
-	int fds[2];
-	if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-		return -1;
-	}
-	daemon_conn_init(peer, &peer->owner_conn, fds[0], owner_msg_in);
-	peer->owner_conn.msg_queue_cleared_cb = nonlocal_dump_gossip;
-	return fds[1];
-}
-
 static struct io_plan *peer_parse_init(struct io_conn *conn,
 				       struct peer *peer, u8 *msg)
 {
 	u8 *gfeatures, *lfeatures;
-	int client_fd;
 
 	if (!fromwire_init(msg, msg, NULL, &gfeatures, &lfeatures)) {
 		peer->error = tal_fmt(msg, "Bad init: %s", tal_hex(msg, msg));
@@ -344,13 +371,6 @@ static struct io_plan *peer_parse_init(struct io_conn *conn,
 		return io_close(conn);
 	}
 
-	client_fd = peer_create_owner_conn(peer);
-
-	if (client_fd == -1) {
-		peer->error = tal_fmt(msg, "Internal error");
-		return io_close(conn);
-	}
-
 	/* BOLT #1:
 	 *
 	 * Each node MUST wait to receive `init` before sending any other
@@ -359,7 +379,6 @@ static struct io_plan *peer_parse_init(struct io_conn *conn,
 	daemon_conn_send(&peer->daemon->master,
 			 take(towire_gossipstatus_peer_ready(msg,
 							     peer->unique_id)));
-	daemon_conn_send_fd(&peer->daemon->master, client_fd);
 
 	/* Need to go duplex here, otherwise backpressure would mean
 	 * we both wait indefinitely */
@@ -420,20 +439,11 @@ static struct io_plan *release_peer(struct io_conn *conn, struct daemon *daemon,
 
 	list_for_each(&daemon->peers, peer, list) {
 		if (peer->unique_id == unique_id) {
-			u8 *out;
+			send_peer_with_fds(peer,
-
+			   take(towire_gossipctl_release_peer_reply(msg,
-			/* Don't talk to this peer any more. */
-			peer->fd = io_conn_fd(peer->conn);
-			tal_steal(daemon, peer);
-			io_close_taken_fd(peer->conn);
-
-			out = towire_gossipctl_release_peer_reply(msg,
 								unique_id,
-								  &peer->pcs.cs);
+								&peer->pcs.cs)));
-			peer->local = false;
+			io_close_taken_fd(peer->conn);
-			daemon_conn_send(&daemon->master, take(out));
-			daemon_conn_send_fd(&daemon->master, peer->fd);
-			peer->fd = -1;
 			return daemon_conn_read_next(conn, &daemon->master);
 		}
 	}
@@ -491,6 +501,7 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master
 	case WIRE_GOSSIPSTATUS_BAD_REQUEST:
 	case WIRE_GOSSIPSTATUS_FDPASS_FAILED:
 	case WIRE_GOSSIPSTATUS_PEER_BAD_MSG:
+	case WIRE_GOSSIPSTATUS_PEER_FAILED:
 	case WIRE_GOSSIPSTATUS_PEER_READY:
 	case WIRE_GOSSIPSTATUS_PEER_NONGOSSIP:
 		break;

lightningd/gossip/gossip_wire.csv

@@ -11,6 +11,12 @@ gossipstatus_peer_bad_msg,0,unique_id,8
 gossipstatus_peer_bad_msg,8,len,2
 gossipstatus_peer_bad_msg,10,err,len*u8
 
+# Misc problems like opening control fd.
+gossipstatus_peer_failed,1001
+gossipstatus_peer_failed,0,unique_id,8
+gossipstatus_peer_failed,8,len,2
+gossipstatus_peer_failed,10,err,len*u8
+
 #include <lightningd/cryptomsg.h>
 
 # These take an fd, but have no response
@@ -23,7 +29,7 @@ gossipctl_new_peer,8,crypto_state,struct crypto_state
 gossipctl_release_peer,2
 gossipctl_release_peer,0,unique_id,8
 
-# This releases the peer and returns the cryptostate (followed by fd)
+# This releases the peer and returns the cryptostate (followed two fds: peer and gossip)
 gossipctl_release_peer_reply,102
 gossipctl_release_peer_reply,0,unique_id,8
 gossipctl_release_peer_reply,8,crypto_state,struct crypto_state
@@ -40,7 +46,7 @@ gossipctl_release_peer_reply,8,crypto_state,struct crypto_state
 gossipstatus_peer_ready,3
 gossipstatus_peer_ready,0,unique_id,8
 
-# Peer can send non-gossip packet (usually an open_channel) (followed by fd)
+# Peer can send non-gossip packet (usually an open_channel) (followed two fds: peer and gossip)
 gossipstatus_peer_nongossip,4
 gossipstatus_peer_nongossip,0,unique_id,8
 gossipstatus_peer_nongossip,10,crypto_state,struct crypto_state

lightningd/gossip_control.c

@@ -42,7 +42,30 @@ static void peer_bad_message(struct subd *gossip, const u8 *msg)
 	tal_free(peer);
 }
 
-static void peer_nongossip(struct subd *gossip, const u8 *msg, int fd)
+static void peer_failed(struct subd *gossip, const u8 *msg)
+{
+	u64 unique_id;
+	struct peer *peer;
+	u8 *err;
+
+	if (!fromwire_gossipstatus_peer_failed(msg, msg, NULL,
+					       &unique_id, &err))
+		fatal("Gossip gave bad PEER_FAILED message %s",
+		      tal_hex(msg, msg));
+
+	peer = peer_by_unique_id(gossip->ld, unique_id);
+	if (!peer)
+		fatal("Gossip gave bad peerid %"PRIu64, unique_id);
+
+	log_unusual(gossip->log, "Peer %s failed: %.*s",
+		    type_to_string(msg, struct pubkey, peer->id),
+		    (int)tal_len(err), (const char *)err);
+	peer_set_condition(peer, "Error during gossip phase");
+	tal_free(peer);
+}
+
+static void peer_nongossip(struct subd *gossip, const u8 *msg,
+			   int peer_fd, int gossip_fd)
 {
 	u64 unique_id;
 	struct peer *peer;
@@ -64,7 +87,8 @@ static void peer_nongossip(struct subd *gossip, const u8 *msg, int fd)
 
 	/* It returned the fd. */
 	assert(peer->fd == -1);
-	peer->fd = fd;
+	peer->fd = peer_fd;
+	peer->gossip_client_fd = gossip_fd;
 
 	peer_set_condition(peer, "Gossip ended up receipt of %s",
 			   wire_type_name(fromwire_peektype(inner)));
@@ -72,7 +96,7 @@ static void peer_nongossip(struct subd *gossip, const u8 *msg, int fd)
 	peer_accept_open(peer, &cs, inner);
 }
 
-static void peer_ready(struct subd *gossip, const u8 *msg, int fd)
+static void peer_ready(struct subd *gossip, const u8 *msg)
 {
 	u64 unique_id;
 	struct peer *peer;
@@ -85,8 +109,9 @@ static void peer_ready(struct subd *gossip, const u8 *msg, int fd)
 	if (!peer)
 		fatal("Gossip gave bad peerid %"PRIu64, unique_id);
 
-	log_debug_struct(gossip->log, "Peer %s ready for channel open",
+	log_debug(gossip->log, "Peer %s (%"PRIu64") ready for channel open",
-			 struct pubkey, peer->id);
+		  type_to_string(msg, struct pubkey, peer->id),
+		  unique_id);
 
 	if (peer->connect_cmd) {
 		struct json_result *response;
@@ -99,8 +124,6 @@ static void peer_ready(struct subd *gossip, const u8 *msg, int fd)
 		peer->connect_cmd = NULL;
 	}
 
-	peer->gossip_client_fd = fd;
-
 	peer_set_condition(peer, "Exchanging gossip");
 }
 
@@ -126,15 +149,16 @@ static size_t gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 	case WIRE_GOSSIPSTATUS_PEER_BAD_MSG:
 		peer_bad_message(gossip, msg);
 		break;
+	case WIRE_GOSSIPSTATUS_PEER_FAILED:
+		peer_failed(gossip, msg);
+		break;
 	case WIRE_GOSSIPSTATUS_PEER_NONGOSSIP:
-		if (tal_count(fds) != 1)
+		if (tal_count(fds) != 2)
-			return 1;
+			return 2;
-		peer_nongossip(gossip, msg, fds[0]);
+		peer_nongossip(gossip, msg, fds[0], fds[1]);
 		break;
 	case WIRE_GOSSIPSTATUS_PEER_READY:
-		if (tal_count(fds) != 1)
+		peer_ready(gossip, msg);
-			return 1;
-		peer_ready(gossip, msg, fds[0]);
 		break;
 	}
 	return 0;

lightningd/peer_control.c

@@ -961,8 +961,9 @@ static bool gossip_peer_released(struct subd *gossip,
 	u8 *msg;
 	struct subd *opening;
 
-	assert(tal_count(fds) == 1);
+	assert(tal_count(fds) == 2);
 	fc->peer->fd = fds[0];
+	fc->peer->gossip_client_fd = fds[1];
 
 	fc->cs = tal(fc, struct crypto_state);
 	if (!fromwire_gossipctl_release_peer_reply(resp, NULL, &id, fc->cs))
@@ -1061,7 +1062,7 @@ static void json_fund_channel(struct command *cmd,
 	/* Tie this fc lifetime (and hence utxo release) to the peer */
 	tal_steal(fc->peer, fc);
 	tal_add_destructor(fc, fail_fundchannel_command);
-	subd_req(ld->gossip, msg, -1, 1, gossip_peer_released, fc);
+	subd_req(ld->gossip, msg, -1, 2, gossip_peer_released, fc);
 }
 
 static const struct json_command fund_channel_command = {