lightningd: remove (most) functions to search channels by status.

This is generally verboten now, since there can be multiple.  There are a
few exceptions:

1. We sometimes want to know if there are *any* active channels.
2. Some dev commands still take peer id when they mean channel_id.
3. We still allow peer id when it's fully determined.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `close` by peer id will fail if there is more than one live channel (use `channel_id` or `short_channel_id` as id arg).

lightningd/channel.c

@@ -549,26 +549,44 @@ const char *channel_state_str(enum channel_state state)
 	return "unknown";
 }
 
-struct channel *peer_unsaved_channel(struct peer *peer)
+struct channel *peer_any_active_channel(struct peer *peer, bool *others)
 {
-	struct channel *channel;
+	struct channel *channel, *ret = NULL;
 
 	list_for_each(&peer->channels, channel, list) {
-		if (channel_unsaved(channel))
+		if (!channel_active(channel))
-			return channel;
+			continue;
+		/* Already found one? */
+		if (ret) {
+			if (others)
+				*others = true;
+		} else {
+			if (others)
+				*others = false;
+			ret = channel;
+		}
 	}
-	return NULL;
+	return ret;
 }
 
-struct channel *peer_active_channel(struct peer *peer)
+struct channel *peer_any_unsaved_channel(struct peer *peer, bool *others)
 {
-	struct channel *channel;
+	struct channel *channel, *ret = NULL;
 
 	list_for_each(&peer->channels, channel, list) {
-		if (channel_active(channel))
+		if (!channel_unsaved(channel))
-			return channel;
+			continue;
+		/* Already found one? */
+		if (ret) {
+			if (others)
+				*others = true;
+		} else {
+			if (others)
+				*others = false;
+			ret = channel;
+		}
 	}
-	return NULL;
+	return ret;
 }
 
 struct channel_inflight *channel_inflight_find(struct channel *channel,
@@ -583,42 +601,6 @@ struct channel_inflight *channel_inflight_find(struct channel *channel,
 	return NULL;
 }
 
-struct channel *peer_normal_channel(struct peer *peer)
-{
-	struct channel *channel;
-
-	list_for_each(&peer->channels, channel, list) {
-		if (channel->state == CHANNELD_NORMAL)
-			return channel;
-	}
-	return NULL;
-}
-
-struct channel *active_channel_by_id(struct lightningd *ld,
-				     const struct node_id *id,
-				     struct uncommitted_channel **uc)
-{
-	struct peer *peer = peer_by_id(ld, id);
-	if (!peer) {
-		if (uc)
-			*uc = NULL;
-		return NULL;
-	}
-
-	if (uc)
-		*uc = peer->uncommitted_channel;
-	return peer_active_channel(peer);
-}
-
-struct channel *unsaved_channel_by_id(struct lightningd *ld,
-				      const struct node_id *id)
-{
-	struct peer *peer = peer_by_id(ld, id);
-	if (!peer)
-		return NULL;
-	return peer_unsaved_channel(peer);
-}
-
 struct channel *active_channel_by_scid(struct lightningd *ld,
 				       const struct short_channel_id *scid)
 {

lightningd/channel.h

@@ -385,23 +385,12 @@ void channel_set_state(struct channel *channel,
 
 const char *channel_change_state_reason_str(enum state_change reason);
 
+/* Find a channel which is not onchain, if any: sets *others if there
+ * is more than one. */
+struct channel *peer_any_active_channel(struct peer *peer, bool *others);
+
 /* Find a channel which is not yet saved to disk */
-struct channel *peer_unsaved_channel(struct peer *peer);
+struct channel *peer_any_unsaved_channel(struct peer *peer, bool *others);
-
-/* Find a channel which is not onchain, if any */
-struct channel *peer_active_channel(struct peer *peer);
-
-/* Find a channel which is in state CHANNELD_NORMAL, if any */
-struct channel *peer_normal_channel(struct peer *peer);
-
-/* Get active channel for peer, optionally any uncommitted_channel. */
-struct channel *active_channel_by_id(struct lightningd *ld,
-				     const struct node_id *id,
-				     struct uncommitted_channel **uc);
-
-/* Get unsaved channel for peer */
-struct channel *unsaved_channel_by_id(struct lightningd *ld,
-				      const struct node_id *id);
 
 struct channel *channel_by_dbid(struct lightningd *ld, const u64 dbid);
 

lightningd/channel_control.c

@@ -1052,6 +1052,7 @@ static struct command_result *json_dev_feerate(struct command *cmd,
 	struct json_stream *response;
 	struct channel *channel;
 	const u8 *msg;
+	bool more_than_one;
 
 	if (!param(cmd, buffer, params,
 		   p_req("id", param_node_id, &id),
@@ -1063,9 +1064,12 @@ static struct command_result *json_dev_feerate(struct command *cmd,
 	if (!peer)
 		return command_fail(cmd, LIGHTNINGD, "Peer not connected");
 
-	channel = peer_active_channel(peer);
+	channel = peer_any_active_channel(peer, &more_than_one);
 	if (!channel || !channel->owner || channel->state != CHANNELD_NORMAL)
 		return command_fail(cmd, LIGHTNINGD, "Peer bad state");
+	/* This is a dev command: fix the api if you need this! */
+	if (more_than_one)
+		return command_fail(cmd, LIGHTNINGD, "More than one channel");
 
 	msg = towire_channeld_feerates(NULL, *feerate,
 				       feerate_min(cmd->ld, NULL),
@@ -1110,6 +1114,7 @@ static struct command_result *json_dev_quiesce(struct command *cmd,
 	struct peer *peer;
 	struct channel *channel;
 	const u8 *msg;
+	bool more_than_one;
 
 	if (!param(cmd, buffer, params,
 		   p_req("id", param_node_id, &id),
@@ -1120,9 +1125,12 @@ static struct command_result *json_dev_quiesce(struct command *cmd,
 	if (!peer)
 		return command_fail(cmd, LIGHTNINGD, "Peer not connected");
 
-	channel = peer_active_channel(peer);
+	channel = peer_any_active_channel(peer, &more_than_one);
 	if (!channel || !channel->owner || channel->state != CHANNELD_NORMAL)
 		return command_fail(cmd, LIGHTNINGD, "Peer bad state");
+	/* This is a dev command: fix the api if you need this! */
+	if (more_than_one)
+		return command_fail(cmd, LIGHTNINGD, "More than one channel");
 
 	msg = towire_channeld_dev_quiesce(NULL);
 	subd_req(channel->owner, channel->owner, take(msg), -1, 0,

lightningd/closing_control.c

@@ -577,9 +577,14 @@ static struct command_result *json_close(struct command *cmd,
 		return command_param_failed();
 
 	peer = peer_from_json(cmd->ld, buffer, idtok);
-	if (peer)
+	if (peer) {
-		channel = peer_active_channel(peer);
+		bool more_than_one;
-	else {
+		channel = peer_any_active_channel(peer, &more_than_one);
+		if (channel && more_than_one) {
+			return command_fail(cmd, LIGHTNINGD,
+					    "Peer has multiple channels: use channel_id or short_channel_id");
+		}
+	} else {
 		struct command_result *res;
 		res = command_find_channel(cmd, buffer, idtok, &channel);
 		if (res)
@@ -587,13 +592,19 @@ static struct command_result *json_close(struct command *cmd,
 	}
 
 	if (!channel && peer) {
+		bool more_than_one;
 		struct uncommitted_channel *uc = peer->uncommitted_channel;
 		if (uc) {
 			/* Easy case: peer can simply be forgotten. */
 			kill_uncommitted_channel(uc, "close command called");
 			goto discard_unopened;
 		}
-		if ((channel = peer_unsaved_channel(peer))) {
+		channel = peer_any_unsaved_channel(peer, &more_than_one);
+		if (channel) {
+			if (more_than_one) {
+				return command_fail(cmd, LIGHTNINGD,
+						    "Peer has multiple channels: use channel_id or short_channel_id");
+			}
 			channel_unsaved_close_conn(channel,
 						   "close command called");
 			goto discard_unopened;

lightningd/connect_control.c

@@ -295,8 +295,7 @@ static void connect_failed(struct lightningd *ld, const u8 *msg)
 	/* If we have an active channel, then reconnect. */
 	peer = peer_by_id(ld, &id);
 	if (peer) {
-		struct channel *channel = peer_active_channel(peer);
+		if (peer_any_active_channel(peer, NULL))
-		if (channel)
 			try_reconnect(peer, peer, seconds_to_delay, addrhint);
 	}
 }
@@ -332,22 +331,34 @@ static void peer_already_connected(struct lightningd *ld, const u8 *msg)
 static void peer_please_disconnect(struct lightningd *ld, const u8 *msg)
 {
 	struct node_id id;
-	struct channel *c;
+	struct peer *peer;
-	struct uncommitted_channel *uc;
+	struct channel *c, **channels;
 
 	if (!fromwire_connectd_reconnected(msg, &id))
 		fatal("Bad msg %s from connectd", tal_hex(tmpctx, msg));
 
-	c = active_channel_by_id(ld, &id, &uc);
+	peer = peer_by_id(ld, &id);
-	if (uc)
+	if (!peer)
-		kill_uncommitted_channel(uc, "Reconnected");
+		return;
-	else if (c) {
+
-		channel_cleanup_commands(c, "Reconnected");
+	/* Freeing channels can free peer, so gather first. */
-		channel_fail_reconnect(c, "Reconnected");
+	channels = tal_arr(tmpctx, struct channel *, 0);
-	}
+	list_for_each(&peer->channels, c, list)
-	else if ((c = unsaved_channel_by_id(ld, &id))) {
+		tal_arr_expand(&channels, c);
-		log_info(c->log, "Killing opening daemon: Reconnected");
+
-		channel_unsaved_close_conn(c, "Reconnected");
+	if (peer->uncommitted_channel)
+		kill_uncommitted_channel(peer->uncommitted_channel,
+					 "Reconnected");
+
+	for (size_t i = 0; i < tal_count(channels); i++) {
+		c = channels[i];
+		if (channel_active(c)) {
+			channel_cleanup_commands(c, "Reconnected");
+			channel_fail_reconnect(c, "Reconnected");
+		} else if (channel_unsaved(c)) {
+			log_info(c->log, "Killing opening daemon: Reconnected");
+			channel_unsaved_close_conn(c, "Reconnected");
+		}
 	}
 }
 

lightningd/dual_open_control.c

@@ -1782,7 +1782,7 @@ static void accepter_got_offer(struct subd *dualopend,
 {
 	struct openchannel2_payload *payload;
 
-	if (peer_active_channel(channel->peer)) {
+	if (peer_any_active_channel(channel->peer, NULL)) {
 		subd_send_msg(dualopend,
 				take(towire_dualopend_fail(NULL,
 					"Already have active channel")));
@@ -2584,13 +2584,13 @@ static struct command_result *json_openchannel_init(struct command *cmd,
 		return command_fail(cmd, FUNDING_UNKNOWN_PEER, "Unknown peer");
 	}
 
-	channel = peer_active_channel(peer);
+	channel = peer_any_active_channel(peer, NULL);
 	if (channel) {
 		return command_fail(cmd, LIGHTNINGD, "Peer already %s",
 				    channel_state_name(channel));
 	}
 
-	channel = peer_unsaved_channel(peer);
+	channel = peer_any_unsaved_channel(peer, NULL);
 	if (!channel) {
 		channel = new_unsaved_channel(peer,
 					      peer->ld->config.fee_base,
@@ -2839,18 +2839,6 @@ static void handle_commit_received(struct subd *dualopend,
 			       total_funding);
 
 	if (channel->state == DUALOPEND_OPEN_INIT) {
-		if (peer_active_channel(channel->peer)) {
-			channel_saved_err_broken_reconn(channel,
-						  "Already have active"
-						  " channel with %s",
-						  type_to_string(tmpctx,
-							 struct node_id,
-							 &channel->peer->id));
-			channel->open_attempt
-				= tal_free(channel->open_attempt);
-			return;
-		}
-
 		if (!(inflight = wallet_commit_channel(ld, channel,
 						       remote_commit,
 						       &remote_commit_sig,
@@ -3091,15 +3079,8 @@ static struct command_result *json_queryrates(struct command *cmd,
 		return command_fail(cmd, FUNDING_PEER_NOT_CONNECTED,
 				    "Peer not connected");
 
-	/* We can't query rates for a peer we have a channel with */
+	/* FIXME: This is wrong: we should always create a new channel? */
-	channel = peer_active_channel(peer);
+	channel = peer_any_unsaved_channel(peer, NULL);
-	if (channel)
-		return command_fail(cmd, LIGHTNINGD, "Peer in state %s,"
-				    " can't query peer's rates if already"
-				    " have a channel",
-				    channel_state_name(channel));
-
-	channel = peer_unsaved_channel(peer);
 	if (!channel) {
 		channel = new_unsaved_channel(peer,
 					      peer->ld->config.fee_base,

lightningd/opening_control.c

@@ -478,7 +478,7 @@ static void opening_fundee_finished(struct subd *openingd,
 	remote_commit->chainparams = chainparams;
 
 	/* openingd should never accept them funding channel in this case. */
-	if (peer_active_channel(uc->peer)) {
+	if (peer_any_active_channel(uc->peer, NULL)) {
 		uncommitted_channel_disconnect(uc,
 					       LOG_BROKEN,
 					       "already have active channel");
@@ -770,7 +770,7 @@ static void opening_got_offer(struct subd *openingd,
 	struct openchannel_hook_payload *payload;
 
 	/* Tell them they can't open, if we already have open channel. */
-	if (peer_active_channel(uc->peer)) {
+	if (peer_any_active_channel(uc->peer, NULL)) {
 		subd_send_msg(openingd,
 			      take(towire_openingd_got_offer_reply(
 					   NULL, "Already have active channel", NULL, NULL)));
@@ -935,7 +935,6 @@ static struct command_result *json_fundchannel_complete(struct command *cmd,
 	struct node_id *id;
 	struct bitcoin_txid *funding_txid;
 	struct peer *peer;
-	struct channel *channel;
 	struct wally_psbt *funding_psbt;
 	u32 *funding_txout_num = NULL;
 	struct funding_channel *fc;
@@ -951,11 +950,6 @@ static struct command_result *json_fundchannel_complete(struct command *cmd,
 		return command_fail(cmd, FUNDING_UNKNOWN_PEER, "Unknown peer");
 	}
 
-	channel = peer_active_channel(peer);
-	if (channel)
-		return command_fail(cmd, LIGHTNINGD, "Peer already %s",
-				    channel_state_name(channel));
-
 	if (!peer->is_connected)
 		return command_fail(cmd, FUNDING_PEER_NOT_CONNECTED,
 				    "Peer not connected");
@@ -1133,7 +1127,7 @@ static struct command_result *json_fundchannel_start(struct command *cmd,
 		return command_fail(cmd, FUNDING_PEER_NOT_CONNECTED,
 				    "Peer not connected");
 
-	channel = peer_active_channel(peer);
+	channel = peer_any_active_channel(peer, NULL);
 	if (channel) {
 		return command_fail(cmd, LIGHTNINGD, "Peer already %s",
 				    channel_state_name(channel));

lightningd/pay.c

@@ -826,6 +826,23 @@ static struct command_result *check_offer_usage(struct command *cmd,
 	return NULL;
 }
 
+static struct channel *find_channel_for_htlc_add(struct lightningd *ld,
+						 const struct node_id *node)
+{
+	struct channel *channel;
+	struct peer *peer = peer_by_id(ld, node);
+	if (!peer)
+		return NULL;
+
+	list_for_each(&peer->channels, channel, list) {
+		if (channel_can_add_htlc(channel)) {
+			return channel;
+		}
+	}
+
+	return NULL;
+}
+
 /* destination/route_channels/route_nodes are NULL (and path_secrets may be NULL)
  * if we're sending a raw onion. */
 static struct command_result *
@@ -1014,8 +1031,8 @@ send_payment_core(struct lightningd *ld,
 	if (offer_err)
 		return offer_err;
 
-	channel = active_channel_by_id(ld, &first_hop->node_id, NULL);
+	channel = find_channel_for_htlc_add(ld, &first_hop->node_id);
-	if (!channel || !channel_can_add_htlc(channel)) {
+	if (!channel) {
 		struct json_stream *data
 			= json_stream_fail(cmd, PAY_TRY_OTHER_ROUTE,
 					   "No connection to first "

lightningd/peer_control.c

@@ -1884,8 +1884,9 @@ static struct command_result *json_disconnect(struct command *cmd,
 	struct node_id *id;
 	struct disconnect_command *dc;
 	struct peer *peer;
-	struct channel *channel;
+	struct channel *channel, **channels;
 	bool *force;
+	bool disconnected = false;
 
 	if (!param(cmd, buffer, params,
 		   p_req("id", param_node_id, &id),
@@ -1900,32 +1901,69 @@ static struct command_result *json_disconnect(struct command *cmd,
 	if (!peer->is_connected) {
 		return command_fail(cmd, LIGHTNINGD, "Peer not connected");
 	}
-	channel = peer_active_channel(peer);
+
-	if (channel) {
+	channel = peer_any_active_channel(peer, NULL);
-		if (*force) {
+	if (channel && !*force) {
-			channel_fail_reconnect(channel,
+		return command_fail(cmd, LIGHTNINGD,
-					       "disconnect command force=true");
+				    "Peer has (at least one) channel in state %s",
-			goto wait_for_connectd;
-		}
-		return command_fail(cmd, LIGHTNINGD, "Peer is in state %s",
 				    channel_state_name(channel));
 	}
-	channel = peer_unsaved_channel(peer);
+
-	if (channel) {
+	/* Careful here!  Disconnecting can free peer! */
-		channel_unsaved_close_conn(channel, "disconnect command");
+	channels = tal_arr(cmd, struct channel *, 0);
-		goto wait_for_connectd;
+	list_for_each(&peer->channels, channel, list) {
+		if (!channel->owner)
+			continue;
+		if (!channel->owner->talks_to_peer)
+			continue;
+
+		switch (channel->state) {
+		case DUALOPEND_OPEN_INIT:
+		case CHANNELD_AWAITING_LOCKIN:
+		case CHANNELD_NORMAL:
+		case CHANNELD_SHUTTING_DOWN:
+		case DUALOPEND_AWAITING_LOCKIN:
+		case CLOSINGD_SIGEXCHANGE:
+			tal_arr_expand(&channels, channel);
+			continue;
+		case CLOSINGD_COMPLETE:
+		case AWAITING_UNILATERAL:
+		case FUNDING_SPEND_SEEN:
+		case ONCHAIN:
+		case CLOSED:
+			/* We don't expect these to have owners who connect! */
+			log_broken(channel->log,
+				   "Don't expect owner %s in state %s",
+				   channel->owner->name,
+				   channel_state_name(channel));
+			continue;
+		}
+		abort();
 	}
+
+	/* This can free peer too! */
 	if (peer->uncommitted_channel) {
 		kill_uncommitted_channel(peer->uncommitted_channel,
 					 "disconnect command");
-		goto wait_for_connectd;
+		disconnected = true;
 	}
 
-	/* It's just sitting in connectd. */
+	for (size_t i = 0; i < tal_count(channels); i++) {
-	subd_send_msg(cmd->ld->connectd,
+		if (channel_unsaved(channels[i]))
-		      take(towire_connectd_discard_peer(NULL, id)));
+			channel_unsaved_close_conn(channels[i],
+						   "disconnect command");
+		else
+			channel_fail_reconnect(channels[i],
+					       "disconnect command");
+		disconnected = true;
+	}
+
+	if (!disconnected) {
+		/* It's just sitting in connectd. */
+		subd_send_msg(cmd->ld->connectd,
+			      take(towire_connectd_discard_peer(NULL, id)));
+	}
 
-wait_for_connectd:
 	/* Connectd tells us when it's finally disconnected */
 	dc = tal(cmd, struct disconnect_command);
 	dc->cmd = cmd;
@@ -2459,6 +2497,7 @@ static struct command_result *json_sign_last_tx(struct command *cmd,
 	struct peer *peer;
 	struct json_stream *response;
 	struct channel *channel;
+	bool more_than_one;
 
 	if (!param(cmd, buffer, params,
 		   p_req("id", param_node_id, &peerid),
@@ -2470,10 +2509,10 @@ static struct command_result *json_sign_last_tx(struct command *cmd,
 		return command_fail(cmd, LIGHTNINGD,
 				    "Could not find peer with that id");
 	}
-	channel = peer_active_channel(peer);
+	channel = peer_any_active_channel(peer, &more_than_one);
-	if (!channel) {
+	if (!channel || more_than_one) {
 		return command_fail(cmd, LIGHTNINGD,
-				    "Could not find active channel");
+				    "Could not find single active channel");
 	}
 
 	response = json_stream_success(cmd);
@@ -2521,6 +2560,7 @@ static struct command_result *json_dev_fail(struct command *cmd,
 	struct node_id *peerid;
 	struct peer *peer;
 	struct channel *channel;
+	bool more_than_one;
 
 	if (!param(cmd, buffer, params,
 		   p_req("id", param_node_id, &peerid),
@@ -2533,10 +2573,10 @@ static struct command_result *json_dev_fail(struct command *cmd,
 				    "Could not find peer with that id");
 	}
 
-	channel = peer_active_channel(peer);
+	channel = peer_any_active_channel(peer, &more_than_one);
-	if (!channel) {
+	if (!channel || more_than_one) {
 		return command_fail(cmd, LIGHTNINGD,
-				    "Could not find active channel with peer");
+				    "Could not find single active channel with peer");
 	}
 
 	channel_fail_permanent(channel,
@@ -2570,6 +2610,7 @@ static struct command_result *json_dev_reenable_commit(struct command *cmd,
 	struct peer *peer;
 	u8 *msg;
 	struct channel *channel;
+	bool more_than_one;
 
 	if (!param(cmd, buffer, params,
 		   p_req("id", param_node_id, &peerid),
@@ -2582,8 +2623,8 @@ static struct command_result *json_dev_reenable_commit(struct command *cmd,
 				    "Could not find peer with that id");
 	}
 
-	channel = peer_active_channel(peer);
+	channel = peer_any_active_channel(peer, &more_than_one);
-	if (!channel) {
+	if (!channel || more_than_one) {
 		return command_fail(cmd, LIGHTNINGD,
 				    "Peer has no active channel");
 	}

lightningd/test/run-invoice-select-inchan.c

@@ -586,9 +586,9 @@ struct command_result *param_u64(struct command *cmd UNNEEDED, const char *name
 				 const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED,
 				 uint64_t **num UNNEEDED)
 { fprintf(stderr, "param_u64 called!\n"); abort(); }
-/* Generated stub for peer_active_channel */
+/* Generated stub for peer_any_active_channel */
-struct channel *peer_active_channel(struct peer *peer UNNEEDED)
+struct channel *peer_any_active_channel(struct peer *peer UNNEEDED, bool *others UNNEEDED)
-{ fprintf(stderr, "peer_active_channel called!\n"); abort(); }
+{ fprintf(stderr, "peer_any_active_channel called!\n"); abort(); }
 /* Generated stub for peer_restart_dualopend */
 void peer_restart_dualopend(struct peer *peer UNNEEDED,
 			    struct peer_fd *peer_fd UNNEEDED,
@@ -609,9 +609,6 @@ 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_unsaved_channel */
-struct channel *peer_unsaved_channel(struct peer *peer UNNEEDED)
-{ fprintf(stderr, "peer_unsaved_channel 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)

tests/test_connection.py

@@ -2586,7 +2586,7 @@ def test_disconnectpeer(node_factory, bitcoind):
     mine_funding_to_announce(bitcoind, [l1, l2, l3])
 
     # disconnecting a non gossiping peer results in error
-    with pytest.raises(RpcError, match=r'Peer is in state CHANNELD_NORMAL'):
+    with pytest.raises(RpcError, match=r'Peer has \(at least one\) channel in state CHANNELD_NORMAL'):
         l1.rpc.disconnect(l3.info['id'])