diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go index 6521b6491..5b4428f5e 100644 --- a/p2p/pex/addrbook.go +++ b/p2p/pex/addrbook.go @@ -626,8 +626,8 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error { ka := a.addrLookup[addr.ID] if ka != nil { - // Already old. - if ka.isOld() { + // If its already old and the addr is the same, ignore it. + if ka.isOld() && ka.Addr.Equals(addr) { return nil } // Already in max new buckets. diff --git a/p2p/pex/known_address.go b/p2p/pex/known_address.go index 0261e4902..5673dec11 100644 --- a/p2p/pex/known_address.go +++ b/p2p/pex/known_address.go @@ -106,7 +106,6 @@ func (ka *knownAddress) removeBucketRef(bucketIdx int) int { All addresses that meet these criteria are assumed to be worthless and not worth keeping hold of. - XXX: so a good peer needs us to call MarkGood before the conditions above are reached! */ func (ka *knownAddress) isBad() bool { // Is Old --> good @@ -115,14 +114,15 @@ func (ka *knownAddress) isBad() bool { } // Has been attempted in the last minute --> good - if ka.LastAttempt.Before(time.Now().Add(-1 * time.Minute)) { + if ka.LastAttempt.After(time.Now().Add(-1 * time.Minute)) { return false } + // TODO: From the future? + // Too old? - // XXX: does this mean if we've kept a connection up for this long we'll disconnect?! - // and shouldn't it be .Before ? - if ka.LastAttempt.After(time.Now().Add(-1 * numMissingDays * time.Hour * 24)) { + // TODO: should be a timestamp of last seen, not just last attempt + if ka.LastAttempt.Before(time.Now().Add(-1 * numMissingDays * time.Hour * 24)) { return true } @@ -132,7 +132,6 @@ func (ka *knownAddress) isBad() bool { } // Hasn't succeeded in too long? - // XXX: does this mean if we've kept a connection up for this long we'll disconnect?! if ka.LastSuccess.Before(time.Now().Add(-1*minBadDays*time.Hour*24)) && ka.Attempts >= maxFailures { return true diff --git a/p2p/switch.go b/p2p/switch.go index ffc1cbb8e..bccae393b 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -290,6 +290,8 @@ func (sw *Switch) stopAndRemovePeer(peer Peer, reason interface{}) { // to the PEX/Addrbook to find the peer with the addr again // NOTE: this will keep trying even if the handshake or auth fails. // TODO: be more explicit with error types so we only retry on certain failures +// - ie. if we're getting ErrDuplicatePeer we can stop +// because the addrbook got us the peer back already func (sw *Switch) reconnectToPeer(addr *NetAddress) { if sw.reconnecting.Has(string(addr.ID)) { return @@ -305,14 +307,14 @@ func (sw *Switch) reconnectToPeer(addr *NetAddress) { } err := sw.DialPeerWithAddress(addr, true) - if err != nil { - sw.Logger.Info("Error reconnecting to peer. Trying again", "tries", i, "err", err, "addr", addr) - // sleep a set amount - sw.randomSleep(reconnectInterval) - continue - } else { - return + if err == nil { + return // success } + + sw.Logger.Info("Error reconnecting to peer. Trying again", "tries", i, "err", err, "addr", addr) + // sleep a set amount + sw.randomSleep(reconnectInterval) + continue } sw.Logger.Error("Failed to reconnect to peer. Beginning exponential backoff", @@ -501,6 +503,8 @@ func (sw *Switch) addInboundPeerWithConfig(conn net.Conn, config *PeerConfig) er // dial the peer; make secret connection; authenticate against the dialed ID; // add the peer. +// if dialing fails, start the reconnect loop. If handhsake fails, its over. +// If peer is started succesffuly, reconnectLoop will start when StopPeerForError is called func (sw *Switch) addOutboundPeerWithConfig(addr *NetAddress, config *PeerConfig, persistent bool) error { sw.Logger.Info("Dialing peer", "address", addr) peerConn, err := newOutboundPeerConn(addr, config, persistent, sw.nodeKey.PrivKey)