From 3ba7442dcaf88aefaccb5646674e1a5cd98c6c16 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Sun, 24 Mar 2019 11:29:29 +0100 Subject: [PATCH] Properly distinguish in/out connections in the PSM (#2102) --- substrate/core/peerset/src/lib.rs | 40 ++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/substrate/core/peerset/src/lib.rs b/substrate/core/peerset/src/lib.rs index 2f39b96a94..74444a2c38 100644 --- a/substrate/core/peerset/src/lib.rs +++ b/substrate/core/peerset/src/lib.rs @@ -31,14 +31,18 @@ pub struct Peerset { struct Inner { /// List of nodes that we know exist but we are not connected to. + /// Elements in this list must never be in `out_slots` or `in_slots`. discovered: Vec, /// List of reserved nodes. reserved: HashSet, /// If true, we only accept reserved nodes. reserved_only: bool, - /// Node slots. Each slot contains either `None` if the node is free, or `Some` if it is - /// assigned to a peer. - slots: Vec>, + /// Node slots for outgoing connections. Each slot contains either `None` if the node is free, + /// or `Some` if it is assigned to a peer. + out_slots: Vec>, + /// Node slots for incoming connections. Each slot contains either `None` if the node is free, + /// or `Some` if it is assigned to a peer. + in_slots: Vec>, } /// Message that can be sent by the peer set manager (PSM). @@ -111,7 +115,8 @@ impl Peerset { discovered: config.bootnodes.into_iter().collect(), reserved: Default::default(), reserved_only: config.reserved_only, - slots: (0 .. (config.in_peers + config.out_peers)).map(|_| None).collect(), + out_slots: (0 .. config.out_peers).map(|_| None).collect(), + in_slots: (0 .. config.in_peers).map(|_| None).collect(), }; alloc_slots(&mut inner, &tx); @@ -147,10 +152,15 @@ impl Peerset { return; } + // Nothing more to do if we're already connected. + if inner.out_slots.iter().chain(inner.in_slots.iter()).any(|s| s.as_ref() == Some(&peer_id)) { + return; + } + // Assign a slot for this reserved peer. - if let Some(pos) = inner.slots.iter().position(|s| s.as_ref().map(|n| !inner.reserved.contains(n)).unwrap_or(true)) { + if let Some(pos) = inner.out_slots.iter().position(|s| s.as_ref().map(|n| !inner.reserved.contains(n)).unwrap_or(true)) { let _ = self.tx.unbounded_send(Message::Connect(peer_id.clone())); - inner.slots[pos] = Some(peer_id); + inner.out_slots[pos] = Some(peer_id); } else { // All slots are filled with reserved peers. @@ -176,7 +186,7 @@ impl Peerset { // Disconnect non-reserved nodes. if reserved_only { - for slot in &mut inner.slots { + for slot in inner.out_slots.iter_mut().chain(inner.in_slots.iter_mut()) { if let Some(peer) = slot.as_ref() { if inner.reserved.contains(peer) { continue; @@ -201,7 +211,7 @@ fn alloc_slots(inner: &mut Inner, tx: &mpsc::UnboundedSender) { return; } - for slot in inner.slots.iter_mut() { + for slot in inner.out_slots.iter_mut() { if slot.is_some() { continue; } @@ -226,12 +236,12 @@ impl PeersetMut { /// peerset is already connected to, in which case it must not answer. pub fn incoming(&self, peer_id: PeerId, index: IncomingIndex) { let mut inner = self.parent.inner.lock(); - if inner.slots.iter().any(|s| s.as_ref() == Some(&peer_id)) { + if inner.out_slots.iter().chain(inner.in_slots.iter()).any(|s| s.as_ref() == Some(&peer_id)) { return } - if let Some(pos) = inner.slots.iter().position(|s| s.is_none()) { - inner.slots[pos] = Some(peer_id); + if let Some(pos) = inner.in_slots.iter().position(|s| s.is_none()) { + inner.in_slots[pos] = Some(peer_id); let _ = self.parent.tx.unbounded_send(Message::Accept(index)); } else { if inner.discovered.iter().all(|p| *p != peer_id) { @@ -247,6 +257,7 @@ impl PeersetMut { /// `PeerId`, or accepted an incoming connection with this `PeerId`. pub fn dropped(&self, peer_id: &PeerId) { let mut inner = self.parent.inner.lock(); + let inner = &mut *inner; // Fixes a borrowing issue. // Automatically connect back if reserved. if inner.reserved.contains(peer_id) { @@ -255,9 +266,10 @@ impl PeersetMut { } // Otherwise, free the slot. - for slot in inner.slots.iter_mut() { + for slot in inner.out_slots.iter_mut().chain(inner.in_slots.iter_mut()) { if slot.as_ref() == Some(peer_id) { *slot = None; + break; } } @@ -266,7 +278,7 @@ impl PeersetMut { if inner.discovered.iter().all(|p| p != peer_id) { inner.discovered.push(peer_id.clone()); } - alloc_slots(&mut inner, &self.parent.tx); + alloc_slots(inner, &self.parent.tx); } /// Adds a discovered peer id to the PSM. @@ -276,7 +288,7 @@ impl PeersetMut { pub fn discovered(&self, peer_id: PeerId) { let mut inner = self.parent.inner.lock(); - if inner.slots.iter().any(|p| p.as_ref() == Some(&peer_id)) { + if inner.out_slots.iter().chain(inner.in_slots.iter()).any(|p| p.as_ref() == Some(&peer_id)) { return; }