gossipd: remove ping/pong handling

To minimize the diffs, we #if 0 the code.  We'll reenable it once
channeld is ready.

We also temporarily disable the ping tests.

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

gossipd/gossipd.c

@@ -18,7 +18,6 @@
 #include <common/ecdh_hsmd.h>
 #include <common/lease_rates.h>
 #include <common/memleak.h>
-#include <common/ping.h>
 #include <common/pseudorand.h>
 #include <common/sphinx.h>
 #include <common/status.h>
@@ -259,36 +258,6 @@ static u8 *handle_channel_update_msg(struct peer *peer, const u8 *msg)
 	return NULL;
 }
 
-/*~ For simplicity, all pings and pongs are forwarded to us here in gossipd. */
-static u8 *handle_ping(struct peer *peer, const u8 *ping)
-{
-	u8 *pong;
-
-	/* This checks the ping packet and makes a pong reply if needed; peer
-	 * can specify it doesn't want a response, to simulate traffic. */
-	if (!check_ping_make_pong(NULL, ping, &pong))
-		return towire_warningfmt(peer, NULL, "Bad ping");
-
-	if (pong)
-		queue_peer_msg(peer, take(pong));
-	return NULL;
-}
-
-/*~ When we get a pong, we tell lightningd about it (it's probably a response
- * to the `ping` JSON RPC command). */
-static const u8 *handle_pong(struct peer *peer, const u8 *pong)
-{
-	const char *err = got_pong(pong, &peer->num_pings_outstanding);
-
-	if (err)
-		return towire_warningfmt(peer, NULL, "%s", err);
-
-	daemon_conn_send(peer->daemon->master,
-			 take(towire_gossipd_ping_reply(NULL, &peer->id, true,
-						       tal_count(pong))));
-	return NULL;
-}
-
 /*~ This is when channeld asks us for a channel_update for a local channel.
  * It does that to fill in the error field when lightningd fails an HTLC and
  * sets the UPDATE bit in the error type.  lightningd is too important to
@@ -845,12 +814,6 @@ static struct io_plan *peer_msg_in(struct io_conn *conn,
 	case WIRE_REPLY_SHORT_CHANNEL_IDS_END:
 		err = handle_reply_short_channel_ids_end(peer, msg);
 		goto handled_relay;
-	case WIRE_PING:
-		err = handle_ping(peer, msg);
-		goto handled_relay;
-	case WIRE_PONG:
-		err = handle_pong(peer, msg);
-		goto handled_relay;
 	case WIRE_OBS_ONION_MESSAGE:
 		err = handle_obs_onion_message(peer, msg);
 		goto handled_relay;
@@ -862,6 +825,8 @@ static struct io_plan *peer_msg_in(struct io_conn *conn,
 	case WIRE_WARNING:
 	case WIRE_INIT:
 	case WIRE_ERROR:
+	case WIRE_PING:
+	case WIRE_PONG:
 	case WIRE_OPEN_CHANNEL:
 	case WIRE_ACCEPT_CHANNEL:
 	case WIRE_FUNDING_CREATED:
@@ -998,7 +963,6 @@ static struct io_plan *connectd_new_peer(struct io_conn *conn,
 	peer->scid_query_outstanding = false;
 	peer->range_replies = NULL;
 	peer->query_channel_range_cb = NULL;
-	peer->num_pings_outstanding = 0;
 
 	/* We keep a list so we can find peer by id */
 	list_add_tail(&peer->daemon->peers, &peer->list);
@@ -1292,61 +1256,6 @@ static struct io_plan *gossip_init(struct io_conn *conn,
 	return daemon_conn_read_next(conn, daemon->master);
 }
 
