cleanup dcutr legacy behavior and add fallback

fallback will get triggered during the last attempt at holepunching.

Co-authored-by: Marco Munizaga <marco@marcopolo.io>

Author: jpserrat

p2p/protocol/holepunch/holepunch_test.go

@@ -250,7 +250,7 @@ func TestEndToEndSimConnect(t *testing.T) {
 
 			h1 := MustNewHost(t,
 				quicSimnet(false, router),
-				libp2p.EnableHolePunching(holepunch.WithTracer(h1tr), holepunch.DirectDialTimeout(100*time.Millisecond), SetLegacyBehavior(useLegacyHolePunchingBehavior)),
+				libp2p.EnableHolePunching(holepunch.WithTracer(h1tr), holepunch.DirectDialTimeout(100*time.Millisecond)),
 				libp2p.ListenAddrs(ma.StringCast("/ip4/2.2.0.1/udp/8000/quic-v1")),
 				libp2p.ResourceManager(&network.NullResourceManager{}),
 				libp2p.ForceReachabilityPrivate(),
@@ -261,7 +261,7 @@ func TestEndToEndSimConnect(t *testing.T) {
 				libp2p.ListenAddrs(ma.StringCast("/ip4/2.2.0.2/udp/8001/quic-v1")),
 				libp2p.ResourceManager(&network.NullResourceManager{}),
 				connectToRelay(&relay),
-				libp2p.EnableHolePunching(holepunch.WithTracer(h2tr), holepunch.DirectDialTimeout(100*time.Millisecond), SetLegacyBehavior(useLegacyHolePunchingBehavior)),
+				libp2p.EnableHolePunching(holepunch.WithTracer(h2tr), holepunch.DirectDialTimeout(100*time.Millisecond)),
 				libp2p.ForceReachabilityPrivate(),
 			)
 
@@ -660,20 +660,6 @@ func waitForHolePunchingSvcActive(t *testing.T, h host.Host) {
 	}, time.Second, 100*time.Millisecond)
 }
 
-// setLegacyBehavior is an option that controls the isClient behavior of the hole punching service.
-// Prior to https://github.com/libp2p/go-libp2p/pull/3044, go-libp2p would
-// pick the opposite roles for client/server a hole punch. Setting this to
-// true preserves that behavior.
-//
-// Currently, only exposed for testing purposes.
-// Do not set this unless you know what you are doing.
-func SetLegacyBehavior(legacyBehavior bool) holepunch.Option {
-	return func(s *holepunch.Service) error {
-		s.SetLegacyBehavior(legacyBehavior)
-		return nil
-	}
-}
-
 // TestEndToEndSimConnectQUICReuse tests that hole punching works if we are
 // reusing the same port for QUIC and WebTransport, and when we have multiple
 // QUIC listeners on different ports.

p2p/protocol/holepunch/holepuncher.go

@@ -48,11 +48,6 @@ type holePuncher struct {
 
 	tracer *tracer
 	filter AddrFilter
-
-	// Prior to https://github.com/libp2p/go-libp2p/pull/3044, go-libp2p would
-	// pick the opposite roles for client/server a hole punch. Setting this to
-	// true preserves that behavior
-	legacyBehavior bool
 }
 
 func newHolePuncher(h host.Host, ids identify.IDService, listenAddrs func() []ma.Multiaddr, tracer *tracer, filter AddrFilter) *holePuncher {
@@ -63,8 +58,6 @@ func newHolePuncher(h host.Host, ids identify.IDService, listenAddrs func() []ma
 		tracer:      tracer,
 		filter:      filter,
 		listenAddrs: listenAddrs,
-
-		legacyBehavior: true,
 	}
 	hp.ctx, hp.ctxCancel = context.WithCancel(context.Background())
 	h.Network().Notify((*netNotifiee)(hp))
@@ -141,6 +134,13 @@ func (hp *holePuncher) directConnect(rp peer.ID) error {
 
 	// hole punch
 	for i := 1; i <= maxRetries; i++ {
+		isClient := false
+		// On the last attempt we switch roles in case the connection is
+		// being made with a client with switched roles. Common for peers
+		// running go-libp2p prior to v0.41.
+		if i == maxRetries {
+			isClient = true
+		}
 		addrs, obsAddrs, rtt, err := hp.initiateHolePunch(rp)
 		if err != nil {
 			hp.tracer.ProtocolError(rp, err)
@@ -161,10 +161,6 @@ func (hp *holePuncher) directConnect(rp peer.ID) error {
 			hp.tracer.StartHolePunch(rp, addrs, rtt)
 			hp.tracer.HolePunchAttempt(pi.ID)
 			ctx, cancel := context.WithTimeout(hp.ctx, hp.directDialTimeout)
-			isClient := true
-			if hp.legacyBehavior {
-				isClient = false
-			}
 			err := holePunchConnect(ctx, hp.host, pi, isClient)
 			cancel()
 			dt := time.Since(start)

p2p/protocol/holepunch/svc.go

@@ -71,17 +71,6 @@ type Service struct {
 	filter AddrFilter
 
 	refCount sync.WaitGroup
-
-	// Prior to https://github.com/libp2p/go-libp2p/pull/3044, go-libp2p would
-	// pick the opposite roles for client/server a hole punch. Setting this to
-	// true preserves that behavior
-	legacyBehavior bool
-}
-
-// SetLegacyBehavior is only exposed for testing purposes.
-// Do not set this unless you know what you are doing.
-func (s *Service) SetLegacyBehavior(legacyBehavior bool) {
-	s.legacyBehavior = legacyBehavior
 }
 
 // NewService creates a new service that can be used for hole punching
@@ -105,7 +94,6 @@ func NewService(h host.Host, ids identify.IDService, listenAddrs func() []ma.Mul
 		listenAddrs:        listenAddrs,
 		hasPublicAddrsChan: make(chan struct{}),
 		directDialTimeout:  defaultDirectDialTimeout,
-		legacyBehavior:     true,
 	}
 
 	for _, opt := range opts {
@@ -161,7 +149,6 @@ func (s *Service) waitForPublicAddr() {
 	}
 	s.holePuncher = newHolePuncher(s.host, s.ids, s.listenAddrs, s.tracer, s.filter)
 	s.holePuncher.directDialTimeout = s.directDialTimeout
-	s.holePuncher.legacyBehavior = s.legacyBehavior
 	s.holePuncherMx.Unlock()
 	close(s.hasPublicAddrsChan)
 }
@@ -284,11 +271,7 @@ func (s *Service) handleNewStream(str network.Stream) {
 	start := time.Now()
 	s.tracer.HolePunchAttempt(pi.ID)
 	ctx, cancel := context.WithTimeout(s.ctx, s.directDialTimeout)
-	isClient := false
-	if s.legacyBehavior {
-		isClient = true
-	}
-	err = holePunchConnect(ctx, s.host, pi, isClient)
+	err = holePunchConnect(ctx, s.host, pi, true) // true (Client)
 	cancel()
 	dt := time.Since(start)
 	s.tracer.EndHolePunch(rp, dt, err)