lightningd: use channel_id when a peer is activated.

Rather than intuiting whether this is a new channel / active channel,
use the channel_id.  This simplifies things and makes them explicit,
and prepares for multiple live channels per peer.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/peer_control.c

@@ -1256,24 +1256,17 @@ void peer_active(struct lightningd *ld, const u8 *msg, int fd)
 		return;
 	}
 
-	channel = peer_active_channel(peer);
+	/* Do we know what channel they're talking about? */
-
+	channel = find_channel_by_id(peer, &channel_id);
-	/* It might be v2 opening, though, since we hang onto these */
-	if (!channel)
-		channel = peer_unsaved_channel(peer);
-
 	if (channel) {
 		switch (channel->state) {
 		case ONCHAIN:
 		case FUNDING_SPEND_SEEN:
 		case CLOSINGD_COMPLETE:
-			/* Channel is supposed to be active!*/
+			goto channel_is_closed;
-			abort();
 		case CLOSED:
 			/* Channel should not have been loaded */
 			abort();
-
-		/* We consider this "active" but we only send an error */
 		case AWAITING_UNILATERAL: {
 			/* channel->error is not saved in db, so this can
 			 * happen if we restart. */
@@ -1340,48 +1333,63 @@ void peer_active(struct lightningd *ld, const u8 *msg, int fd)
 		return;
 	}
 
-	/* It's possible that they want to reestablish a channel, but
+	/* OK, it's an unknown channel.  Create a new one if they're trying. */
-	 * it's closed? */
+	switch (*msgtype) {
-	if (*msgtype == WIRE_CHANNEL_REESTABLISH) {
+	case WIRE_OPEN_CHANNEL:
-		channel = find_channel_by_id(peer, &channel_id);
+		if (dual_fund) {
-		if (channel && channel_closed(channel)) {
+			error = towire_errorfmt(tmpctx, &channel_id,
-			log_debug(channel->log,
+						"OPT_DUAL_FUND: cannot use open_channel");
-				  "Reestablish on %s channel: using channeld to reply",
+			goto send_error;
-				  channel_state_name(channel));
+		}
-			peer_start_channeld(channel, peer_fd, NULL, true, true);
+		if (peer->uncommitted_channel) {
-			return;
+			error = towire_errorfmt(tmpctx, &channel_id,
-		} else {
+						"Multiple simulteneous opens not supported");
-			const u8 *err = towire_errorfmt(tmpctx, &channel_id,
+			goto send_error;
-						"Unknown channel for reestablish");
+		}
-			log_peer_debug(ld->log, &peer->id,
+		peer->uncommitted_channel = new_uncommitted_channel(peer);
-				       "Reestablish on UNKNOWN channel %s",
+		peer_start_openingd(peer, peer_fd);
-				       type_to_string(tmpctx, struct channel_id,
+		break;
-						      &channel_id));
+	case WIRE_OPEN_CHANNEL2:
-			/* Unless we're shutting down, tell connectd to send err */
+		if (!dual_fund) {
-			if (ld->connectd)
+			error = towire_errorfmt(tmpctx, &channel_id,
-				subd_send_msg(ld->connectd,
+						"Didn't negotiate OPT_DUAL_FUND: cannot use open_channel2");
-				      take(towire_connectd_peer_final_msg(NULL,
+			goto send_error;
-									  &peer->id,
-									  err)));
-			else
-				peer->is_connected = false;
-			return;
 		}
-	}
-
-	/* OK, it's unsolicited.  What kind of open do they want? */
-	if (dual_fund) {
 		channel = new_unsaved_channel(peer,
 					      peer->ld->config.fee_base,
 					      peer->ld->config.fee_per_satoshi);
 		channel->cid = channel_id;
 		peer_start_dualopend(peer, peer_fd, channel);
-	} else {
+		break;
-		peer->uncommitted_channel = new_uncommitted_channel(peer);
+	default:
-		peer_start_openingd(peer, peer_fd);
+		log_peer_unusual(ld->log, &peer->id,
+				 "Unknown channel %s for %s",
+				 type_to_string(tmpctx, struct channel_id,
+						&channel_id),
+				 peer_wire_name(*msgtype));
+		error = towire_errorfmt(tmpctx, &channel_id,
+					"Unknown channel for %s", peer_wire_name(*msgtype));
+		goto send_error;
+		break;
 	}
 	return;
 
+channel_is_closed:
+	if (msgtype && *msgtype == WIRE_CHANNEL_REESTABLISH) {
+		log_debug(channel->log,
+			  "Reestablish on %s channel: using channeld to reply",
+			  channel_state_name(channel));
+		peer_start_channeld(channel, peer_fd, NULL, true, true);
+		return;
+	}
+
+	/* Retransmit error if we have one.  Otherwise generic error. */
+	error = channel->error;
+	if (!error)
+		error = towire_errorfmt(tmpctx, &channel_id,
+					"channel in state %s",
+					channel_state_name(channel));
+
 send_error:
 	log_peer_debug(ld->log, &peer->id, "Telling connectd to send error %s",
 		       tal_hex(tmpctx, error));

tests/test_connection.py

@@ -1258,7 +1258,7 @@ def test_funding_external_wallet_corners(node_factory, bitcoind):
 
     # on reconnect, channel should get destroyed
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-    l1.daemon.wait_for_log('Reestablish on UNKNOWN channel')
+    l1.daemon.wait_for_log('Unknown channel .* for WIRE_CHANNEL_REESTABLISH')
     wait_for(lambda: len(l1.rpc.listpeers()['peers']) == 0)
     wait_for(lambda: len(l2.rpc.listpeers()['peers']) == 0)
 

wallet/test/run-wallet.c

@@ -648,6 +648,9 @@ bool peer_start_dualopend(struct peer *peer UNNEEDED, struct peer_fd *peer_fd UN
 bool peer_start_openingd(struct peer *peer UNNEEDED,
 			 struct peer_fd *peer_fd UNNEEDED)
 { fprintf(stderr, "peer_start_openingd called!\n"); abort(); }
+/* Generated stub for peer_wire_name */
+const char *peer_wire_name(int e UNNEEDED)
+{ fprintf(stderr, "peer_wire_name called!\n"); abort(); }
 /* Generated stub for plugin_hook_call_ */
 bool plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
 		       tal_t *cb_arg STEALS UNNEEDED)