diff --git a/channeld/channeld.c b/channeld/channeld.c index 616447b0bcda..b93e618bde1c 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -49,6 +49,15 @@ #define MASTER_FD STDIN_FILENO #define HSM_FD 6 +enum pong_expect_type { + /* We weren't expecting a ping reply */ + PONG_UNEXPECTED = 0, + /* We were expecting a ping reply due to ping command */ + PONG_EXPECTED_COMMAND = 1, + /* We were expecting a ping reply due to ping timer */ + PONG_EXPECTED_PROBING = 2, +}; + struct peer { struct per_peer_state *pps; bool funding_locked[NUM_SIDES]; @@ -104,7 +113,7 @@ struct peer { struct oneshot *ping_timer; /* Are we expecting a pong? */ - bool expecting_pong; + enum pong_expect_type expecting_pong; /* The feerate we want. */ u32 desired_feerate; @@ -1105,13 +1114,13 @@ static void set_ping_timer(struct peer *peer) static void send_ping(struct peer *peer) { /* Already have a ping in flight? */ - if (peer->expecting_pong) { + if (peer->expecting_pong != PONG_UNEXPECTED) { status_debug("Last ping unreturned: hanging up"); exit(0); } sync_crypto_write_no_delay(peer->pps, take(make_ping(NULL, 1, 0))); - peer->expecting_pong = true; + peer->expecting_pong = PONG_EXPECTED_PROBING; set_ping_timer(peer); } @@ -2143,6 +2152,29 @@ static void handle_unexpected_reestablish(struct peer *peer, const u8 *msg) &channel_id)); } +static void handle_ping_reply(struct peer *peer, const u8 *msg) +{ + u8 *ignored; + size_t i; + + /* We print this out because we asked for pong, so can't spam us... */ + if (!fromwire_pong(msg, msg, &ignored)) + status_unusual("Got malformed ping reply %s", + tal_hex(tmpctx, msg)); + + /* We print this because dev versions of c-lightning embed + * version here: see check_ping_make_pong! */ + for (i = 0; i < tal_count(ignored); i++) { + if (ignored[i] < ' ' || ignored[i] == 127) + break; + } + status_debug("Got pong %zu bytes (%.*s...)", + tal_count(ignored), (int)i, (char *)ignored); + wire_sync_write(MASTER_FD, + take(towire_channeld_ping_reply(NULL, true, + tal_bytelen(msg)))); +} + static void peer_in(struct peer *peer, const u8 *msg) { enum peer_wire type = fromwire_peektype(msg); @@ -2236,12 +2268,18 @@ static void peer_in(struct peer *peer, const u8 *msg) case WIRE_ACK_RBF: break; case WIRE_PONG: - if (peer->expecting_pong) { - peer->expecting_pong = false; + switch (peer->expecting_pong) { + case PONG_EXPECTED_COMMAND: + handle_ping_reply(peer, msg); + /* fall thru */ + case PONG_EXPECTED_PROBING: + peer->expecting_pong = PONG_UNEXPECTED; + return; + case PONG_UNEXPECTED: + status_debug("Unexpected pong?"); return; } - status_debug("Unexpected pong?"); - return; + abort(); case WIRE_CHANNEL_REESTABLISH: handle_unexpected_reestablish(peer, msg); @@ -3437,6 +3475,57 @@ static void handle_send_error(struct peer *peer, const u8 *msg) take(towire_channeld_send_error_reply(NULL))); } +static void handle_send_ping(struct peer *peer, const u8 *msg) +{ + u8 *ping; + u16 len, num_pong_bytes; + + if (!fromwire_channeld_ping(msg, &num_pong_bytes, &len)) + master_badmsg(WIRE_CHANNELD_PING, msg); + + /* We're not supposed to send another ping until previous replied */ + if (peer->expecting_pong != PONG_UNEXPECTED) { + wire_sync_write(MASTER_FD, + take(towire_channeld_ping_reply(NULL, false, 0))); + return; + } + + /* It should never ask for an oversize ping. */ + ping = make_ping(NULL, num_pong_bytes, len); + if (tal_count(ping) > 65535) + status_failed(STATUS_FAIL_MASTER_IO, "Oversize ping"); + + sync_crypto_write_no_delay(peer->pps, take(ping)); + + /* Since we're doing this manually, kill and restart timer. */ + status_debug("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) { + wire_sync_write(MASTER_FD, + take(towire_channeld_ping_reply(NULL, + true, 0))); + return; + } + + /* We'll respond to lightningd once the pong comes in */ + peer->expecting_pong = PONG_EXPECTED_COMMAND; + + /* Restart our timed pings now. */ + tal_free(peer->ping_timer); + set_ping_timer(peer); +} + #if DEVELOPER static void handle_dev_reenable_commit(struct peer *peer) { @@ -3535,6 +3624,9 @@ static void req_in(struct peer *peer, const u8 *msg) case WIRE_CHANNELD_SEND_ERROR: handle_send_error(peer, msg); return; + case WIRE_CHANNELD_PING: + handle_send_ping(peer, msg); + return; #if DEVELOPER case WIRE_CHANNELD_DEV_REENABLE_COMMIT: handle_dev_reenable_commit(peer); @@ -3570,6 +3662,7 @@ static void req_in(struct peer *peer, const u8 *msg) case WIRE_CHANNELD_SEND_ERROR_REPLY: case WIRE_CHANNELD_DEV_QUIESCE_REPLY: case WIRE_CHANNELD_UPGRADED: + case WIRE_CHANNELD_PING_REPLY: break; } @@ -3807,7 +3900,7 @@ int main(int argc, char *argv[]) status_setup_sync(MASTER_FD); peer = tal(NULL, struct peer); - peer->expecting_pong = false; + peer->expecting_pong = PONG_UNEXPECTED; timers_init(&peer->timers, time_mono()); peer->commit_timer = NULL; set_ping_timer(peer); diff --git a/channeld/channeld_wire.csv b/channeld/channeld_wire.csv index daf0eaa3e05e..ae96a489c825 100644 --- a/channeld/channeld_wire.csv +++ b/channeld/channeld_wire.csv @@ -238,3 +238,14 @@ msgdata,channeld_upgraded,new_type,channel_type, # Tell peer about our latest and greatest blockheight. msgtype,channeld_blockheight,1012 msgdata,channeld_blockheight,blockheight,u32, + +# Ping/pong test. Waits for a reply if it expects one. +msgtype,channeld_ping,1030 +msgdata,channeld_ping,num_pong_bytes,u16, +msgdata,channeld_ping,len,u16, + +msgtype,channeld_ping_reply,1130 +# False if we there was already a ping in progress. +msgdata,channeld_ping_reply,sent,bool, +# 0 == no pong expected, otherwise length of pong. +msgdata,channeld_ping_reply,totlen,u16, diff --git a/common/ping.c b/common/ping.c index 8322e55a51ae..bcb2f2555603 100644 --- a/common/ping.c +++ b/common/ping.c @@ -66,25 +66,3 @@ u8 *make_ping(const tal_t *ctx, u16 num_pong_bytes, u16 padlen) tal_free(ignored); return ping; } - -const char *got_pong(const u8 *pong, size_t *num_pings_outstanding) -{ - u8 *ignored; - int i; - - if (!fromwire_pong(pong, pong, &ignored)) - return "Bad pong"; - - if (*num_pings_outstanding == 0) - return "Unexpected pong"; - - for (i = 0; i < tal_count(ignored); i++) { - if (ignored[i] < ' ' || ignored[i] == 127) - break; - } - status_debug("Got pong %zu bytes (%.*s...)", - tal_count(ignored), i, (char *)ignored); - - (*num_pings_outstanding)--; - return NULL; -} diff --git a/common/ping.h b/common/ping.h index 490dc4ac47d9..60678f66120c 100644 --- a/common/ping.h +++ b/common/ping.h @@ -10,7 +10,4 @@ bool check_ping_make_pong(const tal_t *ctx, const u8 *ping, u8 **pong); /* Make a ping packet requesting num_pong_bytes */ u8 *make_ping(const tal_t *ctx, u16 num_pong_bytes, u16 padlen); -/* Returns error string if something wrong. */ -const char *got_pong(const u8 *pong, size_t *num_pings_outstanding); - #endif /* LIGHTNING_COMMON_PING_H */ diff --git a/doc/lightning-ping.7.md b/doc/lightning-ping.7.md index 38271f4c31b0..9408b06f2cc4 100644 --- a/doc/lightning-ping.7.md +++ b/doc/lightning-ping.7.md @@ -9,11 +9,15 @@ SYNOPSIS DESCRIPTION ----------- -The **ping** command checks if the node with *id* is ready to talk. It accepts the following parameters: +The **ping** command checks if the node with *id* is ready to talk. +It currently only works for peers we have a channel with. + +It accepts the following parameters: - *id*: A string that represents the node id; - *len*: A integer that represents the length of the ping (default 128); - *pongbytes*: An integer that represents the length of the reply (default 128). + A value of 65532 to 65535 means "don't reply". EXAMPLE JSON REQUEST ------------ @@ -39,7 +43,7 @@ On success, an object is returned, containing: On failure, one of the following error codes may be returned: -- -32602: Error in given parameters or unknown peer. +- -32602: Error in given parameters or we're already waiting for a ping response from peer. EXAMPLE JSON RESPONSE ----- diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index d91cbe698660..66d960929502 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -19,6 +19,7 @@ #include #include #include +#include #include static void update_feerates(struct lightningd *ld, struct channel *channel) @@ -491,6 +492,9 @@ static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds) case WIRE_CHANNELD_SEND_ERROR_REPLY: handle_error_channel(sd->channel, msg); break; + case WIRE_CHANNELD_PING_REPLY: + ping_reply(sd, msg); + break; #if EXPERIMENTAL_FEATURES case WIRE_CHANNELD_UPGRADED: handle_channel_upgrade(sd->channel, msg); @@ -520,6 +524,7 @@ static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds) case WIRE_CHANNELD_DEV_MEMLEAK_REPLY: case WIRE_CHANNELD_SEND_ERROR: case WIRE_CHANNELD_DEV_QUIESCE_REPLY: + case WIRE_CHANNELD_PING: break; } diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 19aaeea6e6c5..77008c226f86 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -14,7 +14,6 @@ #include #include #include -#include static void got_txout(struct bitcoind *bitcoind, const struct bitcoin_tx_output *output, diff --git a/lightningd/ping.c b/lightningd/ping.c index 98dd5870022d..eca50097d29e 100644 --- a/lightningd/ping.c +++ b/lightningd/ping.c @@ -1,9 +1,11 @@ +#include #include #include #include -#include +#include #include #include +#include #include #include @@ -45,33 +47,37 @@ static struct ping_command *new_ping_command(const tal_t *ctx, return pc; } -void ping_reply(struct subd *subd, const u8 *msg) +void ping_reply(struct subd *channeld, 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; + bool sent; struct ping_command *pc; + struct channel *c = channeld->channel; - log_debug(subd->ld->log, "Got ping reply!"); - ok = fromwire_gossipd_ping_reply(msg, &id, &sent, &totlen); - - pc = find_ping_cmd(subd->ld, &id); - assert(pc); + log_debug(channeld->log, "Got ping reply!"); + pc = find_ping_cmd(channeld->ld, &c->peer->id); + if (!pc) { + log_broken(channeld->log, "Unexpected ping reply?"); + return; + } - if (!ok) + if (!fromwire_channeld_ping_reply(msg, &sent, &totlen)) { + log_broken(channeld->log, "Malformed ping reply %s", + tal_hex(tmpctx, msg)); was_pending(command_fail(pc->cmd, LIGHTNINGD, "Bad reply message")); - else if (!sent) - was_pending(command_fail(pc->cmd, LIGHTNINGD, "Unknown peer")); + return; + } + + if (!sent) + was_pending(command_fail(pc->cmd, LIGHTNINGD, + "Ping already pending")); else { struct json_stream *response = json_stream_success(pc->cmd); json_add_num(response, "totlen", totlen); was_pending(command_success(pc->cmd, response)); } -#endif } static struct command_result *json_ping(struct command *cmd, @@ -81,6 +87,9 @@ static struct command_result *json_ping(struct command *cmd, { unsigned int *len, *pongbytes; struct node_id *id; + struct peer *peer; + struct channel *channel; + u8 *msg; if (!param(cmd, buffer, params, p_req("id", param_node_id, &id), @@ -114,14 +123,20 @@ static struct command_result *json_ping(struct command *cmd, "pongbytes %u > 65535", *pongbytes); } + peer = peer_by_id(cmd->ld, id); + if (!peer) + return command_fail(cmd, LIGHTNINGD, "Peer not connected"); + + channel = peer_active_channel(peer); + if (!channel || !channel->owner || channel->state != CHANNELD_NORMAL) + return command_fail(cmd, LIGHTNINGD, "Peer bad state"); + /* 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 + msg = towire_channeld_ping(NULL, *pongbytes, *len); + subd_send_msg(channel->owner, take(msg)); + return command_still_pending(cmd); } diff --git a/tests/test_misc.py b/tests/test_misc.py index 2a8ccf2bf04b..2c3df9cc12e3 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -253,9 +253,8 @@ def mock_getblock(r): 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) + l1, l2 = node_factory.line_graph(2) def ping_tests(l1, l2): # 0-byte pong gives just type + length field. @@ -284,14 +283,6 @@ def ping_tests(l1, l2): with pytest.raises(RpcError, match=r'oversize ping'): l1.rpc.ping(l2.info['id'], 65530, 1) - # Test gossip pinging. - ping_tests(l1, l2) - if DEVELOPER: - l1.daemon.wait_for_log(r'Got pong 1000 bytes \({}\.\.\.\)' - .format(l2.info['version']), timeout=1) - - l1.fundchannel(l2, 10**5) - # channeld pinging ping_tests(l1, l2) if DEVELOPER: