p2p: do not log err if peer is private (#3474)

* add actionable advice for ErrAddrBookNonRoutable err

Should replace https://github.com/tendermint/tendermint/pull/3463

* reorder checks in addrbook#addAddress so

ErrAddrBookPrivate is returned first

and do not log error in DialPeersAsync if the address is private
because it's not an error

p2p/pex/addrbook.go

@@ -586,23 +586,10 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error {
 		return ErrAddrBookNilAddr{addr, src}
 	}
 
-	if a.routabilityStrict && !addr.Routable() {
-		return ErrAddrBookNonRoutable{addr}
-	}
-
-	if !addr.Valid() {
-		return ErrAddrBookInvalidAddr{addr}
-	}
-
 	if !addr.HasID() {
 		return ErrAddrBookInvalidAddrNoID{addr}
 	}
 
-	// TODO: we should track ourAddrs by ID and by IP:PORT and refuse both.
-	if _, ok := a.ourAddrs[addr.String()]; ok {
-		return ErrAddrBookSelf{addr}
-	}
-
 	if _, ok := a.privateIDs[addr.ID]; ok {
 		return ErrAddrBookPrivate{addr}
 	}
@@ -611,6 +598,19 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error {
 		return ErrAddrBookPrivateSrc{src}
 	}
 
+	// TODO: we should track ourAddrs by ID and by IP:PORT and refuse both.
+	if _, ok := a.ourAddrs[addr.String()]; ok {
+		return ErrAddrBookSelf{addr}
+	}
+
+	if a.routabilityStrict && !addr.Routable() {
+		return ErrAddrBookNonRoutable{addr}
+	}
+
+	if !addr.Valid() {
+		return ErrAddrBookInvalidAddr{addr}
+	}
+
 	ka := a.addrLookup[addr.ID]
 	if ka != nil {
 		// If its already old and the addr is the same, ignore it.

p2p/pex/errors.go

@@ -30,6 +30,10 @@ func (err ErrAddrBookPrivate) Error() string {
 	return fmt.Sprintf("Cannot add private peer with address %v", err.Addr)
 }
 
+func (err ErrAddrBookPrivate) PrivateAddr() bool {
+	return true
+}
+
 type ErrAddrBookPrivateSrc struct {
 	Src *p2p.NetAddress
 }
@@ -38,6 +42,10 @@ func (err ErrAddrBookPrivateSrc) Error() string {
 	return fmt.Sprintf("Cannot add peer coming from private peer with address %v", err.Src)
 }
 
+func (err ErrAddrBookPrivateSrc) PrivateAddr() bool {
+	return true
+}
+
 type ErrAddrBookNilAddr struct {
 	Addr *p2p.NetAddress
 	Src  *p2p.NetAddress

p2p/switch.go

@@ -6,6 +6,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/pkg/errors"
+
 	"github.com/tendermint/tendermint/config"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/p2p/conn"
@@ -390,6 +392,15 @@ func (sw *Switch) MarkPeerAsGood(peer Peer) {
 //---------------------------------------------------------------------
 // Dialing
 
+type privateAddr interface {
+	PrivateAddr() bool
+}
+
+func isPrivateAddr(err error) bool {
+	te, ok := errors.Cause(err).(privateAddr)
+	return ok && te.PrivateAddr()
+}
+
 // 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
@@ -412,10 +423,14 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b
 			// do not add our address or ID
 			if !netAddr.Same(ourAddr) {
 				if err := addrBook.AddAddress(netAddr, ourAddr); err != nil {
+					if isPrivateAddr(err) {
+						sw.Logger.Debug("Won't add peer's address to addrbook", "err", err)
+					} else {
 						sw.Logger.Error("Can't add peer's address to addrbook", "err", err)
 					}
 				}
 			}
+		}
 		// Persist some peers to disk right away.
 		// NOTE: integration tests depend on this
 		addrBook.Save()