From aaa81092e75b22d97503ac0944779d3450045285 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Sat, 28 Apr 2018 15:01:33 -0400 Subject: [PATCH] p2p: some comments and a log line --- docs/specification/new-spec/reactors/pex/pex.md | 5 +---- p2p/pex/addrbook.go | 2 ++ p2p/pex/pex_reactor.go | 12 +++++++++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/specification/new-spec/reactors/pex/pex.md b/docs/specification/new-spec/reactors/pex/pex.md index 0173704e2..d24271ebf 100644 --- a/docs/specification/new-spec/reactors/pex/pex.md +++ b/docs/specification/new-spec/reactors/pex/pex.md @@ -83,10 +83,7 @@ Connection attempts are made with exponential backoff (plus jitter). Because the selection process happens every `ensurePeersPeriod`, we might not end up dialing a peer for much longer than the backoff duration. - -TODO -PEX: if we fail to conenct to the peer after 16 tries (with exponential backoff), we remove from address book completely. - +If we fail to conenct to the peer after 16 tries (with exponential backoff), we remove from address book completely. ## Select Peers to Exchange diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go index 08646ae76..aff07b662 100644 --- a/p2p/pex/addrbook.go +++ b/p2p/pex/addrbook.go @@ -295,6 +295,7 @@ func (a *addrBook) MarkBad(addr *p2p.NetAddress) { // GetSelection implements AddrBook. // It randomly selects some addresses (old & new). Suitable for peer-exchange protocols. +// Must never return a nil address. func (a *addrBook) GetSelection() []*p2p.NetAddress { a.mtx.Lock() defer a.mtx.Unlock() @@ -332,6 +333,7 @@ func (a *addrBook) GetSelection() []*p2p.NetAddress { // GetSelectionWithBias implements AddrBook. // It randomly selects some addresses (old & new). Suitable for peer-exchange protocols. +// Must never return a nil address. // // Each address is picked randomly from an old or new bucket according to the // biasTowardsNewAddrs argument, which must be between [0, 100] (or else is truncated to diff --git a/p2p/pex/pex_reactor.go b/p2p/pex/pex_reactor.go index dfe54e0d6..82b772a64 100644 --- a/p2p/pex/pex_reactor.go +++ b/p2p/pex/pex_reactor.go @@ -301,7 +301,7 @@ func (r *PEXReactor) ReceiveAddrs(addrs []*p2p.NetAddress, src Peer) error { srcAddr := src.NodeInfo().NetAddress() for _, netAddr := range addrs { - // TODO: make sure correct nodes never send nil and return error + // NOTE: GetSelection methods should never return nil addrs if netAddr == nil { return cmn.NewError("received nil addr") } @@ -316,7 +316,13 @@ func (r *PEXReactor) ReceiveAddrs(addrs []*p2p.NetAddress, src Peer) error { err := r.book.AddAddress(netAddr, srcAddr) if err != nil { - r.Logger.Error("Failed to add new address", "err", err) + switch err.(type) { + case ErrAddrBookNilAddr: + r.Logger.Error("Failed to add new address", "err", err) + default: + // Could be non-routable, self, or full book. But not worth logging an Error + r.Logger.Debug("Failed to add new address", "err", err) + } } } return nil @@ -410,7 +416,7 @@ func (r *PEXReactor) ensurePeers() { } // TODO: consider moving some checks from toDial into here // so we don't even consider dialing peers that we want to wait - // before dialling again, or have dialled too many times already + // before dialling again, or have dialed too many times already r.Logger.Info("Will dial address", "addr", try) toDial[try.ID] = try }