From d8d4b19f3afd8c4c1b3a89e4de021cb4062c32bf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 9 Aug 2018 09:55:29 +0930 Subject: [PATCH] connectd: remove separate address hint message. Include it as an optional field in the connect_to_peer message (it was added before we had optional fields). The only issue is that reconnects want it too, so again connectd hands it back to master in connectctl_connect_failed. Signed-off-by: Rusty Russell --- connectd/connect.c | 86 +++++++++--------------------------- connectd/connect_wire.csv | 7 +-- lightningd/channel.c | 2 +- lightningd/connect_control.c | 31 +++++++------ lightningd/connect_control.h | 4 +- lightningd/peer_control.c | 7 +-- wallet/test/run-wallet.c | 8 ++-- 7 files changed, 51 insertions(+), 94 deletions(-) diff --git a/connectd/connect.c b/connectd/connect.c index ebf9b5cfc..a79c761bf 100644 --- a/connectd/connect.c +++ b/connectd/connect.c @@ -109,9 +109,6 @@ struct daemon { /* Connection to main daemon. */ struct daemon_conn master; - /* Hacky list of known address hints. */ - struct list_head addrhints; - struct timers timers; /* Local and global features to offer to peers. */ @@ -147,6 +144,9 @@ struct reaching { /* FIXME: Support multiple address. */ struct wireaddr_internal addr; + /* NULL if there wasn't a hint. */ + struct wireaddr_internal *addrhint; + /* How far did we get? */ const char *connstate; @@ -176,15 +176,6 @@ struct peer { struct io_conn *conn; }; -struct addrhint { - /* Off ld->addrhints */ - struct list_node list; - - struct pubkey id; - /* FIXME: use array... */ - struct wireaddr_internal addr; -}; - static struct peer *find_reconnecting_peer(struct daemon *daemon, const struct pubkey *id) { @@ -213,23 +204,6 @@ static void add_reconnecting_peer(struct daemon *daemon, struct peer *peer) tal_add_destructor(peer, destroy_reconnecting_peer); } -static void destroy_addrhint(struct addrhint *a) -{ - list_del(&a->list); -} - -static struct addrhint *find_addrhint(struct daemon *daemon, - const struct pubkey *id) -{ - struct addrhint *a; - - list_for_each(&daemon->addrhints, a, list) { - if (pubkey_eq(&a->id, id)) - return a; - } - return NULL; -} - /** * Some ISP resolvers will reply with a dummy IP to queries that would otherwise * result in an NXDOMAIN reply. This just checks whether we have one such @@ -912,10 +886,11 @@ struct io_plan *connection_out(struct io_conn *conn, struct reaching *reach) handshake_out_success, reach); } -static void PRINTF_FMT(4,5) +static void PRINTF_FMT(5,6) connect_failed(struct daemon *daemon, const struct pubkey *id, u32 seconds_waited, + const struct wireaddr_internal *addrhint, const char *errfmt, ...) { u8 *msg; @@ -935,7 +910,8 @@ static void PRINTF_FMT(4,5) wait_seconds = INITIAL_WAIT_SECONDS; /* Tell any connect command what happened. */ - msg = towire_connectctl_connect_failed(NULL, id, err, wait_seconds); + msg = towire_connectctl_connect_failed(NULL, id, err, wait_seconds, + addrhint); daemon_conn_send(&daemon->master, take(msg)); status_trace("Failed connected out for %s: %s", @@ -946,6 +922,7 @@ static void PRINTF_FMT(4,5) static void destroy_io_conn(struct io_conn *conn, struct reaching *reach) { connect_failed(reach->daemon, &reach->id, reach->seconds_waited, + reach->addrhint, "%s: %s", reach->connstate, strerror(errno)); tal_free(reach); } @@ -1077,12 +1054,13 @@ gossip_resolve_addr(const tal_t *ctx, const struct pubkey *id) return addr; } +/* Consumes addrhint if not NULL */ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id, - u32 seconds_waited) + u32 seconds_waited, + struct wireaddr_internal *addrhint) { struct wireaddr_internal *a; - struct addrhint *hint; int fd, af; struct reaching *reach; bool use_proxy = daemon->use_proxy_always; @@ -1095,12 +1073,8 @@ static void try_reach_peer(struct daemon *daemon, if (find_reaching(daemon, id)) return; - hint = find_addrhint(daemon, id); - if (hint) - a = &hint->addr; - else - a = NULL; - + /* FIXME: Gather *all* the addresses, and iterate through. */ + a = addrhint; if (!a) a = gossip_resolve_addr(tmpctx, id); @@ -1117,7 +1091,8 @@ static void try_reach_peer(struct daemon *daemon, } if (!a) { - connect_failed(daemon, id, seconds_waited, "No address known"); + connect_failed(daemon, id, seconds_waited, addrhint, + "No address known"); return; } @@ -1172,7 +1147,7 @@ static void try_reach_peer(struct daemon *daemon, fd = socket(af, SOCK_STREAM, 0); if (fd < 0) { - connect_failed(daemon, id, seconds_waited, + connect_failed(daemon, id, seconds_waited, addrhint, "Can't open %i socket for %s (%s)", af, type_to_string(tmpctx, struct pubkey, id), @@ -1187,6 +1162,7 @@ static void try_reach_peer(struct daemon *daemon, reach->addr = *a; reach->connstate = "Connection establishment"; reach->seconds_waited = seconds_waited; + reach->addrhint = tal_steal(reach, addrhint); list_add_tail(&daemon->reaching, &reach->list); tal_add_destructor(reach, destroy_reaching); @@ -1201,28 +1177,14 @@ static struct io_plan *connect_to_peer(struct io_conn *conn, { struct pubkey id; u32 seconds_waited; + struct wireaddr_internal *addrhint; - if (!fromwire_connectctl_connect_to_peer(msg, &id, &seconds_waited)) + if (!fromwire_connectctl_connect_to_peer(tmpctx, msg, + &id, &seconds_waited, + &addrhint)) master_badmsg(WIRE_CONNECTCTL_CONNECT_TO_PEER, msg); - try_reach_peer(daemon, &id, seconds_waited); - return daemon_conn_read_next(conn, &daemon->master); -} - -static struct io_plan *addr_hint(struct io_conn *conn, - struct daemon *daemon, const u8 *msg) -{ - struct addrhint *a = tal(daemon, struct addrhint); - - if (!fromwire_connectctl_peer_addrhint(msg, &a->id, &a->addr)) - master_badmsg(WIRE_CONNECTCTL_PEER_ADDRHINT, msg); - - /* Replace any old one. */ - tal_free(find_addrhint(daemon, &a->id)); - - list_add_tail(&daemon->addrhints, &a->list); - tal_add_destructor(a, destroy_addrhint); - + try_reach_peer(daemon, &id, seconds_waited, addrhint); return daemon_conn_read_next(conn, &daemon->master); } @@ -1269,9 +1231,6 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_CONNECTCTL_CONNECT_TO_PEER: return connect_to_peer(conn, daemon, master->msg_in); - case WIRE_CONNECTCTL_PEER_ADDRHINT: - return addr_hint(conn, daemon, master->msg_in); - case WIRE_CONNECTCTL_PEER_DISCONNECTED: return peer_disconnected(conn, daemon, master->msg_in); @@ -1308,7 +1267,6 @@ int main(int argc, char *argv[]) pubkey_set_init(&daemon->peers); list_head_init(&daemon->reconnecting); list_head_init(&daemon->reaching); - list_head_init(&daemon->addrhints); timers_init(&daemon->timers, time_mono()); daemon->broken_resolver_response = NULL; daemon->listen_fds = tal_arr(daemon, struct listen_fd, 0); diff --git a/connectd/connect_wire.csv b/connectd/connect_wire.csv index 6b9f62b62..af6cbf4aa 100644 --- a/connectd/connect_wire.csv +++ b/connectd/connect_wire.csv @@ -36,21 +36,18 @@ connectctl_activate_reply,2125 connect_reconnected,2112 connect_reconnected,,id,struct pubkey -# Master -> connectd: Optional hint for where to find peer. -connectctl_peer_addrhint,2014 -connectctl_peer_addrhint,,id,struct pubkey -connectctl_peer_addrhint,,addr,struct wireaddr_internal - # Master -> connectd: connect to a peer. connectctl_connect_to_peer,2001 connectctl_connect_to_peer,,id,struct pubkey connectctl_connect_to_peer,,seconds_waited,u32 +connectctl_connect_to_peer,,addrhint,?struct wireaddr_internal # Connectd->master: connect failed. connectctl_connect_failed,2020 connectctl_connect_failed,,id,struct pubkey connectctl_connect_failed,,failreason,wirestring connectctl_connect_failed,,seconds_to_delay,u32 +connectctl_connect_failed,,addrhint,?struct wireaddr_internal # Connectd -> master: we got a peer. Two fds: peer and gossip connect_peer_connected,2002 diff --git a/lightningd/channel.c b/lightningd/channel.c index 628cc4ac9..fafc253ec 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -40,7 +40,7 @@ void channel_set_owner(struct channel *channel, struct subd *owner, if (reconnect) { /* Reconnect after 1 second: prevents some spurious * reconnects during tests. */ - delay_then_reconnect(channel, 1); + delay_then_reconnect(channel, 1, &channel->peer->addr); } } channel->connected = connects_to_peer(owner); diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c index 8cbcb8b36..a8faa21a7 100644 --- a/lightningd/connect_control.c +++ b/lightningd/connect_control.c @@ -80,7 +80,7 @@ static void json_connect(struct command *cmd, char *atptr; char *ataddr = NULL; const char *name; - struct wireaddr_internal addr; + struct wireaddr_internal *addr; u8 *msg; const char *err_msg; struct peer *peer; @@ -161,7 +161,8 @@ static void json_connect(struct command *cmd, } else { port = DEFAULT_PORT; } - if (!parse_wireaddr_internal(name, &addr, port, false, + addr = tal(cmd, struct wireaddr_internal); + if (!parse_wireaddr_internal(name, addr, port, false, !cmd->ld->use_proxy_always && !cmd->ld->pure_tor_setup, true, @@ -170,13 +171,10 @@ static void json_connect(struct command *cmd, name, port, err_msg ? err_msg : "port is 0"); return; } + } else + addr = NULL; - /* Tell it about the address. */ - msg = towire_connectctl_peer_addrhint(cmd, &id, &addr); - subd_send_msg(cmd->ld->connectd, take(msg)); - } - - msg = towire_connectctl_connect_to_peer(NULL, &id, 0); + msg = towire_connectctl_connect_to_peer(NULL, &id, 0, addr); subd_send_msg(cmd->ld->connectd, take(msg)); /* Leave this here for peer_connected or connect_failed. */ @@ -195,6 +193,7 @@ AUTODATA(json_command, &connect_command); struct delayed_reconnect { struct channel *channel; u32 seconds_delayed; + struct wireaddr_internal *addrhint; }; static void maybe_reconnect(struct delayed_reconnect *d) @@ -204,13 +203,15 @@ static void maybe_reconnect(struct delayed_reconnect *d) /* Might have gone onchain since we started timer. */ if (channel_active(d->channel)) { u8 *msg = towire_connectctl_connect_to_peer(NULL, &peer->id, - d->seconds_delayed); + d->seconds_delayed, + d->addrhint); subd_send_msg(peer->ld->connectd, take(msg)); } tal_free(d); } -void delay_then_reconnect(struct channel *channel, u32 seconds_delay) +void delay_then_reconnect(struct channel *channel, u32 seconds_delay, + const struct wireaddr_internal *addrhint) { struct delayed_reconnect *d; struct lightningd *ld = channel->peer->ld; @@ -221,6 +222,10 @@ void delay_then_reconnect(struct channel *channel, u32 seconds_delay) d = tal(channel, struct delayed_reconnect); d->channel = channel; d->seconds_delayed = seconds_delay; + if (addrhint) + d->addrhint = tal_dup(d, struct wireaddr_internal, addrhint); + else + d->addrhint = NULL; log_debug(channel->log, "Will try reconnect in %u seconds", seconds_delay); @@ -234,10 +239,11 @@ static void connect_failed(struct lightningd *ld, const u8 *msg) char *err; struct connect *c; u32 seconds_to_delay; + struct wireaddr_internal *addrhint; struct channel *channel; if (!fromwire_connectctl_connect_failed(tmpctx, msg, &id, &err, - &seconds_to_delay)) + &seconds_to_delay, &addrhint)) fatal("Connect gave bad CONNECTCTL_CONNECT_FAILED message %s", tal_hex(msg, msg)); @@ -250,7 +256,7 @@ static void connect_failed(struct lightningd *ld, const u8 *msg) /* If we have an active channel, then reconnect. */ channel = active_channel_by_id(ld, &id, NULL); if (channel) - delay_then_reconnect(channel, seconds_to_delay); + delay_then_reconnect(channel, seconds_to_delay, addrhint); } void connect_succeeded(struct lightningd *ld, const struct pubkey *id) @@ -288,7 +294,6 @@ static unsigned connectd_msg(struct subd *connectd, const u8 *msg, const int *fd /* These are messages we send, not them. */ case WIRE_CONNECTCTL_INIT: case WIRE_CONNECTCTL_ACTIVATE: - case WIRE_CONNECTCTL_PEER_ADDRHINT: case WIRE_CONNECTCTL_CONNECT_TO_PEER: case WIRE_CONNECTCTL_PEER_DISCONNECTED: /* This is a reply, so never gets through to here. */ diff --git a/lightningd/connect_control.h b/lightningd/connect_control.h index 054595bf8..56f203339 100644 --- a/lightningd/connect_control.h +++ b/lightningd/connect_control.h @@ -4,12 +4,14 @@ struct lightningd; struct pubkey; +struct wireaddr_internal; /* Returns fd for gossipd to talk to connectd */ int connectd_init(struct lightningd *ld); void connectd_activate(struct lightningd *ld); -void delay_then_reconnect(struct channel *channel, u32 seconds_delay); +void delay_then_reconnect(struct channel *channel, u32 seconds_delay, + const struct wireaddr_internal *addrhint TAKES); void connect_succeeded(struct lightningd *ld, const struct pubkey *id); void gossip_connect_result(struct lightningd *ld, const u8 *msg); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index adc101b45..443b10118 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -918,15 +918,12 @@ static void activate_peer(struct peer *peer) struct channel *channel; struct lightningd *ld = peer->ld; - /* Pass connectd any addrhints we currently have */ - msg = towire_connectctl_peer_addrhint(peer, &peer->id, &peer->addr); - subd_send_msg(peer->ld->connectd, take(msg)); - /* We can only have one active channel: make sure connectd * knows to try reconnecting. */ channel = peer_active_channel(peer); if (channel && ld->reconnect) { - msg = towire_connectctl_connect_to_peer(NULL, &peer->id, 0); + msg = towire_connectctl_connect_to_peer(NULL, &peer->id, 0, + &peer->addr); subd_send_msg(ld->connectd, take(msg)); } diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 99c3171a3..035a5f20a 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -63,7 +63,8 @@ void command_success(struct command *cmd UNNEEDED, struct json_result *response void connect_succeeded(struct lightningd *ld UNNEEDED, const struct pubkey *id UNNEEDED) { fprintf(stderr, "connect_succeeded called!\n"); abort(); } /* Generated stub for delay_then_reconnect */ -void delay_then_reconnect(struct channel *channel UNNEEDED, u32 seconds_delay UNNEEDED) +void delay_then_reconnect(struct channel *channel UNNEEDED, u32 seconds_delay UNNEEDED, + const struct wireaddr_internal *addrhint TAKES UNNEEDED) { fprintf(stderr, "delay_then_reconnect called!\n"); abort(); } /* Generated stub for fromwire_connect_peer_connected */ bool fromwire_connect_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey *id UNNEEDED, struct wireaddr_internal *addr UNNEEDED, struct crypto_state *crypto_state UNNEEDED, u8 **gfeatures UNNEEDED, u8 **lfeatures UNNEEDED) @@ -325,11 +326,8 @@ u8 *towire_channel_dev_reenable_commit(const tal_t *ctx UNNEEDED) u8 *towire_channel_send_shutdown(const tal_t *ctx UNNEEDED) { fprintf(stderr, "towire_channel_send_shutdown called!\n"); abort(); } /* Generated stub for towire_connectctl_connect_to_peer */ -u8 *towire_connectctl_connect_to_peer(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED, u32 seconds_waited UNNEEDED) +u8 *towire_connectctl_connect_to_peer(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED, u32 seconds_waited UNNEEDED, const struct wireaddr_internal *addrhint UNNEEDED) { fprintf(stderr, "towire_connectctl_connect_to_peer called!\n"); abort(); } -/* Generated stub for towire_connectctl_peer_addrhint */ -u8 *towire_connectctl_peer_addrhint(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED, const struct wireaddr_internal *addr UNNEEDED) -{ fprintf(stderr, "towire_connectctl_peer_addrhint called!\n"); abort(); } /* Generated stub for towire_connectctl_peer_disconnected */ u8 *towire_connectctl_peer_disconnected(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED) { fprintf(stderr, "towire_connectctl_peer_disconnected called!\n"); abort(); }