From eb5cf0f0dd054517a029dc41ee73934de388fef5 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 5 Sep 2018 19:52:22 +0400 Subject: [PATCH] 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 | 3 ++- p2p/pex/pex_reactor.go | 5 +---- p2p/switch.go | 26 +++++++++++++++++--------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 11e5fc8d5..08461ba2c 100644 --- a/CHANGELOG_PENDING.md +++ b/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 diff --git a/p2p/pex/pex_reactor.go b/p2p/pex/pex_reactor.go index b3c50a182..c919794ab 100644 --- a/p2p/pex/pex_reactor.go +++ b/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 diff --git a/p2p/switch.go b/p2p/switch.go index b26e1147e..b5413dabf 100644 --- a/p2p/switch.go +++ b/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} }