From bee795ed68b8899cdae5ecf3273897ba286a5277 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 23 Apr 2018 19:38:01 +0930 Subject: [PATCH] channeld: don't do explicit state update. We missed it in some corner cases where we crashed/were killed between being told of the lockin and sending the channel_normal_operation message. When we were restarted, we were told both sides were locked in already, so we never updated the state. Pull the entire "tell channeld" logic into channel_control.c, and make it clear that we need to keep waching if we cant't tell channeld. I think we did get this correct in practice, since funding_announce_cb has the same test, but it's better to be clear. Signed-off-by: Rusty Russell --- channeld/channel.c | 9 -------- channeld/channel_wire.csv | 3 --- lightningd/channel_control.c | 42 ++++++++++++++++++++++++++++++++---- lightningd/channel_control.h | 6 ++++++ lightningd/peer_control.c | 16 ++------------ wallet/test/run-wallet.c | 11 +++++----- 6 files changed, 51 insertions(+), 36 deletions(-) diff --git a/channeld/channel.c b/channeld/channel.c index 45562d0de..fb31eacbb 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -514,10 +514,6 @@ static void handle_peer_funding_locked(struct peer *peer, const u8 *msg) take(towire_channel_got_funding_locked(NULL, &peer->remote_per_commit))); - if (peer->funding_locked[LOCAL]) { - wire_sync_write(MASTER_FD, - take(towire_channel_normal_operation(NULL))); - } billboard_update(peer); /* Send temporary or final announcements */ @@ -1999,10 +1995,6 @@ static void handle_funding_locked(struct peer *peer, const u8 *msg) enqueue_peer_msg(peer, take(msg)); peer->funding_locked[LOCAL] = true; - if (peer->funding_locked[REMOTE]) { - wire_sync_write(MASTER_FD, - take(towire_channel_normal_operation(NULL))); - } billboard_update(peer); /* Send temporary or final announcements */ @@ -2414,7 +2406,6 @@ static void req_in(struct peer *peer, const u8 *msg) handle_dev_reenable_commit(peer); return; #endif /* DEVELOPER */ - case WIRE_CHANNEL_NORMAL_OPERATION: case WIRE_CHANNEL_INIT: case WIRE_CHANNEL_OFFER_HTLC_REPLY: case WIRE_CHANNEL_PING_REPLY: diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index da809d30e..446f0e5a2 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -1,6 +1,3 @@ -# Received and sent funding_locked -channel_normal_operation,11001 - #include #include diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 93e6c9222..c3413f9cd 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -14,6 +14,15 @@ #include #include +static void lockin_complete(struct channel *channel) +{ + /* We set this once we're locked in. */ + assert(channel->scid); + /* We set this once they're locked in. */ + assert(channel->remote_funding_locked); + channel_set_state(channel, CHANNELD_AWAITING_LOCKIN, CHANNELD_NORMAL); +} + /* We were informed by channeld that it announced the channel and sent * an update, so we can now start sending a node_announcement. The * first step is to build the provisional announcement and ask the HSM @@ -40,6 +49,9 @@ static void peer_got_funding_locked(struct channel *channel, const u8 *msg) log_debug(channel->log, "Got funding_locked"); channel->remote_funding_locked = true; + + if (channel->scid) + lockin_complete(channel); } static void peer_got_shutdown(struct channel *channel, const u8 *msg) @@ -111,10 +123,6 @@ static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds) enum channel_wire_type t = fromwire_peektype(msg); switch (t) { - case WIRE_CHANNEL_NORMAL_OPERATION: - channel_set_state(sd->channel, - CHANNELD_AWAITING_LOCKIN, CHANNELD_NORMAL); - break; case WIRE_CHANNEL_SENDING_COMMITSIG: peer_sending_commitsig(sd->channel, msg); break; @@ -281,3 +289,29 @@ bool peer_start_channeld(struct channel *channel, return true; } + +bool channel_tell_funding_locked(struct lightningd *ld, + struct channel *channel, + const struct bitcoin_txid *txid) +{ + /* If not awaiting lockin, it doesn't care any more */ + if (channel->state != CHANNELD_AWAITING_LOCKIN) { + log_debug(channel->log, + "Funding tx confirmed, but peer in state %s", + channel_state_name(channel)); + return true; + } + + if (!channel->owner) { + log_debug(channel->log, + "Funding tx confirmed, but peer disconnected"); + return false; + } + + subd_send_msg(channel->owner, + take(towire_channel_funding_locked(NULL, channel->scid))); + + if (channel->remote_funding_locked) + lockin_complete(channel); + return true; +} diff --git a/lightningd/channel_control.h b/lightningd/channel_control.h index e2b03eab5..16c54ac72 100644 --- a/lightningd/channel_control.h +++ b/lightningd/channel_control.h @@ -6,6 +6,7 @@ struct channel; struct crypto_state; +struct lightningd; bool peer_start_channeld(struct channel *channel, const struct crypto_state *cs, @@ -14,4 +15,9 @@ bool peer_start_channeld(struct channel *channel, const u8 *funding_signed, bool reconnected); +/* Returns true if subd told, otherwise false. */ +bool channel_tell_funding_locked(struct lightningd *ld, + struct channel *channel, + const struct bitcoin_txid *txid); + #endif /* LIGHTNING_LIGHTNINGD_CHANNEL_CONTROL_H */ diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 783a72ff5..141655ef5 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -628,7 +628,6 @@ static enum watch_result funding_lockin_cb(struct channel *channel, unsigned int depth) { const char *txidstr; - bool channel_ready; struct lightningd *ld = channel->peer->ld; txidstr = type_to_string(channel, struct bitcoin_txid, txid); @@ -652,19 +651,8 @@ static enum watch_result funding_lockin_cb(struct channel *channel, wallet_channel_save(ld->wallet, channel); } - /* In theory, it could have been buried before we got back - * from accepting openingd or disconnected: just wait for next one. */ - channel_ready = (channel->owner && channel->state == CHANNELD_AWAITING_LOCKIN); - if (!channel_ready) { - log_debug(channel->log, - "Funding tx confirmed, but channel state %s %s", - channel_state_name(channel), - channel->owner ? channel->owner->name : "unowned"); - } else { - subd_send_msg(channel->owner, - take(towire_channel_funding_locked(channel, - channel->scid))); - } + if (!channel_tell_funding_locked(ld, channel, txid)) + return KEEP_WATCHING; /* BOLT #7: * diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 8a4b5eb45..076cf1e65 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -44,6 +44,11 @@ void broadcast_tx(struct chain_topology *topo UNNEEDED, int exitstatus UNNEEDED, const char *err)) { fprintf(stderr, "broadcast_tx called!\n"); abort(); } +/* Generated stub for channel_tell_funding_locked */ +bool channel_tell_funding_locked(struct lightningd *ld UNNEEDED, + struct channel *channel UNNEEDED, + const struct bitcoin_txid *txid UNNEEDED) +{ fprintf(stderr, "channel_tell_funding_locked called!\n"); abort(); } /* Generated stub for command_fail */ void command_fail(struct command *cmd UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "command_fail called!\n"); abort(); } @@ -269,9 +274,6 @@ bool json_tok_short_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok void kill_uncommitted_channel(struct uncommitted_channel *uc UNNEEDED, const char *why UNNEEDED) { fprintf(stderr, "kill_uncommitted_channel called!\n"); abort(); } -/* Generated stub for locate_tx */ -struct txlocator *locate_tx(const void *ctx UNNEEDED, const struct chain_topology *topo UNNEEDED, const struct bitcoin_txid *txid UNNEEDED) -{ fprintf(stderr, "locate_tx called!\n"); abort(); } /* Generated stub for log_add */ void log_add(struct log *log UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "log_add called!\n"); abort(); } @@ -354,9 +356,6 @@ u8 *towire_channel_dev_reenable_commit(const tal_t *ctx UNNEEDED) /* Generated stub for towire_channel_funding_announce_depth */ u8 *towire_channel_funding_announce_depth(const tal_t *ctx UNNEEDED) { fprintf(stderr, "towire_channel_funding_announce_depth called!\n"); abort(); } -/* Generated stub for towire_channel_funding_locked */ -u8 *towire_channel_funding_locked(const tal_t *ctx UNNEEDED, const struct short_channel_id *short_channel_id UNNEEDED) -{ fprintf(stderr, "towire_channel_funding_locked called!\n"); abort(); } /* Generated stub for towire_channel_send_shutdown */ u8 *towire_channel_send_shutdown(const tal_t *ctx UNNEEDED) { fprintf(stderr, "towire_channel_send_shutdown called!\n"); abort(); }