lightningd/gossip: Move INIT message handling to handshake daemon.

We need to do this on every connection, whether reconnecting or not,
so it makes sense for the handshake daemon to handle it and return
the feature fields.

Longer term I'm considering having the handshake daemon handle the
listening and connecting, and simply hand the fds back once the peers
are ready.

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

lightningd/gossip/gossip.c

@@ -229,10 +229,6 @@ static struct io_plan *peer_msgin(struct io_conn *conn,
 		handle_gossip_msg(peer->daemon->rstate, msg);
 		return peer_read_message(conn, &peer->pcs, peer_msgin);
 
-	case WIRE_INIT:
-		peer->error = "Duplicate INIT message received";
-		return io_close(conn);
-
 	case WIRE_PING:
 		if (!handle_ping(peer, msg))
 			return io_close(conn);
@@ -258,6 +254,7 @@ static struct io_plan *peer_msgin(struct io_conn *conn,
 	case WIRE_UPDATE_FAIL_MALFORMED_HTLC:
 	case WIRE_COMMITMENT_SIGNED:
 	case WIRE_REVOKE_AND_ACK:
+	case WIRE_INIT:
 		/* Not our place to handle this, so we punt */
 		s = towire_gossipstatus_peer_nongossip(msg, peer->unique_id,
 						       &peer->pcs.cs, msg);
@@ -322,19 +319,6 @@ static struct io_plan *peer_pkt_out(struct io_conn *conn, struct peer *peer)
 	return msg_queue_wait(conn, &peer->peer_out, peer_pkt_out, peer);
 }
 
-static bool has_even_bit(const u8 *bitmap)
-{
-	size_t len = tal_count(bitmap);
-
-	while (len) {
-		if (*bitmap & 0xAA)
-			return true;
-		len--;
-		bitmap++;
-	}
-	return false;
-}
-
 /**
  * owner_msg_in - Called by the `peer->owner_conn` upon receiving a
  * message
@@ -380,43 +364,8 @@ static struct io_plan *nonlocal_dump_gossip(struct io_conn *conn, struct daemon_
 	}
 }
 
-static struct io_plan *peer_parse_init(struct io_conn *conn,
-				       struct peer *peer, u8 *msg)
+static struct io_plan *peer_start_gossip(struct io_conn *conn, struct peer *peer)
 {
-	u8 *gfeatures, *lfeatures;
-
-	if (!fromwire_init(msg, msg, NULL, &gfeatures, &lfeatures)) {
-		peer->error = tal_fmt(msg, "Bad init: %s", tal_hex(msg, msg));
-		return io_close(conn);
-	}
-
-	/* BOLT #1:
-	 *
-	 * The receiving node MUST fail the channels if it receives a
-	 * `globalfeatures` or `localfeatures` with an even bit set which it
-	 * does not understand.
-	 */
-	if (has_even_bit(gfeatures)) {
-		peer->error = tal_fmt(msg, "Bad globalfeatures: %s",
-				      tal_hex(msg, gfeatures));
-		return io_close(conn);
-	}
-
-	if (has_even_bit(lfeatures)) {
-		peer->error = tal_fmt(msg, "Bad localfeatures: %s",
-				      tal_hex(msg, lfeatures));
-		return io_close(conn);
-	}
-
-	/* BOLT #1:
-	 *
-	 * Each node MUST wait to receive `init` before sending any other
-	 * messages.
-	 */
-	daemon_conn_send(&peer->daemon->master,
-			 take(towire_gossipstatus_peer_ready(msg,
-							     peer->unique_id)));
-
 	/* Need to go duplex here, otherwise backpressure would mean
 	 * we both wait indefinitely */
 	return io_duplex(conn,
@@ -424,29 +373,9 @@ static struct io_plan *peer_parse_init(struct io_conn *conn,
 			 peer_pkt_out(conn, peer));
 }
 
-static struct io_plan *peer_init_sent(struct io_conn *conn, struct peer *peer)
-{
-	return peer_read_message(conn, &peer->pcs, peer_parse_init);
-}
-
-static struct io_plan *peer_send_init(struct io_conn *conn, struct peer *peer)
-{
-	/* BOLT #1:
-	 *
-	 * The sending node SHOULD use the minimum lengths required to
-	 * represent the feature fields.  The sending node MUST set feature
-	 * bits corresponding to features it requires the peer to support, and
-	 * SHOULD set feature bits corresponding to features it optionally
-	 * supports.
-	 */
-	return peer_write_message(conn, &peer->pcs,
-				  take(towire_init(peer, NULL, NULL)),
-				  peer_init_sent);
-}
-
 static struct io_plan *new_peer_got_fd(struct io_conn *conn, struct peer *peer)
 {
-	peer->conn = io_new_conn(conn, peer->fd, peer_send_init, peer);
+	peer->conn = io_new_conn(conn, peer->fd, peer_start_gossip, peer);
 	if (!peer->conn) {
 		peer->error = "Could not create connection";
 		tal_free(peer);
@@ -716,7 +645,6 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master
 	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

@@ -46,10 +46,6 @@ gossipctl_release_peer_reply,8,crypto_state,struct crypto_state
 #gossipstatus_peer_features,10+gflen,lflen,2
 #gossipstatus_peer_features,12+gflen,localfeatures,lflen
 
-# Peer init handshake complete (now you can release_peer if you want)
-gossipstatus_peer_ready,3
-gossipstatus_peer_ready,0,unique_id,8
-
 # 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

lightningd/gossip_control.c

@@ -95,26 +95,6 @@ static void peer_nongossip(struct subd *gossip, const u8 *msg,
 	peer_accept_open(peer, &cs, inner);
 }
 
-static void peer_ready(struct subd *gossip, const u8 *msg)
-{
-	u64 unique_id;
-	struct peer *peer;
-
-	if (!fromwire_gossipstatus_peer_ready(msg, NULL, &unique_id))
-		fatal("Gossip gave bad PEER_READY 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_debug(gossip->log, "Peer %s (%"PRIu64") ready for channel open",
-		  type_to_string(msg, struct pubkey, peer->id),
-		  unique_id);
-
-	peer_set_condition(peer, GOSSIPING);
-}
-
 static int gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 {
 	enum gossip_wire_type t = fromwire_peektype(msg);
@@ -155,9 +135,6 @@ static int gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 			return 2;
 		peer_nongossip(gossip, msg, fds[0], fds[1]);
 		break;
-	case WIRE_GOSSIPSTATUS_PEER_READY:
-		peer_ready(gossip, msg);
-		break;
 	}
 	return 0;
 }

lightningd/handshake/handshake.c

@@ -9,6 +9,7 @@
 #include <ccan/read_write_all/read_write_all.h>
 #include <ccan/short_types/short_types.h>
 #include <errno.h>
+#include <lightningd/crypto_sync.h>
 #include <lightningd/debug.h>
 #include <lightningd/handshake/gen_handshake_wire.h>
 #include <lightningd/hsm/client.h>
@@ -20,6 +21,7 @@
 #include <stdio.h>
 #include <unistd.h>
 #include <version.h>
+#include <wire/peer_wire.h>
 #include <wire/wire.h>
 #include <wire/wire_sync.h>
 
@@ -956,6 +958,36 @@ static void responder(int fd,
 }
 
 #ifndef TESTING
+static void exchange_init(int fd, struct crypto_state *cs,
+			  u8 **gfeatures, u8 **lfeatures)
+{
+	/* BOLT #1:
+	 *
+	 * The sending node SHOULD use the minimum lengths required to
+	 * represent the feature fields.  The sending node MUST set feature
+	 * bits corresponding to features it requires the peer to support, and
+	 * SHOULD set feature bits corresponding to features it optionally
+	 * supports.
+	 */
+	u8 *msg = towire_init(NULL, NULL, NULL);
+
+	if (!sync_crypto_write(cs, fd, msg))
+		status_failed(WIRE_INITMSG_WRITE_FAILED, "%s", strerror(errno));
+
+	/* BOLT #1:
+	 *
+	 * Each node MUST wait to receive `init` before sending any other
+	 * messages.
+	 */
+	msg = sync_crypto_read(NULL, cs, fd);
+	if (!msg)
+		status_failed(WIRE_INITMSG_READ_FAILED, "%s", strerror(errno));
+
+	if (!fromwire_init(msg, msg, NULL, gfeatures, lfeatures))
+		status_failed(WIRE_INITMSG_READ_FAILED, "bad init: %s",
+			      tal_hex(msg, msg));
+}
+
 /* We expect hsmfd as fd 3, clientfd as 4 */
 int main(int argc, char *argv[])
 {
@@ -964,6 +996,7 @@ int main(int argc, char *argv[])
 	int hsmfd = 3, clientfd = 4;
 	struct secret ck, rk, sk;
 	struct crypto_state cs;
+	u8 *gfeatures, *lfeatures;
 
 	if (argc == 2 && streq(argv[1], "--version")) {
 		printf("%s\n", version());
@@ -983,14 +1016,18 @@ int main(int argc, char *argv[])
 
 	if (fromwire_handshake_responder(msg, NULL, &my_id)) {
 		responder(clientfd, &my_id, &their_id, &ck, &sk, &rk);
+
 		cs.rn = cs.sn = 0;
 		cs.sk = sk;
 		cs.rk = rk;
 		cs.r_ck = cs.s_ck = ck;
+		exchange_init(clientfd, &cs, &gfeatures, &lfeatures);
 		wire_sync_write(REQ_FD,
 				towire_handshake_responder_reply(msg,
 								 &their_id,
-								 &cs));
+								 &cs,
+								 gfeatures,
+								 lfeatures));
 	} else if (fromwire_handshake_initiator(msg, NULL, &my_id,
 						&their_id)) {
 		initiator(clientfd, &my_id, &their_id, &ck, &sk, &rk);
@@ -998,8 +1035,11 @@ int main(int argc, char *argv[])
 		cs.sk = sk;
 		cs.rk = rk;
 		cs.r_ck = cs.s_ck = ck;
+		exchange_init(clientfd, &cs, &gfeatures, &lfeatures);
 		wire_sync_write(REQ_FD,
-				towire_handshake_initiator_reply(msg, &cs));
+				towire_handshake_initiator_reply(msg, &cs,
+								 gfeatures,
+								 lfeatures));
 	} else
 		status_failed(WIRE_HANDSHAKE_BAD_COMMAND, "%i",
 			      fromwire_peektype(msg));

lightningd/handshake/handshake_wire.csv

@@ -22,6 +22,8 @@ respr_act3_bad_ciphertext,0x8023
 respr_act3_bad_pubkey,0x8024
 respr_act3_bad_ecdh_for_ss,0x8025
 respr_act3_bad_tag,0x8026
+initmsg_write_failed,0x8030
+initmsg_read_failed,0x8031
 
 # FIXME: This is probably too finegrained.
 initr_act_one,1001
@@ -37,6 +39,10 @@ handshake_responder,1,my_id,33
 handshake_responder_reply,101
 handshake_responder_reply,0,initiator_id,33
 handshake_responder_reply,33,cs,struct crypto_state
+handshake_responder_reply,0,gflen,2
+handshake_responder_reply,0,globalfeatures,gflen
+handshake_responder_reply,0,lflen,2
+handshake_responder_reply,0,localfeatures,lflen
 
 handshake_initiator,2
 handshake_initiator,0,my_id,33
@@ -44,4 +50,7 @@ handshake_initiator,33,responder_id,33
 
 handshake_initiator_reply,102
 handshake_initiator_reply,0,cs,struct crypto_state
-
+handshake_initiator_reply,0,gflen,2
+handshake_initiator_reply,0,globalfeatures,gflen
+handshake_initiator_reply,0,lflen,2
+handshake_initiator_reply,0,localfeatures,lflen

lightningd/new_connection.c

@@ -1,3 +1,4 @@
+#include <ccan/tal/str/str.h>
 #include <daemon/jsonrpc.h>
 #include <daemon/log.h>
 #include <errno.h>
@@ -37,6 +38,25 @@ static void connection_destroy(struct connection *c)
 		command_fail(c->cmd, "Failed to connect to peer");
 }
 
