From fae94a44a2d6bbd7fcef548200b6c26ec668795c Mon Sep 17 00:00:00 2001
From: Ethan Buchman <ethan@coinculture.info>
Date: Sat, 28 Apr 2018 20:07:38 -0400
Subject: [PATCH] p2p/pex: some addrbook fixes

* fix before/after in isBad()
* allow multiple IPs per ID even if ID isOld
---
 p2p/pex/addrbook.go      |  4 ++--
 p2p/pex/known_address.go | 11 +++++------
 p2p/switch.go            | 18 +++++++++++-------
 3 files changed, 18 insertions(+), 15 deletions(-)

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)