From c7c1a043e875d00ba1700cf5abef7eb0554fe26a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 25 Apr 2018 22:04:38 +0930 Subject: [PATCH 01/21] test_lightningd.py: make tests more robust by suppressing reconnects. Got some intermittant failures, mainly caused by the tests being slow enough that the peer reconnected. We should always suppress reconnection if we can, and not stress too much in the !DEVELOPER case where we can't. We should turn off dev-no-reconnect *always* unless told we will reconnect, and since we can't if !DEVELOPER, don't do the connection check there. Instead of adding an option to line_graph, we remove it in favor of connect (since we only use it with n=2 anyway). Signed-off-by: Rusty Russell --- tests/test_lightningd.py | 44 +++++++++++----------------------------- 1 file changed, 12 insertions(+), 32 deletions(-) diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 94f615029c23..e75469541b6d 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -130,6 +130,8 @@ def get_node(self, disconnect=None, options=None, may_fail=False, may_reconnect= daemon.env["LIGHTNINGD_DEV_MEMLEAK"] = "1" if VALGRIND: daemon.env["LIGHTNINGD_DEV_NO_BACKTRACE"] = "1" + if not may_reconnect: + daemon.opts["dev-no-reconnect"] = None if fake_bitcoin_cli: cli = os.path.join(lightning_dir, "fake-bitcoin-cli") @@ -243,7 +245,8 @@ def printCrashLog(self, node): return 1 if errors else 0 def checkReconnect(self, node): - if node.may_reconnect: + # Without DEVELOPER, we can't suppress reconnection. + if node.may_reconnect or not DEVELOPER: return 0 if node.daemon.is_in_log('Peer has reconnected'): return 1 @@ -277,16 +280,9 @@ def tearDown(self): class LightningDTests(BaseLightningDTests): - def connect(self): - # Better to have clear failure because they didn't reconnect, than - # catch it at the end that we had an unexpected reconnect. - if DEVELOPER: - opts = {'dev-no-reconnect': None} - else: - opts = None - - l1 = self.node_factory.get_node(options=opts) - l2 = self.node_factory.get_node(options=opts) + def connect(self, may_reconnect=False): + l1 = self.node_factory.get_node(may_reconnect=may_reconnect) + l2 = self.node_factory.get_node(may_reconnect=may_reconnect) ret = l1.rpc.connect(l2.info['id'], 'localhost', l2.info['port']) assert ret['id'] == l2.info['id'] @@ -332,21 +328,6 @@ def fund_channel(self, l1, l2, amount): decoded2 = bitcoind.rpc.decoderawtransaction(tx) raise ValueError("Can't find {} payment in {} (1={} 2={})".format(amount, tx, decoded, decoded2)) - def line_graph(self, n=2): - """Build a line graph of the specified length and fund it. - """ - nodes = [self.node_factory.get_node() for _ in range(n)] - - for i in range(len(nodes) - 1): - nodes[i].rpc.connect( - nodes[i + 1].info['id'], - 'localhost', - nodes[i + 1].info['port'] - ) - self.fund_channel(nodes[i], nodes[i + 1], 10**6) - - return nodes - def pay(self, lsrc, ldst, amt, label=None, async=False): if not label: label = ''.join(random.choice(string.ascii_letters + string.digits) for _ in range(20)) @@ -1262,9 +1243,7 @@ def test_closing(self): wait_forget_channels(l2) def test_closing_while_disconnected(self): - l1 = self.node_factory.get_node(may_reconnect=True) - l2 = self.node_factory.get_node(may_reconnect=True) - l1.rpc.connect(l2.info['id'], 'localhost', l2.info['port']) + l1, l2 = self.connect(may_reconnect=True) self.fund_channel(l1, l2, 10**6) self.pay(l1, l2, 200000000) @@ -2254,7 +2233,8 @@ def test_permfail_htlc_out(self): wait_forget_channels(l2) def test_gossip_jsonrpc(self): - l1, l2 = self.line_graph(n=2) + l1, l2 = self.connect() + self.fund_channel(l1, l2, 10**6) # Shouldn't send announce signatures until 6 deep. assert not l1.daemon.is_in_log('peer_out WIRE_ANNOUNCEMENT_SIGNATURES') @@ -3962,8 +3942,8 @@ def test_waitanyinvoice_reversed(self): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1 for --dev-broadcast-interval") def test_channel_reenable(self): - l1, l2 = self.line_graph(n=2) - l1.may_reconnect = l2.may_reconnect = True + l1, l2 = self.connect(may_reconnect=True) + self.fund_channel(l1, l2, 10**6) l1.bitcoin.generate_block(6) l1.daemon.wait_for_log('Received node_announcement for node {}'.format(l2.info['id'])) From 3524a3244a2fb3f3439c9b10613e39178cd886c1 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 25 Apr 2018 22:05:38 +0930 Subject: [PATCH 02/21] gossipd: exponential backoff for reconnect (5 minute ceiling). Signed-off-by: Rusty Russell --- gossipd/gossip.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index d795da26209f..a2f56da83489 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -1584,10 +1584,17 @@ static void connect_failed(struct io_conn *conn, struct reaching *reach) NULL, &reach->id, diff, reach->attempts, false))); tal_free(reach); } else { - status_trace("Failed connected out for %s, will try again", - type_to_string(tmpctx, struct pubkey, &reach->id)); - /* FIXME: Configurable timer! */ - new_reltimer(&reach->daemon->timers, reach, time_from_sec(5), + unsigned int secs; + + /* Exponential backoff, then every 5 minutes */ + if (reach->attempts < 9) + secs = 1 << reach->attempts; + else + secs = 300; + status_trace("Failed connected out for %s, will try again in %u seconds", + type_to_string(tmpctx, struct pubkey, &reach->id), + secs); + new_reltimer(&reach->daemon->timers, reach, time_from_sec(secs), try_connect, reach); } } From f000f523a12fc834c5c9894112bc03bb396f15a2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 25 Apr 2018 22:06:38 +0930 Subject: [PATCH 03/21] gossipd: explicitly track which peers are important. These don't have a maximum number of reconnect attempts, and ensure that we try to reconnect when the peer dies. Signed-off-by: Rusty Russell --- gossipd/gossip.c | 89 ++++++++++++++++++++++++++----------- gossipd/gossip_wire.csv | 5 +++ lightningd/gossip_control.c | 1 + 3 files changed, 70 insertions(+), 25 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index a2f56da83489..7e3b886c6c75 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -102,12 +102,12 @@ struct reaching { /* Did we succeed? */ bool succeeded; + /* Is this an important peer to keep connected? */ + bool keep_connected; + /* How many times have we attempted to connect? */ u32 attempts; - /* How many times to attempt */ - u32 max_attempts; - /* Timestamp of the first attempt */ u32 first_attempt; }; @@ -158,7 +158,7 @@ struct peer { bool gossip_sync; /* If we die, should we reach again? */ - bool reach_again; + bool keep_connected; /* Only one of these is set: */ struct local_peer_state *local; @@ -179,13 +179,14 @@ static struct io_plan *peer_start_gossip(struct io_conn *conn, struct peer *peer); static bool send_peer_with_fds(struct peer *peer, const u8 *msg); static void wake_pkt_out(struct peer *peer); -static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id); +static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id, + bool keep_connected); static void destroy_peer(struct peer *peer) { list_del_from(&peer->daemon->peers, &peer->list); - if (peer->reach_again) - try_reach_peer(peer->daemon, &peer->id); + if (peer->keep_connected) + try_reach_peer(peer->daemon, &peer->id, true); } static struct peer *find_peer(struct daemon *daemon, const struct pubkey *id) @@ -242,7 +243,7 @@ static struct peer *new_peer(const tal_t *ctx, peer->daemon = daemon; peer->local = new_local_peer_state(peer, cs); peer->remote = NULL; - peer->reach_again = false; + peer->keep_connected = false; return peer; } @@ -273,18 +274,21 @@ static struct reaching *find_reaching(struct daemon *daemon, return NULL; } -static void reached_peer(struct daemon *daemon, const struct pubkey *id, - struct io_conn *conn) +static void reached_peer(struct peer *peer, struct io_conn *conn) { - struct reaching *r = find_reaching(daemon, id); + struct reaching *r = find_reaching(peer->daemon, &peer->id); if (!r) return; + /* If this peer was important, remember, so we reconnect. */ + if (r->keep_connected) + peer->keep_connected = true; + /* OK, we've reached the peer successfully, stop retrying. */ /* Don't free conn with reach. */ - tal_steal(daemon, conn); + tal_steal(peer->daemon, conn); /* Don't call connect_failed */ io_set_finish(conn, NULL, NULL); @@ -350,7 +354,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn, return io_close(conn); } - reached_peer(peer->daemon, &peer->id, conn); + reached_peer(peer, conn); /* BOLT #7: * @@ -1574,7 +1578,7 @@ static void connect_failed(struct io_conn *conn, struct reaching *reach) u32 diff = time_now().ts.tv_sec - reach->first_attempt; reach->attempts++; - if (reach->attempts >= reach->max_attempts) { + if (!reach->keep_connected && reach->attempts >= 10) { status_info("Failed to connect after %d attempts, giving up " "after %d seconds", reach->attempts, diff); @@ -1724,25 +1728,31 @@ static void try_connect(struct reaching *reach) } /* Returns true if we're already connected. */ -static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id) +static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id, + bool keep_connected) { struct reaching *reach; struct peer *peer; - if (find_reaching(daemon, id)) { - /* FIXME: Perhaps kick timer in this case? */ + reach = find_reaching(daemon, id); + if (reach) { + /* May not have been important before */ + if (keep_connected) + reach->keep_connected = true; status_trace("try_reach_peer: already trying to reach %s", type_to_string(tmpctx, struct pubkey, id)); return false; } - /* Master might find out before we do that a peer is dead; if we - * seem to be connected just mark it for reconnect. */ + /* Master might find out before we do that a peer is dead. */ peer = find_peer(daemon, id); if (peer) { - status_trace("reach_peer: have %s, will retry if it dies", - type_to_string(tmpctx, struct pubkey, id)); - peer->reach_again = true; + /* May not have been important before */ + if (keep_connected) + peer->keep_connected = true; + status_trace("reach_peer: have peer %s%s", + type_to_string(tmpctx, struct pubkey, id), + peer->keep_connected ? " (will retry if it dies)" : ""); return true; } @@ -1752,7 +1762,7 @@ static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id) reach->id = *id; reach->first_attempt = time_now().ts.tv_sec; reach->attempts = 0; - reach->max_attempts = 10; + reach->keep_connected = keep_connected; list_add_tail(&daemon->reaching, &reach->list); tal_add_destructor(reach, destroy_reaching); @@ -1760,7 +1770,6 @@ static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id) return false; } -/* This catches all kinds of failures, like network errors. */ static struct io_plan *reach_peer(struct io_conn *conn, struct daemon *daemon, const u8 *msg) { @@ -1770,7 +1779,7 @@ static struct io_plan *reach_peer(struct io_conn *conn, master_badmsg(WIRE_GOSSIPCTL_REACH_PEER, msg); /* Master can't check this itself, because that's racy. */ - if (try_reach_peer(daemon, &id)) { + if (try_reach_peer(daemon, &id, false)) { daemon_conn_send(&daemon->master, take(towire_gossip_peer_already_connected(NULL, &id))); @@ -1796,6 +1805,33 @@ static struct io_plan *addr_hint(struct io_conn *conn, return daemon_conn_read_next(conn, &daemon->master); } +static struct io_plan *peer_important(struct io_conn *conn, + struct daemon *daemon, const u8 *msg) +{ + struct pubkey id; + bool important; + struct reaching *r; + struct peer *p; + + if (!fromwire_gossipctl_peer_important(msg, &id, &important)) + master_badmsg(WIRE_GOSSIPCTL_REACH_PEER, msg); + + r = find_reaching(daemon, &id); + p = find_peer(daemon, &id); + + /* Override keep_connected flag everywhere */ + if (r) + r->keep_connected = important; + if (p) + p->keep_connected = important; + + /* If it's important and we're not connected/connecting, do so now. */ + if (important && !r && !p) + try_reach_peer(daemon, &id, true); + + return daemon_conn_read_next(conn, &daemon->master); +} + static struct io_plan *get_peers(struct io_conn *conn, struct daemon *daemon, const u8 *msg) { @@ -2044,6 +2080,9 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_GOSSIPCTL_PEER_ADDRHINT: return addr_hint(conn, daemon, master->msg_in); + case WIRE_GOSSIPCTL_PEER_IMPORTANT: + return peer_important(conn, daemon, master->msg_in); + case WIRE_GOSSIP_GETPEERS_REQUEST: return get_peers(conn, daemon, master->msg_in); diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 412e6ce16b23..3399b7bd7444 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -32,6 +32,11 @@ gossipctl_peer_addrhint,,addr,struct wireaddr gossipctl_reach_peer,3001 gossipctl_reach_peer,,id,struct pubkey +# Master -> gossipd: try to always maintain connection to this peer (or not) +gossipctl_peer_important,3010 +gossipctl_peer_important,,id,struct pubkey +gossipctl_peer_important,,important,bool + # Gossipd -> master: we got a peer. Two fds: peer and gossip gossip_peer_connected,3002 gossip_peer_connected,,id,struct pubkey diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 48036ff9086e..f81aac36ca29 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -137,6 +137,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) case WIRE_GOSSIP_ROUTING_FAILURE: case WIRE_GOSSIP_MARK_CHANNEL_UNROUTABLE: case WIRE_GOSSIPCTL_PEER_DISCONNECT: + case WIRE_GOSSIPCTL_PEER_IMPORTANT: /* This is a reply, so never gets through to here. */ case WIRE_GOSSIPCTL_INIT_REPLY: case WIRE_GOSSIP_GET_UPDATE_REPLY: From 832fbf387514e6bf479700eb2ab3e4cc9fa1d99a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 25 Apr 2018 22:07:38 +0930 Subject: [PATCH 04/21] openingd: tell gossipd that the peer is important once funding tx in place. And on channel_fail_permanent and closing (the two places we drop to chain), we tell gossipd it's no longer important. Fixes: #1316 Signed-off-by: Rusty Russell --- gossipd/gossip.c | 15 ++++++++++++++- gossipd/gossip_wire.csv | 2 ++ lightningd/channel.c | 9 +++++---- lightningd/closing_control.c | 5 +++++ lightningd/gossip_control.c | 7 ++++++- lightningd/opening_control.c | 21 +++++++++++++++++++++ lightningd/peer_control.c | 2 +- lightningd/peer_control.h | 3 --- wallet/test/run-wallet.c | 3 +++ 9 files changed, 57 insertions(+), 10 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 7e3b886c6c75..d263636bd228 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -84,6 +84,9 @@ struct daemon { /* To make sure our node_announcement timestamps increase */ u32 last_announce_timestamp; + + /* Only matters if DEVELOPER defined */ + bool no_reconnect; }; /* Peers we're trying to reach. */ @@ -185,6 +188,10 @@ static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id, static void destroy_peer(struct peer *peer) { list_del_from(&peer->daemon->peers, &peer->list); +#if DEVELOPER + if (peer->daemon->no_reconnect) + return; +#endif if (peer->keep_connected) try_reach_peer(peer->daemon, &peer->id, true); } @@ -1498,7 +1505,7 @@ static struct io_plan *gossip_init(struct daemon_conn *master, daemon, msg, &daemon->broadcast_interval, &chain_hash, &daemon->id, &port, &daemon->globalfeatures, &daemon->localfeatures, &daemon->wireaddrs, daemon->rgb, - daemon->alias, &update_channel_interval)) { + daemon->alias, &update_channel_interval, &daemon->no_reconnect)) { master_badmsg(WIRE_GOSSIPCTL_INIT, msg); } /* Prune time is twice update time */ @@ -1825,6 +1832,12 @@ static struct io_plan *peer_important(struct io_conn *conn, if (p) p->keep_connected = important; +#if DEVELOPER + /* With --dev-no-reconnect, we only want explicit connects */ + if (daemon->no_reconnect) + important = false; +#endif + /* If it's important and we're not connected/connecting, do so now. */ if (important && !r && !p) try_reach_peer(daemon, &id, true); diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 3399b7bd7444..32600c543c1b 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -18,6 +18,8 @@ gossipctl_init,,wireaddrs,num_wireaddrs*struct wireaddr gossipctl_init,,rgb,3*u8 gossipctl_init,,alias,32*u8 gossipctl_init,,update_channel_interval,u32 +# DEVELOPER only +gossipctl_init,,no_reconnect,bool # Gossipd->master, I am ready. gossipctl_init_reply,3100 diff --git a/lightningd/channel.c b/lightningd/channel.c index 952e046b75b6..c2ee3d678586 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -309,6 +309,7 @@ void channel_fail_permanent(struct channel *channel, const char *fmt, ...) va_list ap; char *why; struct channel_id cid; + u8 *msg; va_start(ap, fmt); why = tal_vfmt(channel, fmt, ap); @@ -333,6 +334,10 @@ void channel_fail_permanent(struct channel *channel, const char *fmt, ...) channel->error = towire_errorfmt(channel, &cid, "%s", why); } + /* Tell gossipd we no longer need to keep connection to this peer */ + msg = towire_gossipctl_peer_important(NULL, &channel->peer->id, false); + subd_send_msg(ld->gossip, take(msg)); + channel_set_owner(channel, NULL); /* Drop non-cooperatively (unilateral) to chain. */ drop_to_chain(ld, channel, false); @@ -397,8 +402,4 @@ void channel_fail_transient(struct channel *channel, const char *fmt, ...) #endif channel_set_owner(channel, NULL); - - /* Reconnect unless we've dropped/are dropping to chain. */ - if (channel_active(channel)) - try_reconnect(channel->peer); } diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 225c888b4818..ed9357373cf7 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -93,6 +94,10 @@ static void peer_closing_complete(struct channel *channel, const u8 *msg) if (channel->state == CLOSINGD_COMPLETE) return; + /* Tell gossipd we no longer need to keep connection to this peer */ + msg = towire_gossipctl_peer_important(NULL, &channel->peer->id, false); + subd_send_msg(channel->peer->ld->gossip, take(msg)); + /* Channel gets dropped to chain cooperatively. */ drop_to_chain(channel->peer->ld, channel, true); channel_set_state(channel, CLOSINGD_SIGEXCHANGE, CLOSINGD_COMPLETE); diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index f81aac36ca29..6d62659c70a4 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -194,6 +194,11 @@ void gossip_init(struct lightningd *ld) u8 *msg; int hsmfd; u64 capabilities = HSM_CAP_ECDH | HSM_CAP_SIGN_GOSSIP; +#if DEVELOPER + bool no_reconnect = ld->no_reconnect; +#else + bool no_reconnect = false; +#endif msg = towire_hsm_client_hsmfd(tmpctx, &ld->id, capabilities); if (!wire_sync_write(ld->hsm_fd, msg)) @@ -218,7 +223,7 @@ void gossip_init(struct lightningd *ld) &get_chainparams(ld)->genesis_blockhash, &ld->id, ld->portnum, get_offered_global_features(tmpctx), get_offered_local_features(tmpctx), ld->wireaddrs, ld->rgb, - ld->alias, ld->config.channel_update_interval); + ld->alias, ld->config.channel_update_interval, no_reconnect); subd_req(ld->gossip, ld->gossip, msg, -1, 0, gossip_init_done, NULL); /* Wait for init done */ diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index b5434db78f37..18953540b6b8 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -255,6 +255,23 @@ static void funding_broadcast_failed(struct channel *channel, exitstatus, err); } +static void tell_gossipd_peer_is_important(struct lightningd *ld, + const struct channel *channel) +{ + u8 *msg; + +#if DEVELOPER + /* Don't schedule an attempt if we disabled reconnections with + * the `--dev-no-reconnect` flag */ + if (ld->no_reconnect) + return; +#endif /* DEVELOPER */ + + /* Tell gossipd we need to keep connection to this peer */ + msg = towire_gossipctl_peer_important(NULL, &channel->peer->id, true); + subd_send_msg(ld->gossip, take(msg)); +} + static void opening_funder_finished(struct subd *openingd, const u8 *resp, const int *fds, struct funding_channel *fc) @@ -410,6 +427,8 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, channel_watch_funding(ld, channel); + tell_gossipd_peer_is_important(ld, channel); + /* Start normal channel daemon. */ peer_start_channeld(channel, &cs, gossip_index, fds[0], fds[1], NULL, false); @@ -515,6 +534,8 @@ static void opening_fundee_finished(struct subd *openingd, channel_watch_funding(ld, channel); + tell_gossipd_peer_is_important(ld, channel); + /* On to normal operation! */ peer_start_channeld(channel, &cs, gossip_index, fds[0], fds[1], funding_signed, false); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index ae74e54e9da3..09727c4eeabe 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1090,7 +1090,7 @@ static const struct json_command close_command = { }; AUTODATA(json_command, &close_command); -void try_reconnect(struct peer *peer) +static void try_reconnect(struct peer *peer) { struct lightningd *ld = peer->ld; u8 *msg; diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 4cac8e46d480..dd8b524f11d7 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -61,9 +61,6 @@ struct peer *new_peer(struct lightningd *ld, u64 dbid, /* Also removes from db. */ void delete_peer(struct peer *peer); -/* Tell gossipd to try to reconnect (unless --dev-no-reconnect) */ -void try_reconnect(struct peer *peer); - struct peer *peer_by_id(struct lightningd *ld, const struct pubkey *id); struct peer *peer_from_json(struct lightningd *ld, const char *buffer, diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 6f4514832602..641526f92197 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -382,6 +382,9 @@ u8 *towire_gossipctl_peer_addrhint(const tal_t *ctx UNNEEDED, const struct pubke /* Generated stub for towire_gossipctl_peer_disconnect */ u8 *towire_gossipctl_peer_disconnect(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED) { fprintf(stderr, "towire_gossipctl_peer_disconnect called!\n"); abort(); } +/* Generated stub for towire_gossipctl_peer_important */ +u8 *towire_gossipctl_peer_important(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED, bool important UNNEEDED) +{ fprintf(stderr, "towire_gossipctl_peer_important called!\n"); abort(); } /* Generated stub for towire_gossipctl_reach_peer */ u8 *towire_gossipctl_reach_peer(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED) { fprintf(stderr, "towire_gossipctl_reach_peer called!\n"); abort(); } From e0686ba102a1f8686404807d435cc4a7e8199e0c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 25 Apr 2018 22:08:38 +0930 Subject: [PATCH 05/21] lightningd: tell gossipd that peers we load from db are important. Signed-off-by: Rusty Russell --- lightningd/opening_control.c | 4 ++-- lightningd/opening_control.h | 3 +++ lightningd/peer_control.c | 23 ++++------------------- wallet/test/run-wallet.c | 4 ++++ 4 files changed, 13 insertions(+), 21 deletions(-) diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 18953540b6b8..2f894563de2d 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -255,8 +255,8 @@ static void funding_broadcast_failed(struct channel *channel, exitstatus, err); } -static void tell_gossipd_peer_is_important(struct lightningd *ld, - const struct channel *channel) +void tell_gossipd_peer_is_important(struct lightningd *ld, + const struct channel *channel) { u8 *msg; diff --git a/lightningd/opening_control.h b/lightningd/opening_control.h index 82e4591db548..d57b7050f5ea 100644 --- a/lightningd/opening_control.h +++ b/lightningd/opening_control.h @@ -41,4 +41,7 @@ bool handle_opening_channel(struct lightningd *ld, void kill_uncommitted_channel(struct uncommitted_channel *uc, const char *why); + +void tell_gossipd_peer_is_important(struct lightningd *ld, + const struct channel *channel); #endif /* LIGHTNING_LIGHTNINGD_OPENING_CONTROL_H */ diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 09727c4eeabe..dc3dc99c0e9a 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1090,22 +1090,6 @@ static const struct json_command close_command = { }; AUTODATA(json_command, &close_command); -static void try_reconnect(struct peer *peer) -{ - struct lightningd *ld = peer->ld; - u8 *msg; - -#if DEVELOPER - /* Don't schedule an attempt if we disabled reconnections with - * the `--dev-no-reconnect` flag */ - if (ld->no_reconnect) - return; -#endif /* DEVELOPER */ - - msg = towire_gossipctl_reach_peer(NULL, &peer->id); - subd_send_msg(ld->gossip, take(msg)); -} - static void activate_peer(struct peer *peer) { u8 *msg; @@ -1116,10 +1100,11 @@ static void activate_peer(struct peer *peer) msg = towire_gossipctl_peer_addrhint(peer, &peer->id, &peer->addr); subd_send_msg(peer->ld->gossip, take(msg)); - /* We can only have one active channel: reconnect if not already. */ + /* We can only have one active channel: make sure gossipd + * knows to reconnect. */ channel = peer_active_channel(peer); - if (channel && !channel->owner) - try_reconnect(peer); + if (channel) + tell_gossipd_peer_is_important(ld, channel); list_for_each(&peer->channels, channel, list) { /* Watching lockin may be unnecessary, but it's harmless. */ diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 641526f92197..0dc51f5604c4 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -359,6 +359,10 @@ void subd_send_fd(struct subd *sd UNNEEDED, int fd UNNEEDED) /* Generated stub for subd_send_msg */ void subd_send_msg(struct subd *sd UNNEEDED, const u8 *msg_out UNNEEDED) { fprintf(stderr, "subd_send_msg called!\n"); abort(); } +/* Generated stub for tell_gossipd_peer_is_important */ +void tell_gossipd_peer_is_important(struct lightningd *ld UNNEEDED, + const struct channel *channel UNNEEDED) +{ fprintf(stderr, "tell_gossipd_peer_is_important called!\n"); abort(); } /* Generated stub for towire_channel_dev_reenable_commit */ u8 *towire_channel_dev_reenable_commit(const tal_t *ctx UNNEEDED) { fprintf(stderr, "towire_channel_dev_reenable_commit called!\n"); abort(); } From 7e90e4e7745962f6d335d3695c49c90774b7efc1 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 25 Apr 2018 22:09:38 +0930 Subject: [PATCH 06/21] gossipd: maintain a separate structure to track important peers. Rather than using a flag in reaching/peer; we make it self-contained as the next patch puts it straight into a timer callback. Also remove unused 'succeeded' field from struct peer. Signed-off-by: Rusty Russell --- gossipd/gossip.c | 144 +++++++++++++++++++++++++++-------------------- 1 file changed, 83 insertions(+), 61 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index d263636bd228..850a9805a67c 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -52,6 +53,36 @@ #define HSM_FD 3 +struct important_peerid { + struct daemon *daemon; + + struct pubkey id; +}; + +/* We keep a set of peer ids we're always trying to reach. */ +static const struct pubkey * +important_peerid_keyof(const struct important_peerid *imp) +{ + return &imp->id; +} + +static bool important_peerid_eq(const struct important_peerid *imp, + const struct pubkey *key) +{ + return pubkey_eq(&imp->id, key); +} + +static size_t important_peerid_hash(const struct pubkey *id) +{ + return siphash24(siphash_seed(), id, sizeof(*id)); +} + +HTABLE_DEFINE_TYPE(struct important_peerid, + important_peerid_keyof, + important_peerid_hash, + important_peerid_eq, + important_peerid_map); + struct daemon { /* Who am I? */ struct pubkey id; @@ -75,6 +106,9 @@ struct daemon { u32 broadcast_interval; + /* Important peers */ + struct important_peerid_map important_peerids; + /* Local and global features to offer to peers. */ u8 *localfeatures, *globalfeatures; @@ -102,12 +136,6 @@ struct reaching { /* Where I'm reaching to. */ struct wireaddr addr; - /* Did we succeed? */ - bool succeeded; - - /* Is this an important peer to keep connected? */ - bool keep_connected; - /* How many times have we attempted to connect? */ u32 attempts; @@ -160,9 +188,6 @@ struct peer { /* Is it time to continue the staggered broadcast? */ bool gossip_sync; - /* If we die, should we reach again? */ - bool keep_connected; - /* Only one of these is set: */ struct local_peer_state *local; struct daemon_conn *remote; @@ -182,18 +207,17 @@ static struct io_plan *peer_start_gossip(struct io_conn *conn, struct peer *peer); static bool send_peer_with_fds(struct peer *peer, const u8 *msg); static void wake_pkt_out(struct peer *peer); -static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id, - bool keep_connected); +static void retry_important(struct important_peerid *imp); static void destroy_peer(struct peer *peer) { + struct important_peerid *imp; + list_del_from(&peer->daemon->peers, &peer->list); -#if DEVELOPER - if (peer->daemon->no_reconnect) - return; -#endif - if (peer->keep_connected) - try_reach_peer(peer->daemon, &peer->id, true); + imp = important_peerid_map_get(&peer->daemon->important_peerids, + &peer->id); + if (imp) + retry_important(imp); } static struct peer *find_peer(struct daemon *daemon, const struct pubkey *id) @@ -250,7 +274,6 @@ static struct peer *new_peer(const tal_t *ctx, peer->daemon = daemon; peer->local = new_local_peer_state(peer, cs); peer->remote = NULL; - peer->keep_connected = false; return peer; } @@ -283,17 +306,12 @@ static struct reaching *find_reaching(struct daemon *daemon, static void reached_peer(struct peer *peer, struct io_conn *conn) { + /* OK, we've reached the peer successfully, stop retrying. */ struct reaching *r = find_reaching(peer->daemon, &peer->id); if (!r) return; - /* If this peer was important, remember, so we reconnect. */ - if (r->keep_connected) - peer->keep_connected = true; - - /* OK, we've reached the peer successfully, stop retrying. */ - /* Don't free conn with reach. */ tal_steal(peer->daemon, conn); /* Don't call connect_failed */ @@ -1585,7 +1603,8 @@ static void connect_failed(struct io_conn *conn, struct reaching *reach) u32 diff = time_now().ts.tv_sec - reach->first_attempt; reach->attempts++; - if (!reach->keep_connected && reach->attempts >= 10) { + if (!important_peerid_map_get(&reach->daemon->important_peerids, &reach->id) + && reach->attempts >= 10) { status_info("Failed to connect after %d attempts, giving up " "after %d seconds", reach->attempts, diff); @@ -1735,17 +1754,13 @@ static void try_connect(struct reaching *reach) } /* Returns true if we're already connected. */ -static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id, - bool keep_connected) +static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id) { struct reaching *reach; struct peer *peer; reach = find_reaching(daemon, id); if (reach) { - /* May not have been important before */ - if (keep_connected) - reach->keep_connected = true; status_trace("try_reach_peer: already trying to reach %s", type_to_string(tmpctx, struct pubkey, id)); return false; @@ -1754,22 +1769,16 @@ static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id, /* Master might find out before we do that a peer is dead. */ peer = find_peer(daemon, id); if (peer) { - /* May not have been important before */ - if (keep_connected) - peer->keep_connected = true; - status_trace("reach_peer: have peer %s%s", - type_to_string(tmpctx, struct pubkey, id), - peer->keep_connected ? " (will retry if it dies)" : ""); + status_trace("reach_peer: have peer %s", + type_to_string(tmpctx, struct pubkey, id)); return true; } reach = tal(daemon, struct reaching); - reach->succeeded = false; reach->daemon = daemon; reach->id = *id; reach->first_attempt = time_now().ts.tv_sec; reach->attempts = 0; - reach->keep_connected = keep_connected; list_add_tail(&daemon->reaching, &reach->list); tal_add_destructor(reach, destroy_reaching); @@ -1777,6 +1786,18 @@ static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id, return false; } +/* Reconnect to peer. */ +static void retry_important(struct important_peerid *imp) +{ +#if DEVELOPER + /* With --dev-no-reconnect, we only want explicit + * connects */ + if (imp->daemon->no_reconnect) + return; +#endif + try_reach_peer(imp->daemon, &imp->id); +} + static struct io_plan *reach_peer(struct io_conn *conn, struct daemon *daemon, const u8 *msg) { @@ -1786,7 +1807,7 @@ static struct io_plan *reach_peer(struct io_conn *conn, master_badmsg(WIRE_GOSSIPCTL_REACH_PEER, msg); /* Master can't check this itself, because that's racy. */ - if (try_reach_peer(daemon, &id, false)) { + if (try_reach_peer(daemon, &id)) { daemon_conn_send(&daemon->master, take(towire_gossip_peer_already_connected(NULL, &id))); @@ -1817,30 +1838,30 @@ static struct io_plan *peer_important(struct io_conn *conn, { struct pubkey id; bool important; - struct reaching *r; - struct peer *p; + struct important_peerid *imp; if (!fromwire_gossipctl_peer_important(msg, &id, &important)) - master_badmsg(WIRE_GOSSIPCTL_REACH_PEER, msg); - - r = find_reaching(daemon, &id); - p = find_peer(daemon, &id); - - /* Override keep_connected flag everywhere */ - if (r) - r->keep_connected = important; - if (p) - p->keep_connected = important; - -#if DEVELOPER - /* With --dev-no-reconnect, we only want explicit connects */ - if (daemon->no_reconnect) - important = false; -#endif - - /* If it's important and we're not connected/connecting, do so now. */ - if (important && !r && !p) - try_reach_peer(daemon, &id, true); + master_badmsg(WIRE_GOSSIPCTL_PEER_IMPORTANT, msg); + + imp = important_peerid_map_get(&daemon->important_peerids, &id); + if (important) { + if (!imp) { + imp = tal(daemon, struct important_peerid); + imp->id = id; + imp->daemon = daemon; + important_peerid_map_add(&daemon->important_peerids, + imp); + /* Start trying to reaching it now. */ + retry_important(imp); + } + } else { + if (imp) { + important_peerid_map_del(&daemon->important_peerids, + imp); + /* Stop trying to reach it (if we are) */ + tal_free(find_reaching(daemon, &imp->id)); + } + } return daemon_conn_read_next(conn, &daemon->master); } @@ -2163,6 +2184,7 @@ int main(int argc, char *argv[]) list_head_init(&daemon->peers); list_head_init(&daemon->reaching); list_head_init(&daemon->addrhints); + important_peerid_map_init(&daemon->important_peerids); timers_init(&daemon->timers, time_mono()); daemon->broadcast_interval = 30000; daemon->last_announce_timestamp = 0; From b40b8f0428907013bb5c83ced72b4309a74462f0 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:20:58 +0930 Subject: [PATCH 07/21] gossipd: keep reaching struct only when we're actively connecting, and don't retry 1. Lifetime of 'struct reaching' now only while we're actively doing connect. 2. Always free after a single attempt: if it's an important peer, retry on a timer. 3. Have a single response message to master, rather than relying on peer_connected on success and other msgs on failure. 4. If we are actively connecting and we get another command for the same id, just increment the counter The result is much simpler in the master daemon, and much nicer for reconnection: if they say to connect they get an immediate response, rather than waiting for 10 retries. Even if it's an important peer, it fires off another reconnect attempt, unless it's actively connecting now. This removes exponential backoff: that's restored in next patch. It also doesn't handle multiple addresses for a single peer. Signed-off-by: Rusty Russell --- gossipd/gossip.c | 200 +++++++++++++++++------------------ gossipd/gossip_wire.csv | 27 +++-- lightningd/connect_control.c | 92 ++++++---------- lightningd/connect_control.h | 10 +- lightningd/gossip_control.c | 11 +- lightningd/peer_control.c | 25 +---- tests/test_lightningd.py | 49 ++++++++- wallet/test/run-wallet.c | 14 --- 8 files changed, 193 insertions(+), 235 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 850a9805a67c..fdf968a65bfa 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -53,6 +53,7 @@ #define HSM_FD 3 +/* We put everything in this struct (redundantly) to pass it to timer cb */ struct important_peerid { struct daemon *daemon; @@ -133,14 +134,11 @@ struct reaching { /* The ID of the peer (not necessarily unique, in transit!) */ struct pubkey id; - /* Where I'm reaching to. */ + /* FIXME: Support multiple address. */ struct wireaddr addr; - /* How many times have we attempted to connect? */ - u32 attempts; - - /* Timestamp of the first attempt */ - u32 first_attempt; + /* How many (if any) connect commands are waiting for the result. */ + size_t num_master_responses; }; /* Things we need when we're talking direct to the peer. */ @@ -306,17 +304,24 @@ static struct reaching *find_reaching(struct daemon *daemon, static void reached_peer(struct peer *peer, struct io_conn *conn) { - /* OK, we've reached the peer successfully, stop retrying. */ + /* OK, we've reached the peer successfully, tell everyone. */ struct reaching *r = find_reaching(peer->daemon, &peer->id); + u8 *msg; if (!r) return; - /* Don't free conn with reach. */ - tal_steal(peer->daemon, conn); /* Don't call connect_failed */ io_set_finish(conn, NULL, NULL); + /* Don't free conn with reach */ + tal_steal(peer->daemon, conn); + + /* Tell any connect commands what happened. */ + msg = towire_gossipctl_connect_to_peer_result(r, &r->id, true, ""); + for (size_t i = 0; i < r->num_master_responses; i++) + daemon_conn_send(&peer->daemon->master, msg); + tal_free(r); } @@ -1596,37 +1601,32 @@ static struct io_plan *connection_out(struct io_conn *conn, handshake_out_success, reach); } -static void try_connect(struct reaching *reach); - static void connect_failed(struct io_conn *conn, struct reaching *reach) { - u32 diff = time_now().ts.tv_sec - reach->first_attempt; - reach->attempts++; - - if (!important_peerid_map_get(&reach->daemon->important_peerids, &reach->id) - && reach->attempts >= 10) { - status_info("Failed to connect after %d attempts, giving up " - "after %d seconds", - reach->attempts, diff); - daemon_conn_send( - &reach->daemon->master, - take(towire_gossip_peer_connection_failed( - NULL, &reach->id, diff, reach->attempts, false))); - tal_free(reach); - } else { - unsigned int secs; - - /* Exponential backoff, then every 5 minutes */ - if (reach->attempts < 9) - secs = 1 << reach->attempts; - else - secs = 300; - status_trace("Failed connected out for %s, will try again in %u seconds", - type_to_string(tmpctx, struct pubkey, &reach->id), - secs); - new_reltimer(&reach->daemon->timers, reach, time_from_sec(secs), - try_connect, reach); + u8 *msg; + struct important_peerid *imp; + + /* Tell any connect commands what happened. */ + msg = towire_gossipctl_connect_to_peer_result(reach, &reach->id, + false, strerror(errno)); + for (size_t i = 0; i < reach->num_master_responses; i++) + daemon_conn_send(&reach->daemon->master, msg); + + status_trace("Failed connected out for %s", + type_to_string(tmpctx, struct pubkey, &reach->id)); + + /* If we want to keep trying, do so. */ + imp = important_peerid_map_get(&reach->daemon->important_peerids, + &reach->id); + if (imp) { + /* FIXME: Exponential backoff! */ + status_trace("...will try again in %u seconds", 5); + /* If important_id freed, this will be removed too */ + new_reltimer(&reach->daemon->timers, imp, + time_from_sec(5), retry_important, imp); } + tal_free(reach); + return; } static struct io_plan *conn_init(struct io_conn *conn, struct reaching *reach) @@ -1695,34 +1695,50 @@ seed_resolve_addr(const tal_t *ctx, const struct pubkey *id, const u16 port) } } -static void try_connect(struct reaching *reach) +static void try_reach_peer(struct daemon *daemon, const struct pubkey *id, + bool master_needs_response) { struct addrhint *a; int fd; + struct reaching *reach; + u8 *msg; + struct peer *peer = find_peer(daemon, id); - /* Already succeeded somehow? */ - if (find_peer(reach->daemon, &reach->id)) { - status_trace("Already reached %s, not retrying", - type_to_string(tmpctx, struct pubkey, &reach->id)); - tal_free(reach); + if (peer) { + status_debug("try_reach_peer: have peer %s", + type_to_string(tmpctx, struct pubkey, id)); + if (master_needs_response) { + msg = towire_gossipctl_connect_to_peer_result(NULL, id, + true, + ""); + daemon_conn_send(&daemon->master, take(msg)); + } return; } - a = find_addrhint(reach->daemon, &reach->id); + /* If we're trying to reach it right now, that's OK. */ + reach = find_reaching(daemon, id); + if (reach) { + /* Please tell us too. */ + if (master_needs_response) + reach->num_master_responses++; + return; + } + + a = find_addrhint(daemon, id); if (!a) - a = seed_resolve_addr(reach, &reach->id, 9735); + a = seed_resolve_addr(tmpctx, id, 9735); if (!a) { - status_info("No address known for %s, giving up", - type_to_string(tmpctx, struct pubkey, &reach->id)); - daemon_conn_send( - &reach->daemon->master, - take(towire_gossip_peer_connection_failed( - NULL, &reach->id, - time_now().ts.tv_sec - reach->first_attempt, - reach->attempts, true))); - tal_free(reach); + status_debug("No address known for %s, giving up", + type_to_string(tmpctx, struct pubkey, id)); + if (master_needs_response) { + msg = towire_gossipctl_connect_to_peer_result(NULL, id, + false, + "No address known, giving up"); + daemon_conn_send(&daemon->master, take(msg)); + } return; } @@ -1741,52 +1757,33 @@ static void try_connect(struct reaching *reach) } if (fd < 0) { - status_broken("Can't open %i socket for %s (%s), giving up", - a->addr.type, - type_to_string(tmpctx, struct pubkey, &reach->id), - strerror(errno)); - tal_free(reach); + char *err = tal_fmt(tmpctx, + "Can't open %i socket for %s (%s), giving up", + a->addr.type, + type_to_string(tmpctx, struct pubkey, id), + strerror(errno)); + status_debug("%s", err); + if (master_needs_response) { + msg = towire_gossipctl_connect_to_peer_result(NULL, id, + false, err); + daemon_conn_send(&daemon->master, take(msg)); + } return; } - reach->addr = a->addr; - io_new_conn(reach, fd, conn_init, reach); -} - -/* Returns true if we're already connected. */ -static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id) -{ - struct reaching *reach; - struct peer *peer; - - reach = find_reaching(daemon, id); - if (reach) { - status_trace("try_reach_peer: already trying to reach %s", - type_to_string(tmpctx, struct pubkey, id)); - return false; - } - - /* Master might find out before we do that a peer is dead. */ - peer = find_peer(daemon, id); - if (peer) { - status_trace("reach_peer: have peer %s", - type_to_string(tmpctx, struct pubkey, id)); - return true; - } - + /* Start connecting to it */ reach = tal(daemon, struct reaching); reach->daemon = daemon; reach->id = *id; - reach->first_attempt = time_now().ts.tv_sec; - reach->attempts = 0; + reach->addr = a->addr; + reach->num_master_responses = master_needs_response; list_add_tail(&daemon->reaching, &reach->list); tal_add_destructor(reach, destroy_reaching); - try_connect(reach); - return false; + io_new_conn(reach, fd, conn_init, reach); } -/* Reconnect to peer. */ +/* Called from timer, so needs single-arg declaration */ static void retry_important(struct important_peerid *imp) { #if DEVELOPER @@ -1795,24 +1792,18 @@ static void retry_important(struct important_peerid *imp) if (imp->daemon->no_reconnect) return; #endif - try_reach_peer(imp->daemon, &imp->id); + try_reach_peer(imp->daemon, &imp->id, false); } -static struct io_plan *reach_peer(struct io_conn *conn, - struct daemon *daemon, const u8 *msg) +static struct io_plan *connect_to_peer(struct io_conn *conn, + struct daemon *daemon, const u8 *msg) { struct pubkey id; - if (!fromwire_gossipctl_reach_peer(msg, &id)) - master_badmsg(WIRE_GOSSIPCTL_REACH_PEER, msg); - - /* Master can't check this itself, because that's racy. */ - if (try_reach_peer(daemon, &id)) { - daemon_conn_send(&daemon->master, - take(towire_gossip_peer_already_connected(NULL, - &id))); - } + if (!fromwire_gossipctl_connect_to_peer(msg, &id)) + master_badmsg(WIRE_GOSSIPCTL_CONNECT_TO_PEER, msg); + try_reach_peer(daemon, &id, true); return daemon_conn_read_next(conn, &daemon->master); } @@ -2108,8 +2099,8 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_GOSSIPCTL_HAND_BACK_PEER: return hand_back_peer(conn, daemon, master->msg_in); - case WIRE_GOSSIPCTL_REACH_PEER: - return reach_peer(conn, daemon, master->msg_in); + case WIRE_GOSSIPCTL_CONNECT_TO_PEER: + return connect_to_peer(conn, daemon, master->msg_in); case WIRE_GOSSIPCTL_PEER_ADDRHINT: return addr_hint(conn, daemon, master->msg_in); @@ -2149,8 +2140,7 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_GOSSIP_PING_REPLY: case WIRE_GOSSIP_RESOLVE_CHANNEL_REPLY: case WIRE_GOSSIP_PEER_CONNECTED: - case WIRE_GOSSIP_PEER_ALREADY_CONNECTED: - case WIRE_GOSSIP_PEER_CONNECTION_FAILED: + case WIRE_GOSSIPCTL_CONNECT_TO_PEER_RESULT: case WIRE_GOSSIP_PEER_NONGOSSIP: case WIRE_GOSSIP_GET_UPDATE: case WIRE_GOSSIP_GET_UPDATE_REPLY: diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 32600c543c1b..b9f48ed6a2c2 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -29,10 +29,18 @@ gossipctl_peer_addrhint,3014 gossipctl_peer_addrhint,,id,struct pubkey gossipctl_peer_addrhint,,addr,struct wireaddr -# Master -> gossipd: connect to a peer. We may get a peer_connected or -# peer_already_connected -gossipctl_reach_peer,3001 -gossipctl_reach_peer,,id,struct pubkey +# Master -> gossipd: connect to a peer. +gossipctl_connect_to_peer,3001 +gossipctl_connect_to_peer,,id,struct pubkey + +# Gossipd->master: result (not a reply since it can be out-of-order, but +# you will get one reply for every request). +gossipctl_connect_to_peer_result,3020 +gossipctl_connect_to_peer_result,,id,struct pubkey +# True it connected. +gossipctl_connect_to_peer_result,,connected,bool +# Otherwise, why we can't reach them. +gossipctl_connect_to_peer_result,,failreason,wirestring # Master -> gossipd: try to always maintain connection to this peer (or not) gossipctl_peer_important,3010 @@ -50,17 +58,6 @@ gossip_peer_connected,,gfeatures,gflen*u8 gossip_peer_connected,,lflen,u16 gossip_peer_connected,,lfeatures,lflen*u8 -# Gossipd -> master: you asked to reach a peer, we already had. -gossip_peer_already_connected,3015 -gossip_peer_already_connected,,id,struct pubkey - -# gossipd -> master: attempted to connect, unsuccessful, gave up -gossip_peer_connection_failed,3020 -gossip_peer_connection_failed,,id,struct pubkey -gossip_peer_connection_failed,,timeout,u32 -gossip_peer_connection_failed,,attempts,u32 -gossip_peer_connection_failed,,addr_unknown,bool - # Gossipd -> master: peer sent non-gossip packet. Two fds: peer and gossip gossip_peer_nongossip,3003 gossip_peer_nongossip,,id,struct pubkey diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c index ef213d0fe3fc..e9a3669db427 100644 --- a/lightningd/connect_control.c +++ b/lightningd/connect_control.c @@ -28,79 +28,51 @@ static struct connect *new_connect(struct lightningd *ld, struct connect *c = tal(cmd, struct connect); c->id = *id; c->cmd = cmd; - list_add(&ld->connects, &c->list); + list_add_tail(&ld->connects, &c->list); tal_add_destructor(c, destroy_connect); return c; } -void connect_succeeded(struct lightningd *ld, const struct pubkey *id) +/* Finds first command which matches. */ +static struct connect *find_connect(struct lightningd *ld, + const struct pubkey *id) { - struct connect *i, *next; + struct connect *i; - /* Careful! Completing command frees connect. */ - list_for_each_safe(&ld->connects, i, next, list) { - struct json_result *response; - - if (!pubkey_eq(&i->id, id)) - continue; - - response = new_json_result(i->cmd); - json_object_start(response, NULL); - json_add_pubkey(response, "id", id); - json_object_end(response); - command_success(i->cmd, response); - } -} - -void connect_failed(struct lightningd *ld, const struct pubkey *id, - const char *error) -{ - struct connect *i, *next; - - /* Careful! Completing command frees connect. */ - list_for_each_safe(&ld->connects, i, next, list) { + list_for_each(&ld->connects, i, list) { if (pubkey_eq(&i->id, id)) - command_fail(i->cmd, "%s", error); + return i; } + return NULL; } -void peer_connection_failed(struct lightningd *ld, const u8 *msg) +void gossip_connect_result(struct lightningd *ld, const u8 *msg) { struct pubkey id; - u32 attempts, timediff; - bool addr_unknown; - char *error; - - if (!fromwire_gossip_peer_connection_failed(msg, &id, &timediff, - &attempts, &addr_unknown)) - fatal( - "Gossip gave bad GOSSIP_PEER_CONNECTION_FAILED message %s", - tal_hex(msg, msg)); - - if (addr_unknown) { - error = tal_fmt( - msg, "No address known for node %s, please provide one", - type_to_string(msg, struct pubkey, &id)); - } else { - error = tal_fmt(msg, "Could not connect to %s after %d seconds and %d attempts", - type_to_string(msg, struct pubkey, &id), timediff, - attempts); - } - - connect_failed(ld, &id, error); -} + bool connected; + char *err; + struct connect *c; + + if (!fromwire_gossipctl_connect_to_peer_result(tmpctx, msg, + &id, + &connected, + &err)) + fatal("Gossip gave bad GOSSIPCTL_CONNECT_TO_PEER_RESULT message %s", + tal_hex(msg, msg)); -/* Gossipd tells us peer was already connected. */ -void peer_already_connected(struct lightningd *ld, const u8 *msg) -{ - struct pubkey id; - if (!fromwire_gossip_peer_already_connected(msg, &id)) - fatal("Gossip gave bad GOSSIP_PEER_ALREADY_CONNECTED message %s", - tal_hex(msg, msg)); + c = find_connect(ld, &id); + assert(c); - /* If we were waiting for connection, we succeeded. */ - connect_succeeded(ld, &id); + if (connected) { + struct json_result *response = new_json_result(c->cmd); + json_object_start(response, NULL); + json_add_pubkey(response, "id", &id); + json_object_end(response); + command_success(c->cmd, response); + } else { + command_fail(c->cmd, "%s", err); + } } static void json_connect(struct command *cmd, @@ -191,10 +163,10 @@ static void json_connect(struct command *cmd, } /* Now tell it to try reaching it. */ - msg = towire_gossipctl_reach_peer(cmd, &id); + msg = towire_gossipctl_connect_to_peer(NULL, &id); subd_send_msg(cmd->ld->gossip, take(msg)); - /* Leave this here for gossip_peer_connected */ + /* Leave this here for gossip_connect_result */ new_connect(cmd->ld, &id, cmd); command_still_pending(cmd); } diff --git a/lightningd/connect_control.h b/lightningd/connect_control.h index d1b3399727a3..23c8bdf5f730 100644 --- a/lightningd/connect_control.h +++ b/lightningd/connect_control.h @@ -5,14 +5,6 @@ struct lightningd; struct pubkey; -void connect_succeeded(struct lightningd *ld, const struct pubkey *id); -void connect_failed(struct lightningd *ld, const struct pubkey *id, - const char *error); - -/* Gossipd was unable to connect to the peer */ -void peer_connection_failed(struct lightningd *ld, const u8 *msg); - -/* This simply means we asked to reach a peer, but we already have it */ -void peer_already_connected(struct lightningd *ld, const u8 *msg); +void gossip_connect_result(struct lightningd *ld, const u8 *msg); #endif /* LIGHTNING_LIGHTNINGD_CONNECT_CONTROL_H */ diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 6d62659c70a4..062833431c52 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -125,7 +125,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) case WIRE_GOSSIP_GETPEERS_REQUEST: case WIRE_GOSSIP_PING: case WIRE_GOSSIP_RESOLVE_CHANNEL_REQUEST: - case WIRE_GOSSIPCTL_REACH_PEER: + case WIRE_GOSSIPCTL_CONNECT_TO_PEER: case WIRE_GOSSIPCTL_HAND_BACK_PEER: case WIRE_GOSSIPCTL_RELEASE_PEER: case WIRE_GOSSIPCTL_PEER_ADDRHINT: @@ -160,12 +160,6 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) return 2; peer_connected(gossip->ld, msg, fds[0], fds[1]); break; - case WIRE_GOSSIP_PEER_ALREADY_CONNECTED: - peer_already_connected(gossip->ld, msg); - break; - case WIRE_GOSSIP_PEER_CONNECTION_FAILED: - peer_connection_failed(gossip->ld, msg); - break; case WIRE_GOSSIP_PEER_NONGOSSIP: if (tal_count(fds) != 2) return 2; @@ -174,6 +168,9 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) case WIRE_GOSSIP_GET_TXOUT: get_txout(gossip, msg); break; + case WIRE_GOSSIPCTL_CONNECT_TO_PEER_RESULT: + gossip_connect_result(gossip->ld, msg); + break; } return 0; } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index dc3dc99c0e9a..71752d5dce9a 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -492,7 +492,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, peer_start_channeld(channel, &cs, gossip_index, peer_fd, gossip_fd, NULL, true); - goto connected; + return; case CLOSINGD_SIGEXCHANGE: /* Stop any existing daemon, without triggering error @@ -503,26 +503,17 @@ void peer_connected(struct lightningd *ld, const u8 *msg, peer_start_closingd(channel, &cs, gossip_index, peer_fd, gossip_fd, true, NULL); - goto connected; + return; } abort(); } return_to_gossipd: - /* Otherwise, we hand back to gossipd, to continue. */ - msg = towire_gossipctl_hand_back_peer(msg, &id, &cs, gossip_index, NULL); - subd_send_msg(ld->gossip, take(msg)); - subd_send_fd(ld->gossip, peer_fd); - subd_send_fd(ld->gossip, gossip_fd); - -connected: - /* If we were waiting for connection, we succeeded. */ - connect_succeeded(ld, &id); - return; + /* No err, all good. */ + error = NULL; send_error: /* Hand back to gossipd, with an error packet. */ - connect_failed(ld, &id, sanitize_error(msg, error, NULL)); msg = towire_gossipctl_hand_back_peer(msg, &id, &cs, gossip_index, error); subd_send_msg(ld->gossip, take(msg)); @@ -610,7 +601,6 @@ void peer_sent_nongossip(struct lightningd *ld, send_error: /* Hand back to gossipd, with an error packet. */ - connect_failed(ld, id, sanitize_error(tmpctx, error, NULL)); msg = towire_gossipctl_hand_back_peer(ld, id, cs, gossip_index, error); subd_send_msg(ld->gossip, take(msg)); subd_send_fd(ld->gossip, peer_fd); @@ -1068,13 +1058,6 @@ static void json_close(struct command *cmd, subd_send_msg(channel->owner, take(towire_channel_send_shutdown(channel))); } - /* If channel has no owner, it means the peer is disconnected, - * so make a nominal effort to contact it now. - */ - if (!channel->owner) - subd_send_msg(cmd->ld->gossip, - take(towire_gossipctl_reach_peer(NULL, - &channel->peer->id))); /* Register this command for later handling. */ register_close_command(cmd->ld, cmd, channel, timeout, force); diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index e75469541b6d..dc7529930e4b 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -78,7 +78,9 @@ def wait_forget_channels(node): """This node is closing all of its channels, check we are forgetting them """ node.daemon.wait_for_log(r'onchaind complete, forgetting peer') - assert node.rpc.listpeers()['peers'] == [] + # May have reconnected, but should merely be gossiping. + for peer in node.rpc.listpeers()['peers']: + assert peer['state'] == 'GOSSIPING' assert node.db_query("SELECT * FROM channels") == [] @@ -610,6 +612,11 @@ def test_connect(self): assert len(l1.rpc.listpeers()) == 1 assert len(l2.rpc.listpeers()) == 1 + # Should get reasonable error if unknown addr for peer. + self.assertRaisesRegex(ValueError, + "No address known", + l1.rpc.connect, '032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e') + def test_connect_standard_addr(self): """Test standard node@host:port address """ @@ -629,6 +636,34 @@ def test_connect_standard_addr(self): # ret = l1.rpc.connect("{}@[::1]:{}".format(l3.info['id'], l3.info['port'])) # assert ret['id'] == l3.info['id'] + def test_reconnect_channel_peers(self): + l1 = self.node_factory.get_node(may_reconnect=True) + l2 = self.node_factory.get_node(may_reconnect=True) + l1.rpc.connect(l2.info['id'], 'localhost', l2.info['port']) + + self.fund_channel(l1, l2, 10**6) + l2.stop() + l2.daemon.start() + + # Should reconnect. + wait_for(lambda: l1.rpc.listpeers(l2.info['id'])['peers'][0]['connected']) + wait_for(lambda: l2.rpc.listpeers(l1.info['id'])['peers'][0]['connected']) + # Connect command should succeed. + l1.rpc.connect(l2.info['id'], 'localhost', l2.info['port']) + + # Stop l2 and wait for l1 to notice. + l2.stop() + wait_for(lambda: not l1.rpc.listpeers(l2.info['id'])['peers'][0]['connected']) + + # Now should fail. + self.assertRaisesRegex(ValueError, + "Connection refused", + l1.rpc.connect, l2.info['id'], 'localhost', l2.info['port']) + + # It should now succeed when it restarts. + l2.daemon.start() + l1.rpc.connect(l2.info['id'], 'localhost', l2.info['port']) + def test_balance(self): l1, l2 = self.connect() @@ -2955,18 +2990,24 @@ def test_htlc_in_timeout(self): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_disconnect(self): - # These should all make us fail, and retry. - # FIXME: Configure short timeout for reconnect! + # These should all make us fail disconnects = ['-WIRE_INIT', '@WIRE_INIT', '+WIRE_INIT'] l1 = self.node_factory.get_node(disconnect=disconnects) l2 = self.node_factory.get_node() + + self.assertRaises(ValueError, l1.rpc.connect, + l2.info['id'], 'localhost', l2.info['port']) + self.assertRaises(ValueError, l1.rpc.connect, + l2.info['id'], 'localhost', l2.info['port']) + self.assertRaises(ValueError, l1.rpc.connect, + l2.info['id'], 'localhost', l2.info['port']) l1.rpc.connect(l2.info['id'], 'localhost', l2.info['port']) # Should have 3 connect fails. for d in disconnects: - l1.daemon.wait_for_log('Failed connected out for {}, will try again' + l1.daemon.wait_for_log('Failed connected out for {}' .format(l2.info['id'])) # Should still only have one peer! diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 0dc51f5604c4..0aa18e579dce 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -58,13 +58,6 @@ void command_still_pending(struct command *cmd UNNEEDED) /* Generated stub for command_success */ void command_success(struct command *cmd UNNEEDED, struct json_result *response UNNEEDED) { fprintf(stderr, "command_success called!\n"); abort(); } -/* Generated stub for connect_failed */ -void connect_failed(struct lightningd *ld UNNEEDED, const struct pubkey *id UNNEEDED, - const char *error UNNEEDED) -{ fprintf(stderr, "connect_failed called!\n"); abort(); } -/* Generated stub for connect_succeeded */ -void connect_succeeded(struct lightningd *ld UNNEEDED, const struct pubkey *id UNNEEDED) -{ fprintf(stderr, "connect_succeeded called!\n"); abort(); } /* Generated stub for derive_basepoints */ bool derive_basepoints(const struct privkey *seed UNNEEDED, struct pubkey *funding_pubkey UNNEEDED, @@ -338,10 +331,6 @@ void peer_start_closingd(struct channel *channel UNNEEDED, bool reconnected UNNEEDED, const u8 *channel_reestablish UNNEEDED) { fprintf(stderr, "peer_start_closingd called!\n"); abort(); } -/* Generated stub for sanitize_error */ -char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED, - struct channel_id *channel_id UNNEEDED) -{ fprintf(stderr, "sanitize_error called!\n"); abort(); } /* Generated stub for subd_release_channel */ void subd_release_channel(struct subd *owner UNNEEDED, void *channel UNNEEDED) { fprintf(stderr, "subd_release_channel called!\n"); abort(); } @@ -389,9 +378,6 @@ u8 *towire_gossipctl_peer_disconnect(const tal_t *ctx UNNEEDED, const struct pub /* Generated stub for towire_gossipctl_peer_important */ u8 *towire_gossipctl_peer_important(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED, bool important UNNEEDED) { fprintf(stderr, "towire_gossipctl_peer_important called!\n"); abort(); } -/* Generated stub for towire_gossipctl_reach_peer */ -u8 *towire_gossipctl_reach_peer(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED) -{ fprintf(stderr, "towire_gossipctl_reach_peer called!\n"); abort(); } /* Generated stub for towire_gossip_disable_channel */ u8 *towire_gossip_disable_channel(const tal_t *ctx UNNEEDED, const struct short_channel_id *short_channel_id UNNEEDED, u8 direction UNNEEDED, bool active UNNEEDED) { fprintf(stderr, "towire_gossip_disable_channel called!\n"); abort(); } From 39574c02db5cea7e88c0ba9588a5218e73775eab Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 08/21] closing: don't go into temporary failure because we completed negotiation. It only lasts until the next block, but it's weird. Signed-off-by: Rusty Russell --- lightningd/closing_control.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index ed9357373cf7..979dfbcd2c2d 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -90,6 +90,11 @@ static void peer_closing_complete(struct channel *channel, const u8 *msg) return; } + /* Don't report spurious failure when closingd exits. */ + channel_set_owner(channel, NULL); + /* Clear any transient negotiation messages */ + channel_set_billboard(channel, false, NULL); + /* Retransmission only, ignore closing. */ if (channel->state == CLOSINGD_COMPLETE) return; From 66239b55514eb2d6ab4ee49909941fe27b000e93 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 09/21] gossipd: drain fd instead of passing around gossip index. (This was sitting in my gossip-enchancement patch queue, but it simplifies this set too, so I moved it here). In 94711969f we added an explicit gossip_index so when gossipd gets peers back from other daemons, it knows what gossip it has sent (since gossipd can send gossip after the other daemon is already complete). This solution is insufficient for the more general case where gossipd wants to send other messages reliably, so replace it with the other solution: have gossipd drain the "gossip fd" which the daemon returns. This turns out to be quite simple, and is probably how I should have done it originally :( Signed-off-by: Rusty Russell --- channeld/channel.c | 62 ++++++------------------------------ channeld/channel_wire.csv | 2 -- closingd/closing.c | 53 +++++++++++++----------------- closingd/closing_wire.csv | 2 -- common/peer_failed.c | 8 ++--- common/peer_failed.h | 12 +++---- common/peer_status_wire.csv | 1 - common/read_peer_msg.c | 5 ++- common/read_peer_msg.h | 8 ++--- gossipd/gossip.c | 44 ++++++++++++++++--------- gossipd/gossip_wire.csv | 5 --- lightningd/channel_control.c | 9 ++---- lightningd/channel_control.h | 1 - lightningd/closing_control.c | 7 +--- lightningd/closing_control.h | 1 - lightningd/gossip_control.c | 5 ++- lightningd/onchain_control.c | 1 - lightningd/opening_control.c | 30 +++++------------ lightningd/opening_control.h | 2 -- lightningd/peer_control.c | 23 ++++++------- lightningd/peer_control.h | 2 -- lightningd/subd.c | 9 ++---- lightningd/subd.h | 3 -- openingd/opening.c | 35 ++++++++++---------- openingd/opening_wire.csv | 3 -- wallet/test/run-wallet.c | 8 ++--- 26 files changed, 118 insertions(+), 223 deletions(-) diff --git a/channeld/channel.c b/channeld/channel.c index fb31eacbb92b..4ac546ea2a02 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -161,9 +161,6 @@ struct peer { bool announce_depth_reached; - /* Where we got up to in gossip broadcasts. */ - u64 gossip_index; - /* Make sure timestamps move forward. */ u32 last_update_timestamp; }; @@ -304,23 +301,18 @@ static void enqueue_peer_msg(struct peer *peer, const u8 *msg TAKES) static void gossip_in(struct peer *peer, const u8 *msg) { u8 *gossip; - u64 gossip_index; - if (!fromwire_gossip_send_gossip(msg, msg, &gossip_index, &gossip)) + if (!fromwire_gossip_send_gossip(msg, msg, &gossip)) status_failed(STATUS_FAIL_GOSSIP_IO, "Got bad message from gossipd: %s", tal_hex(msg, msg)); - /* Zero is a special index meaning this is unindexed gossip. */ - if (gossip_index != 0) - peer->gossip_index = gossip_index; - if (is_msg_for_gossipd(gossip)) enqueue_peer_msg(peer, gossip); else if (fromwire_peektype(gossip) == WIRE_ERROR) { struct channel_id channel_id; char *what = sanitize_error(msg, msg, &channel_id); - peer_failed(&peer->cs, peer->gossip_index, &channel_id, + peer_failed(&peer->cs, &channel_id, "gossipd said: %s", what); } else status_failed(STATUS_FAIL_GOSSIP_IO, @@ -497,12 +489,12 @@ static void handle_peer_funding_locked(struct peer *peer, const u8 *msg) peer->old_remote_per_commit = peer->remote_per_commit; if (!fromwire_funding_locked(msg, &chanid, &peer->remote_per_commit)) - peer_failed(&peer->cs, peer->gossip_index, + peer_failed(&peer->cs, &peer->channel_id, "Bad funding_locked %s", tal_hex(msg, msg)); if (!structeq(&chanid, &peer->channel_id)) - peer_failed(&peer->cs, peer->gossip_index, + peer_failed(&peer->cs, &peer->channel_id, "Wrong channel id in %s (expected %s)", tal_hex(tmpctx, msg), @@ -529,7 +521,7 @@ static void check_short_ids_match(struct peer *peer) if (!structeq(&peer->short_channel_ids[LOCAL], &peer->short_channel_ids[REMOTE])) - peer_failed(&peer->cs, peer->gossip_index, + peer_failed(&peer->cs, &peer->channel_id, "We disagree on short_channel_ids:" " I have %s, you say %s", @@ -561,14 +553,14 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg &peer->short_channel_ids[REMOTE], &peer->announcement_node_sigs[REMOTE], &peer->announcement_bitcoin_sigs[REMOTE])) - peer_failed(&peer->cs, peer->gossip_index, + peer_failed(&peer->cs, &peer->channel_id, "Bad announcement_signatures %s", tal_hex(msg, msg)); /* Make sure we agree on the channel ids */ if (!structeq(&chanid, &peer->channel_id)) { - peer_failed(&peer->cs, peer->gossip_index, + peer_failed(&peer->cs, &peer->channel_id, "Wrong channel_id: expected %s, got %s", type_to_string(tmpctx, struct channel_id, @@ -627,7 +619,6 @@ static void handle_peer_add_htlc(struct peer *peer, const u8 *msg) &payment_hash, &cltv_expiry, onion_routing_packet)) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad peer_add_htlc %s", tal_hex(msg, msg)); @@ -636,7 +627,6 @@ static void handle_peer_add_htlc(struct peer *peer, const u8 *msg) onion_routing_packet, &htlc); if (add_err != CHANNEL_ERR_ADD_OK) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad peer_add_htlc: %s", channel_add_err_name(add_err)); @@ -654,7 +644,6 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) if (!fromwire_update_fee(msg, &channel_id, &feerate)) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad update_fee %s", tal_hex(msg, msg)); } @@ -666,7 +655,6 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) */ if (peer->channel->funder != REMOTE) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "update_fee from non-funder?"); @@ -680,7 +668,6 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) */ if (feerate < peer->feerate_min || feerate > peer->feerate_max) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "update_fee %u outside range %u-%u", feerate, peer->feerate_min, peer->feerate_max); @@ -694,7 +681,6 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) */ if (!channel_update_feerate(peer->channel, feerate)) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "update_fee %u unaffordable", feerate); @@ -1175,7 +1161,6 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) * does not include any updates. */ peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "commit_sig with no changes"); } @@ -1189,7 +1174,6 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) if (!fromwire_commitment_signed(tmpctx, msg, &channel_id, &commit_sig, &htlc_sigs)) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad commit_sig %s", tal_hex(msg, msg)); @@ -1221,7 +1205,6 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) &peer->channel->funding_pubkey[REMOTE], &commit_sig)) { dump_htlcs(peer->channel, "receiving commit_sig"); peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad commit_sig signature %"PRIu64" %s for tx %s wscript %s key %s", peer->next_index[LOCAL], @@ -1242,7 +1225,6 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) */ if (tal_count(htlc_sigs) != tal_count(txs) - 1) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Expected %zu htlc sigs, not %zu", tal_count(txs) - 1, tal_count(htlc_sigs)); @@ -1257,7 +1239,6 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) if (!check_tx_sig(txs[1+i], 0, NULL, wscripts[1+i], &remote_htlckey, &htlc_sigs[i])) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad commit_sig signature %s for htlc %s wscript %s key %s", type_to_string(msg, secp256k1_ecdsa_signature, &htlc_sigs[i]), @@ -1316,14 +1297,12 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg) if (!fromwire_revoke_and_ack(msg, &channel_id, &old_commit_secret, &next_per_commit)) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad revoke_and_ack %s", tal_hex(msg, msg)); } if (peer->revocations_received != peer->next_index[REMOTE] - 2) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Unexpected revoke_and_ack"); } @@ -1337,14 +1316,12 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg) memcpy(&privkey, &old_commit_secret, sizeof(privkey)); if (!pubkey_from_privkey(&privkey, &per_commit_point)) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad privkey %s", type_to_string(msg, struct privkey, &privkey)); } if (!pubkey_eq(&per_commit_point, &peer->old_remote_per_commit)) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Wrong privkey %s for %"PRIu64" %s", type_to_string(msg, struct privkey, &privkey), @@ -1389,7 +1366,6 @@ static void handle_peer_fulfill_htlc(struct peer *peer, const u8 *msg) if (!fromwire_update_fulfill_htlc(msg, &channel_id, &id, &preimage)) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad update_fulfill_htlc %s", tal_hex(msg, msg)); } @@ -1409,7 +1385,6 @@ static void handle_peer_fulfill_htlc(struct peer *peer, const u8 *msg) case CHANNEL_ERR_HTLC_NOT_IRREVOCABLE: case CHANNEL_ERR_BAD_PREIMAGE: peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad update_fulfill_htlc: failed to fulfill %" PRIu64 " error %s", id, channel_remove_err_name(e)); @@ -1428,7 +1403,6 @@ static void handle_peer_fail_htlc(struct peer *peer, const u8 *msg) if (!fromwire_update_fail_htlc(msg, msg, &channel_id, &id, &reason)) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad update_fulfill_htlc %s", tal_hex(msg, msg)); } @@ -1447,7 +1421,6 @@ static void handle_peer_fail_htlc(struct peer *peer, const u8 *msg) case CHANNEL_ERR_HTLC_NOT_IRREVOCABLE: case CHANNEL_ERR_BAD_PREIMAGE: peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad update_fail_htlc: failed to remove %" PRIu64 " error %s", id, @@ -1470,7 +1443,6 @@ static void handle_peer_fail_malformed_htlc(struct peer *peer, const u8 *msg) &sha256_of_onion, &failure_code)) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad update_fail_malformed_htlc %s", tal_hex(msg, msg)); @@ -1483,7 +1455,6 @@ static void handle_peer_fail_malformed_htlc(struct peer *peer, const u8 *msg) */ if (!(failure_code & BADONION)) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad update_fail_malformed_htlc failure code %u", failure_code); @@ -1521,7 +1492,6 @@ static void handle_peer_fail_malformed_htlc(struct peer *peer, const u8 *msg) case CHANNEL_ERR_HTLC_NOT_IRREVOCABLE: case CHANNEL_ERR_BAD_PREIMAGE: peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad update_fail_malformed_htlc: failed to remove %" PRIu64 " error %s", id, channel_remove_err_name(e)); @@ -1534,7 +1504,6 @@ 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->gossip_index, &peer->channel_id, "%s", err); @@ -1552,7 +1521,6 @@ static void handle_peer_shutdown(struct peer *peer, const u8 *shutdown) if (!fromwire_shutdown(peer, shutdown, &channel_id, &scriptpubkey)) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Bad shutdown %s", tal_hex(peer, shutdown)); @@ -1589,7 +1557,6 @@ static void peer_in(struct peer *peer, const u8 *msg) && type != WIRE_PONG && type != WIRE_SHUTDOWN) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "%s (%u) before funding locked", wire_type_name(type), type); @@ -1650,7 +1617,6 @@ static void peer_in(struct peer *peer, const u8 *msg) } peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Peer sent unknown message %u (%s)", type, wire_type_name(type)); @@ -1692,7 +1658,6 @@ static void send_fail_or_fulfill(struct peer *peer, const struct htlc *h) h->r); } else peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "HTLC %"PRIu64" state %s not failed/fulfilled", h->id, htlc_state_name(h->state)); @@ -1725,7 +1690,6 @@ static void resend_commitment(struct peer *peer, const struct changed_htlc *last * then they asked for a retransmit */ if (!h) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "Can't find HTLC %"PRIu64" to resend", last[i].id); @@ -1778,7 +1742,7 @@ static bool channeld_send_reply(struct crypto_state *cs UNUSED, static u8 *channeld_read_peer_msg(struct peer *peer) { - return read_peer_msg(peer, &peer->cs, peer->gossip_index, + return read_peer_msg(peer, &peer->cs, &peer->channel_id, channeld_send_reply, channeld_io_error, @@ -1824,7 +1788,6 @@ static void peer_reconnect(struct peer *peer) &next_local_commitment_number, &next_remote_revocation_number)) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "bad reestablish msg: %s %s", wire_type_name(fromwire_peektype(msg)), @@ -1873,7 +1836,6 @@ static void peer_reconnect(struct peer *peer) /* Don't try to retransmit revocation index -1! */ if (peer->next_index[LOCAL] < 2) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "bad reestablish revocation_number: %" PRIu64, @@ -1882,7 +1844,6 @@ static void peer_reconnect(struct peer *peer) retransmit_revoke_and_ack = true; } else if (next_remote_revocation_number != peer->next_index[LOCAL] - 1) { peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "bad reestablish revocation_number: %"PRIu64 " vs %"PRIu64, @@ -1907,7 +1868,6 @@ static void peer_reconnect(struct peer *peer) /* We completed opening, we don't re-transmit that one! */ if (next_local_commitment_number == 0) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "bad reestablish commitment_number: %" PRIu64, @@ -1923,7 +1883,6 @@ static void peer_reconnect(struct peer *peer) */ } else if (next_local_commitment_number != peer->next_index[REMOTE]) peer_failed(&peer->cs, - peer->gossip_index, &peer->channel_id, "bad reestablish commitment_number: %"PRIu64 " vs %"PRIu64, @@ -2477,7 +2436,6 @@ static void init_channel(struct peer *peer) &peer->feerate_min, &peer->feerate_max, &peer->their_commit_sig, &peer->cs, - &peer->gossip_index, &funding_pubkey[REMOTE], &points[REMOTE].revocation, &points[REMOTE].payment, @@ -2592,9 +2550,7 @@ static void send_shutdown_complete(struct peer *peer) /* Now we can tell master shutdown is complete. */ wire_sync_write(MASTER_FD, - take(towire_channel_shutdown_complete(NULL, - &peer->cs, - peer->gossip_index))); + take(towire_channel_shutdown_complete(NULL, &peer->cs))); fdpass_send(MASTER_FD, PEER_FD); fdpass_send(MASTER_FD, GOSSIP_FD); close(MASTER_FD); diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index 446f0e5a252e..75d5af20c906 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -15,7 +15,6 @@ channel_init,,feerate_min,u32 channel_init,,feerate_max,u32 channel_init,,first_commit_sig,secp256k1_ecdsa_signature channel_init,,crypto_state,struct crypto_state -channel_init,,gossip_index,u64 channel_init,,remote_fundingkey,struct pubkey channel_init,,remote_revocation_basepoint,struct pubkey channel_init,,remote_payment_basepoint,struct pubkey @@ -173,7 +172,6 @@ channel_got_shutdown,,scriptpubkey,scriptpubkey_len*u8 # Shutdown is complete, ready for closing negotiation. + peer_fd & gossip_fd. channel_shutdown_complete,1025 channel_shutdown_complete,,crypto_state,struct crypto_state -channel_shutdown_complete,,gossip_index,u64 # Re-enable commit timer. channel_dev_reenable_commit,1026 diff --git a/closingd/closing.c b/closingd/closing.c index d0b75f855ea9..c4b2b8b937aa 100644 --- a/closingd/closing.c +++ b/closingd/closing.c @@ -29,7 +29,6 @@ static struct bitcoin_tx *close_tx(const tal_t *ctx, struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, u8 *scriptpubkey[NUM_SIDES], const struct bitcoin_txid *funding_txid, @@ -43,7 +42,7 @@ static struct bitcoin_tx *close_tx(const tal_t *ctx, struct bitcoin_tx *tx; if (satoshi_out[funder] < fee) - peer_failed(cs, gossip_index, channel_id, + peer_failed(cs, channel_id, "Funder cannot afford fee %"PRIu64 " (%"PRIu64" and %"PRIu64")", fee, satoshi_out[LOCAL], @@ -62,7 +61,7 @@ static struct bitcoin_tx *close_tx(const tal_t *ctx, satoshi_out[REMOTE] - (funder == REMOTE ? fee : 0), dust_limit); if (!tx) - peer_failed(cs, gossip_index, channel_id, + peer_failed(cs, channel_id, "Both outputs below dust limit:" " funding = %"PRIu64 " fee = %"PRIu64 @@ -78,7 +77,6 @@ static struct bitcoin_tx *close_tx(const tal_t *ctx, } static void do_reconnect(struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, const u64 next_index[NUM_SIDES], u64 revocations_received, @@ -111,7 +109,7 @@ static void do_reconnect(struct crypto_state *cs, /* Wait for them to say something interesting */ channel_reestablish - = read_peer_msg(tmpctx, cs, gossip_index, channel_id, + = read_peer_msg(tmpctx, cs, channel_id, sync_crypto_write_arg, status_fail_io, NULL); @@ -120,7 +118,7 @@ static void do_reconnect(struct crypto_state *cs, if (!fromwire_channel_reestablish(channel_reestablish, &their_channel_id, &next_local_commitment_number, &next_remote_revocation_number)) { - peer_failed(cs, gossip_index, channel_id, + peer_failed(cs, channel_id, "bad reestablish msg: %s %s", wire_type_name(fromwire_peektype(channel_reestablish)), tal_hex(tmpctx, channel_reestablish)); @@ -144,7 +142,6 @@ static void do_reconnect(struct crypto_state *cs, } static void send_offer(struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, const struct pubkey funding_pubkey[NUM_SIDES], const u8 *funding_wscript, @@ -168,7 +165,7 @@ static void send_offer(struct crypto_state *cs, * the close transaction as specified in [BOLT * #3](03-transactions.md#closing-transaction). */ - tx = close_tx(tmpctx, cs, gossip_index, channel_id, + tx = close_tx(tmpctx, cs, channel_id, scriptpubkey, funding_txid, funding_txout, @@ -214,7 +211,6 @@ static void tell_master_their_offer(const secp256k1_ecdsa_signature *their_sig, /* Returns fee they offered. */ static uint64_t receive_offer(struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, const struct pubkey funding_pubkey[NUM_SIDES], const u8 *funding_wscript, @@ -237,7 +233,7 @@ static uint64_t receive_offer(struct crypto_state *cs, do { clean_tmpctx(); - msg = read_peer_msg(tmpctx, cs, gossip_index, channel_id, + msg = read_peer_msg(tmpctx, cs, channel_id, sync_crypto_write_arg, status_fail_io, NULL); @@ -262,7 +258,7 @@ static uint64_t receive_offer(struct crypto_state *cs, if (!fromwire_closing_signed(msg, &their_channel_id, &received_fee, &their_sig)) - peer_failed(cs, gossip_index, channel_id, + peer_failed(cs, channel_id, "Expected closing_signed: %s", tal_hex(tmpctx, msg)); @@ -273,7 +269,7 @@ static uint64_t receive_offer(struct crypto_state *cs, * #3](03-transactions.md#closing-transaction), and MUST fail * the connection if it is not. */ - tx = close_tx(tmpctx, cs, gossip_index, channel_id, + tx = close_tx(tmpctx, cs, channel_id, scriptpubkey, funding_txid, funding_txout, @@ -299,7 +295,7 @@ static uint64_t receive_offer(struct crypto_state *cs, * then remove any output below its own `dust_limit_satoshis`, * and MAY also eliminate its own output. */ - trimmed = close_tx(tmpctx, cs, gossip_index, channel_id, + trimmed = close_tx(tmpctx, cs, channel_id, scriptpubkey, funding_txid, funding_txout, @@ -309,7 +305,7 @@ static uint64_t receive_offer(struct crypto_state *cs, if (!trimmed || !check_tx_sig(trimmed, 0, NULL, funding_wscript, &funding_pubkey[REMOTE], &their_sig)) { - peer_failed(cs, gossip_index, channel_id, + peer_failed(cs, channel_id, "Bad closing_signed signature for" " %s (and trimmed version %s)", type_to_string(tmpctx, @@ -368,14 +364,13 @@ static void init_feerange(struct feerange *feerange, } static void adjust_feerange(struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, struct feerange *feerange, u64 offer, enum side side) { if (offer < feerange->min || offer > feerange->max) { if (!feerange->allow_mistakes || side != REMOTE) - peer_failed(cs, gossip_index, channel_id, + peer_failed(cs, channel_id, "%s offer %"PRIu64 " not between %"PRIu64" and %"PRIu64, side == LOCAL ? "local" : "remote", @@ -402,7 +397,6 @@ static void adjust_feerange(struct crypto_state *cs, /* Figure out what we should offer now. */ static u64 adjust_offer(struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, const struct feerange *feerange, u64 remote_offer, @@ -414,7 +408,7 @@ static u64 adjust_offer(struct crypto_state *cs, /* Max is below our minimum acceptable? */ if (feerange->max < min_fee_to_accept) - peer_failed(cs, gossip_index, channel_id, + peer_failed(cs, channel_id, "Feerange %"PRIu64"-%"PRIu64 " below minimum acceptable %"PRIu64, feerange->min, feerange->max, @@ -446,7 +440,6 @@ int main(int argc, char *argv[]) struct secrets secrets; bool reconnected; u64 next_index[NUM_SIDES], revocations_received; - u64 gossip_index; enum side whose_turn; bool deprecated_api; u8 *channel_reestablish; @@ -457,7 +450,7 @@ int main(int argc, char *argv[]) msg = wire_sync_read(tmpctx, REQ_FD); if (!fromwire_closing_init(ctx, msg, - &cs, &gossip_index, &seed, + &cs, &seed, &funding_txid, &funding_txout, &funding_satoshi, &funding_pubkey[REMOTE], @@ -490,7 +483,7 @@ int main(int argc, char *argv[]) &funding_pubkey[REMOTE]); if (reconnected) - do_reconnect(&cs, gossip_index, &channel_id, + do_reconnect(&cs, &channel_id, next_index, revocations_received, channel_reestablish); @@ -508,7 +501,7 @@ int main(int argc, char *argv[]) whose_turn = funder; for (size_t i = 0; i < 2; i++, whose_turn = !whose_turn) { if (whose_turn == LOCAL) { - send_offer(&cs, gossip_index, + send_offer(&cs, &channel_id, funding_pubkey, funding_wscript, scriptpubkey, &funding_txid, funding_txout, @@ -524,7 +517,7 @@ int main(int argc, char *argv[]) " ours was %"PRIu64" satoshi", offer[LOCAL]); offer[REMOTE] - = receive_offer(&cs, gossip_index, + = receive_offer(&cs, &channel_id, funding_pubkey, funding_wscript, scriptpubkey, &funding_txid, @@ -539,8 +532,7 @@ int main(int argc, char *argv[]) init_feerange(&feerange, commitment_fee, offer); /* Apply (and check) funder offer now. */ - adjust_feerange(&cs, gossip_index, &channel_id, - &feerange, offer[funder], funder); + adjust_feerange(&cs, &channel_id, &feerange, offer[funder], funder); /* Older spec clients would make offers independently, so allow */ feerange.allow_mistakes = deprecated_api; @@ -548,16 +540,16 @@ int main(int argc, char *argv[]) /* Now any extra rounds required. */ while (offer[LOCAL] != offer[REMOTE]) { /* Still don't agree: adjust feerange based on previous offer */ - adjust_feerange(&cs, gossip_index, &channel_id, + adjust_feerange(&cs, &channel_id, &feerange, offer[!whose_turn], !whose_turn); if (whose_turn == LOCAL) { - offer[LOCAL] = adjust_offer(&cs, gossip_index, + offer[LOCAL] = adjust_offer(&cs, &channel_id, &feerange, offer[REMOTE], min_fee_to_accept); - send_offer(&cs, gossip_index, &channel_id, + send_offer(&cs, &channel_id, funding_pubkey, funding_wscript, scriptpubkey, &funding_txid, funding_txout, @@ -570,7 +562,7 @@ int main(int argc, char *argv[]) " theirs was %"PRIu64" satoshi,", offer[LOCAL], offer[REMOTE]); offer[REMOTE] - = receive_offer(&cs, gossip_index, &channel_id, + = receive_offer(&cs, &channel_id, funding_pubkey, funding_wscript, scriptpubkey, &funding_txid, @@ -587,8 +579,7 @@ int main(int argc, char *argv[]) offer[LOCAL]); /* We're done! */ - wire_sync_write(REQ_FD, - take(towire_closing_complete(NULL, gossip_index))); + wire_sync_write(REQ_FD, take(towire_closing_complete(NULL))); tal_free(ctx); daemon_shutdown(); diff --git a/closingd/closing_wire.csv b/closingd/closing_wire.csv index 21e9350120f2..964e48d4abf7 100644 --- a/closingd/closing_wire.csv +++ b/closingd/closing_wire.csv @@ -3,7 +3,6 @@ # Begin! (passes peer fd, gossipd-client fd) closing_init,2001 closing_init,,crypto_state,struct crypto_state -closing_init,,gossip_index,u64 closing_init,,seed,struct privkey closing_init,,funding_txid,struct bitcoin_txid closing_init,,funding_txout,u16 @@ -38,4 +37,3 @@ closing_received_signature_reply,2102 # Negotiations complete, we're exiting. closing_complete,2004 -closing_complete,,gossip_index,u64 diff --git a/common/peer_failed.c b/common/peer_failed.c index f40d27a8257a..8dca86f3639f 100644 --- a/common/peer_failed.c +++ b/common/peer_failed.c @@ -9,7 +9,7 @@ /* We only support one channel per peer anyway */ void peer_failed_(int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, + struct crypto_state *cs, const struct channel_id *channel_id, const char *fmt, ...) { @@ -22,7 +22,7 @@ void peer_failed_(int peer_fd, int gossip_fd, va_end(ap); msg = towire_status_peer_error(NULL, channel_id, - desc, cs, gossip_index, + desc, cs, towire_errorfmt(desc, channel_id, "%s", desc)); peer_billboard(true, desc); @@ -32,12 +32,12 @@ void peer_failed_(int peer_fd, int gossip_fd, /* We're failing because peer sent us an error message */ void peer_failed_received_errmsg(int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, + struct crypto_state *cs, const char *desc, const struct channel_id *channel_id) { u8 *msg = towire_status_peer_error(NULL, channel_id, - desc, cs, gossip_index, NULL); + desc, cs, NULL); peer_billboard(true, "Received error from peer: %s", desc); status_send_fatal(take(msg), peer_fd, gossip_fd); } diff --git a/common/peer_failed.h b/common/peer_failed.h index 709468c90894..342be57d0f52 100644 --- a/common/peer_failed.h +++ b/common/peer_failed.h @@ -9,23 +9,21 @@ struct channel_id; /** * peer_failed - Exit with error for peer. * @cs: the peer's current crypto state. - * @gossip_index: the peer's current gossip_index. * @channel_id: channel with error, or NULL for all. * @fmt...: format as per status_failed(STATUS_FAIL_PEER_BAD) */ -#define peer_failed(cs, gossip_index, channel_id, ...) \ - peer_failed_(PEER_FD, GOSSIP_FD, (cs), (gossip_index), (channel_id), \ - __VA_ARGS__) +#define peer_failed(cs, channel_id, ...) \ + peer_failed_(PEER_FD, GOSSIP_FD, (cs), (channel_id), __VA_ARGS__) void peer_failed_(int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, + struct crypto_state *cs, const struct channel_id *channel_id, const char *fmt, ...) - PRINTF_FMT(6,7) NORETURN; + PRINTF_FMT(5,6) NORETURN; /* We're failing because peer sent us an error message */ void peer_failed_received_errmsg(int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, + struct crypto_state *cs, const char *desc, const struct channel_id *channel_id) NORETURN; diff --git a/common/peer_status_wire.csv b/common/peer_status_wire.csv index 135b2256cda9..f69467f2b2ce 100644 --- a/common/peer_status_wire.csv +++ b/common/peer_status_wire.csv @@ -6,6 +6,5 @@ status_peer_error,0xFFF4 status_peer_error,,channel,struct channel_id status_peer_error,,desc,wirestring status_peer_error,,crypto_state,struct crypto_state -status_peer_error,,gossip_index,u64 status_peer_error,,len,u16 status_peer_error,,error_for_them,len*u8 diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index 94717bbf84ab..9193efa76154 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -40,7 +40,7 @@ static void handle_ping(const u8 *msg, u8 *read_peer_msg_(const tal_t *ctx, int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, + struct crypto_state *cs, const struct channel_id *channel, bool (*send_reply)(struct crypto_state *cs, int fd, const u8 *TAKES, void *arg), @@ -86,8 +86,7 @@ u8 *read_peer_msg_(const tal_t *ctx, */ if (structeq(&chanid, channel) || channel_id_is_all(&chanid)) peer_failed_received_errmsg(peer_fd, gossip_fd, - cs, gossip_index, - err, &chanid); + cs, err, &chanid); return tal_free(msg); } diff --git a/common/read_peer_msg.h b/common/read_peer_msg.h index 5c83995095eb..83f50c394b16 100644 --- a/common/read_peer_msg.h +++ b/common/read_peer_msg.h @@ -12,7 +12,6 @@ struct channel_id; * read_peer_msg - read & decode in a peer message, handling common ones. * @ctx: context to allocate return packet from. * @cs: the cryptostate (updated) - * @gossip_index: the gossip_index * @chanid: the channel id (for identifying errors) * @send_reply: the way to send a reply packet (eg. sync_crypto_write_arg) * @io_error: what to do if there's an IO error (eg. status_fail_io) @@ -21,9 +20,8 @@ struct channel_id; * This returns NULL if it handled the message, so it's normally called in * a loop. */ -#define read_peer_msg(ctx, cs, gossip_index, chanid, send_reply, \ - io_error, arg) \ - read_peer_msg_((ctx), PEER_FD, GOSSIP_FD, (cs), (gossip_index), \ +#define read_peer_msg(ctx, cs, chanid, send_reply, io_error, arg) \ + read_peer_msg_((ctx), PEER_FD, GOSSIP_FD, (cs), \ (chanid), \ typesafe_cb_preargs(bool, void *, (send_reply), (arg), \ struct crypto_state *, int, \ @@ -40,7 +38,7 @@ void status_fail_io(void *unused); u8 *read_peer_msg_(const tal_t *ctx, int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, + struct crypto_state *cs, const struct channel_id *channel, bool (*send_reply)(struct crypto_state *cs, int fd, const u8 *TAKES, void *arg), diff --git a/gossipd/gossip.c b/gossipd/gossip.c index fdf968a65bfa..5312949355d2 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -330,8 +330,7 @@ static void queue_peer_msg(struct peer *peer, const u8 *msg TAKES) if (peer->local) { msg_enqueue(&peer->local->peer_out, msg); } else { - /* Use gossip_index 0 meaning don't update index */ - const u8 *send = towire_gossip_send_gossip(NULL, 0, msg); + const u8 *send = towire_gossip_send_gossip(NULL, msg); if (taken(msg)) tal_free(msg); daemon_conn_send(peer->remote, take(send)); @@ -406,7 +405,6 @@ static struct io_plan *peer_init_received(struct io_conn *conn, /* We will not have anything queued, since we're not duplex. */ msg = towire_gossip_peer_connected(peer, &peer->id, &peer->addr, &peer->local->pcs.cs, - peer->broadcast_index, peer->gfeatures, peer->lfeatures); if (!send_peer_with_fds(peer, msg)) return io_close(conn); @@ -599,7 +597,6 @@ static struct io_plan *ready_for_master(struct io_conn *conn, struct peer *peer) msg = towire_gossip_peer_nongossip(peer, &peer->id, &peer->addr, &peer->local->pcs.cs, - peer->broadcast_index, peer->gfeatures, peer->lfeatures, peer->local->nongossip_msg); @@ -607,7 +604,6 @@ static struct io_plan *ready_for_master(struct io_conn *conn, struct peer *peer) msg = towire_gossipctl_release_peer_reply(peer, &peer->addr, &peer->local->pcs.cs, - peer->broadcast_index, peer->gfeatures, peer->lfeatures); @@ -925,9 +921,7 @@ static bool nonlocal_dump_gossip(struct io_conn *conn, struct daemon_conn *dc) peer->gossip_sync = false; return false; } else { - u8 *msg = towire_gossip_send_gossip(NULL, - peer->broadcast_index, - next); + u8 *msg = towire_gossip_send_gossip(NULL, next); daemon_conn_send(peer->remote, take(msg)); return true; } @@ -953,11 +947,33 @@ struct returning_peer { struct daemon *daemon; struct pubkey id; struct crypto_state cs; - u64 gossip_index; u8 *inner_msg; int peer_fd, gossip_fd; }; +static void drain_and_forward_gossip(struct peer *peer, int gossip_fd) +{ + u8 *msg; + + /* Be careful: what if they handed wrong fd? Make it non-blocking. */ + if (!io_fd_block(gossip_fd, false)) { + status_unusual("NONBLOCK failed for gossip_fd from peer %s: %s", + type_to_string(tmpctx, struct pubkey, &peer->id), + strerror(errno)); + return; + } + + /* It's sync, but not blocking. */ + while ((msg = wire_sync_read(tmpctx, gossip_fd)) != NULL) { + u8 *gossip; + if (!fromwire_gossip_send_gossip(NULL, msg, &gossip)) + break; + msg_enqueue(&peer->local->peer_out, take(gossip)); + } + + close(gossip_fd); +} + static struct io_plan *handle_returning_peer(struct io_conn *conn, struct returning_peer *rpeer) { @@ -970,15 +986,12 @@ static struct io_plan *handle_returning_peer(struct io_conn *conn, "hand_back_peer unknown peer: %s", type_to_string(tmpctx, struct pubkey, &rpeer->id)); - /* We don't need the gossip_fd; we know what gossip it got - * from gossip_index */ - close(rpeer->gossip_fd); - /* Possible if there's a reconnect: ignore handed back. */ if (peer->local) { status_trace("hand_back_peer %s: reconnected, dropping handback", type_to_string(tmpctx, struct pubkey, &rpeer->id)); + close(rpeer->gossip_fd); close(rpeer->peer_fd); tal_free(rpeer); return daemon_conn_read_next(conn, &daemon->master); @@ -993,7 +1006,9 @@ static struct io_plan *handle_returning_peer(struct io_conn *conn, peer->local = new_local_peer_state(peer, &rpeer->cs); peer->local->fd = rpeer->peer_fd; - peer->broadcast_index = rpeer->gossip_index; + + /* Forward any gossip we sent while fd wasn't being read */ + drain_and_forward_gossip(peer, rpeer->gossip_fd); /* If they told us to send a message, queue it now */ if (tal_len(rpeer->inner_msg)) @@ -1019,7 +1034,6 @@ static struct io_plan *hand_back_peer(struct io_conn *conn, rpeer->daemon = daemon; if (!fromwire_gossipctl_hand_back_peer(msg, msg, &rpeer->id, &rpeer->cs, - &rpeer->gossip_index, &rpeer->inner_msg)) master_badmsg(WIRE_GOSSIPCTL_HAND_BACK_PEER, msg); diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index b9f48ed6a2c2..e021e3029e8b 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -52,7 +52,6 @@ gossip_peer_connected,3002 gossip_peer_connected,,id,struct pubkey gossip_peer_connected,,addr,struct wireaddr gossip_peer_connected,,crypto_state,struct crypto_state -gossip_peer_connected,,gossip_index,u64 gossip_peer_connected,,gflen,u16 gossip_peer_connected,,gfeatures,gflen*u8 gossip_peer_connected,,lflen,u16 @@ -63,7 +62,6 @@ gossip_peer_nongossip,3003 gossip_peer_nongossip,,id,struct pubkey gossip_peer_nongossip,,addr,struct wireaddr gossip_peer_nongossip,,crypto_state,struct crypto_state -gossip_peer_nongossip,,gossip_index,u64 gossip_peer_nongossip,,gflen,u16 gossip_peer_nongossip,,gfeatures,gflen*u8 gossip_peer_nongossip,,lflen,u16 @@ -79,7 +77,6 @@ gossipctl_release_peer,,id,struct pubkey gossipctl_release_peer_reply,3104 gossipctl_release_peer_reply,,addr,struct wireaddr gossipctl_release_peer_reply,,crypto_state,struct crypto_state -gossipctl_release_peer_reply,,gossip_index,u64 gossipctl_release_peer_reply,,gflen,u16 gossipctl_release_peer_reply,,gfeatures,gflen*u8 gossipctl_release_peer_reply,,lflen,u16 @@ -92,7 +89,6 @@ gossipctl_release_peer_replyfail,3204 gossipctl_hand_back_peer,3013 gossipctl_hand_back_peer,,id,struct pubkey gossipctl_hand_back_peer,,crypto_state,struct crypto_state -gossipctl_hand_back_peer,,gossip_index,u64 gossipctl_hand_back_peer,,len,u16 gossipctl_hand_back_peer,,msg,len*u8 @@ -175,7 +171,6 @@ gossip_get_update_reply,,update,len*u8 # Gossipd can tell channeld etc about gossip to fwd. gossip_send_gossip,3016 -gossip_send_gossip,,gossip_index,u64 gossip_send_gossip,,len,u16 gossip_send_gossip,,gossip,len*u8 diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 87ad602564f0..207c74b0e866 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -102,20 +102,18 @@ static void peer_start_closingd_after_shutdown(struct channel *channel, const int *fds) { struct crypto_state cs; - u64 gossip_index; /* We expect 2 fds. */ assert(tal_count(fds) == 2); - if (!fromwire_channel_shutdown_complete(msg, &cs, &gossip_index)) { + if (!fromwire_channel_shutdown_complete(msg, &cs)) { channel_internal_error(channel, "bad shutdown_complete: %s", tal_hex(msg, msg)); return; } /* This sets channel->owner, closes down channeld. */ - peer_start_closingd(channel, &cs, gossip_index, fds[0], fds[1], - false, NULL); + peer_start_closingd(channel, &cs, fds[0], fds[1], false, NULL); channel_set_state(channel, CHANNELD_SHUTTING_DOWN, CLOSINGD_SIGEXCHANGE); } @@ -172,7 +170,6 @@ static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds) bool peer_start_channeld(struct channel *channel, const struct crypto_state *cs, - u64 gossip_index, int peer_fd, int gossip_fd, const u8 *funding_signed, bool reconnected) @@ -248,7 +245,7 @@ bool peer_start_channeld(struct channel *channel, feerate_min(ld), feerate_max(ld), &channel->last_sig, - cs, gossip_index, + cs, &channel->channel_info.remote_fundingkey, &channel->channel_info.theirbase.revocation, &channel->channel_info.theirbase.payment, diff --git a/lightningd/channel_control.h b/lightningd/channel_control.h index 16c54ac72c1d..de24fe274489 100644 --- a/lightningd/channel_control.h +++ b/lightningd/channel_control.h @@ -10,7 +10,6 @@ struct lightningd; bool peer_start_channeld(struct channel *channel, const struct crypto_state *cs, - u64 gossip_index, int peer_fd, int gossip_fd, const u8 *funding_signed, bool reconnected); diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 979dfbcd2c2d..a41c0a29164b 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -81,10 +81,7 @@ static void peer_received_closing_signature(struct channel *channel, static void peer_closing_complete(struct channel *channel, const u8 *msg) { - /* FIXME: We should save this, to return to gossipd */ - u64 gossip_index; - - if (!fromwire_closing_complete(msg, &gossip_index)) { + if (!fromwire_closing_complete(msg)) { channel_internal_error(channel, "Bad closing_complete %s", tal_hex(msg, msg)); return; @@ -132,7 +129,6 @@ static unsigned closing_msg(struct subd *sd, const u8 *msg, const int *fds UNUSE void peer_start_closingd(struct channel *channel, const struct crypto_state *cs, - u64 gossip_index, int peer_fd, int gossip_fd, bool reconnected, const u8 *channel_reestablish) @@ -197,7 +193,6 @@ void peer_start_closingd(struct channel *channel, their_msatoshi = funding_msatoshi - our_msatoshi; initmsg = towire_closing_init(tmpctx, cs, - gossip_index, &channel->seed, &channel->funding_txid, channel->funding_outnum, diff --git a/lightningd/closing_control.h b/lightningd/closing_control.h index 56b25b7ff736..2611d19a2b53 100644 --- a/lightningd/closing_control.h +++ b/lightningd/closing_control.h @@ -8,7 +8,6 @@ struct crypto_state; void peer_start_closingd(struct channel *channel, const struct crypto_state *cs, - u64 gossip_index, int peer_fd, int gossip_fd, bool reconnected, const u8 *channel_reestablish); diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 062833431c52..518c3e237bd1 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -37,10 +37,9 @@ static void peer_nongossip(struct subd *gossip, const u8 *msg, struct crypto_state cs; struct wireaddr addr; u8 *gfeatures, *lfeatures, *in_pkt; - u64 gossip_index; if (!fromwire_gossip_peer_nongossip(msg, msg, - &id, &addr, &cs, &gossip_index, + &id, &addr, &cs, &gfeatures, &lfeatures, &in_pkt)) @@ -58,7 +57,7 @@ static void peer_nongossip(struct subd *gossip, const u8 *msg, return; } - peer_sent_nongossip(gossip->ld, &id, &addr, &cs, gossip_index, + peer_sent_nongossip(gossip->ld, &id, &addr, &cs, gfeatures, lfeatures, peer_fd, gossip_fd, in_pkt); } diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 52ad3f4eb80b..66af23b97a99 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -357,7 +357,6 @@ static bool tell_if_missing(const struct channel *channel, static void onchain_error(struct channel *channel, int peer_fd UNUSED, int gossip_fd UNUSED, const struct crypto_state *cs UNUSED, - u64 gossip_index UNUSED, const struct channel_id *channel_id UNUSED, const char *desc, const u8 *err_for_them UNUSED) diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 2f894563de2d..08bdee102cb0 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -101,7 +101,6 @@ static void remove_funding_channel_from_list(struct funding_channel *fc) static void uncommitted_channel_to_gossipd(struct lightningd *ld, struct uncommitted_channel *uc, const struct crypto_state *cs, - u64 gossip_index, int peer_fd, int gossip_fd, const u8 *errorpkt, const char *fmt, @@ -121,7 +120,6 @@ static void uncommitted_channel_to_gossipd(struct lightningd *ld, /* Hand back to gossipd, (maybe) with an error packet to send. */ msg = towire_gossipctl_hand_back_peer(errstr, &uc->peer->id, cs, - gossip_index, errorpkt); subd_send_msg(ld->gossip, take(msg)); subd_send_fd(ld->gossip, peer_fd); @@ -286,7 +284,6 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, secp256k1_ecdsa_signature remote_commit_sig; struct bitcoin_tx *remote_commit; u16 funding_outnum; - u64 gossip_index; u32 feerate; u64 change_satoshi; struct channel *channel; @@ -304,7 +301,6 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, &remote_commit, &remote_commit_sig, &cs, - &gossip_index, &channel_info.theirbase.revocation, &channel_info.theirbase.payment, &channel_info.theirbase.htlc, @@ -430,8 +426,7 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, tell_gossipd_peer_is_important(ld, channel); /* Start normal channel daemon. */ - peer_start_channeld(channel, &cs, gossip_index, - fds[0], fds[1], NULL, false); + peer_start_channeld(channel, &cs, fds[0], fds[1], NULL, false); wallet_confirm_utxos(ld->wallet, fc->utxomap); @@ -470,7 +465,6 @@ static void opening_fundee_finished(struct subd *openingd, u8 *funding_signed; struct channel_info channel_info; struct crypto_state cs; - u64 gossip_index; secp256k1_ecdsa_signature remote_commit_sig; struct bitcoin_tx *remote_commit; struct lightningd *ld = openingd->ld; @@ -492,7 +486,6 @@ static void opening_fundee_finished(struct subd *openingd, &remote_commit, &remote_commit_sig, &cs, - &gossip_index, &channel_info.theirbase.revocation, &channel_info.theirbase.payment, &channel_info.theirbase.htlc, @@ -537,7 +530,7 @@ static void opening_fundee_finished(struct subd *openingd, tell_gossipd_peer_is_important(ld, channel); /* On to normal operation! */ - peer_start_channeld(channel, &cs, gossip_index, + peer_start_channeld(channel, &cs, fds[0], fds[1], funding_signed, false); subd_release_channel(openingd, uc); @@ -548,7 +541,6 @@ static void opening_fundee_finished(struct subd *openingd, static void opening_channel_errmsg(struct uncommitted_channel *uc, int peer_fd, int gossip_fd, const struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id UNUSED, const char *desc, const u8 *err_for_them) @@ -562,7 +554,7 @@ static void opening_channel_errmsg(struct uncommitted_channel *uc, const char *errsrc = err_for_them ? "sent" : "received"; uncommitted_channel_to_gossipd(uc->peer->ld, uc, - cs, gossip_index, + cs, peer_fd, gossip_fd, err_for_them, "%s ERROR %s", errsrc, desc); @@ -687,7 +679,6 @@ u8 *peer_accept_channel(const tal_t *ctx, const struct pubkey *peer_id, const struct wireaddr *addr, const struct crypto_state *cs, - u64 gossip_index, const u8 *gfeatures UNUSED, const u8 *lfeatures UNUSED, int peer_fd, int gossip_fd, const struct channel_id *channel_id, @@ -722,7 +713,7 @@ u8 *peer_accept_channel(const tal_t *ctx, errpkt = towire_errorfmt(uc, channel_id, "%s", errmsg); uncommitted_channel_to_gossipd(ld, uc, - cs, gossip_index, + cs, peer_fd, gossip_fd, errpkt, "%s", errmsg); tal_free(uc); @@ -745,7 +736,7 @@ u8 *peer_accept_channel(const tal_t *ctx, &uc->our_config, max_to_self_delay, min_effective_htlc_capacity_msat, - cs, gossip_index, &uc->seed); + cs, &uc->seed); subd_send_msg(uc->openingd, take(msg)); @@ -769,7 +760,6 @@ static void peer_offer_channel(struct lightningd *ld, struct funding_channel *fc, const struct wireaddr *addr, const struct crypto_state *cs, - u64 gossip_index, const u8 *gfeatures UNUSED, const u8 *lfeatures UNUSED, int peer_fd, int gossip_fd) { @@ -806,7 +796,6 @@ static void peer_offer_channel(struct lightningd *ld, /* We don't send them an error packet: for them, nothing * happened! */ uncommitted_channel_to_gossipd(ld, fc->uc, NULL, - gossip_index, peer_fd, gossip_fd, NULL, "Failed to launch openingd: %s", @@ -824,7 +813,7 @@ static void peer_offer_channel(struct lightningd *ld, &fc->uc->our_config, max_to_self_delay, min_effective_htlc_capacity_msat, - cs, gossip_index, &fc->uc->seed); + cs, &fc->uc->seed); subd_send_msg(fc->uc->openingd, take(msg)); msg = towire_opening_funder(fc, fc->funding_satoshi, @@ -848,7 +837,6 @@ static void gossip_peer_released(struct subd *gossip, { struct lightningd *ld = gossip->ld; struct crypto_state cs; - u64 gossip_index; u8 *gfeatures, *lfeatures; struct wireaddr addr; struct channel *c; @@ -861,7 +849,6 @@ static void gossip_peer_released(struct subd *gossip, c = active_channel_by_id(ld, &fc->peerid, &uc); if (!fromwire_gossipctl_release_peer_reply(fc, resp, &addr, &cs, - &gossip_index, &gfeatures, &lfeatures)) { if (!fromwire_gossipctl_release_peer_replyfail(resp)) { fatal("Gossip daemon gave invalid reply %s", @@ -884,7 +871,7 @@ static void gossip_peer_released(struct subd *gossip, assert(!uc); /* OK, offer peer a channel. */ - peer_offer_channel(ld, fc, &addr, &cs, gossip_index, + peer_offer_channel(ld, fc, &addr, &cs, gfeatures, lfeatures, fds[0], fds[1]); } @@ -896,7 +883,6 @@ bool handle_opening_channel(struct lightningd *ld, const struct pubkey *id, const struct wireaddr *addr, const struct crypto_state *cs, - u64 gossip_index, const u8 *gfeatures, const u8 *lfeatures, int peer_fd, int gossip_fd) { @@ -905,7 +891,7 @@ bool handle_opening_channel(struct lightningd *ld, if (!fc) return false; - peer_offer_channel(ld, fc, addr, cs, gossip_index, gfeatures, lfeatures, + peer_offer_channel(ld, fc, addr, cs, gfeatures, lfeatures, peer_fd, gossip_fd); return true; } diff --git a/lightningd/opening_control.h b/lightningd/opening_control.h index d57b7050f5ea..bbd2781af30e 100644 --- a/lightningd/opening_control.h +++ b/lightningd/opening_control.h @@ -23,7 +23,6 @@ u8 *peer_accept_channel(const tal_t *ctx, const struct pubkey *peer_id, const struct wireaddr *addr, const struct crypto_state *cs, - u64 gossip_index, const u8 *gfeatures, const u8 *lfeatures, int peer_fd, int gossip_fd, const struct channel_id *channel_id, @@ -35,7 +34,6 @@ bool handle_opening_channel(struct lightningd *ld, const struct pubkey *id, const struct wireaddr *addr, const struct crypto_state *cs, - u64 gossip_index, const u8 *gfeatures, const u8 *lfeatures, int peer_fd, int gossip_fd); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 71752d5dce9a..40e7381aa785 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -346,7 +346,6 @@ void drop_to_chain(struct lightningd *ld, struct channel *channel, void channel_errmsg(struct channel *channel, int peer_fd, int gossip_fd, const struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id UNUSED, const char *desc, const u8 *err_for_them) @@ -393,8 +392,7 @@ void channel_errmsg(struct channel *channel, /* Hand back to gossipd, with any error packet. */ msg = towire_gossipctl_hand_back_peer(NULL, &channel->peer->id, - cs, gossip_index, - err_for_them); + cs, err_for_them); subd_send_msg(ld->gossip, take(msg)); subd_send_fd(ld->gossip, peer_fd); subd_send_fd(ld->gossip, gossip_fd); @@ -412,11 +410,10 @@ void peer_connected(struct lightningd *ld, const u8 *msg, u8 *local_features; struct channel *channel; struct wireaddr addr; - u64 gossip_index; struct uncommitted_channel *uc; if (!fromwire_gossip_peer_connected(msg, msg, - &id, &addr, &cs, &gossip_index, + &id, &addr, &cs, &gfeatures, &lfeatures)) fatal("Gossip gave bad GOSSIP_PEER_CONNECTED message %s", tal_hex(msg, msg)); @@ -441,7 +438,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, } /* Were we trying to open a channel, and we've raced? */ - if (handle_opening_channel(ld, &id, &addr, &cs, gossip_index, + if (handle_opening_channel(ld, &id, &addr, &cs, gfeatures, lfeatures, peer_fd, gossip_fd)) return; @@ -489,7 +486,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, channel_set_owner(channel, NULL); channel->peer->addr = addr; - peer_start_channeld(channel, &cs, gossip_index, + peer_start_channeld(channel, &cs, peer_fd, gossip_fd, NULL, true); return; @@ -500,7 +497,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, channel_set_owner(channel, NULL); channel->peer->addr = addr; - peer_start_closingd(channel, &cs, gossip_index, + peer_start_closingd(channel, &cs, peer_fd, gossip_fd, true, NULL); return; @@ -514,8 +511,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, send_error: /* Hand back to gossipd, with an error packet. */ - msg = towire_gossipctl_hand_back_peer(msg, &id, &cs, gossip_index, - error); + msg = towire_gossipctl_hand_back_peer(msg, &id, &cs, error); subd_send_msg(ld->gossip, take(msg)); subd_send_fd(ld->gossip, peer_fd); subd_send_fd(ld->gossip, gossip_fd); @@ -543,7 +539,6 @@ void peer_sent_nongossip(struct lightningd *ld, const struct pubkey *id, const struct wireaddr *addr, const struct crypto_state *cs, - u64 gossip_index, const u8 *gfeatures, const u8 *lfeatures, int peer_fd, int gossip_fd, @@ -563,7 +558,7 @@ void peer_sent_nongossip(struct lightningd *ld, /* Open request? */ if (fromwire_peektype(in_msg) == WIRE_OPEN_CHANNEL) { error = peer_accept_channel(tmpctx, - ld, id, addr, cs, gossip_index, + ld, id, addr, cs, gfeatures, lfeatures, peer_fd, gossip_fd, channel_id, in_msg); @@ -588,7 +583,7 @@ void peer_sent_nongossip(struct lightningd *ld, if (fromwire_peektype(in_msg) == WIRE_CHANNEL_REESTABLISH && channel && channel->state == CLOSINGD_COMPLETE) { - peer_start_closingd(channel, cs, gossip_index, + peer_start_closingd(channel, cs, peer_fd, gossip_fd, true, in_msg); return; } @@ -601,7 +596,7 @@ void peer_sent_nongossip(struct lightningd *ld, send_error: /* Hand back to gossipd, with an error packet. */ - msg = towire_gossipctl_hand_back_peer(ld, id, cs, gossip_index, error); + msg = towire_gossipctl_hand_back_peer(ld, id, cs, error); subd_send_msg(ld->gossip, take(msg)); subd_send_fd(ld->gossip, peer_fd); subd_send_fd(ld->gossip, gossip_fd); diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index dd8b524f11d7..eb56bcb64440 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -80,7 +80,6 @@ void peer_sent_nongossip(struct lightningd *ld, const struct pubkey *id, const struct wireaddr *addr, const struct crypto_state *cs, - u64 gossip_index, const u8 *gfeatures, const u8 *lfeatures, int peer_fd, int gossip_fd, @@ -92,7 +91,6 @@ void peer_sent_nongossip(struct lightningd *ld, void channel_errmsg(struct channel *channel, int peer_fd, int gossip_fd, const struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, const char *desc, const u8 *err_for_them); diff --git a/lightningd/subd.c b/lightningd/subd.c index 363a0c9910d0..27e1e626d684 100644 --- a/lightningd/subd.c +++ b/lightningd/subd.c @@ -396,17 +396,16 @@ static bool handle_peer_error(struct subd *sd, const u8 *msg, int fds[2]) struct channel_id channel_id; char *desc; struct crypto_state cs; - u64 gossip_index; u8 *err_for_them; if (!fromwire_status_peer_error(msg, msg, &channel_id, &desc, - &cs, &gossip_index, &err_for_them)) + &cs, &err_for_them)) return false; /* Don't free sd; we're may be about to free channel. */ sd->channel = NULL; - sd->errcb(channel, fds[0], fds[1], &cs, gossip_index, + sd->errcb(channel, fds[0], fds[1], &cs, &channel_id, desc, err_for_them); return true; } @@ -587,7 +586,7 @@ static void destroy_subd(struct subd *sd) if (!outer_transaction) db_begin_transaction(db); if (sd->errcb) - sd->errcb(channel, -1, -1, NULL, 0, NULL, + sd->errcb(channel, -1, -1, NULL, NULL, tal_fmt(sd, "Owning subdaemon %s died (%i)", sd->name, status), NULL); @@ -638,7 +637,6 @@ static struct subd *new_subd(struct lightningd *ld, void (*errcb)(void *channel, int peer_fd, int gossip_fd, const struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, const char *desc, const u8 *err_for_them), @@ -726,7 +724,6 @@ struct subd *new_channel_subd_(struct lightningd *ld, void (*errcb)(void *channel, int peer_fd, int gossip_fd, const struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, const char *desc, const u8 *err_for_them), diff --git a/lightningd/subd.h b/lightningd/subd.h index 6afe6a8234c9..5a9b8a08687a 100644 --- a/lightningd/subd.h +++ b/lightningd/subd.h @@ -44,7 +44,6 @@ struct subd { void (*errcb)(void *channel, int peer_fd, int gossip_fd, const struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, const char *desc, const u8 *err_for_them); @@ -116,7 +115,6 @@ struct subd *new_channel_subd_(struct lightningd *ld, void (*errcb)(void *channel, int peer_fd, int gossip_fd, const struct crypto_state *cs, - u64 gossip_index, const struct channel_id *channel_id, const char *desc, const u8 *err_for_them), @@ -130,7 +128,6 @@ struct subd *new_channel_subd_(struct lightningd *ld, typesafe_cb_postargs(void, void *, (errcb), \ (channel), int, int, \ const struct crypto_state *, \ - u64, \ const struct channel_id *, \ const char *, const u8 *), \ typesafe_cb_postargs(void, void *, (billboardcb), \ diff --git a/openingd/opening.c b/openingd/opening.c index cae0cb49ab7c..8ca4aecf0942 100644 --- a/openingd/opening.c +++ b/openingd/opening.c @@ -40,7 +40,6 @@ struct state { struct crypto_state cs; - u64 gossip_index; struct pubkey next_per_commit[NUM_SIDES]; /* Initially temporary, then final channel id. */ @@ -82,14 +81,14 @@ static void negotiation_failed(struct state *state, const char *fmt, ...) peer_billboard(true, errmsg); msg = towire_status_peer_error(NULL, &state->channel_id, - errmsg, &state->cs, state->gossip_index, + errmsg, &state->cs, towire_errorfmt(errmsg, &state->channel_id, "You gave bad parameters:%s", errmsg)); tal_free(errmsg); status_send_fatal(take(msg), PEER_FD, GOSSIP_FD); - peer_failed(&state->cs, state->gossip_index, &state->channel_id, + peer_failed(&state->cs, &state->channel_id, "You gave bad parameters: %s", errmsg); } @@ -230,7 +229,7 @@ static u8 *opening_read_peer_msg(struct state *state) { u8 *msg; - while ((msg = read_peer_msg(state, &state->cs, state->gossip_index, + while ((msg = read_peer_msg(state, &state->cs, &state->channel_id, sync_crypto_write_arg, status_fail_io, @@ -329,7 +328,7 @@ static u8 *funder_channel(struct state *state, &theirs.delayed_payment, &theirs.htlc, &state->next_per_commit[REMOTE])) - peer_failed(&state->cs, state->gossip_index, + peer_failed(&state->cs, &state->channel_id, "Parsing accept_channel %s", tal_hex(msg, msg)); @@ -338,7 +337,7 @@ static u8 *funder_channel(struct state *state, * The `temporary_channel_id` MUST be the same as the * `temporary_channel_id` in the `open_channel` message. */ if (!structeq(&id_in, &state->channel_id)) - peer_failed(&state->cs, state->gossip_index, + peer_failed(&state->cs, &state->channel_id, "accept_channel ids don't match: sent %s got %s", type_to_string(msg, struct channel_id, &id_in), @@ -419,7 +418,7 @@ static u8 *funder_channel(struct state *state, &their_funding_pubkey, LOCAL); if (!state->channel) - peer_failed(&state->cs, state->gossip_index, + peer_failed(&state->cs, &state->channel_id, "could not create channel with given config"); @@ -466,7 +465,7 @@ static u8 *funder_channel(struct state *state, msg = opening_read_peer_msg(state); if (!fromwire_funding_signed(msg, &id_in, &sig)) - peer_failed(&state->cs, state->gossip_index, + peer_failed(&state->cs, &state->channel_id, "Parsing funding_signed: %s", tal_hex(msg, msg)); @@ -482,7 +481,7 @@ static u8 *funder_channel(struct state *state, &state->funding_txid, state->funding_txout); if (!structeq(&id_in, &state->channel_id)) - peer_failed(&state->cs, state->gossip_index, &id_in, + peer_failed(&state->cs, &id_in, "funding_signed ids don't match: expected %s got %s", type_to_string(msg, struct channel_id, &state->channel_id), @@ -499,7 +498,7 @@ static u8 *funder_channel(struct state *state, "Could not meet our fees and reserve"); if (!check_tx_sig(tx, 0, NULL, wscript, &their_funding_pubkey, &sig)) { - peer_failed(&state->cs, state->gossip_index, + peer_failed(&state->cs, &state->channel_id, "Bad signature %s on tx %s using key %s", type_to_string(tmpctx, secp256k1_ecdsa_signature, @@ -518,7 +517,7 @@ static u8 *funder_channel(struct state *state, state->remoteconf, tx, &sig, - &state->cs, state->gossip_index, + &state->cs, &theirs.revocation, &theirs.payment, &theirs.htlc, @@ -574,7 +573,7 @@ static u8 *fundee_channel(struct state *state, &theirs.htlc, &state->next_per_commit[REMOTE], &channel_flags)) - peer_failed(&state->cs, state->gossip_index, NULL, + peer_failed(&state->cs, NULL, "Bad open_channel %s", tal_hex(peer_msg, peer_msg)); @@ -607,7 +606,7 @@ static u8 *fundee_channel(struct state *state, * greater than `funding_satoshis` * 1000. */ if (state->push_msat > state->funding_satoshis * 1000) - peer_failed(&state->cs, state->gossip_index, + peer_failed(&state->cs, &state->channel_id, "Our push_msat %"PRIu64 " would be too large for funding_satoshis %"PRIu64, @@ -684,7 +683,7 @@ static u8 *fundee_channel(struct state *state, &state->funding_txid, &state->funding_txout, &theirsig)) - peer_failed(&state->cs, state->gossip_index, + peer_failed(&state->cs, &state->channel_id, "Parsing funding_created"); @@ -693,7 +692,7 @@ static u8 *fundee_channel(struct state *state, * The sender MUST set `temporary_channel_id` the same as the * `temporary_channel_id` in the `open_channel` message. */ if (!structeq(&id_in, &state->channel_id)) - peer_failed(&state->cs, state->gossip_index, &id_in, + peer_failed(&state->cs, &id_in, "funding_created ids don't match: sent %s got %s", type_to_string(msg, struct channel_id, &state->channel_id), @@ -712,7 +711,7 @@ static u8 *fundee_channel(struct state *state, &their_funding_pubkey, REMOTE); if (!state->channel) - peer_failed(&state->cs, state->gossip_index, + peer_failed(&state->cs, &state->channel_id, "We could not create channel with given config"); @@ -728,7 +727,7 @@ static u8 *fundee_channel(struct state *state, if (!check_tx_sig(their_commit, 0, NULL, wscript, &their_funding_pubkey, &theirsig)) { - peer_failed(&state->cs, state->gossip_index, + peer_failed(&state->cs, &state->channel_id, "Bad signature %s on tx %s using key %s", type_to_string(tmpctx, secp256k1_ecdsa_signature, @@ -776,7 +775,6 @@ static u8 *fundee_channel(struct state *state, their_commit, &theirsig, &state->cs, - state->gossip_index, &theirs.revocation, &theirs.payment, &theirs.htlc, @@ -820,7 +818,6 @@ int main(int argc, char *argv[]) &state->max_to_self_delay, &state->min_effective_htlc_capacity_msat, &state->cs, - &state->gossip_index, &seed)) master_badmsg(WIRE_OPENING_INIT, msg); diff --git a/openingd/opening_wire.csv b/openingd/opening_wire.csv index eced430fafe5..c55bc3edd4e5 100644 --- a/openingd/opening_wire.csv +++ b/openingd/opening_wire.csv @@ -9,7 +9,6 @@ opening_init,,our_config,struct channel_config opening_init,,max_to_self_delay,u32 opening_init,,min_effective_htlc_capacity_msat,u64 opening_init,,crypto_state,struct crypto_state -opening_init,,gossip_index,u64 # Seed to generate all the keys from opening_init,,seed,struct privkey @@ -35,7 +34,6 @@ opening_funder_reply,,their_config,struct channel_config opening_funder_reply,,first_commit,struct bitcoin_tx opening_funder_reply,,first_commit_sig,secp256k1_ecdsa_signature opening_funder_reply,,crypto_state,struct crypto_state -opening_funder_reply,,gossip_index,u64 opening_funder_reply,,revocation_basepoint,struct pubkey opening_funder_reply,,payment_basepoint,struct pubkey opening_funder_reply,,htlc_basepoint,struct pubkey @@ -60,7 +58,6 @@ opening_fundee_reply,,their_config,struct channel_config opening_fundee_reply,,first_commit,struct bitcoin_tx opening_fundee_reply,,first_commit_sig,secp256k1_ecdsa_signature opening_fundee_reply,,crypto_state,struct crypto_state -opening_fundee_reply,,gossip_index,u64 opening_fundee_reply,,revocation_basepoint,struct pubkey opening_fundee_reply,,payment_basepoint,struct pubkey opening_fundee_reply,,htlc_basepoint,struct pubkey diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 0aa18e579dce..bbbf9bab4e8e 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -81,7 +81,7 @@ bool fromwire_gossipctl_peer_disconnect_replyfail(const void *p UNNEEDED, bool * bool fromwire_gossip_getpeers_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey **id UNNEEDED, struct wireaddr **addr UNNEEDED, struct gossip_getnodes_entry ***nodes UNNEEDED) { fprintf(stderr, "fromwire_gossip_getpeers_reply called!\n"); abort(); } /* Generated stub for fromwire_gossip_peer_connected */ -bool fromwire_gossip_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey *id UNNEEDED, struct wireaddr *addr UNNEEDED, struct crypto_state *crypto_state UNNEEDED, u64 *gossip_index UNNEEDED, u8 **gfeatures UNNEEDED, u8 **lfeatures UNNEEDED) +bool fromwire_gossip_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey *id UNNEEDED, struct wireaddr *addr UNNEEDED, struct crypto_state *crypto_state UNNEEDED, u8 **gfeatures UNNEEDED, u8 **lfeatures UNNEEDED) { fprintf(stderr, "fromwire_gossip_peer_connected called!\n"); abort(); } /* Generated stub for get_feerate */ u32 get_feerate(const struct chain_topology *topo UNNEEDED, enum feerate feerate UNNEEDED) @@ -97,7 +97,6 @@ bool handle_opening_channel(struct lightningd *ld UNNEEDED, const struct pubkey *id UNNEEDED, const struct wireaddr *addr UNNEEDED, const struct crypto_state *cs UNNEEDED, - u64 gossip_index UNNEEDED, const u8 *gfeatures UNNEEDED, const u8 *lfeatures UNNEEDED, int peer_fd UNNEEDED, int gossip_fd UNNEEDED) { fprintf(stderr, "handle_opening_channel called!\n"); abort(); } @@ -309,7 +308,6 @@ u8 *peer_accept_channel(const tal_t *ctx UNNEEDED, const struct pubkey *peer_id UNNEEDED, const struct wireaddr *addr UNNEEDED, const struct crypto_state *cs UNNEEDED, - u64 gossip_index UNNEEDED, const u8 *gfeatures UNNEEDED, const u8 *lfeatures UNNEEDED, int peer_fd UNNEEDED, int gossip_fd UNNEEDED, const struct channel_id *channel_id UNNEEDED, @@ -318,7 +316,6 @@ u8 *peer_accept_channel(const tal_t *ctx UNNEEDED, /* Generated stub for peer_start_channeld */ bool peer_start_channeld(struct channel *channel UNNEEDED, const struct crypto_state *cs UNNEEDED, - u64 gossip_index UNNEEDED, int peer_fd UNNEEDED, int gossip_fd UNNEEDED, const u8 *funding_signed UNNEEDED, bool reconnected UNNEEDED) @@ -326,7 +323,6 @@ bool peer_start_channeld(struct channel *channel UNNEEDED, /* Generated stub for peer_start_closingd */ void peer_start_closingd(struct channel *channel UNNEEDED, const struct crypto_state *cs UNNEEDED, - u64 gossip_index UNNEEDED, int peer_fd UNNEEDED, int gossip_fd UNNEEDED, bool reconnected UNNEEDED, const u8 *channel_reestablish UNNEEDED) @@ -367,7 +363,7 @@ u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "towire_errorfmt called!\n"); abort(); } /* Generated stub for towire_gossipctl_hand_back_peer */ -u8 *towire_gossipctl_hand_back_peer(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED, const struct crypto_state *crypto_state UNNEEDED, u64 gossip_index UNNEEDED, const u8 *msg UNNEEDED) +u8 *towire_gossipctl_hand_back_peer(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED, const struct crypto_state *crypto_state UNNEEDED, const u8 *msg UNNEEDED) { fprintf(stderr, "towire_gossipctl_hand_back_peer called!\n"); abort(); } /* Generated stub for towire_gossipctl_peer_addrhint */ u8 *towire_gossipctl_peer_addrhint(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED, const struct wireaddr *addr UNNEEDED) From c75ecd954c3266a8a52af8376da5614030985365 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 10/21] read_peer_msg: handle incoming gossip from gossipd. This means that openingd and closingd now forward our gossip. But the real reason we want to do this is that it gives an easy way for gossipd to kill any active daemon, by closing its fd: previously closingd and openingd didn't read the fd, so tended not to notice. Signed-off-by: Rusty Russell --- channeld/channel.c | 34 ++++++-------------------- closingd/Makefile | 5 +++- common/read_peer_msg.c | 55 ++++++++++++++++++++++++++++++++++++++++++ common/read_peer_msg.h | 18 ++++++++++++++ openingd/Makefile | 5 +++- 5 files changed, 89 insertions(+), 28 deletions(-) diff --git a/channeld/channel.c b/channeld/channel.c index 4ac546ea2a02..a789f8f86d73 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -298,29 +298,6 @@ static void enqueue_peer_msg(struct peer *peer, const u8 *msg TAKES) msg_enqueue(&peer->peer_out, msg); } -static void gossip_in(struct peer *peer, const u8 *msg) -{ - u8 *gossip; - - if (!fromwire_gossip_send_gossip(msg, msg, &gossip)) - status_failed(STATUS_FAIL_GOSSIP_IO, - "Got bad message from gossipd: %s", - tal_hex(msg, msg)); - - if (is_msg_for_gossipd(gossip)) - enqueue_peer_msg(peer, gossip); - else if (fromwire_peektype(gossip) == WIRE_ERROR) { - struct channel_id channel_id; - char *what = sanitize_error(msg, msg, &channel_id); - peer_failed(&peer->cs, &channel_id, - "gossipd said: %s", what); - } else - status_failed(STATUS_FAIL_GOSSIP_IO, - "Got bad message type %s from gossipd: %s", - wire_type_name(fromwire_peektype(gossip)), - tal_hex(msg, msg)); -} - /* Send a temporary `channel_announcement` and `channel_update`. These * are unsigned and mainly used to tell gossip about the channel * before we have reached the `announcement_depth`, not being signed @@ -2632,8 +2609,10 @@ int main(int argc, char *argv[]) if (msg) { status_trace("Now dealing with deferred gossip %u", fromwire_peektype(msg)); - gossip_in(peer, msg); - tal_free(msg); + handle_gossip_msg(take(msg), &peer->cs, + channeld_send_reply, + channeld_io_error, + peer); continue; } @@ -2687,7 +2666,10 @@ int main(int argc, char *argv[]) status_failed(STATUS_FAIL_GOSSIP_IO, "Can't read command: %s", strerror(errno)); - gossip_in(peer, msg); + handle_gossip_msg(msg, &peer->cs, + channeld_send_reply, + channeld_io_error, + peer); } else if (FD_ISSET(PEER_FD, &rfds)) { /* This could take forever, but who cares? */ msg = channeld_read_peer_msg(peer); diff --git a/closingd/Makefile b/closingd/Makefile index fb5d5c2a2742..3834f5074f7a 100644 --- a/closingd/Makefile +++ b/closingd/Makefile @@ -66,7 +66,10 @@ CLOSINGD_COMMON_OBJS := \ common/type_to_string.o \ common/utils.o \ common/version.o \ - common/wire_error.o + common/wire_error.o \ + common/wireaddr.o \ + gossipd/gen_gossip_wire.o \ + lightningd/gossip_msg.o closingd/gen_closing_wire.h: $(WIRE_GEN) closingd/closing_wire.csv $(WIRE_GEN) --header $@ closing_wire_type < closingd/closing_wire.csv > $@ diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index 9193efa76154..a90952fd6c0e 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -38,6 +40,38 @@ static void handle_ping(const u8 *msg, io_error(arg); } +void handle_gossip_msg_(const u8 *msg TAKES, int peer_fd, + struct crypto_state *cs, + bool (*send_msg)(struct crypto_state *cs, int fd, + const u8 *TAKES, void *arg), + void (*io_error)(void *arg), + void *arg) +{ + u8 *gossip; + + if (!fromwire_gossip_send_gossip(tmpctx, msg, &gossip)) { + status_broken("Got bad message from gossipd: %s", + tal_hex(msg, msg)); + io_error(arg); + } + + /* Gossipd can send us gossip messages, OR errors */ + if (is_msg_for_gossipd(gossip)) { + if (!send_msg(cs, peer_fd, gossip, arg)) + io_error(arg); + } else if (fromwire_peektype(gossip) == WIRE_ERROR) { + status_debug("Gossipd old us to send error"); + send_msg(cs, peer_fd, gossip, arg); + io_error(arg); + } else { + status_broken("Gossipd gave us bad send_gossip message %s", + tal_hex(msg, msg)); + io_error(arg); + } + if (taken(msg)) + tal_free(msg); +} + u8 *read_peer_msg_(const tal_t *ctx, int peer_fd, int gossip_fd, struct crypto_state *cs, @@ -49,6 +83,27 @@ u8 *read_peer_msg_(const tal_t *ctx, { u8 *msg; struct channel_id chanid; + fd_set readfds; + + FD_ZERO(&readfds); + FD_SET(peer_fd, &readfds); + FD_SET(gossip_fd, &readfds); + + select(peer_fd > gossip_fd ? peer_fd + 1 : gossip_fd + 1, + &readfds, NULL, NULL, NULL); + + if (FD_ISSET(gossip_fd, &readfds)) { + /* gossipd uses this to kill us, so not a surprise if it + happens. */ + msg = wire_sync_read(NULL, gossip_fd); + if (!msg) { + status_debug("Error reading gossip msg"); + io_error(arg); + } + + handle_gossip_msg_(msg, peer_fd, cs, send_reply, io_error, arg); + return NULL; + } msg = sync_crypto_read(ctx, cs, peer_fd); if (!msg) diff --git a/common/read_peer_msg.h b/common/read_peer_msg.h index 83f50c394b16..78ee219dbdab 100644 --- a/common/read_peer_msg.h +++ b/common/read_peer_msg.h @@ -36,6 +36,24 @@ bool sync_crypto_write_arg(struct crypto_state *cs, int fd, const u8 *TAKES, /* Helper: calls peer_failed_connection_lost. */ void status_fail_io(void *unused); +/* Handler for a gossip msg; used by channeld since it queues them. */ +#define handle_gossip_msg(msg, cs, send_reply, io_error, arg) \ + handle_gossip_msg_((msg), PEER_FD, (cs), \ + typesafe_cb_preargs(bool, void *, \ + (send_reply), (arg), \ + struct crypto_state *, int, \ + const u8 *), \ + typesafe_cb(void, void *, (io_error), (arg)), \ + arg) + +void handle_gossip_msg_(const u8 *msg TAKES, + int peer_fd, + struct crypto_state *cs, + bool (*send_msg)(struct crypto_state *cs, int fd, + const u8 *TAKES, void *arg), + void (*io_error)(void *arg), + void *arg); + u8 *read_peer_msg_(const tal_t *ctx, int peer_fd, int gossip_fd, struct crypto_state *cs, diff --git a/openingd/Makefile b/openingd/Makefile index d2245a44eca8..d4842f6bc6f2 100644 --- a/openingd/Makefile +++ b/openingd/Makefile @@ -67,7 +67,10 @@ OPENINGD_COMMON_OBJS := \ common/utils.o \ common/utxo.o \ common/version.o \ - common/wire_error.o + common/wire_error.o \ + common/wireaddr.o \ + gossipd/gen_gossip_wire.o \ + lightningd/gossip_msg.o $(LIGHTNINGD_OPENING_OBJS): $(LIGHTNINGD_HEADERS) From 174e5781c0846d2cbfd600857da31b808dfa7e8c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 11/21] subd: record which ones connect to a peer. This comes in useful for the next patch. Signed-off-by: Rusty Russell --- lightningd/channel_control.c | 5 +++-- lightningd/closing_control.c | 5 +++-- lightningd/onchain_control.c | 2 +- lightningd/opening_control.c | 15 ++++++++------- lightningd/subd.c | 7 +++++-- lightningd/subd.h | 9 +++++++-- 6 files changed, 27 insertions(+), 16 deletions(-) diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 207c74b0e866..2c0f71b5528f 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -199,9 +199,10 @@ bool peer_start_channeld(struct channel *channel, if (hsmfd < 0) fatal("Could not read fd from HSM: %s", strerror(errno)); - channel_set_owner(channel, new_channel_subd(ld, + channel_set_owner(channel, + new_channel_subd(ld, "lightning_channeld", channel, - channel->log, + channel->log, true, channel_wire_type_name, channel_msg, channel_errmsg, diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index a41c0a29164b..bd2006d36570 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -145,9 +145,10 @@ void peer_start_closingd(struct channel *channel, return; } - channel_set_owner(channel, new_channel_subd(ld, + channel_set_owner(channel, + new_channel_subd(ld, "lightning_closingd", - channel, channel->log, + channel, channel->log, true, closing_wire_type_name, closing_msg, channel_errmsg, channel_set_billboard, diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 66af23b97a99..199f44dd0444 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -387,7 +387,7 @@ enum watch_result onchaind_funding_spent(struct channel *channel, channel_set_owner(channel, new_channel_subd(ld, "lightning_onchaind", channel, - channel->log, + channel->log, false, onchain_wire_type_name, onchain_msg, onchain_error, diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 08bdee102cb0..360916ec05d6 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -698,7 +698,7 @@ u8 *peer_accept_channel(const tal_t *ctx, "Multiple channels unsupported"); uc->openingd = new_channel_subd(ld, "lightning_openingd", uc, uc->log, - opening_wire_type_name, NULL, + true, opening_wire_type_name, NULL, opening_channel_errmsg, opening_channel_set_billboard, take(&peer_fd), take(&gossip_fd), @@ -786,12 +786,13 @@ static void peer_offer_channel(struct lightningd *ld, tal_steal(fc->uc, fc); fc->uc->openingd = new_channel_subd(ld, - "lightning_openingd", fc->uc, fc->uc->log, - opening_wire_type_name, NULL, - opening_channel_errmsg, - opening_channel_set_billboard, - take(&peer_fd), take(&gossip_fd), - NULL); + "lightning_openingd", + fc->uc, fc->uc->log, + true, opening_wire_type_name, NULL, + opening_channel_errmsg, + opening_channel_set_billboard, + take(&peer_fd), take(&gossip_fd), + NULL); if (!fc->uc->openingd) { /* We don't send them an error packet: for them, nothing * happened! */ diff --git a/lightningd/subd.c b/lightningd/subd.c index 27e1e626d684..f4c69cb9241a 100644 --- a/lightningd/subd.c +++ b/lightningd/subd.c @@ -631,6 +631,7 @@ static struct subd *new_subd(struct lightningd *ld, const char *name, void *channel, struct log *base_log, + bool talks_to_peer, const char *(*msgname)(int msgtype), unsigned int (*msgcb)(struct subd *, const u8 *, const int *fds), @@ -674,6 +675,7 @@ static struct subd *new_subd(struct lightningd *ld, sd->name = name; sd->must_not_exit = false; + sd->talks_to_peer = talks_to_peer; sd->msgname = msgname; sd->msgcb = msgcb; sd->errcb = errcb; @@ -707,7 +709,7 @@ struct subd *new_global_subd(struct lightningd *ld, struct subd *sd; va_start(ap, msgcb); - sd = new_subd(ld, name, NULL, NULL, msgname, msgcb, NULL, NULL, &ap); + sd = new_subd(ld, name, NULL, NULL, false, msgname, msgcb, NULL, NULL, &ap); va_end(ap); sd->must_not_exit = true; @@ -718,6 +720,7 @@ struct subd *new_channel_subd_(struct lightningd *ld, const char *name, void *channel, struct log *base_log, + bool talks_to_peer, const char *(*msgname)(int msgtype), unsigned int (*msgcb)(struct subd *, const u8 *, const int *fds), @@ -735,7 +738,7 @@ struct subd *new_channel_subd_(struct lightningd *ld, struct subd *sd; va_start(ap, billboardcb); - sd = new_subd(ld, name, channel, base_log, msgname, + sd = new_subd(ld, name, channel, base_log, talks_to_peer, msgname, msgcb, errcb, billboardcb, &ap); va_end(ap); return sd; diff --git a/lightningd/subd.h b/lightningd/subd.h index 5a9b8a08687a..037e9c7c20b1 100644 --- a/lightningd/subd.h +++ b/lightningd/subd.h @@ -61,6 +61,9 @@ struct subd { /* For global daemons: we fail if they fail. */ bool must_not_exit; + /* Do we talk to a peer? ie. not onchaind */ + bool talks_to_peer; + /* Messages queue up here. */ struct msg_queue outq; @@ -109,6 +112,7 @@ struct subd *new_channel_subd_(struct lightningd *ld, const char *name, void *channel, struct log *base_log, + bool talks_to_peer, const char *(*msgname)(int msgtype), unsigned int (*msgcb)(struct subd *, const u8 *, const int *fds), @@ -122,9 +126,10 @@ struct subd *new_channel_subd_(struct lightningd *ld, const char *happenings), ...); -#define new_channel_subd(ld, name, channel, log, msgname, \ +#define new_channel_subd(ld, name, channel, log, talks_to_peer, msgname, \ msgcb, errcb, billboardcb, ...) \ - new_channel_subd_((ld), (name), (channel), (log), (msgname), (msgcb), \ + new_channel_subd_((ld), (name), (channel), (log), (talks_to_peer), \ + (msgname), (msgcb), \ typesafe_cb_postargs(void, void *, (errcb), \ (channel), int, int, \ const struct crypto_state *, \ From 2b24e641042d1518e7e4aa06741ca8115c9bb540 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 12/21] gossipd: have master explicitly tell us when peer is disconnected. Currently we intuit it from the fd being closed, but that may happen out of order with when the master thinks it's dead. So now if the gossip fd closes we just ignore it, and we'll get a notification from the master when the peer is disconnected. The notification is slightly ugly in that we have to disable it for a channel when we manually hand the channel back to gossipd. Note: as stands, this is racy with reconnects. See the next patch. Signed-off-by: Rusty Russell --- gossipd/gossip.c | 51 +++++++++++++++++++++++++----------- gossipd/gossip_wire.csv | 4 +++ lightningd/channel.c | 19 ++++++++++++-- lightningd/channel.h | 6 ++++- lightningd/gossip_control.c | 1 + lightningd/opening_control.c | 6 ++++- lightningd/peer_control.c | 3 +++ wallet/test/run-wallet.c | 3 +++ wallet/wallet.c | 4 ++- 9 files changed, 76 insertions(+), 21 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 5312949355d2..164bc37db410 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -398,8 +398,8 @@ static struct io_plan *peer_init_received(struct io_conn *conn, peer->broadcast_index = peer->daemon->rstate->broadcasts->next_index; - /* This is a full peer now; we keep it around until its - * gossipfd closed (forget_peer) or reconnect. */ + /* This is a full peer now; we keep it around until master says + * it's dead, or reconnect. */ peer_finalized(peer); /* We will not have anything queued, since we're not duplex. */ @@ -845,25 +845,12 @@ static struct io_plan *owner_msg_in(struct io_conn *conn, status_broken("peer %s: send us unknown msg of type %s", type_to_string(tmpctx, struct pubkey, &peer->id), gossip_wire_type_name(type)); - /* Calls forget_peer */ return io_close(conn); } return daemon_conn_read_next(conn, dc); } -static void forget_peer(struct io_conn *conn UNUSED, struct daemon_conn *dc) -{ - struct peer *peer = dc->ctx; - - status_trace("Forgetting %s peer %s", - peer->local ? "local" : "remote", - type_to_string(tmpctx, struct pubkey, &peer->id)); - - /* Free peer. */ - tal_free(dc->ctx); -} - /* When a peer is to be owned by another daemon, we create a socket * pair to send/receive gossip from it */ static bool send_peer_with_fds(struct peer *peer, const u8 *msg) @@ -884,7 +871,7 @@ static bool send_peer_with_fds(struct peer *peer, const u8 *msg) peer->local = tal_free(peer->local); peer->remote = tal(peer, struct daemon_conn); daemon_conn_init(peer, peer->remote, fds[0], - owner_msg_in, forget_peer); + owner_msg_in, NULL); peer->remote->msg_queue_cleared_cb = nonlocal_dump_gossip; /* Peer stays around, even though caller will close conn. */ @@ -1871,6 +1858,35 @@ static struct io_plan *peer_important(struct io_conn *conn, return daemon_conn_read_next(conn, &daemon->master); } +static struct io_plan *peer_disconnected(struct io_conn *conn, + struct daemon *daemon, const u8 *msg) +{ + struct pubkey id; + struct peer *peer; + + if (!fromwire_gossipctl_peer_disconnected(msg, &id)) + master_badmsg(WIRE_GOSSIPCTL_PEER_DISCONNECTED, msg); + + peer = find_peer(daemon, &id); + if (!peer) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "peer_disconnected unknown peer: %s", + type_to_string(tmpctx, struct pubkey, &id)); + + /* Possible if there's a reconnect: ignore disonnect. */ + if (peer->local) { + status_trace("peer_disconnected %s: reconnected, ignoring", + type_to_string(tmpctx, struct pubkey, &id)); + return daemon_conn_read_next(conn, &daemon->master); + } + + status_trace("Forgetting remote peer %s", + type_to_string(tmpctx, struct pubkey, &peer->id)); + + tal_free(peer); + return daemon_conn_read_next(conn, &daemon->master); +} + static struct io_plan *get_peers(struct io_conn *conn, struct daemon *daemon, const u8 *msg) { @@ -2122,6 +2138,9 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_GOSSIPCTL_PEER_IMPORTANT: return peer_important(conn, daemon, master->msg_in); + case WIRE_GOSSIPCTL_PEER_DISCONNECTED: + return peer_disconnected(conn, daemon, master->msg_in); + case WIRE_GOSSIP_GETPEERS_REQUEST: return get_peers(conn, daemon, master->msg_in); diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index e021e3029e8b..2c258922856b 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -92,6 +92,10 @@ gossipctl_hand_back_peer,,crypto_state,struct crypto_state gossipctl_hand_back_peer,,len,u16 gossipctl_hand_back_peer,,msg,len*u8 +# master -> gossipd: peer has disconnected. +gossipctl_peer_disconnected,3015 +gossipctl_peer_disconnected,,id,struct pubkey + # Pass JSON-RPC getnodes call through gossip_getnodes_request,3005 # Can be 0 or 1 currently diff --git a/lightningd/channel.c b/lightningd/channel.c index c2ee3d678586..54c4d27db5fd 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -12,13 +12,26 @@ #include #include +static bool connects_to_peer(struct subd *owner) +{ + return owner && owner->talks_to_peer; +} + void channel_set_owner(struct channel *channel, struct subd *owner) { struct subd *old_owner = channel->owner; channel->owner = owner; - if (old_owner) + if (old_owner) { subd_release_channel(old_owner, channel); + if (channel->connected && !connects_to_peer(owner)) { + u8 *msg = towire_gossipctl_peer_disconnected(NULL, + &channel->peer->id); + subd_send_msg(channel->peer->ld->gossip, take(msg)); + channel->connected = false; + } + } + channel->connected = connects_to_peer(owner); } struct htlc_out *channel_has_htlc_out(struct channel *channel) @@ -155,7 +168,8 @@ struct channel *new_channel(struct peer *peer, u64 dbid, struct changed_htlc *last_sent_commit, u32 first_blocknum, u32 min_possible_feerate, - u32 max_possible_feerate) + u32 max_possible_feerate, + bool connected) { struct channel *channel = tal(peer->ld, struct channel); @@ -212,6 +226,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, channel->first_blocknum = first_blocknum; channel->min_possible_feerate = min_possible_feerate; channel->max_possible_feerate = max_possible_feerate; + channel->connected = connected; derive_channel_seed(peer->ld, &channel->seed, &peer->id, channel->dbid); list_add_tail(&peer->channels, &channel->list); diff --git a/lightningd/channel.h b/lightningd/channel.h index 291d037cd662..2684c15a5572 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -96,6 +96,9 @@ struct channel { /* Feerate range */ u32 min_possible_feerate, max_possible_feerate; + + /* Does gossipd need to know if the owner dies? (ie. not onchaind) */ + bool connected; }; struct channel *new_channel(struct peer *peer, u64 dbid, @@ -136,7 +139,8 @@ struct channel *new_channel(struct peer *peer, u64 dbid, struct changed_htlc *last_sent_commit, u32 first_blocknum, u32 min_possible_feerate, - u32 max_possible_feerate); + u32 max_possible_feerate, + bool connected); void delete_channel(struct channel *channel); diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 518c3e237bd1..5928b4f9e2db 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -137,6 +137,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) case WIRE_GOSSIP_MARK_CHANNEL_UNROUTABLE: case WIRE_GOSSIPCTL_PEER_DISCONNECT: case WIRE_GOSSIPCTL_PEER_IMPORTANT: + case WIRE_GOSSIPCTL_PEER_DISCONNECTED: /* This is a reply, so never gets through to here. */ case WIRE_GOSSIPCTL_INIT_REPLY: case WIRE_GOSSIP_GET_UPDATE_REPLY: diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 360916ec05d6..dec49908e6c1 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -237,7 +237,9 @@ wallet_commit_channel(struct lightningd *ld, final_key_idx, false, NULL, /* No commit sent yet */ uc->first_blocknum, - feerate, feerate); + feerate, feerate, + /* We are connected */ + true); /* Now we finally put it in the database. */ wallet_channel_insert(ld->wallet, channel); @@ -546,7 +548,9 @@ static void opening_channel_errmsg(struct uncommitted_channel *uc, const u8 *err_for_them) { if (peer_fd == -1) { + u8 *msg = towire_gossipctl_peer_disconnected(tmpctx, &uc->peer->id); log_info(uc->log, "%s", desc); + subd_send_msg(uc->peer->ld->gossip, msg); if (uc->fc) command_fail(uc->fc->cmd, "%s", desc); } else { diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 40e7381aa785..90055e5dd9a9 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -366,6 +366,9 @@ void channel_errmsg(struct channel *channel, err_for_them, tal_len(err_for_them), 0); + /* Make sure channel_fail_permanent doesn't tell gossipd we died! */ + channel->connected = false; + /* BOLT #1: * * A sending node: diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index bbbf9bab4e8e..c55b6da6e071 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -371,6 +371,9 @@ u8 *towire_gossipctl_peer_addrhint(const tal_t *ctx UNNEEDED, const struct pubke /* Generated stub for towire_gossipctl_peer_disconnect */ u8 *towire_gossipctl_peer_disconnect(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED) { fprintf(stderr, "towire_gossipctl_peer_disconnect called!\n"); abort(); } +/* Generated stub for towire_gossipctl_peer_disconnected */ +u8 *towire_gossipctl_peer_disconnected(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED) +{ fprintf(stderr, "towire_gossipctl_peer_disconnected called!\n"); abort(); } /* Generated stub for towire_gossipctl_peer_important */ u8 *towire_gossipctl_peer_important(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED, bool important UNNEEDED) { fprintf(stderr, "towire_gossipctl_peer_important called!\n"); abort(); } diff --git a/wallet/wallet.c b/wallet/wallet.c index 1f1bfc124f8c..ae7b5d118d03 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -651,7 +651,9 @@ static struct channel *wallet_stmt2channel(const tal_t *ctx, struct wallet *w, s last_sent_commit, sqlite3_column_int64(stmt, 35), sqlite3_column_int(stmt, 36), - sqlite3_column_int(stmt, 37)); + sqlite3_column_int(stmt, 37), + /* Not connected */ + false); return chan; } From ad6f7654b9fc8e4aaef7a9b49a997ca2705ef078 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 13/21] gossipd: make sure master only ever sees one active connection. When we get a reconnection, kill the current remote peer, and wait for the master to tell us it's dead. Then we hand it the new peer. Previously, we would end up with gossipd holding multiple peers, and the logging was really hard to interpret; I'm not completely convinced that we did the right thing when one terminated, either. Note that this now means we can have peers with neither ->local nor ->remote populated, so we check that more carefully. Signed-off-by: Rusty Russell --- channeld/channel.c | 7 +- gossipd/gossip.c | 147 ++++++++++++++++++++++++++++++-------- lightningd/peer_control.c | 19 ++--- tests/test_lightningd.py | 5 +- 4 files changed, 130 insertions(+), 48 deletions(-) diff --git a/channeld/channel.c b/channeld/channel.c index a789f8f86d73..0949587376a7 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -2661,11 +2661,10 @@ int main(int argc, char *argv[]) req_in(peer, msg); } else if (FD_ISSET(GOSSIP_FD, &rfds)) { msg = wire_sync_read(peer, GOSSIP_FD); - + /* Gossipd hangs up on us to kill us when a new + * connection comes in. */ if (!msg) - status_failed(STATUS_FAIL_GOSSIP_IO, - "Can't read command: %s", - strerror(errno)); + peer_conn_broken(peer); handle_gossip_msg(msg, &peer->cs, channeld_send_reply, channeld_io_error, diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 164bc37db410..9a3f013bbe9b 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -88,9 +88,12 @@ struct daemon { /* Who am I? */ struct pubkey id; - /* Peers we have directly or indirectly */ + /* Peers we have directly or indirectly: id is unique */ struct list_head peers; + /* Peers reconnecting now (waiting for current peer to die). */ + struct list_head reconnecting; + /* Peers we are trying to reach */ struct list_head reaching; @@ -228,6 +231,34 @@ static struct peer *find_peer(struct daemon *daemon, const struct pubkey *id) return NULL; } +static struct peer *find_reconnecting_peer(struct daemon *daemon, + const struct pubkey *id) +{ + struct peer *peer; + + list_for_each(&daemon->reconnecting, peer, list) + if (pubkey_eq(&peer->id, id)) + return peer; + return NULL; +} + +static void destroy_reconnecting_peer(struct peer *peer) +{ + list_del_from(&peer->daemon->reconnecting, &peer->list); + /* This is safe even if we're being destroyed because of peer->conn, + * since tal_free protects against loops. */ + io_close(peer->local->conn); +} + +static void add_reconnecting_peer(struct daemon *daemon, struct peer *peer) +{ + /* Drop any previous connecting peer */ + tal_free(find_reconnecting_peer(peer->daemon, &peer->id)); + + list_add_tail(&daemon->reconnecting, &peer->list); + tal_add_destructor(peer, destroy_reconnecting_peer); +} + static void destroy_addrhint(struct addrhint *a) { list_del(&a->list); @@ -329,7 +360,7 @@ static void queue_peer_msg(struct peer *peer, const u8 *msg TAKES) { if (peer->local) { msg_enqueue(&peer->local->peer_out, msg); - } else { + } else if (peer->remote) { const u8 *send = towire_gossip_send_gossip(NULL, msg); if (taken(msg)) tal_free(msg); @@ -372,15 +403,44 @@ static struct io_plan *peer_close_after_error(struct io_conn *conn, return io_close(conn); } -static struct io_plan *peer_init_received(struct io_conn *conn, - struct peer *peer, - u8 *msg) +/* Mutual recursion */ +static struct io_plan *peer_connected(struct io_conn *conn, struct peer *peer); +static struct io_plan *retry_peer_connected(struct io_conn *conn, + struct peer *peer) { - if (!fromwire_init(peer, msg, &peer->gfeatures, &peer->lfeatures)){ - status_trace("peer %s bad fromwire_init '%s', closing", + status_trace("peer %s: processing now old peer gone", + type_to_string(tmpctx, struct pubkey, &peer->id)); + + /* Clean up reconnecting state, try again */ + list_del_from(&peer->daemon->reconnecting, &peer->list); + tal_del_destructor(peer, destroy_reconnecting_peer); + + return peer_connected(conn, peer); +} + +static struct io_plan *peer_connected(struct io_conn *conn, struct peer *peer) +{ + struct peer *old_peer; + u8 *msg; + + /* Now, is this a reconnect? */ + old_peer = find_peer(peer->daemon, &peer->id); + if (old_peer) { + status_trace("peer %s: reconnect for %s", type_to_string(tmpctx, struct pubkey, &peer->id), - tal_hex(tmpctx, msg)); - return io_close(conn); + old_peer->local ? "local peer" : "active peer"); + if (!old_peer->local) { + /* If not already closed, close it: it will + * fail, and master will peer_died to us */ + if (old_peer->remote) { + daemon_conn_clear(old_peer->remote); + old_peer->remote = tal_free(old_peer->remote); + } + add_reconnecting_peer(peer->daemon, peer); + return io_wait(conn, peer, retry_peer_connected, peer); + } + /* Local peers can just be discarded when they reconnect */ + tal_free(old_peer); } reached_peer(peer, conn); @@ -399,7 +459,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn, = peer->daemon->rstate->broadcasts->next_index; /* This is a full peer now; we keep it around until master says - * it's dead, or reconnect. */ + * it's dead. */ peer_finalized(peer); /* We will not have anything queued, since we're not duplex. */ @@ -410,14 +470,25 @@ static struct io_plan *peer_init_received(struct io_conn *conn, return io_close(conn); /* Start the gossip flowing. */ - /* FIXME: This is a bit wasteful in the common case where master - * simply hands it straight back to us and we restart the peer and - * restart gossip broadcast... */ wake_pkt_out(peer); return io_close_taken_fd(conn); } +static struct io_plan *peer_init_received(struct io_conn *conn, + struct peer *peer, + u8 *msg) +{ + if (!fromwire_init(peer, msg, &peer->gfeatures, &peer->lfeatures)) { + status_trace("peer %s bad fromwire_init '%s', closing", + type_to_string(tmpctx, struct pubkey, &peer->id), + tal_hex(tmpctx, msg)); + return io_close(conn); + } + + return peer_connected(conn, peer); +} + static struct io_plan *read_init(struct io_conn *conn, struct peer *peer) { /* BOLT #1: @@ -715,7 +786,7 @@ static void wake_pkt_out(struct peer *peer) if (peer->local) /* Notify the peer-write loop */ msg_wake(&peer->local->peer_out); - else + else if (peer->remote) /* Notify the daemon_conn-write loop */ msg_wake(&peer->remote->out); } @@ -851,6 +922,13 @@ static struct io_plan *owner_msg_in(struct io_conn *conn, return daemon_conn_read_next(conn, dc); } +static void free_peer_remote(struct io_conn *conn, struct daemon_conn *dc) +{ + struct peer *peer = dc->ctx; + + peer->remote = tal_free(peer->remote); +} + /* When a peer is to be owned by another daemon, we create a socket * pair to send/receive gossip from it */ static bool send_peer_with_fds(struct peer *peer, const u8 *msg) @@ -871,12 +949,15 @@ static bool send_peer_with_fds(struct peer *peer, const u8 *msg) peer->local = tal_free(peer->local); peer->remote = tal(peer, struct daemon_conn); daemon_conn_init(peer, peer->remote, fds[0], - owner_msg_in, NULL); + owner_msg_in, free_peer_remote); peer->remote->msg_queue_cleared_cb = nonlocal_dump_gossip; /* Peer stays around, even though caller will close conn. */ tal_steal(peer->daemon, peer); + status_debug("peer %s now remote", + type_to_string(tmpctx, struct pubkey, &peer->id)); + daemon_conn_send(&peer->daemon->master, msg); daemon_conn_send_fd(&peer->daemon->master, peer_fd); daemon_conn_send_fd(&peer->daemon->master, fds[1]); @@ -965,7 +1046,7 @@ static struct io_plan *handle_returning_peer(struct io_conn *conn, struct returning_peer *rpeer) { struct daemon *daemon = rpeer->daemon; - struct peer *peer; + struct peer *peer, *connecting; peer = find_peer(daemon, &rpeer->id); if (!peer) @@ -973,14 +1054,20 @@ static struct io_plan *handle_returning_peer(struct io_conn *conn, "hand_back_peer unknown peer: %s", type_to_string(tmpctx, struct pubkey, &rpeer->id)); - /* Possible if there's a reconnect: ignore handed back. */ - if (peer->local) { - status_trace("hand_back_peer %s: reconnected, dropping handback", - type_to_string(tmpctx, struct pubkey, &rpeer->id)); + assert(!peer->local); + + /* Corner case: we got a reconnection while master was handing this + * back. We would have killed it immediately if it was local previously + * so do that now */ + connecting = find_reconnecting_peer(daemon, &rpeer->id); + if (connecting) { + status_trace("Forgetting handed back peer %s", + type_to_string(tmpctx, struct pubkey, &peer->id)); + + tal_free(peer); + /* Now connecting peer can go ahead. */ + io_wake(connecting); - close(rpeer->gossip_fd); - close(rpeer->peer_fd); - tal_free(rpeer); return daemon_conn_read_next(conn, &daemon->master); } @@ -1873,17 +1960,18 @@ static struct io_plan *peer_disconnected(struct io_conn *conn, "peer_disconnected unknown peer: %s", type_to_string(tmpctx, struct pubkey, &id)); - /* Possible if there's a reconnect: ignore disonnect. */ - if (peer->local) { - status_trace("peer_disconnected %s: reconnected, ignoring", - type_to_string(tmpctx, struct pubkey, &id)); - return daemon_conn_read_next(conn, &daemon->master); - } + assert(!peer->local); status_trace("Forgetting remote peer %s", type_to_string(tmpctx, struct pubkey, &peer->id)); tal_free(peer); + + /* If there was a connecting peer waiting, wake it now */ + peer = find_reconnecting_peer(daemon, &id); + if (peer) + io_wake(peer); + return daemon_conn_read_next(conn, &daemon->master); } @@ -2205,6 +2293,7 @@ int main(int argc, char *argv[]) daemon = tal(NULL, struct daemon); list_head_init(&daemon->peers); + list_head_init(&daemon->reconnecting); list_head_init(&daemon->reaching); list_head_init(&daemon->addrhints); important_peerid_map_init(&daemon->important_peerids); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 90055e5dd9a9..1b84b31c1a86 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -401,7 +401,8 @@ void channel_errmsg(struct channel *channel, subd_send_fd(ld->gossip, gossip_fd); } -/* Gossipd tells us a peer has connected */ +/* Gossipd tells us a peer has connected: it never hands us duplicates, since + * it holds them until we say peer_died. */ void peer_connected(struct lightningd *ld, const u8 *msg, int peer_fd, int gossip_fd) { @@ -450,11 +451,8 @@ void peer_connected(struct lightningd *ld, const u8 *msg, * channel. */ channel = active_channel_by_id(ld, &id, &uc); - /* Opening now? Kill it */ - if (uc) { - kill_uncommitted_channel(uc, "Peer reconnected"); - goto return_to_gossipd; - } + /* Can't be opening now, since we wouldn't have sent peer_died. */ + assert(!uc); if (channel) { log_debug(channel->log, "Peer has reconnected, state %s", @@ -484,9 +482,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, case CHANNELD_AWAITING_LOCKIN: case CHANNELD_NORMAL: case CHANNELD_SHUTTING_DOWN: - /* Stop any existing daemon, without triggering error - * on this peer. */ - channel_set_owner(channel, NULL); + assert(!channel->owner); channel->peer->addr = addr; peer_start_channeld(channel, &cs, @@ -495,9 +491,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, return; case CLOSINGD_SIGEXCHANGE: - /* Stop any existing daemon, without triggering error - * on this peer. */ - channel_set_owner(channel, NULL); + assert(!channel->owner); channel->peer->addr = addr; peer_start_closingd(channel, &cs, @@ -508,7 +502,6 @@ void peer_connected(struct lightningd *ld, const u8 *msg, abort(); } -return_to_gossipd: /* No err, all good. */ error = NULL; diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index dc7529930e4b..212f5e2629cc 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -3125,8 +3125,9 @@ def test_reconnect_openingd(self): # Reconnect. l1.rpc.connect(l2.info['id'], 'localhost', l2.info['port']) - # We should get a message about reconnecting. - l2.daemon.wait_for_log('Killing openingd: Peer reconnected') + # We should get a message about reconnecting, but order unsynced. + l2.daemon.wait_for_logs(['gossipd.*reconnect for active peer', + 'openingd.*Error reading gossip msg']) # Should work fine. l1.rpc.fundchannel(l2.info['id'], 20000) From 60e9d66e252bd9142fc558a86aad34c67d5dd806 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 14/21] gossipd: use exponential backoff on reconnect for important peers. We start at 1 second, back off to 5 minutes. Signed-off-by: Rusty Russell --- gossipd/gossip.c | 22 ++++++++++++++++++---- tests/test_lightningd.py | 12 +++++++++++- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 9a3f013bbe9b..4dde30b03cac 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -53,11 +53,17 @@ #define HSM_FD 3 +#define INITIAL_WAIT_SECONDS 1 +#define MAX_WAIT_SECONDS 300 + /* We put everything in this struct (redundantly) to pass it to timer cb */ struct important_peerid { struct daemon *daemon; struct pubkey id; + + /* How long to wait after failed connect */ + unsigned int wait_seconds; }; /* We keep a set of peer ids we're always trying to reach. */ @@ -217,8 +223,10 @@ static void destroy_peer(struct peer *peer) list_del_from(&peer->daemon->peers, &peer->list); imp = important_peerid_map_get(&peer->daemon->important_peerids, &peer->id); - if (imp) + if (imp) { + imp->wait_seconds = INITIAL_WAIT_SECONDS; retry_important(imp); + } } static struct peer *find_peer(struct daemon *daemon, const struct pubkey *id) @@ -1707,11 +1715,16 @@ static void connect_failed(struct io_conn *conn, struct reaching *reach) imp = important_peerid_map_get(&reach->daemon->important_peerids, &reach->id); if (imp) { - /* FIXME: Exponential backoff! */ - status_trace("...will try again in %u seconds", 5); + imp->wait_seconds *= 2; + if (imp->wait_seconds > MAX_WAIT_SECONDS) + imp->wait_seconds = MAX_WAIT_SECONDS; + + status_trace("...will try again in %u seconds", + imp->wait_seconds); /* If important_id freed, this will be removed too */ new_reltimer(&reach->daemon->timers, imp, - time_from_sec(5), retry_important, imp); + time_from_sec(imp->wait_seconds), + retry_important, imp); } tal_free(reach); return; @@ -1928,6 +1941,7 @@ static struct io_plan *peer_important(struct io_conn *conn, imp = tal(daemon, struct important_peerid); imp->id = id; imp->daemon = daemon; + imp->wait_seconds = INITIAL_WAIT_SECONDS; important_peerid_map_add(&daemon->important_peerids, imp); /* Start trying to reaching it now. */ diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 212f5e2629cc..4528872bc487 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -660,9 +660,19 @@ def test_reconnect_channel_peers(self): "Connection refused", l1.rpc.connect, l2.info['id'], 'localhost', l2.info['port']) + # Wait for exponential backoff to give us a 2 second window. + l1.daemon.wait_for_log('...will try again in 2 seconds') + # It should now succeed when it restarts. l2.daemon.start() - l1.rpc.connect(l2.info['id'], 'localhost', l2.info['port']) + + # Multiples should be fine! + fut1 = self.executor.submit(l1.rpc.connect, l2.info['id'], 'localhost', l2.info['port']) + fut2 = self.executor.submit(l1.rpc.connect, l2.info['id'], 'localhost', l2.info['port']) + fut3 = self.executor.submit(l1.rpc.connect, l2.info['id'], 'localhost', l2.info['port']) + fut1.result(10) + fut2.result(10) + fut3.result(10) def test_balance(self): l1, l2 = self.connect() From c34ac8b4ed15ac9fcb8dcdb2a6a12651833df4f5 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 15/21] json_listpeers: use channel connected flag for JSON. If a channel is active (ie. not onchaind) and has an owner, this should be equivalent. Signed-off-by: Rusty Russell --- lightningd/peer_control.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 1b84b31c1a86..8cb6ba5802c7 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -764,7 +764,7 @@ static void gossipd_getpeers_complete(struct subd *gossip, const u8 *msg, connected = true; else { channel = peer_active_channel(p); - connected = channel && channel->owner; + connected = channel && channel->connected; } json_add_bool(response, "connected", connected); From ea71a2a0cda5ba80c91bed057e733ffedeb9a680 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 16/21] gossipd: give more distinct errors. At least say whether we failed to connect at all, or failed cryptographic handshake, or failed reading/writing init messages. The errno can be "Operation now in progress" if the other end closes the socket on us: this happens when we handshake with the wrong key and it hangs up on us. Fixing this would require work on ccan/io though. Signed-off-by: Rusty Russell --- gossipd/gossip.c | 11 ++++++++++- gossipd/handshake.c | 1 + tests/test_lightningd.py | 10 ++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 4dde30b03cac..b04b37f12762 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -148,6 +148,9 @@ struct reaching { /* How many (if any) connect commands are waiting for the result. */ size_t num_master_responses; + + /* How far did we get? */ + const char *connstate; }; /* Things we need when we're talking direct to the peer. */ @@ -1681,6 +1684,7 @@ static struct io_plan *handshake_out_success(struct io_conn *conn, const struct crypto_state *cs, struct reaching *reach) { + reach->connstate = "Exchanging init messages"; return init_new_peer(conn, id, addr, cs, reach->daemon); } @@ -1692,6 +1696,7 @@ static struct io_plan *connection_out(struct io_conn *conn, status_trace("Connected out for %s", type_to_string(tmpctx, struct pubkey, &reach->id)); + reach->connstate = "Cryptographic handshake"; return initiator_handshake(conn, &reach->daemon->id, &reach->id, &reach->addr, handshake_out_success, reach); @@ -1701,10 +1706,13 @@ static void connect_failed(struct io_conn *conn, struct reaching *reach) { u8 *msg; struct important_peerid *imp; + const char *err = tal_fmt(tmpctx, "%s: %s", + reach->connstate, + strerror(errno)); /* Tell any connect commands what happened. */ msg = towire_gossipctl_connect_to_peer_result(reach, &reach->id, - false, strerror(errno)); + false, err); for (size_t i = 0; i < reach->num_master_responses; i++) daemon_conn_send(&reach->daemon->master, msg); @@ -1878,6 +1886,7 @@ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id, reach->id = *id; reach->addr = a->addr; reach->num_master_responses = master_needs_response; + reach->connstate = "Connection establishment"; list_add_tail(&daemon->reaching, &reach->list); tal_add_destructor(reach, destroy_reaching); diff --git a/gossipd/handshake.c b/gossipd/handshake.c index 04b61feb29c1..26dbf1c9cd40 100644 --- a/gossipd/handshake.c +++ b/gossipd/handshake.c @@ -341,6 +341,7 @@ static struct io_plan *handshake_failed_(struct io_conn *conn, status_trace("%s: handshake failed %s:%u", h->side == RESPONDER ? "Responder" : "Initiator", function, line); + errno = EPROTO; return io_close(conn); } #define handshake_failed(conn, h) \ diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 4528872bc487..31eabb676329 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -617,6 +617,16 @@ def test_connect(self): "No address known", l1.rpc.connect, '032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e') + # Should get reasonable error if connection refuse. + self.assertRaisesRegex(ValueError, + "Connection establishment: Connection refused", + l1.rpc.connect, '032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e', 'localhost', 1) + + # Should get reasonable error if wrong key for peer. + self.assertRaisesRegex(ValueError, + "Cryptographic handshake: ", + l1.rpc.connect, '032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e', 'localhost', l2.info['port']) + def test_connect_standard_addr(self): """Test standard node@host:port address """ From a02d466b46aa3b1d4445b43468bbecd3d639df7c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:01 +0930 Subject: [PATCH 17/21] gossipd: don't create a new chain of timers on every connect command. When a connect fails, if it's an important peer, we set a timer. If we have a manual connect command, this means we do this again, leading to another timer. For a manual command, free any existing timer; the normal fail logic will start another if necessary. Reported-by: @ZmnSCPxj Signed-off-by: Rusty Russell --- gossipd/gossip.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index b04b37f12762..9cc76a4915e2 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -64,6 +64,9 @@ struct important_peerid { /* How long to wait after failed connect */ unsigned int wait_seconds; + + /* The timer we're using to reconnect */ + struct oneshot *reconnect_timer; }; /* We keep a set of peer ids we're always trying to reach. */ @@ -1730,9 +1733,10 @@ static void connect_failed(struct io_conn *conn, struct reaching *reach) status_trace("...will try again in %u seconds", imp->wait_seconds); /* If important_id freed, this will be removed too */ - new_reltimer(&reach->daemon->timers, imp, - time_from_sec(imp->wait_seconds), - retry_important, imp); + imp->reconnect_timer + = new_reltimer(&reach->daemon->timers, imp, + time_from_sec(imp->wait_seconds), + retry_important, imp); } tal_free(reach); return; @@ -1896,6 +1900,9 @@ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id, /* Called from timer, so needs single-arg declaration */ static void retry_important(struct important_peerid *imp) { + /* In case we've come off a timer, don't leave dangling pointer */ + imp->reconnect_timer = NULL; + #if DEVELOPER /* With --dev-no-reconnect, we only want explicit * connects */ @@ -1909,10 +1916,15 @@ static struct io_plan *connect_to_peer(struct io_conn *conn, struct daemon *daemon, const u8 *msg) { struct pubkey id; + struct important_peerid *imp; if (!fromwire_gossipctl_connect_to_peer(msg, &id)) master_badmsg(WIRE_GOSSIPCTL_CONNECT_TO_PEER, msg); + /* If this is an important peer, free any outstanding timer */ + imp = important_peerid_map_get(&daemon->important_peerids, &id); + if (imp) + imp->reconnect_timer = tal_free(imp->reconnect_timer); try_reach_peer(daemon, &id, true); return daemon_conn_read_next(conn, &daemon->master); } From 442105025f5ba29ec2b61b0f741bcb79718c8e55 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:02 +0930 Subject: [PATCH 18/21] channeld: get told when announce depth already reached. If channeld dies for some reason (eg, reconnect) and we didn't yet announce the channel, we can miss doing so. This is unusual, because if lightningd restarts it rearms the callback which gives us funding_locked, so it only happens if just channel dies before sending the announcement message. This problem applies to both temporary announcement (for gossipd) and the real one. For the temporary one, simply re-send on startup, and remote the error msg gossipd gives if it sees a second one. For the real one, we need a flag to tell us the depth is sufficient; the peer will ignore re-sends anyway. Signed-off-by: Rusty Russell --- channeld/channel.c | 11 ++++++++++- channeld/channel_wire.csv | 1 + gossipd/routing.c | 3 ++- lightningd/channel_control.c | 12 ++++++++++-- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/channeld/channel.c b/channeld/channel.c index 0949587376a7..daf498a92d73 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -1940,6 +1940,10 @@ static void handle_funding_locked(struct peer *peer, const u8 *msg) static void handle_funding_announce_depth(struct peer *peer) { + /* This can happen if we got told already at init time */ + if (peer->announce_depth_reached) + return; + peer->announce_depth_reached = true; send_announcement_signatures(peer); @@ -2449,7 +2453,8 @@ static void init_channel(struct peer *peer) &peer->shutdown_sent[REMOTE], &peer->final_scriptpubkey, &peer->channel_flags, - &funding_signed)) + &funding_signed, + &peer->announce_depth_reached)) master_badmsg(WIRE_CHANNEL_INIT, msg); status_trace("init %s: remote_per_commit = %s, old_remote_per_commit = %s" @@ -2516,6 +2521,10 @@ static void init_channel(struct peer *peer) if (funding_signed) enqueue_peer_msg(peer, take(funding_signed)); + /* It's possible that we died previously before doing these. */ + send_temporary_announcement(peer); + send_announcement_signatures(peer); + billboard_update(peer); tal_free(msg); } diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index 75d5af20c906..d7ff88496354 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -58,6 +58,7 @@ channel_init,,final_scriptpubkey,final_scriptpubkey_len*u8 channel_init,,flags,u8 channel_init,,init_peer_pkt_len,u16 channel_init,,init_peer_pkt,init_peer_pkt_len*u8 +channel_init,,reached_announce_depth,bool # Tx is deep enough, go! channel_funding_locked,1002 diff --git a/gossipd/routing.c b/gossipd/routing.c index e87eaaed4f66..3d2ef944e26f 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1516,8 +1516,9 @@ void handle_local_add_channel(struct routing_state *rstate, u8 *msg) return; } + /* Can happen on channeld restart. */ if (get_channel(rstate, &scid)) { - status_broken("Attempted to local_add_channel a known channel"); + status_trace("Attempted to local_add_channel a known channel"); return; } diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 2c0f71b5528f..6bc694e7c058 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -186,6 +186,7 @@ bool peer_start_channeld(struct channel *channel, u64 num_revocations; struct lightningd *ld = channel->peer->ld; const struct config *cfg = &ld->config; + bool reached_announce_depth; msg = towire_hsm_client_hsmfd(tmpctx, &channel->peer->id, HSM_CAP_SIGN_GOSSIP | HSM_CAP_ECDH); if (!wire_sync_write(ld->hsm_fd, take(msg))) @@ -223,10 +224,16 @@ bool peer_start_channeld(struct channel *channel, if (channel->scid) { funding_channel_id = *channel->scid; - log_debug(channel->log, "Already have funding locked in"); + reached_announce_depth + = (short_channel_id_blocknum(&funding_channel_id) + + ANNOUNCE_MIN_DEPTH <= get_block_height(ld->topology)); + log_debug(channel->log, "Already have funding locked in%s", + reached_announce_depth + ? " (and ready to announce)" : ""); } else { log_debug(channel->log, "Waiting for funding confirmations"); memset(&funding_channel_id, 0, sizeof(funding_channel_id)); + reached_announce_depth = false; } num_revocations = revocations_received(&channel->their_shachain.chain); @@ -281,7 +288,8 @@ bool peer_start_channeld(struct channel *channel, p2wpkh_for_keyidx(tmpctx, ld, channel->final_key_idx), channel->channel_flags, - funding_signed); + funding_signed, + reached_announce_depth); /* We don't expect a response: we are triggered by funding_depth_cb. */ subd_send_msg(channel->owner, take(initmsg)); From 69fa1cc2d2cd358c810f8db9692a38b4909a20fa Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:02 +0930 Subject: [PATCH 19/21] lightningd: move "tell gossipd peer is no longer important" to drop_to_chain. Reported-by: @ZmnSCPxj Signed-off-by: Rusty Russell --- lightningd/channel.c | 5 ----- lightningd/closing_control.c | 4 ---- lightningd/peer_control.c | 6 ++++++ 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 54c4d27db5fd..26d74a368b4d 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -324,7 +324,6 @@ void channel_fail_permanent(struct channel *channel, const char *fmt, ...) va_list ap; char *why; struct channel_id cid; - u8 *msg; va_start(ap, fmt); why = tal_vfmt(channel, fmt, ap); @@ -349,10 +348,6 @@ void channel_fail_permanent(struct channel *channel, const char *fmt, ...) channel->error = towire_errorfmt(channel, &cid, "%s", why); } - /* Tell gossipd we no longer need to keep connection to this peer */ - msg = towire_gossipctl_peer_important(NULL, &channel->peer->id, false); - subd_send_msg(ld->gossip, take(msg)); - channel_set_owner(channel, NULL); /* Drop non-cooperatively (unilateral) to chain. */ drop_to_chain(ld, channel, false); diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index bd2006d36570..861579ade97c 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -96,10 +96,6 @@ static void peer_closing_complete(struct channel *channel, const u8 *msg) if (channel->state == CLOSINGD_COMPLETE) return; - /* Tell gossipd we no longer need to keep connection to this peer */ - msg = towire_gossipctl_peer_important(NULL, &channel->peer->id, false); - subd_send_msg(channel->peer->ld->gossip, take(msg)); - /* Channel gets dropped to chain cooperatively. */ drop_to_chain(channel->peer->ld, channel, true); channel_set_state(channel, CLOSINGD_SIGEXCHANGE, CLOSINGD_COMPLETE); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 8cb6ba5802c7..0c114d5f7df3 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -332,6 +332,12 @@ register_close_command(struct lightningd *ld, void drop_to_chain(struct lightningd *ld, struct channel *channel, bool cooperative) { + u8 *msg; + + /* Tell gossipd we no longer need to keep connection to this peer */ + msg = towire_gossipctl_peer_important(NULL, &channel->peer->id, false); + subd_send_msg(ld->gossip, take(msg)); + sign_last_tx(channel); /* Keep broadcasting until we say stop (can fail due to dup, From e9da2e16dbc1b81f620ca0dce2453cae8b3a1c64 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:02 +0930 Subject: [PATCH 20/21] common: typo fix. Old gossip is rarely interesting. Reported-by: @ZmnSCPxj Signed-off-by: Rusty Russell --- common/read_peer_msg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index a90952fd6c0e..03fa05b03ebe 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -60,7 +60,7 @@ void handle_gossip_msg_(const u8 *msg TAKES, int peer_fd, if (!send_msg(cs, peer_fd, gossip, arg)) io_error(arg); } else if (fromwire_peektype(gossip) == WIRE_ERROR) { - status_debug("Gossipd old us to send error"); + status_debug("Gossipd told us to send error"); send_msg(cs, peer_fd, gossip, arg); io_error(arg); } else { From a553b449f0519c37ee1d64a922009f8a3c2d24a1 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 Apr 2018 14:21:02 +0930 Subject: [PATCH 21/21] gossipd: don't handle multiple connect requests, combine them in lightningd. Christian points out that this is the pattern used elsewhere, for example. Signed-off-by: Rusty Russell --- gossipd/gossip.c | 37 +++++++++++++++++++++--------------- lightningd/connect_control.c | 32 ++++++++++++++++--------------- 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 9cc76a4915e2..3686ffdeb873 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -149,8 +149,8 @@ struct reaching { /* FIXME: Support multiple address. */ struct wireaddr addr; - /* How many (if any) connect commands are waiting for the result. */ - size_t num_master_responses; + /* Whether connect command is waiting for the result. */ + bool master_needs_response; /* How far did we get? */ const char *connstate; @@ -362,10 +362,12 @@ static void reached_peer(struct peer *peer, struct io_conn *conn) /* Don't free conn with reach */ tal_steal(peer->daemon, conn); - /* Tell any connect commands what happened. */ - msg = towire_gossipctl_connect_to_peer_result(r, &r->id, true, ""); - for (size_t i = 0; i < r->num_master_responses; i++) - daemon_conn_send(&peer->daemon->master, msg); + /* Tell any connect command what happened. */ + if (r->master_needs_response) { + msg = towire_gossipctl_connect_to_peer_result(NULL, &r->id, + true, ""); + daemon_conn_send(&peer->daemon->master, take(msg)); + } tal_free(r); } @@ -1713,11 +1715,12 @@ static void connect_failed(struct io_conn *conn, struct reaching *reach) reach->connstate, strerror(errno)); - /* Tell any connect commands what happened. */ - msg = towire_gossipctl_connect_to_peer_result(reach, &reach->id, - false, err); - for (size_t i = 0; i < reach->num_master_responses; i++) - daemon_conn_send(&reach->daemon->master, msg); + /* Tell any connect command what happened. */ + if (reach->master_needs_response) { + msg = towire_gossipctl_connect_to_peer_result(NULL, &reach->id, + false, err); + daemon_conn_send(&reach->daemon->master, take(msg)); + } status_trace("Failed connected out for %s", type_to_string(tmpctx, struct pubkey, &reach->id)); @@ -1832,9 +1835,13 @@ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id, /* If we're trying to reach it right now, that's OK. */ reach = find_reaching(daemon, id); if (reach) { - /* Please tell us too. */ - if (master_needs_response) - reach->num_master_responses++; + /* Please tell us too. Master should not ask twice (we'll + * only respond once, and so one request will get stuck) */ + if (reach->master_needs_response) + status_failed(STATUS_FAIL_MASTER_IO, + "Already reaching %s", + type_to_string(tmpctx, struct pubkey, id)); + reach->master_needs_response = true; return; } @@ -1889,7 +1896,7 @@ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id, reach->daemon = daemon; reach->id = *id; reach->addr = a->addr; - reach->num_master_responses = master_needs_response; + reach->master_needs_response = master_needs_response; reach->connstate = "Connection establishment"; list_add_tail(&daemon->reaching, &reach->list); tal_add_destructor(reach, destroy_reaching); diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c index e9a3669db427..3fa915e3ccd4 100644 --- a/lightningd/connect_control.c +++ b/lightningd/connect_control.c @@ -61,17 +61,18 @@ void gossip_connect_result(struct lightningd *ld, const u8 *msg) tal_hex(msg, msg)); - c = find_connect(ld, &id); - assert(c); - - if (connected) { - struct json_result *response = new_json_result(c->cmd); - json_object_start(response, NULL); - json_add_pubkey(response, "id", &id); - json_object_end(response); - command_success(c->cmd, response); - } else { - command_fail(c->cmd, "%s", err); + /* We can have multiple connect commands: complete them all */ + while ((c = find_connect(ld, &id)) != NULL) { + if (connected) { + struct json_result *response = new_json_result(c->cmd); + json_object_start(response, NULL); + json_add_pubkey(response, "id", &id); + json_object_end(response); + command_success(c->cmd, response); + } else { + command_fail(c->cmd, "%s", err); + } + /* They delete themselves from list */ } } @@ -162,10 +163,11 @@ static void json_connect(struct command *cmd, subd_send_msg(cmd->ld->gossip, take(msg)); } - /* Now tell it to try reaching it. */ - msg = towire_gossipctl_connect_to_peer(NULL, &id); - subd_send_msg(cmd->ld->gossip, take(msg)); - + /* If there isn't already a connect command, tell gossipd */ + if (!find_connect(cmd->ld, &id)) { + msg = towire_gossipctl_connect_to_peer(NULL, &id); + subd_send_msg(cmd->ld->gossip, take(msg)); + } /* Leave this here for gossip_connect_result */ new_connect(cmd->ld, &id, cmd); command_still_pending(cmd);