do not even try to dial ourselves

also, remove address from the book (plus mark it as our address)
and return an error if we fail to parse peers list

p2p/pex/addrbook.go

@@ -33,13 +33,15 @@ type AddrBook interface {
 
 	// Add our own addresses so we don't later add ourselves
 	AddOurAddress(*p2p.NetAddress)
-
 	// Check if it is our address
 	OurAddress(*p2p.NetAddress) bool
 
 	// Add and remove an address
 	AddAddress(addr *p2p.NetAddress, src *p2p.NetAddress) error
-	RemoveAddress(addr *p2p.NetAddress)
+	RemoveAddress(*p2p.NetAddress)
+
+	// Check if the address is in the book
+	HasAddress(*p2p.NetAddress) bool
 
 	// Do we need more peers?
 	NeedMoreAddrs() bool
@@ -196,6 +198,14 @@ func (a *addrBook) IsGood(addr *p2p.NetAddress) bool {
 	return a.addrLookup[addr.ID].isOld()
 }
 
+// HasAddress returns true if the address is in the book.
+func (a *addrBook) HasAddress(addr *p2p.NetAddress) bool {
+	a.mtx.Lock()
+	defer a.mtx.Unlock()
+	ka := a.addrLookup[addr.ID]
+	return ka != nil
+}
+
 // NeedMoreAddrs implements AddrBook - returns true if there are not have enough addresses in the book.
 func (a *addrBook) NeedMoreAddrs() bool {
 	return a.Size() < needAddressThreshold

p2p/pex/addrbook_test.go

@@ -338,3 +338,19 @@ func TestAddrBookGetSelectionWithBias(t *testing.T) {
 		t.Fatalf("expected more good peers (%% got: %d, %% expected: %d, number of good addrs: %d, total: %d)", got, expected, good, len(selection))
 	}
 }
+
+func TestAddrBookHasAddress(t *testing.T) {
+	fname := createTempFileName("addrbook_test")
+	defer deleteTempFile(fname)
+
+	book := NewAddrBook(fname, true)
+	book.SetLogger(log.TestingLogger())
+	addr := randIPv4Address(t)
+	book.AddAddress(addr, addr)
+
+	assert.True(t, book.HasAddress(addr))
+
+	book.RemoveAddress(addr)
+
+	assert.False(t, book.HasAddress(addr))
+}

p2p/switch.go

@@ -40,6 +40,8 @@ type AddrBook interface {
 	AddOurAddress(*NetAddress)
 	OurAddress(*NetAddress) bool
 	MarkGood(*NetAddress)
+	RemoveAddress(*NetAddress)
+	HasAddress(*NetAddress) bool
 	Save()
 }
 
@@ -351,18 +353,21 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b
 	for _, err := range errs {
 		sw.Logger.Error("Error in peer's address", "err", err)
 	}
+	if len(errs) > 0 {
+		return errors.New("Errors in peer addresses (see errors above)")
+	}
 
+	ourAddr := sw.nodeInfo.NetAddress()
+
+	// TODO: move this out of here ?
 	if addrBook != nil {
 		// add peers to `addrBook`
-		ourAddr := sw.nodeInfo.NetAddress()
 		for _, netAddr := range netAddrs {
 			// do not add our address or ID
-			if netAddr.Same(ourAddr) {
+			if !netAddr.Same(ourAddr) {
-				continue
-			}
-			// TODO: move this out of here ?
 				addrBook.AddAddress(netAddr, ourAddr)
 			}
+		}
 		// Persist some peers to disk right away.
 		// NOTE: integration tests depend on this
 		addrBook.Save()
@@ -372,8 +377,14 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b
 	perm := sw.rng.Perm(len(netAddrs))
 	for i := 0; i < len(perm); i++ {
 		go func(i int) {
-			sw.randomSleep(0)
 			j := perm[i]
+
+			// do not dial ourselves
+			if netAddrs[j].Same(ourAddr) {
+				return
+			}
+
+			sw.randomSleep(0)
 			err := sw.DialPeerWithAddress(netAddrs[j], persistent)
 			if err != nil {
 				sw.Logger.Error("Error dialing peer", "err", err)
@@ -386,11 +397,6 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b
 // DialPeerWithAddress dials the given peer and runs sw.addPeer if it connects and authenticates successfully.
 // If `persistent == true`, the switch will always try to reconnect to this peer if the connection ever fails.
 func (sw *Switch) DialPeerWithAddress(addr *NetAddress, persistent bool) error {
-	// do not dial ourselves
-	if sw.addrBook.OurAddress(addr) {
-		return ErrSwitchConnectToSelf
-	}
-
 	sw.dialing.Set(string(addr.ID), addr)
 	defer sw.dialing.Delete(string(addr.ID))
 	return sw.addOutboundPeerWithConfig(addr, sw.peerConfig, persistent)
@@ -532,9 +538,14 @@ func (sw *Switch) addPeer(pc peerConn) error {
 
 	// Avoid self
 	if sw.nodeKey.ID() == peerID {
-		// add given address to the address book to avoid dialing ourselves again
+		addr := peerNodeInfo.NetAddress()
-		// this is our public address
+
-		sw.addrBook.AddOurAddress(peerNodeInfo.NetAddress())
+		// remove the given address from the address book if we're added it earlier
+		sw.addrBook.RemoveAddress(addr)
+
+		// add the given address to the address book to avoid dialing ourselves
+		// again this is our public address
+		sw.addrBook.AddOurAddress(addr)
 
 		return ErrSwitchConnectToSelf
 	}

p2p/switch_test.go

@@ -90,7 +90,9 @@ func MakeSwitchPair(t testing.TB, initSwitch func(int, *Switch) *Switch) (*Switc
 }
 
 func initSwitchFunc(i int, sw *Switch) *Switch {
-	sw.SetAddrBook(&addrBookMock{ourAddrs: make(map[string]struct{})})
+	sw.SetAddrBook(&addrBookMock{
+		addrs:    make(map[string]struct{}, 0),
+		ourAddrs: make(map[string]struct{}, 0)})
 
 	// Make two reactors of two channels each
 	sw.AddReactor("foo", NewTestReactor([]*conn.ChannelDescriptor{
@@ -180,32 +182,24 @@ func TestConnAddrFilter(t *testing.T) {
 
 func TestSwitchFiltersOutItself(t *testing.T) {
 	s1 := MakeSwitch(config, 1, "127.0.0.2", "123.123.123", initSwitchFunc)
-	addr := s1.NodeInfo().NetAddress()
+	// addr := s1.NodeInfo().NetAddress()
 
-	// add ourselves like we do in node.go#427
+	// // add ourselves like we do in node.go#427
-	s1.addrBook.AddOurAddress(addr)
+	// s1.addrBook.AddOurAddress(addr)
-
-	// addr should be rejected immediately because of the same IP & port
-	err := s1.DialPeerWithAddress(addr, false)
-	if assert.Error(t, err) {
-		assert.Equal(t, ErrSwitchConnectToSelf, err)
-	}
 
 	// simulate s1 having a public IP by creating a remote peer with the same ID
 	rp := &remotePeer{PrivKey: s1.nodeKey.PrivKey, Config: DefaultPeerConfig()}
 	rp.Start()
 
 	// addr should be rejected in addPeer based on the same ID
-	err = s1.DialPeerWithAddress(rp.Addr(), false)
+	err := s1.DialPeerWithAddress(rp.Addr(), false)
 	if assert.Error(t, err) {
 		assert.Equal(t, ErrSwitchConnectToSelf, err)
 	}
 
-	// addr should be rejected immediately because during previous step we changed node's public IP
+	assert.True(t, s1.addrBook.OurAddress(rp.Addr()))
-	err = s1.DialPeerWithAddress(rp.Addr(), false)
+
-	if assert.Error(t, err) {
+	assert.False(t, s1.addrBook.HasAddress(rp.Addr()))
-		assert.Equal(t, ErrSwitchConnectToSelf, err)
-	}
 
 	rp.Stop()
 
@@ -379,16 +373,27 @@ func BenchmarkSwitchBroadcast(b *testing.B) {
 }
 
 type addrBookMock struct {
+	addrs    map[string]struct{}
 	ourAddrs map[string]struct{}
 }
 
 var _ AddrBook = (*addrBookMock)(nil)
 
-func (book *addrBookMock) AddAddress(addr *NetAddress, src *NetAddress) error { return nil }
+func (book *addrBookMock) AddAddress(addr *NetAddress, src *NetAddress) error {
+	book.addrs[addr.String()] = struct{}{}
+	return nil
+}
 func (book *addrBookMock) AddOurAddress(addr *NetAddress) { book.ourAddrs[addr.String()] = struct{}{} }
 func (book *addrBookMock) OurAddress(addr *NetAddress) bool {
 	_, ok := book.ourAddrs[addr.String()]
 	return ok
 }
 func (book *addrBookMock) MarkGood(*NetAddress) {}
+func (book *addrBookMock) HasAddress(addr *NetAddress) bool {
+	_, ok := book.addrs[addr.String()]
+	return ok
+}
+func (book *addrBookMock) RemoveAddress(addr *NetAddress) {
+	delete(book.addrs, addr.String())
+}
 func (book *addrBookMock) Save() {}