ignore existing peers in DialPeersAsync (#2327)

* ignore existing peers in DialPeersAsync

Fixes #2253

* rename HasPeerWithAddress to IsDialingOrExistingAddress

[breaking] remove Switch#IsDialing

* check if addrBook is nil

to be consistent with other usages of addrBook across Switch

* different log messages for 2 use-cases

CHANGELOG_PENDING.md

@@ -20,7 +20,7 @@ BREAKING CHANGES:
     - Remove PubKey from `Validator` and introduce `ValidatorUpdate`
     - InitChain and EndBlock use ValidatorUpdate
     - Update field names and types in BeginBlock
-- [state] Implement BFT time    
+- [state] Implement BFT time
 - [p2p] update secret connection to use a little endian encoded nonce
 - [libs/clist] Panics if list extends beyond MaxLength
 - [common] SplitAndTrim was deleted
@@ -42,3 +42,4 @@ BUG FIXES:
 - [mempool] No longer possible to fill up linked list without getting caching
   benefits [#2180](https://github.com/tendermint/tendermint/issues/2180)
 - [state] kv store index tx.height to support search
+- [rpc] /dial_peers does not try to dial existing peers

p2p/pex/pex_reactor.go

@@ -392,10 +392,7 @@ func (r *PEXReactor) ensurePeers() {
 		if _, selected := toDial[try.ID]; selected {
 			continue
 		}
-		if dialling := r.Switch.IsDialing(try.ID); dialling {
-			continue
-		}
-		if connected := r.Switch.Peers().Has(try.ID); connected {
+		if r.Switch.IsDialingOrExistingAddress(try) {
 			continue
 		}
 		// TODO: consider moving some checks from toDial into here

p2p/switch.go

@@ -376,11 +376,6 @@ func (sw *Switch) MarkPeerAsGood(peer Peer) {
 //---------------------------------------------------------------------
 // Dialing
 
-// IsDialing returns true if the switch is currently dialing the given ID.
-func (sw *Switch) IsDialing(id ID) bool {
-	return sw.dialing.Has(string(id))
-}
-
 // DialPeersAsync dials a list of peers asynchronously in random order (optionally, making them persistent).
 // Used to dial peers from config on startup or from unsafe-RPC (trusted sources).
 // TODO: remove addrBook arg since it's now set on the switch
@@ -417,10 +412,13 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b
 	for i := 0; i < len(perm); i++ {
 		go func(i int) {
 			j := perm[i]
-
 			addr := netAddrs[j]
-			// do not dial ourselves
+
 			if addr.Same(ourAddr) {
+				sw.Logger.Debug("Ignore attempt to connect to ourselves", "addr", addr, "ourAddr", ourAddr)
+				return
+			} else if sw.IsDialingOrExistingAddress(addr) {
+				sw.Logger.Debug("Ignore attempt to connect to an existing peer", "addr", addr)
 				return
 			}
 
@@ -453,6 +451,14 @@ func (sw *Switch) randomSleep(interval time.Duration) {
 	time.Sleep(r + interval)
 }
 
+// IsDialingOrExistingAddress returns true if switch has a peer with the given
+// address or dialing it at the moment.
+func (sw *Switch) IsDialingOrExistingAddress(addr *NetAddress) bool {
+	return sw.dialing.Has(string(addr.ID)) ||
+		sw.peers.Has(addr.ID) ||
+		(!sw.config.AllowDuplicateIP && sw.peers.HasIP(addr.IP))
+}
+
 //------------------------------------------------------------------------------------
 // Connection filtering
 
@@ -607,8 +613,10 @@ func (sw *Switch) addPeer(pc peerConn) error {
 		addr := peerNodeInfo.NetAddress()
 		// remove the given address from the address book
 		// and add to our addresses to avoid dialing again
-		sw.addrBook.RemoveAddress(addr)
-		sw.addrBook.AddOurAddress(addr)
+		if sw.addrBook != nil {
+			sw.addrBook.RemoveAddress(addr)
+			sw.addrBook.AddOurAddress(addr)
+		}
 		return ErrSwitchConnectToSelf{addr}
 	}