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 <rusty@rustcorp.com.au>

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:

channeld/channel_wire.csv

@@ -1,6 +1,3 @@
-# Received and sent funding_locked
-channel_normal_operation,11001
-
 #include <common/cryptomsg.h>
 #include <common/channel_config.h>
 

lightningd/channel_control.c

@@ -14,6 +14,15 @@
 #include <lightningd/subd.h>
 #include <wire/wire_sync.h>
 
+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;
+}

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 */

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
+	if (!channel_tell_funding_locked(ld, channel, txid))
-	 * from accepting openingd or disconnected: just wait for next one. */
+		return KEEP_WATCHING;
-	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)));
-	}
 
 	/* BOLT #7:
 	 *

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(); }