Prefer outgoing connections from higher to lower peer IDs

I think it may have been wrong all this time.

diff --git a/peerconn.go b/peerconn.go
index 4fefd6101b954c0a0347108e914653425c1b234b..71a38925fd02e9ff55048f8ce07b83663a0572b9 100644 (file)
--- a/peerconn.go
+++ b/peerconn.go
@@ -100,10 +100,12 @@ func (cn *PeerConn) ipv6() bool {
        return len(ip) == net.IPv6len
 }
 
-// Returns true the if the dialer/initiator has the lower client peer ID. TODO: Find the
-// specification for this.
+// Returns true the if the dialer/initiator has the higher client peer ID. See
+// https://github.com/arvidn/libtorrent/blame/272828e1cc37b042dfbbafa539222d8533e99755/src/bt_peer_connection.cpp#L3536-L3557.
+// As far as I can tell, Transmission just keeps the oldest connection.
 func (cn *PeerConn) isPreferredDirection() bool {
-       return bytes.Compare(cn.t.cl.peerID[:], cn.PeerID[:]) < 0 == cn.outgoing
+       // True if our client peer ID is higher than the remote's peer ID.
+       return bytes.Compare(cn.PeerID[:], cn.t.cl.peerID[:]) < 0 == cn.outgoing
 }
 
 // Returns whether the left connection should be preferred over the right one,

diff --git a/peerconn_test.go b/peerconn_test.go
index 42f8fe273131b29dda29d4d7e666975d2858e023..c68ca6f8cfb77ca41f0aa69cfb5e6e9a1fda9a69 100644 (file)
--- a/peerconn_test.go
+++ b/peerconn_test.go
@@ -257,7 +257,10 @@ func TestApplyRequestStateWriteBufferConstraints(t *testing.T) {
        c.Logf("max local to remote requests: %v", maxLocalToRemoteRequests)
 }
 
-func peerConnForPreferredNetworkDirection(localPeerId, remotePeerId int, outgoing, utp, ipv6 bool) *PeerConn {
+func peerConnForPreferredNetworkDirection(
+       localPeerId, remotePeerId int,
+       outgoing, utp, ipv6 bool,
+) *PeerConn {
        pc := PeerConn{}
        pc.outgoing = outgoing
        if utp {
@@ -278,16 +281,37 @@ func peerConnForPreferredNetworkDirection(localPeerId, remotePeerId int, outgoin
 func TestPreferredNetworkDirection(t *testing.T) {
        pc := peerConnForPreferredNetworkDirection
        c := qt.New(t)
-       // Prefer outgoing to higher peer ID
-       c.Assert(pc(1, 2, true, false, false).hasPreferredNetworkOver(pc(1, 2, false, false, false)), qt.IsTrue)
-       c.Assert(pc(1, 2, false, false, false).hasPreferredNetworkOver(pc(1, 2, true, false, false)), qt.IsFalse)
-       c.Assert(pc(2, 1, false, false, false).hasPreferredNetworkOver(pc(2, 1, true, false, false)), qt.IsTrue)
+
+       // Prefer outgoing to lower peer ID
+
+       c.Check(
+               pc(1, 2, true, false, false).hasPreferredNetworkOver(pc(1, 2, false, false, false)),
+               qt.IsFalse,
+       )
+       c.Check(
+               pc(1, 2, false, false, false).hasPreferredNetworkOver(pc(1, 2, true, false, false)),
+               qt.IsTrue,
+       )
+       c.Check(
+               pc(2, 1, false, false, false).hasPreferredNetworkOver(pc(2, 1, true, false, false)),
+               qt.IsFalse,
+       )
+
        // Don't prefer uTP
-       c.Assert(pc(1, 2, false, true, false).hasPreferredNetworkOver(pc(1, 2, false, false, false)), qt.IsFalse)
+       c.Check(
+               pc(1, 2, false, true, false).hasPreferredNetworkOver(pc(1, 2, false, false, false)),
+               qt.IsFalse,
+       )
        // Prefer IPv6
-       c.Assert(pc(1, 2, false, false, false).hasPreferredNetworkOver(pc(1, 2, false, false, true)), qt.IsFalse)
+       c.Check(
+               pc(1, 2, false, false, false).hasPreferredNetworkOver(pc(1, 2, false, false, true)),
+               qt.IsFalse,
+       )
        // No difference
-       c.Assert(pc(1, 2, false, false, false).hasPreferredNetworkOver(pc(1, 2, false, false, false)), qt.IsFalse)
+       c.Check(
+               pc(1, 2, false, false, false).hasPreferredNetworkOver(pc(1, 2, false, false, false)),
+               qt.IsFalse,
+       )
 }
 
 func TestReceiveLargeRequest(t *testing.T) {