+static void
+PRINTF_FMT(3,4) connection_failed(struct connection *c, struct log *log,
+				  const char *fmt, ...)
+{
+	const char *msg;
+	va_list ap;
+
+	va_start(ap, fmt);
+	msg = tal_vfmt(c, fmt, ap);
+	va_end(ap);
+	log_info(log, "%s", msg);
+	if (c->cmd) {
+		command_fail(c->cmd, "%s", msg);
+		/* Don't fail in destructor, too. */
+		c->cmd = NULL;
+	}
+	tal_free(c);
+}
+
 struct connection *new_connection(const tal_t *ctx,
 				  struct lightningd *ld,
 				  struct command *cmd,
@@ -58,12 +78,26 @@ struct connection *new_connection(const tal_t *ctx,
 	return c;
 }
 
+static bool has_even_bit(const u8 *bitmap)
+{
+	size_t len = tal_count(bitmap);
+
+	while (len) {
+		if (*bitmap & 0xAA)
+			return true;
+		len--;
+		bitmap++;
+	}
+	return false;
+}
+
 static bool handshake_succeeded(struct subd *handshaked,
 				const u8 *msg, const int *fds,
 				struct connection *c)
 {
 	struct crypto_state cs;
 	struct pubkey *id;
+	u8 *globalfeatures, *localfeatures;
 
 	assert(tal_count(fds) == 1);
 
@@ -71,18 +105,44 @@ static bool handshake_succeeded(struct subd *handshaked,
 
 	if (!c->known_id) {
 		id = tal(msg, struct pubkey);
-		if (!fromwire_handshake_responder_reply(msg, NULL, id, &cs))
+		if (!fromwire_handshake_responder_reply(c, msg, NULL, id, &cs,
+							&globalfeatures,
+							&localfeatures))
 			goto err;
 		log_info_struct(handshaked->log, "Peer in from %s",
 				struct pubkey, id);
 	} else {
 		id = c->known_id;
-		if (!fromwire_handshake_initiator_reply(msg, NULL, &cs))
+		if (!fromwire_handshake_initiator_reply(c, msg, NULL, &cs,
+							&globalfeatures,
+							&localfeatures))
 			goto err;
 		log_info_struct(handshaked->log, "Peer out to %s",
 				struct pubkey, id);
 	}
 
+	/* BOLT #1:
+	 *
+	 * The receiving node MUST fail the channels if it receives a
+	 * `globalfeatures` or `localfeatures` with an even bit set which it
+	 * does not understand.
+	 */
+	if (has_even_bit(globalfeatures)) {
+		connection_failed(c, handshaked->log,
+				  "peer %s: bad globalfeatures: %s",
+				  type_to_string(c, struct pubkey, id),
+				  tal_hex(msg, globalfeatures));
+		return true;
+	}
+
+	if (has_even_bit(localfeatures)) {
+		connection_failed(c, handshaked->log,
+				  "peer %s: bad localfeatures: %s",
+				  type_to_string(c, struct pubkey, id),
+				  tal_hex(msg, localfeatures));
+		return true;
+	}
+
 	if (c->cmd) {
 		struct json_result *response;
 		response = new_json_result(c->cmd);

tests/test_lightningd.py

@@ -163,8 +163,8 @@ class LightningDTests(BaseLightningDTests):
 
         assert ret['id'] == l2.info['id']
 
-        l1.daemon.wait_for_log('WIRE_GOSSIPSTATUS_PEER_READY')
-        l2.daemon.wait_for_log('WIRE_GOSSIPSTATUS_PEER_READY')
+        l1.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER')
+        l2.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER')
         return l1,l2
 
     def fund_channel(self,l1,l2,amount):
@@ -398,7 +398,7 @@ class LightningDTests(BaseLightningDTests):
 
         assert ret['id'] == l3.info['id']
 
-        l3.daemon.wait_for_log('WIRE_GOSSIPSTATUS_PEER_READY')
+        l3.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER')
         self.fund_channel(l1, l2, 10**6)
         self.fund_channel(l2, l3, 10**6)