diff --git a/lightningd/channel.c b/lightningd/channel.c index e0535212b..49ea854d4 100644 --- a/lightningd/channel.c +++ b/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)) - return channel; + if (!channel_active(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)) - return channel; + if (!channel_unsaved(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) { diff --git a/lightningd/channel.h b/lightningd/channel.h index 2583070c1..7c42752bf 100644 --- a/lightningd/channel.h +++ b/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); - -/* 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 *peer_any_unsaved_channel(struct peer *peer, bool *others); struct channel *channel_by_dbid(struct lightningd *ld, const u64 dbid); diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index c3721ef10..f8f369733 100644 --- a/lightningd/channel_control.c +++ b/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, diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index ef85657f7..285c5dd26 100644 --- a/lightningd/closing_control.c +++ b/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) - channel = peer_active_channel(peer); - else { + if (peer) { + bool more_than_one; + 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; diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c index 8407c1ff0..a31a611a6 100644 --- a/lightningd/connect_control.c +++ b/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 (channel) + if (peer_any_active_channel(peer, NULL)) 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 uncommitted_channel *uc; + struct peer *peer; + 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); - if (uc) - kill_uncommitted_channel(uc, "Reconnected"); - else if (c) { - channel_cleanup_commands(c, "Reconnected"); - channel_fail_reconnect(c, "Reconnected"); - } - else if ((c = unsaved_channel_by_id(ld, &id))) { - log_info(c->log, "Killing opening daemon: Reconnected"); - channel_unsaved_close_conn(c, "Reconnected"); + peer = peer_by_id(ld, &id); + if (!peer) + return; + + /* Freeing channels can free peer, so gather first. */ + channels = tal_arr(tmpctx, struct channel *, 0); + list_for_each(&peer->channels, c, list) + tal_arr_expand(&channels, c); + + 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"); + } } } diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index a968409f7..bab3e3fd4 100644 --- a/lightningd/dual_open_control.c +++ b/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 */ - channel = peer_active_channel(peer); - 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); + /* FIXME: This is wrong: we should always create a new channel? */ + channel = peer_any_unsaved_channel(peer, NULL); if (!channel) { channel = new_unsaved_channel(peer, peer->ld->config.fee_base, diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 23c351ce0..f2e68186e 100644 --- a/lightningd/opening_control.c +++ b/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)); diff --git a/lightningd/pay.c b/lightningd/pay.c index 41f47f42c..00acdece7 100644 --- a/lightningd/pay.c +++ b/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); - if (!channel || !channel_can_add_htlc(channel)) { + channel = find_channel_for_htlc_add(ld, &first_hop->node_id); + if (!channel) { struct json_stream *data = json_stream_fail(cmd, PAY_TRY_OTHER_ROUTE, "No connection to first " diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 3deece449..759f58208 100644 --- a/lightningd/peer_control.c +++ b/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) { - if (*force) { - channel_fail_reconnect(channel, - "disconnect command force=true"); - goto wait_for_connectd; - } - return command_fail(cmd, LIGHTNINGD, "Peer is in state %s", + + channel = peer_any_active_channel(peer, NULL); + if (channel && !*force) { + return command_fail(cmd, LIGHTNINGD, + "Peer has (at least one) channel in state %s", channel_state_name(channel)); } - channel = peer_unsaved_channel(peer); - if (channel) { - channel_unsaved_close_conn(channel, "disconnect command"); - goto wait_for_connectd; + + /* Careful here! Disconnecting can free peer! */ + channels = tal_arr(cmd, struct channel *, 0); + 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. */ - subd_send_msg(cmd->ld->connectd, - take(towire_connectd_discard_peer(NULL, id))); + for (size_t i = 0; i < tal_count(channels); i++) { + if (channel_unsaved(channels[i])) + 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); - if (!channel) { + channel = peer_any_active_channel(peer, &more_than_one); + 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); - if (!channel) { + channel = peer_any_active_channel(peer, &more_than_one); + 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); - if (!channel) { + channel = peer_any_active_channel(peer, &more_than_one); + if (!channel || more_than_one) { return command_fail(cmd, LIGHTNINGD, "Peer has no active channel"); } diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 17e3c9d1b..ec4c220d2 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/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 */ -struct channel *peer_active_channel(struct peer *peer UNNEEDED) -{ fprintf(stderr, "peer_active_channel called!\n"); abort(); } +/* Generated stub for peer_any_active_channel */ +struct channel *peer_any_active_channel(struct peer *peer UNNEEDED, bool *others UNNEEDED) +{ 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) diff --git a/tests/test_connection.py b/tests/test_connection.py index 97e9f91e5..0af12c9cf 100644 --- a/tests/test_connection.py +++ b/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'])