Skip to content

Commit

Permalink
p2p: do not log err if peer is private (tendermint#3474)
Browse files Browse the repository at this point in the history
* add actionable advice for ErrAddrBookNonRoutable err

Should replace tendermint#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
  • Loading branch information
melekes authored and brapse committed Jun 5, 2019
1 parent 1b7de6a commit 4e50130
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 11 deletions.
20 changes: 10 additions & 10 deletions p2p/pex/addrbook.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,29 +586,29 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error {
return ErrAddrBookNilAddr{addr, src}
}

if a.routabilityStrict && !addr.Routable() {
return ErrAddrBookNonRoutable{addr}
if !addr.HasID() {
return ErrAddrBookInvalidAddrNoID{addr}
}

if !addr.Valid() {
return ErrAddrBookInvalidAddr{addr}
if _, ok := a.privateIDs[addr.ID]; ok {
return ErrAddrBookPrivate{addr}
}

if !addr.HasID() {
return ErrAddrBookInvalidAddrNoID{addr}
if _, ok := a.privateIDs[src.ID]; ok {
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 _, ok := a.privateIDs[addr.ID]; ok {
return ErrAddrBookPrivate{addr}
if a.routabilityStrict && !addr.Routable() {
return ErrAddrBookNonRoutable{addr}
}

if _, ok := a.privateIDs[src.ID]; ok {
return ErrAddrBookPrivateSrc{src}
if !addr.Valid() {
return ErrAddrBookInvalidAddr{addr}
}

ka := a.addrLookup[addr.ID]
Expand Down
8 changes: 8 additions & 0 deletions p2p/pex/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down
17 changes: 16 additions & 1 deletion p2p/switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -412,7 +423,11 @@ 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 {
sw.Logger.Error("Can't add peer's address to addrbook", "err", err)
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)
}
}
}
}
Expand Down

0 comments on commit 4e50130

Please sign in to comment.