fix: prefer non-transient connections

Given two relay connections, prefer the one that's non-transient.
Otherwise, a transient connection could prevent us from opening streams
even if we have a second non-transient connection through a better relay.

swarm.go

@@ -385,6 +385,38 @@ func (s *Swarm) ConnsToPeer(p peer.ID) []network.Conn {
 	return output
 }
 
+func isBetterConn(a, b *Conn) bool {
+	// If one is transient and not the other, prefer the non-transient connection.
+	aTransient := a.Stat().Transient
+	bTransient := b.Stat().Transient
+	if aTransient != bTransient {
+		return !aTransient
+	}
+
+	// If one is direct and not the other, prefer the direct connection.
+	aDirect := isDirectConn(a)
+	bDirect := isDirectConn(b)
+	if aDirect != bDirect {
+		return aDirect
+	}
+
+	// Otherwise, prefer the connection with more open streams.
+	a.streams.Lock()
+	aLen := len(a.streams.m)
+	a.streams.Unlock()
+
+	b.streams.Lock()
+	bLen := len(b.streams.m)
+	b.streams.Unlock()
+
+	if aLen != bLen {
+		return aLen > bLen
+	}
+
+	// finally, pick the last connection.
+	return true
+}
+
 // bestConnToPeer returns the best connection to peer.
 func (s *Swarm) bestConnToPeer(p peer.ID) *Conn {
 
@@ -395,26 +427,13 @@ func (s *Swarm) bestConnToPeer(p peer.ID) *Conn {
 	defer s.conns.RUnlock()
 
 	var best *Conn
-	bestLen := 0
 	for _, c := range s.conns.m[p] {
 		if c.conn.IsClosed() {
 			// We *will* garbage collect this soon anyways.
 			continue
 		}
-		c.streams.Lock()
-		cLen := len(c.streams.m)
-		c.streams.Unlock()
-
-		// We will never prefer a Relayed connection over a direct connection.
-		if isDirectConn(best) && !isDirectConn(c) {
-			continue
-		}
-
-		// 1. Always prefer a direct connection over a relayed connection.
-		// 2. If both conns are direct or relayed, pick the one with as many or more streams.
-		if (!isDirectConn(best) && isDirectConn(c)) || (cLen >= bestLen) {
+		if best == nil || isBetterConn(c, best) {
 			best = c
-			bestLen = cLen
 		}
 	}
 	return best