Skip to content

Commit

Permalink
fix(comms): minor edge-case fix to handle inbound connection while di…
Browse files Browse the repository at this point in the history
…aling (#3785)

Description
---
- Notifies the dialler if an inbound connection has come in, so that it can notify the waiting diallers of the new connection

Motivation and Context
---
Noticed this edge case when a wallet start up and dials a base node, I then quickly manually dial the wallet from the same base node. The bn->wallet connection finishes first but the wallet still is in connecting state. The wallet does not immediately flip into connected state because it is waiting for the dial to finish, however this will also cause a tie-break resolution which may take longer. This PR resolves this by notifying the dialler of all inbound connections so that it can notify any waiting dials of the new connection immediately.

How Has This Been Tested?
---
Partially by existing unit tests, manually
  • Loading branch information
sdbondi committed Feb 4, 2022
1 parent b3cc6f2 commit 2f9603b
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
29 changes: 21 additions & 8 deletions comms/src/connection_manager/dialer.rs
Expand Up @@ -73,6 +73,7 @@ pub(crate) enum DialerRequest {
Option<oneshot::Sender<Result<PeerConnection, ConnectionManagerError>>>,
),
CancelPendingDial(NodeId),
NotifyNewInboundConnection(PeerConnection),
}

pub struct Dialer<TTransport, TBackoff> {
Expand Down Expand Up @@ -168,13 +169,29 @@ where
self.handle_dial_peer_request(pending_dials, peer, reply_tx);
},
CancelPendingDial(peer_id) => {
if let Some(mut s) = self.cancel_signals.remove(&peer_id) {
let _ = s.trigger();
self.cancel_dial(&peer_id);
},

NotifyNewInboundConnection(conn) => {
if conn.is_connected() {
self.resolve_pending_dials(conn);
}
},
}
}

fn cancel_dial(&mut self, peer_id: &NodeId) {
if let Some(mut s) = self.cancel_signals.remove(peer_id) {
let _ = s.trigger();
}
}

fn resolve_pending_dials(&mut self, conn: PeerConnection) {
let peer = conn.peer_node_id().clone();
self.reply_to_pending_requests(&peer, Ok(conn));
self.cancel_dial(&peer);
}

fn is_pending_dial(&self, node_id: &NodeId) -> bool {
self.cancel_signals.contains_key(node_id)
}
Expand Down Expand Up @@ -223,12 +240,8 @@ where
);
}

if self.pending_dial_requests.contains_key(&node_id) {
self.reply_to_pending_requests(&node_id, dial_result);
}

// Drop cancel signal
let _ = self.cancel_signals.remove(&node_id);
self.reply_to_pending_requests(&node_id, dial_result);
self.cancel_dial(&node_id);
}

pub async fn notify_connection_manager(&mut self, event: ConnectionManagerEvent) {
Expand Down
7 changes: 7 additions & 0 deletions comms/src/connection_manager/manager.rs
Expand Up @@ -424,6 +424,13 @@ where
},

PeerConnected(conn) => {
if conn.direction().is_inbound() {
// Notify the dialer that we have an inbound connection, so that is can resolve any pending dials.
let _ = self
.dialer_tx
.send(DialerRequest::NotifyNewInboundConnection(conn.clone()))
.await;
}
metrics::successful_connections(conn.peer_node_id(), conn.direction()).inc();
self.publish_event(PeerConnected(conn));
},
Expand Down

0 comments on commit 2f9603b

Please sign in to comment.