Add stats for connections that only occur due to holepunching

client-stats.go

@@ -7,4 +7,11 @@ type ClientStats struct {
 	// to hole-punch connect requests. The total may not match the sum of attempts for all Torrents
 	// if a Torrent is dropped while there are outstanding dials.
 	ActiveHalfOpenAttempts int
+
+	// Number of unique peer addresses that were dialed after receiving a holepunch connect message,
+	// that have previously been undialable without any hole-punching attempts.
+	NumPeersUndialableWithoutHolepunchDialedAfterHolepunchConnect int
+	// Number of unique peer addresses that were successfully dialed and connected after a holepunch
+	// connect message and previously failing to connect without holepunching.
+	NumPeersDialableOnlyAfterHolepunch int
 }

client.go

@@ -18,6 +18,8 @@ import (
 	"strconv"
 	"time"
 
+	"github.com/anacrolix/torrent/internal/panicif"
+
 	"github.com/anacrolix/chansync"
 	"github.com/anacrolix/chansync/events"
 	"github.com/anacrolix/dht/v2"
@@ -89,6 +91,10 @@ type Client struct {
 
 	activeAnnounceLimiter limiter.Instance
 	httpClient            *http.Client
+
+	undialableWithoutHolepunch                                   map[netip.AddrPort]struct{}
+	undialableWithoutHolepunchDialAttemptedAfterHolepunchConnect map[netip.AddrPort]struct{}
+	dialableOnlyAfterHolepunch                                   map[netip.AddrPort]struct{}
 }
 
 type ipStr string
@@ -748,12 +754,8 @@ func (cl *Client) waitForRendezvousConnect(ctx context.Context, rz *utHolepunchR
 
 // Returns nil connection and nil error if no connection could be established for valid reasons.
 func (cl *Client) initiateRendezvousConnect(
-	t *Torrent, addr PeerRemoteAddr,
+	t *Torrent, holepunchAddr netip.AddrPort,
 ) (ok bool, err error) {
-	holepunchAddr, err := addrPortFromPeerRemoteAddr(addr)
-	if err != nil {
-		return
-	}
 	cl.lock()
 	defer cl.unlock()
 	rz, err := t.startHolepunchRendezvous(holepunchAddr)
@@ -783,14 +785,18 @@ func (cl *Client) establishOutgoingConnEx(
 ) {
 	t := opts.t
 	addr := opts.addr
-	var rzOk bool
-	if !opts.skipHolepunchRendezvous {
-		rzOk, err = cl.initiateRendezvousConnect(t, addr)
-		if err != nil {
-			err = fmt.Errorf("initiating rendezvous connect: %w", err)
+	holepunchAddr, err := addrPortFromPeerRemoteAddr(addr)
+	var sentRendezvous bool
+	if err == nil {
+		if !opts.skipHolepunchRendezvous {
+			sentRendezvous, err = cl.initiateRendezvousConnect(t, holepunchAddr)
+			if err != nil {
+				err = fmt.Errorf("initiating rendezvous connect: %w", err)
+			}
 		}
 	}
-	if opts.requireRendezvous && !rzOk {
+	gotHolepunchConnect := (err == nil && sentRendezvous) || opts.receivedHolepunchConnect
+	if opts.requireRendezvous && !sentRendezvous {
 		return nil, err
 	}
 	if err != nil {
@@ -804,12 +810,38 @@ func (cl *Client) establishOutgoingConnEx(
 	defer cancel()
 	dr := cl.dialFirst(dialCtx, addr.String())
 	nc := dr.Conn
+	cl.lock()
+	if gotHolepunchConnect && g.MapContains(cl.undialableWithoutHolepunch, holepunchAddr) {
+		g.MakeMapIfNilAndSet(
+			&cl.undialableWithoutHolepunchDialAttemptedAfterHolepunchConnect,
+			holepunchAddr,
+			struct{}{},
+		)
+	}
+	cl.unlock()
 	if nc == nil {
+		if !sentRendezvous && !gotHolepunchConnect {
+			cl.lock()
+			g.MakeMapIfNilAndSet(&cl.undialableWithoutHolepunch, holepunchAddr, struct{}{})
+			cl.unlock()
+		}
 		if dialCtx.Err() != nil {
 			return nil, fmt.Errorf("dialing: %w", dialCtx.Err())
 		}
 		return nil, errors.New("dial failed")
 	}
+	if gotHolepunchConnect {
+		panicif.False(holepunchAddr.IsValid())
+		cl.lock()
+		if g.MapContains(cl.undialableWithoutHolepunchDialAttemptedAfterHolepunchConnect, holepunchAddr) {
+			g.MakeMapIfNilAndSet(
+				&cl.dialableOnlyAfterHolepunch,
+				holepunchAddr,
+				struct{}{},
+			)
+		}
+		cl.unlock()
+	}
 	addrIpPort, _ := tryIpPortFromNetAddr(addr)
 	c, err := cl.initiateProtocolHandshakes(
 		context.Background(), nc, t, obfuscatedHeader,
@@ -859,6 +891,8 @@ type outgoingConnOpts struct {
 	requireRendezvous bool
 	// Don't send rendezvous requests to eligible relays.
 	skipHolepunchRendezvous bool
+	// Outgoing connection attempt is in response to holepunch connect message.
+	receivedHolepunchConnect bool
 }
 
 // Called to dial out and run a connection. The addr we're given is already
@@ -1795,5 +1829,9 @@ func (cl *Client) Stats() ClientStats {
 func (cl *Client) statsLocked() (stats ClientStats) {
 	stats.ConnStats = cl.connStats.Copy()
 	stats.ActiveHalfOpenAttempts = cl.numHalfOpen
+	stats.NumPeersUndialableWithoutHolepunchDialedAfterHolepunchConnect =
+		len(cl.undialableWithoutHolepunchDialAttemptedAfterHolepunchConnect)
+	stats.NumPeersDialableOnlyAfterHolepunch =
+		len(cl.dialableOnlyAfterHolepunch)
 	return
 }

internal/panicif/panicif.go

@@ -7,3 +7,15 @@ func NotEqual[T comparable](a, b T) {
 		panic(fmt.Sprintf("%v != %v", a, b))
 	}
 }
+
+func False(b bool) {
+	if !b {
+		panic("is false")
+	}
+}
+
+func True(b bool) {
+	if b {
+		panic("is true")
+	}
+}

torrent.go

@@ -1383,7 +1383,7 @@ func (t *Torrent) openNewConns() (initiated int) {
 			return
 		}
 		p := t.peers.PopMax()
-		t.initiateConn(p, false, false, false)
+		t.initiateConn(p, false, false, false, false)
 		initiated++
 	}
 	return
@@ -2403,6 +2403,7 @@ func (t *Torrent) initiateConn(
 	requireRendezvous bool,
 	skipHolepunchRendezvous bool,
 	ignoreLimits bool,
+	receivedHolepunchConnect bool,
 ) {
 	if peer.Id == t.cl.peerID {
 		return
@@ -2424,10 +2425,11 @@ func (t *Torrent) initiateConn(
 	t.addHalfOpen(addrStr, attemptKey)
 	go t.cl.outgoingConnection(
 		outgoingConnOpts{
-			t:                       t,
-			addr:                    peer.Addr,
-			requireRendezvous:       requireRendezvous,
-			skipHolepunchRendezvous: skipHolepunchRendezvous,
+			t:                        t,
+			addr:                     peer.Addr,
+			requireRendezvous:        requireRendezvous,
+			skipHolepunchRendezvous:  skipHolepunchRendezvous,
+			receivedHolepunchConnect: receivedHolepunchConnect,
 		},
 		peer.Source,
 		peer.Trusted,
@@ -2814,7 +2816,7 @@ func (t *Torrent) handleReceivedUtHolepunchMsg(msg utHolepunch.Msg, sender *Peer
 			t.initiateConn(PeerInfo{
 				Addr:   msg.AddrPort,
 				Source: PeerSourceUtHolepunch,
-			}, false, true, true)
+			}, false, true, true, true)
 		}
 		return nil
 	case utHolepunch.Error:

ut-holepunching_test.go

@@ -94,9 +94,20 @@ func TestHolepunchConnect(t *testing.T) {
 	log.Printf("trying to initiate to %v", targetAddr)
 	llg.initiateConn(PeerInfo{
 		Addr: targetAddr,
-	}, true, false, false)
+	}, true, false, false, false)
 	llg.cl.unlock()
 	wg.Wait()
+	// These checks would require that the leecher leecher first attempt to connect without
+	// holepunching.
+
+	//llClientStats := leecherLeecher.Stats()
+	//c := qt.New(t)
+	//c.Check(llClientStats.NumPeersDialedRequiringHolepunch, qt.Not(qt.Equals), 0)
+	//c.Check(
+	//	llClientStats.NumPeersDialedRequiringHolepunch,
+	//	qt.Equals,
+	//	llClientStats.NumPeersUndiableWithoutHolepunch,
+	//)
 }
 
 func waitForConns(t *Torrent) {