From a52d522525d52507fd54f7c14ebdfeb720471682 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 24 Jul 2018 09:56:43 +0930 Subject: [PATCH] gossipd: handle ping messages for remote peers too. This simplifies our ping handling: make gossipd always do it. Signed-off-by: Rusty Russell --- channeld/Makefile | 1 - channeld/channel.c | 52 +----------------------------------- channeld/channel_wire.csv | 8 ------ closingd/Makefile | 1 - common/read_peer_msg.c | 32 ---------------------- gossipd/gossip.c | 22 ++++++++------- lightningd/channel_control.c | 2 -- lightningd/dev_ping.c | 34 ++++------------------- openingd/Makefile | 1 - wire/peer_wire.c | 4 +-- 10 files changed, 21 insertions(+), 136 deletions(-) diff --git a/channeld/Makefile b/channeld/Makefile index c63071534..115413636 100644 --- a/channeld/Makefile +++ b/channeld/Makefile @@ -54,7 +54,6 @@ CHANNELD_COMMON_OBJS := \ common/key_derive.o \ common/memleak.o \ common/msg_queue.o \ - common/ping.o \ common/peer_billboard.o \ common/peer_failed.o \ common/permute_tx.o \ diff --git a/channeld/channel.c b/channeld/channel.c index b0d5f88b3..8cffb5c76 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -1603,18 +1603,6 @@ static void handle_peer_fail_malformed_htlc(struct peer *peer, const u8 *msg) abort(); } -static void handle_pong(struct peer *peer, const u8 *pong) -{ - const char *err = got_pong(pong, &peer->num_pings_outstanding); - if (err) - peer_failed(&peer->cs, - &peer->channel_id, - "%s", err); - - wire_sync_write(MASTER_FD, - take(towire_channel_ping_reply(NULL, tal_len(pong)))); -} - static void handle_peer_shutdown(struct peer *peer, const u8 *shutdown) { struct channel_id channel_id; @@ -1695,9 +1683,6 @@ static void peer_in(struct peer *peer, const u8 *msg) case WIRE_UPDATE_FAIL_MALFORMED_HTLC: handle_peer_fail_malformed_htlc(peer, msg); return; - case WIRE_PONG: - handle_pong(peer, msg); - return; case WIRE_SHUTDOWN: handle_peer_shutdown(peer, msg); return; @@ -1721,6 +1706,7 @@ static void peer_in(struct peer *peer, const u8 *msg) case WIRE_GOSSIP_TIMESTAMP_FILTER: case WIRE_REPLY_SHORT_CHANNEL_IDS_END: case WIRE_PING: + case WIRE_PONG: case WIRE_ERROR: abort(); } @@ -2266,38 +2252,6 @@ static void handle_fail(struct peer *peer, const u8 *inmsg) abort(); } -static void handle_ping_cmd(struct peer *peer, const u8 *inmsg) -{ - u16 num_pong_bytes, ping_len; - u8 *ping; - - if (!fromwire_channel_ping(inmsg, &num_pong_bytes, &ping_len)) - master_badmsg(WIRE_CHANNEL_PING, inmsg); - - ping = make_ping(NULL, num_pong_bytes, ping_len); - if (tal_len(ping) > 65535) - status_failed(STATUS_FAIL_MASTER_IO, "Oversize channel_ping"); - - enqueue_peer_msg(peer, take(ping)); - - status_trace("sending ping expecting %sresponse", - num_pong_bytes >= 65532 ? "no " : ""); - - /* BOLT #1: - * - * - 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) - wire_sync_write(MASTER_FD, - take(towire_channel_ping_reply(NULL, 0))); - else - peer->num_pings_outstanding++; -} - static void handle_shutdown_cmd(struct peer *peer, const u8 *inmsg) { if (!fromwire_channel_send_shutdown(inmsg)) @@ -2339,9 +2293,6 @@ static void req_in(struct peer *peer, const u8 *msg) case WIRE_CHANNEL_FAIL_HTLC: handle_fail(peer, msg); return; - case WIRE_CHANNEL_PING: - handle_ping_cmd(peer, msg); - return; case WIRE_CHANNEL_SEND_SHUTDOWN: handle_shutdown_cmd(peer, msg); return; @@ -2352,7 +2303,6 @@ static void req_in(struct peer *peer, const u8 *msg) #endif /* DEVELOPER */ case WIRE_CHANNEL_INIT: case WIRE_CHANNEL_OFFER_HTLC_REPLY: - case WIRE_CHANNEL_PING_REPLY: case WIRE_CHANNEL_SENDING_COMMITSIG: case WIRE_CHANNEL_GOT_COMMITSIG: case WIRE_CHANNEL_GOT_REVOKE: diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index 7cb9b9704..2899ec2b2 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -96,14 +96,6 @@ channel_fail_htlc,,errcode,u16 # If errcode & UPDATE, this says which outgoing channel failed. channel_fail_htlc,,which_channel,struct short_channel_id -# Ping/pong test. -channel_ping,1011 -channel_ping,,num_pong_bytes,u16 -channel_ping,,len,u16 - -channel_ping_reply,1111 -channel_ping_reply,,totlen,u16 - # When we receive funding_locked. channel_got_funding_locked,1019 channel_got_funding_locked,,next_per_commit_point,struct pubkey diff --git a/closingd/Makefile b/closingd/Makefile index 9a2ad77fb..64cc6a16a 100644 --- a/closingd/Makefile +++ b/closingd/Makefile @@ -60,7 +60,6 @@ CLOSINGD_COMMON_OBJS := \ common/peer_billboard.o \ common/peer_failed.o \ common/permute_tx.o \ - common/ping.o \ common/read_peer_msg.o \ common/socket_close.o \ common/status.o \ diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index 384002911..9a3896895 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -1,6 +1,5 @@ #include #include -#include #include #include #include @@ -12,32 +11,6 @@ #include #include -static void handle_ping(const u8 *msg, - int peer_fd, - struct crypto_state *cs, - const struct channel_id *channel, - bool (*send_reply)(struct crypto_state *, int, - const u8 *, void *), - void *arg) -{ - u8 *pong; - - if (!check_ping_make_pong(msg, msg, &pong)) { - send_reply(cs, peer_fd, - take(towire_errorfmt(NULL, channel, - "Bad ping %s", - tal_hex(msg, msg))), arg); - peer_failed_connection_lost(); - } - - status_debug("Got ping, sending %s", pong ? - wire_type_name(fromwire_peektype(pong)) - : "nothing"); - - if (pong && !send_reply(cs, peer_fd, pong, arg)) - peer_failed_connection_lost(); -} - void handle_gossip_msg_(const u8 *msg TAKES, int peer_fd, struct crypto_state *cs, bool (*send_msg)(struct crypto_state *cs, int fd, @@ -111,11 +84,6 @@ u8 *read_peer_msg_(const tal_t *ctx, return NULL; } - if (fromwire_peektype(msg) == WIRE_PING) { - handle_ping(msg, peer_fd, cs, channel, send_reply, arg); - return tal_free(msg); - } - if (fromwire_peektype(msg) == WIRE_ERROR) { char *err = sanitize_error(msg, msg, &chanid); diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 582237568..fbc2358f8 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -225,9 +225,6 @@ struct local_peer_state { /* If we're exiting due to non-gossip msg, otherwise release */ u8 *nongossip_msg; - /* How many pongs are we expecting? */ - size_t num_pings_outstanding; - /* Message queue for outgoing. */ struct msg_queue peer_out; }; @@ -267,6 +264,9 @@ struct peer { /* How many query responses are we expecting? */ size_t num_scid_queries_outstanding; + /* How many pongs are we expecting? */ + size_t num_pings_outstanding; + /* Map of outstanding channel_range requests. */ u8 *query_channel_blocks; u32 first_channel_range; @@ -368,7 +368,6 @@ new_local_peer_state(struct peer *peer, const struct crypto_state *cs) init_peer_crypto_state(peer, &lps->pcs); lps->pcs.cs = *cs; lps->return_to_master = false; - lps->num_pings_outstanding = 0; msg_queue_init(&lps->peer_out, lps); return lps; @@ -427,6 +426,7 @@ static struct peer *new_peer(const tal_t *ctx, peer->query_channel_blocks = NULL; peer->gossip_timestamp_min = 0; peer->gossip_timestamp_max = UINT32_MAX; + peer->num_pings_outstanding = 0; return peer; } @@ -1101,18 +1101,18 @@ static void handle_ping(struct peer *peer, u8 *ping) { u8 *pong; - if (!check_ping_make_pong(peer, ping, &pong)) { + if (!check_ping_make_pong(NULL, ping, &pong)) { peer_error(peer, "Bad ping"); return; } if (pong) - msg_enqueue(&peer->local->peer_out, take(pong)); + queue_peer_msg(peer, take(pong)); } static void handle_pong(struct peer *peer, const u8 *pong) { - const char *err = got_pong(pong, &peer->local->num_pings_outstanding); + const char *err = got_pong(pong, &peer->num_pings_outstanding); if (err) { peer_error(peer, "%s", err); @@ -1889,6 +1889,10 @@ static struct io_plan *owner_msg_in(struct io_conn *conn, handle_query_channel_range(peer, dc->msg_in); } else if (type == WIRE_REPLY_CHANNEL_RANGE) { handle_reply_channel_range(peer, dc->msg_in); + } else if (type == WIRE_PING) { + handle_ping(peer, dc->msg_in); + } else if (type == WIRE_PONG) { + handle_pong(peer, dc->msg_in); } else { status_broken("peer %s: send us unknown msg of type %s", type_to_string(tmpctx, struct pubkey, &peer->id), @@ -2314,7 +2318,7 @@ static struct io_plan *ping_req(struct io_conn *conn, struct daemon *daemon, if (tal_len(ping) > 65535) status_failed(STATUS_FAIL_MASTER_IO, "Oversize ping"); - msg_enqueue(&peer->local->peer_out, take(ping)); + queue_peer_msg(peer, take(ping)); status_trace("sending ping expecting %sresponse", num_pong_bytes >= 65532 ? "no " : ""); @@ -2332,7 +2336,7 @@ static struct io_plan *ping_req(struct io_conn *conn, struct daemon *daemon, daemon_conn_send(&daemon->master, take(towire_gossip_ping_reply(NULL, true, 0))); else - peer->local->num_pings_outstanding++; + peer->num_pings_outstanding++; out: return daemon_conn_read_next(conn, &daemon->master); diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index af2ef3dfd..8cee73528 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -154,7 +154,6 @@ static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds) case WIRE_CHANNEL_OFFER_HTLC: case WIRE_CHANNEL_FULFILL_HTLC: case WIRE_CHANNEL_FAIL_HTLC: - case WIRE_CHANNEL_PING: case WIRE_CHANNEL_GOT_COMMITSIG_REPLY: case WIRE_CHANNEL_GOT_REVOKE_REPLY: case WIRE_CHANNEL_SENDING_COMMITSIG_REPLY: @@ -163,7 +162,6 @@ static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds) case WIRE_CHANNEL_FEERATES: /* Replies go to requests. */ case WIRE_CHANNEL_OFFER_HTLC_REPLY: - case WIRE_CHANNEL_PING_REPLY: case WIRE_CHANNEL_DEV_REENABLE_COMMIT_REPLY: break; } diff --git a/lightningd/dev_ping.c b/lightningd/dev_ping.c index 37cbc2773..8e962b629 100644 --- a/lightningd/dev_ping.c +++ b/lightningd/dev_ping.c @@ -19,10 +19,7 @@ static void ping_reply(struct subd *subd, const u8 *msg, const int *fds UNUSED, bool ok, sent = true; log_debug(subd->ld->log, "Got ping reply!"); - if (streq(subd->name, "lightning_channeld")) - ok = fromwire_channel_ping_reply(msg, &totlen); - else - ok = fromwire_gossip_ping_reply(msg, &sent, &totlen); + ok = fromwire_gossip_ping_reply(msg, &sent, &totlen); if (!ok) command_fail(cmd, LIGHTNINGD, "Bad reply message"); @@ -41,11 +38,9 @@ static void ping_reply(struct subd *subd, const u8 *msg, const int *fds UNUSED, static void json_dev_ping(struct command *cmd, const char *buffer, const jsmntok_t *params) { - struct peer *peer; u8 *msg; unsigned int len, pongbytes; struct pubkey id; - struct subd *owner; if (!param(cmd, buffer, params, p_req("id", json_tok_pubkey, &id), @@ -81,29 +76,10 @@ static void json_dev_ping(struct command *cmd, return; } - /* First, see if it's in channeld. */ - peer = peer_by_id(cmd->ld, &id); - if (peer) { - struct channel *channel = peer_active_channel(peer); - - if (!channel - || !channel->owner - || !streq(channel->owner->name, "lightning_channeld")) { - command_fail(cmd, LIGHTNINGD, "Peer in %s", - channel && channel->owner - ? channel->owner->name - : "unattached"); - return; - } - msg = towire_channel_ping(cmd, pongbytes, len); - owner = channel->owner; - } else { - /* We assume it's in gossipd. */ - msg = towire_gossip_ping(cmd, &id, pongbytes, len); - owner = cmd->ld->gossip; - } - - subd_req(owner, owner, take(msg), -1, 0, ping_reply, cmd); + /* gossipd handles all pinging, even if it's in another daemon. */ + msg = towire_gossip_ping(NULL, &id, pongbytes, len); + subd_req(cmd->ld->gossip, cmd->ld->gossip, + take(msg), -1, 0, ping_reply, cmd); command_still_pending(cmd); } diff --git a/openingd/Makefile b/openingd/Makefile index 27883c375..c834cea41 100644 --- a/openingd/Makefile +++ b/openingd/Makefile @@ -55,7 +55,6 @@ OPENINGD_COMMON_OBJS := \ common/keyset.o \ common/memleak.o \ common/msg_queue.o \ - common/ping.o \ common/peer_billboard.o \ common/peer_failed.o \ common/permute_tx.o \ diff --git a/wire/peer_wire.c b/wire/peer_wire.c index 777b85dc6..db88fe4a1 100644 --- a/wire/peer_wire.c +++ b/wire/peer_wire.c @@ -47,6 +47,8 @@ bool is_msg_for_gossipd(const u8 *cursor) case WIRE_QUERY_CHANNEL_RANGE: case WIRE_REPLY_CHANNEL_RANGE: case WIRE_GOSSIP_TIMESTAMP_FILTER: + case WIRE_PING: + case WIRE_PONG: return true; case WIRE_INIT: case WIRE_ERROR: @@ -64,8 +66,6 @@ bool is_msg_for_gossipd(const u8 *cursor) case WIRE_COMMITMENT_SIGNED: case WIRE_REVOKE_AND_ACK: case WIRE_UPDATE_FEE: - case WIRE_PING: - case WIRE_PONG: case WIRE_CHANNEL_REESTABLISH: case WIRE_ANNOUNCEMENT_SIGNATURES: break;