From ebdecebb1a89f7dcd8daa53c57ec58af32f7c40d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 23 Oct 2017 14:41:38 +1030 Subject: [PATCH] channeld: send channel_announce and initial update to master, not gossipd. There is a race we see sometimes under valgrind on Travis which shows gossipd receiving the node_announce from master before it reads the channel_announce from channeld, and thus fails. The simplest solution is to send the channel_announce and channel_update to master as well, so it can ensure it sends them to gossipd in order Signed-off-by: Rusty Russell --- channeld/channel.c | 42 +++++++++++++++++++-------------------- channeld/channel_wire.csv | 8 ++++++-- lightningd/peer_control.c | 18 +++++++++++++---- 3 files changed, 40 insertions(+), 28 deletions(-) diff --git a/channeld/channel.c b/channeld/channel.c index 463aeeddb..11c01eb9d 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -231,9 +231,10 @@ static void send_announcement_signatures(struct peer *peer) tal_free(tmpctx); } -static void send_channel_update(struct peer *peer, bool disabled) +static u8 *create_channel_update(const tal_t *ctx, + struct peer *peer, bool disabled) { - tal_t *tmpctx = tal_tmpctx(peer); + tal_t *tmpctx = tal_tmpctx(ctx); u32 timestamp = time_now().ts.tv_sec; u16 flags; u8 *cupdate, *msg; @@ -257,14 +258,12 @@ static void send_channel_update(struct peer *peer, bool disabled) strerror(errno)); msg = wire_sync_read(tmpctx, HSM_FD); - if (!msg || !fromwire_hsm_cupdate_sig_reply(tmpctx, msg, NULL, &cupdate)) + if (!msg || !fromwire_hsm_cupdate_sig_reply(ctx, msg, NULL, &cupdate)) status_failed(STATUS_FAIL_HSM_IO, "Reading cupdate_sig_req: %s", strerror(errno)); - - daemon_conn_send(&peer->gossip_client, cupdate); - msg_enqueue(&peer->peer_out, cupdate); tal_free(tmpctx); + return cupdate; } /* Tentatively create a channel_announcement, possibly with invalid @@ -297,15 +296,6 @@ static u8 *create_channel_announcement(const tal_t *ctx, struct peer *peer) return cannounce; } -static void send_channel_announcement(struct peer *peer) -{ - u8 *msg = create_channel_announcement(peer, peer); - /* Makes a copy */ - msg_enqueue(&peer->peer_out, msg); - /* Takes ownership */ - daemon_conn_send(&peer->gossip_client, take(msg)); -} - static struct io_plan *peer_out(struct io_conn *conn, struct peer *peer) { const u8 *out = msg_dequeue(&peer->peer_out); @@ -362,11 +352,16 @@ static struct io_plan *handle_peer_funding_locked(struct io_conn *conn, static void announce_channel(struct peer *peer) { - send_channel_announcement(peer); - send_channel_update(peer, false); - /* Tell the master that we just announced the channel, - * so it may announce the node */ - wire_sync_write(MASTER_FD, take(towire_channel_announced(peer))); + u8 *cannounce, *cupdate; + + cannounce = create_channel_announcement(peer, peer); + cupdate = create_channel_update(cannounce, peer, false); + + /* Tell the master that we to announce channel (it does node) */ + wire_sync_write(MASTER_FD, take(towire_channel_announce(peer, + cannounce, + cupdate))); + tal_free(cannounce); } static struct io_plan *handle_peer_announcement_signatures(struct io_conn *conn, @@ -1406,7 +1401,10 @@ static void peer_conn_broken(struct io_conn *conn, struct peer *peer) { /* If we have signatures, send an update to say we're disabled. */ if (peer->have_sigs[LOCAL] && peer->have_sigs[REMOTE]) { - send_channel_update(peer, true); + u8 *cupdate = create_channel_update(conn, peer, true); + + daemon_conn_send(&peer->gossip_client, cupdate); + msg_enqueue(&peer->peer_out, take(cupdate)); /* Make sure gossipd actually gets this message before dying */ daemon_conn_sync_flush(&peer->gossip_client); @@ -1954,7 +1952,7 @@ static void req_in(struct peer *peer, const u8 *msg) case WIRE_CHANNEL_INIT: case WIRE_CHANNEL_OFFER_HTLC_REPLY: case WIRE_CHANNEL_PING_REPLY: - case WIRE_CHANNEL_ANNOUNCED: + case WIRE_CHANNEL_ANNOUNCE: case WIRE_CHANNEL_SENDING_COMMITSIG: case WIRE_CHANNEL_GOT_COMMITSIG: case WIRE_CHANNEL_GOT_REVOKE: diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index 5ad595fde..8c761e852 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -102,8 +102,12 @@ channel_ping,,len,u16 channel_ping_reply,1111 channel_ping_reply,,totlen,u16 -# Channeld tells the master that the channel has been announced -channel_announced,1012 +# Channeld tells the master to announce the channel (with first update) +channel_announce,1012 +channel_announce,,announce_len,u16 +channel_announce,,announce,announce_len*u8 +channel_announce,,update_len,u16 +channel_announce,,update,update_len*u8 # When we receive funding_locked. channel_got_funding_locked,1019 diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index d29084750..0284fa4a7 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1546,15 +1546,16 @@ static u8 *create_node_announcement(const tal_t *ctx, struct lightningd *ld, * an update, so we can now start sending a node_announcement. The * first step is to build the provisional announcement and ask the HSM * to sign it. */ -static void peer_channel_announced(struct peer *peer, const u8 *msg) +static void peer_channel_announce(struct peer *peer, const u8 *msg) { struct lightningd *ld = peer->ld; tal_t *tmpctx = tal_tmpctx(peer); secp256k1_ecdsa_signature sig; + u8 *cannounce, *cupdate; u8 *announcement, *wrappedmsg; u32 timestamp = time_now().ts.tv_sec; - if (!fromwire_channel_announced(msg, NULL)) { + if (!fromwire_channel_announce(msg, msg, NULL, &cannounce, &cupdate)) { peer_internal_error(peer, "bad fromwire_channel_announced %s", tal_hex(peer, msg)); return; @@ -1574,6 +1575,15 @@ static void peer_channel_announced(struct peer *peer, const u8 *msg) * from the HSM, create the real announcement and forward it to * gossipd so it can take care of forwarding it. */ announcement = create_node_announcement(tmpctx, ld, &sig, timestamp); + + /* We have to send channel_announce before channel_update and + * node_announcement */ + wrappedmsg = towire_gossip_forwarded_msg(tmpctx, cannounce); + subd_send_msg(ld->gossip, take(wrappedmsg)); + + wrappedmsg = towire_gossip_forwarded_msg(tmpctx, cupdate); + subd_send_msg(ld->gossip, take(wrappedmsg)); + wrappedmsg = towire_gossip_forwarded_msg(tmpctx, announcement); subd_send_msg(ld->gossip, take(wrappedmsg)); tal_free(tmpctx); @@ -1956,8 +1966,8 @@ static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds) case WIRE_CHANNEL_GOT_REVOKE: peer_got_revoke(sd->peer, msg); break; - case WIRE_CHANNEL_ANNOUNCED: - peer_channel_announced(sd->peer, msg); + case WIRE_CHANNEL_ANNOUNCE: + peer_channel_announce(sd->peer, msg); break; case WIRE_CHANNEL_GOT_FUNDING_LOCKED: peer_got_funding_locked(sd->peer, msg);