-/*~ We currently have a JSON command to ping a peer: it ends up here, where
- * gossipd generates the actual ping and sends it like any other gossip. */
-static struct io_plan *ping_req(struct io_conn *conn, struct daemon *daemon,
-				const u8 *msg)
-{
-	struct node_id id;
-	u16 num_pong_bytes, len;
-	struct peer *peer;
-	u8 *ping;
-
-	if (!fromwire_gossipd_ping(msg, &id, &num_pong_bytes, &len))
-		master_badmsg(WIRE_GOSSIPD_PING, msg);
-
-	/* Even if lightningd were to check for valid ids, there's a race
-	 * where it might vanish before we read this command; cleaner to
-	 * handle it here with 'sent' = false. */
-	peer = find_peer(daemon, &id);
-	if (!peer) {
-		daemon_conn_send(daemon->master,
-				 take(towire_gossipd_ping_reply(NULL, &id,
-							       false, 0)));
-		goto out;
-	}
-
-	/* It should never ask for an oversize ping. */
-	ping = make_ping(peer, num_pong_bytes, len);
-	if (tal_count(ping) > 65535)
-		status_failed(STATUS_FAIL_MASTER_IO, "Oversize ping");
-
-	queue_peer_msg(peer, take(ping));
-	status_peer_debug(&peer->id, "sending ping expecting %sresponse",
-			  num_pong_bytes >= 65532 ? "no " : "");
-
-	/* BOLT #1:
-	 *
-	 * A node receiving a `ping` message:
-	 *...
-	 *  - if `num_pong_bytes` is less than 65532:
-	 *    - MUST respond by sending a `pong` message, with `byteslen` equal
-	 *      to `num_pong_bytes`.
-	 *  - otherwise (`num_pong_bytes` is **not** less than 65532):
-	 *    - MUST ignore the `ping`.
-	 */
-	if (num_pong_bytes >= 65532)
-		daemon_conn_send(daemon->master,
-				 take(towire_gossipd_ping_reply(NULL, &id,
-							       true, 0)));
-	else
-		/* We'll respond to lightningd once the pong comes in */
-		peer->num_pings_outstanding++;
-
-out:
-	return daemon_conn_read_next(conn, daemon->master);
-}
-
 static struct io_plan *new_blockheight(struct io_conn *conn,
 				       struct daemon *daemon,
 				       const u8 *msg)
