lightningd/subd: explicit failure reply support.

We had a terrible hack in gossip when a peer didn't exist.  Formalize
a pattern when code+200 is a failure (with no fds passed), and use it
here.

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

lightningd/gossip/gossip.c

@@ -422,21 +422,11 @@ static struct io_plan *release_peer(struct io_conn *conn, struct daemon *daemon,
 	if (!peer) {
 		/* This can happen with a reconnect vs connect race.
 		 * See gossip_peer_released in master daemon. */
-		struct crypto_state dummy;
-
-		status_trace("release_peer: Unknown peer %"PRIu64, unique_id);
-		memset(&dummy, 0, sizeof(dummy));
 		daemon_conn_send(&daemon->master,
-				 take(towire_gossipctl_release_peer_reply(msg,
+				 take(towire_gossipctl_release_peer_replyfail(msg)));
-									  ~unique_id,
-									  &dummy)));
-		/* Needs two fds, send dummies. */
-		daemon_conn_send_fd(&daemon->master, dup(STDOUT_FILENO));
-		daemon_conn_send_fd(&daemon->master, dup(STDOUT_FILENO));
 	} else {
 		send_peer_with_fds(peer,
 				   take(towire_gossipctl_release_peer_reply(msg,
-								unique_id,
 								&peer->pcs.cs)));
 		io_close_taken_fd(peer->conn);
 	}
@@ -679,6 +669,7 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master
 		handle_forwarded_msg(conn, daemon, daemon->master.msg_in);
 		return daemon_conn_read_next(conn, &daemon->master);
 	case WIRE_GOSSIPCTL_RELEASE_PEER_REPLY:
+	case WIRE_GOSSIPCTL_RELEASE_PEER_REPLYFAIL:
 	case WIRE_GOSSIP_GETNODES_REPLY:
 	case WIRE_GOSSIP_GETROUTE_REPLY:
 	case WIRE_GOSSIP_GETCHANNELS_REPLY:

lightningd/gossip/gossip_wire.csv

@@ -36,9 +36,11 @@ gossipctl_release_peer,,unique_id,8
 
 # This releases the peer and returns the cryptostate (followed two fds: peer and gossip)
 gossipctl_release_peer_reply,102
-gossipctl_release_peer_reply,,unique_id,8
 gossipctl_release_peer_reply,,crypto_state,struct crypto_state
 
+# This is if we couldn't find the peer.
+gossipctl_release_peer_replyfail,202
+
 # This is where we save a peer's features.
 #gossipstatus_peer_features,1
 #gossipstatus_peer_features,,unique_id,8

lightningd/gossip_control.c

@@ -118,6 +118,7 @@ static int gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 	case WIRE_GOSSIP_FORWARDED_MSG:
 	/* This is a reply, so never gets through to here. */
 	case WIRE_GOSSIPCTL_RELEASE_PEER_REPLY:
+	case WIRE_GOSSIPCTL_RELEASE_PEER_REPLYFAIL:
 	case WIRE_GOSSIP_GETNODES_REPLY:
 	case WIRE_GOSSIP_GETROUTE_REPLY:
 	case WIRE_GOSSIP_GETCHANNELS_REPLY:

lightningd/peer_control.c

@@ -1391,31 +1391,26 @@ static bool gossip_peer_released(struct subd *gossip,
 	struct lightningd *ld = fc->peer->ld;
 	u32 max_to_self_delay, max_minimum_depth;
 	u64 min_effective_htlc_capacity_msat;
-	u64 id;
 	u8 *msg;
 	struct subd *opening;
 	struct utxo *utxos;
 	u8 *bip32_base;
 	struct crypto_state cs;
 
+	if (!fromwire_gossipctl_release_peer_reply(resp, NULL, &cs)) {
+		if (!fromwire_gossipctl_release_peer_replyfail(resp, NULL)) {
+			fatal("Gossup daemon gave invalid reply %s",
+			      tal_hex(gossip, resp));
+		}
+		tal_del_destructor(fc, fail_fundchannel_command);
+		command_fail(fc->cmd, "Peer reconnected, try again");
+		return true;
+	}
+
 	assert(tal_count(fds) == 2);
 	fc->peer->fd = fds[0];
 	fc->peer->gossip_client_fd = fds[1];
 
-	if (!fromwire_gossipctl_release_peer_reply(resp, NULL, &id, &cs))
-		fatal("Gossup daemon gave invalid reply %s",
-		      tal_hex(gossip, resp));
-
-	/* This is how gossipd handles a reconnect (gossipctl_fail_peer) racing
-	 * with us trying to connect. */
-	if (id != fc->peer->unique_id) {
-		tal_del_destructor(fc, fail_fundchannel_command);
-		command_fail(fc->cmd, "Peer reconnected, try again");
-		close(fds[0]);
-		close(fds[1]);
-		return true;
-	}
-
 	peer_set_condition(fc->peer, GOSSIPD, OPENINGD);
 	opening = new_subd(fc->peer->ld, ld,
 			   "lightningd_opening", fc->peer,

lightningd/subd.c

@@ -44,7 +44,7 @@ struct subd_req {
 	struct list_node list;
 
 	/* Callback for a reply. */
-	int reply_type;
+	int type;
 	bool (*replycb)(struct subd *, const u8 *, const int *, void *);
 	void *replycb_data;
 
@@ -87,7 +87,7 @@ static void add_req(const tal_t *ctx,
 {
 	struct subd_req *sr = tal(sd, struct subd_req);
 
-	sr->reply_type = type + SUBD_REPLY_OFFSET;
+	sr->type = type;
 	sr->replycb = replycb;
 	sr->replycb_data = replycb_data;
 	sr->num_reply_fds = num_fds_in;
@@ -100,7 +100,7 @@ static void add_req(const tal_t *ctx,
 		tal_add_destructor2(sr->disabler, disable_cb, sr);
 	} else
 		sr->disabler = NULL;
-	assert(strends(sd->msgname(sr->reply_type), "_REPLY"));
+	assert(strends(sd->msgname(sr->type + SUBD_REPLY_OFFSET), "_REPLY"));
 
 	/* Keep in FIFO order: we sent in order, so replies will be too. */
 	list_add_tail(&sd->reqs, &sr->list);
@@ -113,8 +113,14 @@ static struct subd_req *get_req(struct subd *sd, int reply_type)
 	struct subd_req *sr;
 
 	list_for_each(&sd->reqs, sr, list) {
-		if (sr->reply_type == reply_type)
+		if (sr->type + SUBD_REPLY_OFFSET == reply_type)
 			return sr;
+		/* If it's a fail, and that's a valid type. */
+		if (sr->type + SUBD_REPLYFAIL_OFFSET == reply_type
+		    && strends(sd->msgname(reply_type), "_REPLYFAIL")) {
+			sr->num_reply_fds = 0;
+			return sr;
+		}
 	}
 	return NULL;
 }

lightningd/subd.h

@@ -11,6 +11,8 @@ struct io_conn;
 
 /* By convention, replies are requests + 100 */
 #define SUBD_REPLY_OFFSET 100
+/* And reply failures are requests + 200 */
+#define SUBD_REPLYFAIL_OFFSET 200
 
 /* One of our subds. */
 struct subd {
@@ -92,11 +94,13 @@ void subd_send_fd(struct subd *sd, int fd);
  * @sd: subdaemon to request
  * @msg_out: request message (can be take)
  * @fd_out: if >=0 fd to pass at the end of the message (closed after)
- * @num_fds_in: how many fds to read in to hand to @replycb.
+ * @num_fds_in: how many fds to read in to hand to @replycb if it's a reply.
  * @replycb: callback when reply comes in, returns false to shutdown daemon.
  * @replycb_data: final arg to hand to @replycb
  *
  * @replycb cannot free @sd, so it returns false to remove it.
+ * Note that @replycb is called for replies of type @msg_out + SUBD_REPLY_OFFSET
+ * with @num_fds_in fds, or type @msg_out + SUBD_REPLYFAIL_OFFSET with no fds.
  */
 #define subd_req(ctx, sd, msg_out, fd_out, num_fds_in, replycb, replycb_data) \
 	subd_req_((ctx), (sd), (msg_out), (fd_out), (num_fds_in),	\