pex: various follow-ups (#3605)

* p2p: merge switch cases

also improve the error msg in privval

* pex: refactor code plus update specification

follow-up to https://github.com/tendermint/tendermint/pull/3603

* Update docs/spec/reactors/pex/pex.md

Co-Authored-By: melekes <anton.kalyaev@gmail.com>

docs/spec/reactors/pex/pex.md

@@ -21,17 +21,20 @@ inbound (they dialed our public address) or outbound (we dialed them).
 ## Discovery
 
 Peer discovery begins with a list of seeds.
-When we have no peers, or have been unable to find enough peers from existing ones,
-we dial a randomly selected seed to get a list of peers to dial.
+
+When we don't have enough peers, we
+
+1. ask existing peers
+2. dial seeds if we're not dialing anyone currently
 
 On startup, we will also immediately dial the given list of `persistent_peers`,
-and will attempt to maintain persistent connections with them. If the connections die, or we fail to dial,
-we will redial every 5s for a few minutes, then switch to an exponential backoff schedule,
-and after about a day of trying, stop dialing the peer.
+and will attempt to maintain persistent connections with them. If the
+connections die, or we fail to dial, we will redial every 5s for a few minutes,
+then switch to an exponential backoff schedule, and after about a day of
+trying, stop dialing the peer.
 
-So long as we have less than `MaxNumOutboundPeers`, we periodically request additional peers
-from each of our own. If sufficient time goes by and we still can't find enough peers,
-we try the seeds again.
+As long as we have less than `MaxNumOutboundPeers`, we periodically request
+additional peers from each of our own and try seeds.
 
 ## Listening
 

p2p/pex/pex_reactor.go

@@ -471,9 +471,7 @@ func (r *PEXReactor) ensurePeers() {
 			err := r.dialPeer(addr)
 			if err != nil {
 				switch err.(type) {
-				case errMaxAttemptsToDial:
-					r.Logger.Debug(err.Error(), "addr", addr)
-				case errTooEarlyToDial:
+				case errMaxAttemptsToDial, errTooEarlyToDial:
 					r.Logger.Debug(err.Error(), "addr", addr)
 				default:
 					r.Logger.Error(err.Error(), "addr", addr)
@@ -482,8 +480,8 @@ func (r *PEXReactor) ensurePeers() {
 		}(addr)
 	}
 
-	// If we need more addresses, pick a random peer and ask for more.
 	if r.book.NeedMoreAddrs() {
+		// 1) Pick a random peer and ask for more.
 		peers := r.Switch.Peers().List()
 		peersCount := len(peers)
 		if peersCount > 0 {
@@ -491,13 +489,14 @@ func (r *PEXReactor) ensurePeers() {
 			r.Logger.Info("We need more addresses. Sending pexRequest to random peer", "peer", peer)
 			r.RequestAddrs(peer)
 		}
-	}
 
-	// If we are not dialing anyone and need more addresses - dial a seed
-	// This is done in addition to asking a peer for addresses to work-around peers not participating in PEX
-	if r.book.NeedMoreAddrs() && len(toDial) == 0 {
-		r.Logger.Info("No addresses to dial. Falling back to seeds")
-		r.dialSeeds()
+		// 2) Dial seeds if we are not dialing anyone.
+		// This is done in addition to asking a peer for addresses to work-around
+		// peers not participating in PEX.
+		if len(toDial) == 0 {
+			r.Logger.Info("No addresses to dial. Falling back to seeds")
+			r.dialSeeds()
+		}
 	}
 }
 
@@ -664,9 +663,7 @@ func (r *PEXReactor) crawlPeers(addrs []*p2p.NetAddress) {
 		err := r.dialPeer(addr)
 		if err != nil {
 			switch err.(type) {
-			case errMaxAttemptsToDial:
-				r.Logger.Debug(err.Error(), "addr", addr)
-			case errTooEarlyToDial:
+			case errMaxAttemptsToDial, errTooEarlyToDial:
 				r.Logger.Debug(err.Error(), "addr", addr)
 			default:
 				r.Logger.Error(err.Error(), "addr", addr)

privval/socket_listeners_test.go

@@ -130,8 +130,8 @@ func TestListenerTimeoutReadWrite(t *testing.T) {
 			t.Errorf("for %s listener, have %v, want %v", tc.description, have, want)
 		}
 
-		if have, want := opErr.Timeout(), true; have != want {
-			t.Errorf("for %s listener, got unexpected error: have %v, want %v", tc.description, have, want)
+		if !opErr.Timeout() {
+			t.Errorf("for %s listener, got unexpected error: have %v, want Timeout error", tc.description, opErr)
 		}
 	}
 }