@@ -1649,9 +1558,6 @@ static struct io_plan *recv_req(struct io_conn *conn,
 	case WIRE_GOSSIPD_LOCAL_CHANNEL_CLOSE:
 		return handle_local_channel_close(conn, daemon, msg);
 
-	case WIRE_GOSSIPD_PING:
-		return ping_req(conn, daemon, msg);
-
 	case WIRE_GOSSIPD_NEW_BLOCKHEIGHT:
 		return new_blockheight(conn, daemon, msg);
 
@@ -1688,7 +1594,6 @@ static struct io_plan *recv_req(struct io_conn *conn,
 		return onionmsg_req(conn, daemon, msg);
 
 	/* We send these, we don't receive them */
-	case WIRE_GOSSIPD_PING_REPLY:
 	case WIRE_GOSSIPD_INIT_REPLY:
 	case WIRE_GOSSIPD_GET_STRIPPED_CUPDATE_REPLY:
 	case WIRE_GOSSIPD_GET_TXOUT:

gossipd/gossipd.h

@@ -97,9 +97,6 @@ struct peer {
 	bool scid_query_outstanding;
 	void (*scid_query_cb)(struct peer *peer, bool complete);
 
-	/* How many pongs are we expecting? */
-	size_t num_pings_outstanding;
-
 	/* What we're querying: [range_first_blocknum, range_end_blocknum) */
 	u32 range_first_blocknum, range_end_blocknum;
 	u32 range_blocks_outstanding;

gossipd/gossipd_wire.csv

@@ -23,19 +23,6 @@ msgtype,gossipd_init_reply,3100
 msgtype,gossipd_dev_set_time,3001
 msgdata,gossipd_dev_set_time,dev_gossip_time,u32,
 
-# Ping/pong test.  Waits for a reply if it expects one.
-msgtype,gossipd_ping,3008
-msgdata,gossipd_ping,id,node_id,
-msgdata,gossipd_ping,num_pong_bytes,u16,
-msgdata,gossipd_ping,len,u16,
-
-msgtype,gossipd_ping_reply,3108
-msgdata,gossipd_ping_reply,id,node_id,
-# False if id in gossip_ping was unknown.
-msgdata,gossipd_ping_reply,sent,bool,
-# 0 == no pong expected
-msgdata,gossipd_ping_reply,totlen,u16,
-
 # Set artificial maximum reply_channel_range size.  Master->gossipd
 msgtype,gossipd_dev_set_max_scids_encode_size,3030
 msgdata,gossipd_dev_set_max_scids_encode_size,max,u32,

lightningd/gossip_control.c

@@ -113,7 +113,6 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 	switch (t) {
 	/* These are messages we send, not them. */
 	case WIRE_GOSSIPD_INIT:
-	case WIRE_GOSSIPD_PING:
 	case WIRE_GOSSIPD_GET_STRIPPED_CUPDATE:
 	case WIRE_GOSSIPD_GET_TXOUT_REPLY:
 	case WIRE_GOSSIPD_OUTPOINT_SPENT:
@@ -145,10 +144,6 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 	case WIRE_GOSSIPD_GOT_ONIONMSG_TO_US:
 		handle_onionmsg_to_us(gossip->ld, msg);
 		break;
-	case WIRE_GOSSIPD_PING_REPLY:
-		ping_reply(gossip, msg);
-		break;
-
 	case WIRE_GOSSIPD_GET_TXOUT:
 		get_txout(gossip, msg);
 		break;

lightningd/ping.c

@@ -47,6 +47,8 @@ static struct ping_command *new_ping_command(const tal_t *ctx,
 
 void ping_reply(struct subd *subd, const u8 *msg)
 {
+	(void)find_ping_cmd;
+#if 0 /* Disabled until channeld sends us ping info */
 	u16 totlen;
 	bool ok, sent = true;
 	struct node_id id;
@@ -69,6 +71,7 @@ void ping_reply(struct subd *subd, const u8 *msg)
 		json_add_num(response, "totlen", totlen);
 		was_pending(command_success(pc->cmd, response));
 	}
+#endif
 }
 
 static struct command_result *json_ping(struct command *cmd,
@@ -76,7 +79,6 @@ static struct command_result *json_ping(struct command *cmd,
 					const jsmntok_t *obj UNNEEDED,
 					const jsmntok_t *params)
 {
-	u8 *msg;
 	unsigned int *len, *pongbytes;
 	struct node_id *id;
 
@@ -115,9 +117,11 @@ static struct command_result *json_ping(struct command *cmd,
 	/* parent is cmd, so when we complete cmd, we free this. */
 	new_ping_command(cmd, cmd->ld, id, cmd);
 
+#if 0 /* FIXME: make channeld take this message */
 	/* gossipd handles all pinging, even if it's in another daemon. */
 	msg = towire_gossipd_ping(NULL, id, *pongbytes, *len);
 	subd_send_msg(cmd->ld->gossip, take(msg));
+#endif
 	return command_still_pending(cmd);
 }
 

tests/test_connection.py

@@ -532,7 +532,7 @@ def test_reconnect_openingd(node_factory):
 @pytest.mark.developer
 def test_reconnect_gossiping(node_factory):
     # connectd thinks we're still gossiping; peer reconnects.
-    disconnects = ['0WIRE_PING']
+    disconnects = ['0INVALID 33333']
     l1 = node_factory.get_node(may_reconnect=True)
     l2 = node_factory.get_node(disconnect=disconnects,
                                may_reconnect=True)
@@ -540,7 +540,7 @@ def test_reconnect_gossiping(node_factory):
     # Make sure l2 knows about l1
     wait_for(lambda: l2.rpc.listpeers(l1.info['id'])['peers'] != [])
 
-    l2.rpc.ping(l1.info['id'], 1, 65532)
+    l2.rpc.sendcustommsg(l1.info['id'], bytes([0x82, 0x35]).hex())
     wait_for(lambda: l1.rpc.listpeers(l2.info['id'])['peers'] == [])
 
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)

tests/test_misc.py

@@ -253,6 +253,7 @@ def test_lightningd_still_loading(node_factory, bitcoind, executor):
     l1.pay(l2, 1000)
 
 
+@pytest.mark.skip(reason="FIXME: channeld needs to handle pings")
 def test_ping(node_factory):
     l1, l2 = node_factory.line_graph(2, fundchannel=False)
 

wire/peer_wire.c

@@ -63,13 +63,13 @@ bool is_msg_for_gossipd(const u8 *cursor)
 	case WIRE_REPLY_SHORT_CHANNEL_IDS_END:
 	case WIRE_QUERY_CHANNEL_RANGE:
 	case WIRE_REPLY_CHANNEL_RANGE:
-	case WIRE_PING:
-	case WIRE_PONG:
 	case WIRE_ONION_MESSAGE:
 	case WIRE_OBS_ONION_MESSAGE:
 		return true;
 	case WIRE_WARNING:
 	case WIRE_INIT:
+	case WIRE_PING:
+	case WIRE_PONG:
 	case WIRE_ERROR:
 	case WIRE_OPEN_CHANNEL:
 	case WIRE_ACCEPT_CHANNEL: