Repository navigation
Fail create_channel early if peer is disconnected #2437
Description
We need to ensure in the case where a peer is disconnected that we fail early when we invoke ChannelManager::create_channel for their_network_key.
The only thing we currently do is check if we know about the peer at all (they're in the per_peer_state):
rust-lightning/lightning/src/ln/channelmanager.rs
Line 2195 in e4c44f3
We also need to check for .is_connected if we know about the peer.
See #2382 (comment)
Activity
TheBlueMatt commentedon Jul 24, 2023
Bonus points if we instead address #2096 (ie don't fail but rebroadcast the open_channel message on reconnect, failing in a minute or two from a timeout instead).
henghonglee commentedon Jul 24, 2023
@dunxen i can try this one
dunxen commentedon Jul 24, 2023
Great, thanks! Matt’s bonus points would be good to handle from his comment above alternatively :)
nafisalawalidris commentedon Sep 15, 2023
To ensure robustness, I propose adding a check for peer disconnection within this function. Currently, we only verify if we know about the peer through the per_peer_state. However, it is crucial to also consider the peer's connection status by using .is_connected. This enhancement will enable us to fail early when attempting to invoke ChannelManager::create_channel for a disconnected peer. Here's the proposed code modification:
// Existing code
let peer_state_mutex = per_peer_state.get(&their_network_key)
// Proposed modification
if let Some(peer_state) = per_peer_state.get(&their_network_key) {
if peer_state.is_connected {
// Proceed with create_channel logic
} else {
// Handle the case where the peer is disconnected
// (e.g., return an error or take appropriate action)
}
}
By implementing this check, we can proactively address issues related to disconnected peers when using ChannelManager::create_channel.
shaavan commentedon Nov 4, 2023
Hi! I couldn't help but notice the PR has been open for a little while. I've given it a shot at solving the issue. I would absolutely love to hear your thoughts on what I've come up with. Thanks a bunch! :)
TheBlueMatt commentedon Nov 12, 2023
Oh, oops, dup of #2096.