Merge pull request #1396 from libp2p/autorelay-v1-handling

improve AutoRelay v1 handling

p2p/host/autorelay/autorelay_test.go

@@ -64,6 +64,29 @@ func newRelay(t *testing.T) host.Host {
 	return h
 }
 
+func newRelayV1(t *testing.T) host.Host {
+	t.Helper()
+	h, err := libp2p.New(
+		libp2p.DisableRelay(),
+		libp2p.ForceReachabilityPublic(),
+		libp2p.AddrsFactory(func(addrs []ma.Multiaddr) []ma.Multiaddr {
+			for i, addr := range addrs {
+				saddr := addr.String()
+				if strings.HasPrefix(saddr, "/ip4/127.0.0.1/") {
+					addrNoIP := strings.TrimPrefix(saddr, "/ip4/127.0.0.1")
+					addrs[i] = ma.StringCast("/dns4/localhost" + addrNoIP)
+				}
+			}
+			return addrs
+		}),
+	)
+	require.NoError(t, err)
+	r, err := relayv1.NewRelay(h)
+	require.NoError(t, err)
+	t.Cleanup(func() { r.Close() })
+	return h
+}
+
 // creates a node that speaks the relay v2 protocol, but doesn't accept any reservations for the first workAfter tries
 func newBrokenRelay(t *testing.T, workAfter int) host.Host {
 	t.Helper()
@@ -242,3 +265,42 @@ func TestStaticRelays(t *testing.T) {
 		return len(ma.FilterAddrs(h.Addrs(), isRelayAddr)) > 0
 	}, 2*time.Second, 50*time.Millisecond)
 }
+
+func TestRelayV1(t *testing.T) {
+	t.Run("relay v1 support disabled", func(t *testing.T) {
+		peerChan := make(chan peer.AddrInfo)
+		go func() {
+			r := newRelayV1(t)
+			peerChan <- peer.AddrInfo{ID: r.ID(), Addrs: r.Addrs()}
+			t.Cleanup(func() { r.Close() })
+		}()
+		h := newPrivateNode(t,
+			autorelay.WithPeerSource(peerChan),
+			autorelay.WithBootDelay(0),
+		)
+		defer h.Close()
+
+		require.Never(t, func() bool {
+			return len(ma.FilterAddrs(h.Addrs(), isRelayAddr)) > 0
+		}, 3*time.Second, 100*time.Millisecond)
+	})
+
+	t.Run("relay v1 support enabled", func(t *testing.T) {
+		peerChan := make(chan peer.AddrInfo)
+		go func() {
+			r := newRelayV1(t)
+			peerChan <- peer.AddrInfo{ID: r.ID(), Addrs: r.Addrs()}
+			t.Cleanup(func() { r.Close() })
+		}()
+		h := newPrivateNode(t,
+			autorelay.WithPeerSource(peerChan),
+			autorelay.WithBootDelay(0),
+			autorelay.WithCircuitV1Support(),
+		)
+		defer h.Close()
+
+		require.Eventually(t, func() bool {
+			return len(ma.FilterAddrs(h.Addrs(), isRelayAddr)) > 0
+		}, 3*time.Second, 100*time.Millisecond)
+	})
+}

p2p/host/autorelay/options.go

@@ -25,6 +25,7 @@ type config struct {
 	// Number of relays we strive to obtain a reservation with.
 	desiredRelays    int
 	setMinCandidates bool
+	enableCircuitV1  bool
 }
 
 var defaultConfig = config{
@@ -144,3 +145,11 @@ func WithMaxAttempts(n int) Option {
 		return nil
 	}
 }
+
+// WithCircuitV1Support enables support for circuit v1 relays.
+func WithCircuitV1Support() Option {
+	return func(c *config) error {
+		c.enableCircuitV1 = true
+		return nil
+	}
+}

p2p/host/autorelay/relay_finder.go

@@ -264,19 +264,36 @@ func (rf *relayFinder) tryNode(ctx context.Context, pi peer.AddrInfo) (supportsR
 	}
 
 	// If the node speaks both, prefer circuit v2
-	var supportsV1, supportsV2 bool
+	var maybeSupportsV1, supportsV2 bool
 	for _, proto := range protos {
 		switch proto {
 		case protoIDv1:
-			supportsV1 = true
+			maybeSupportsV1 = true
 		case protoIDv2:
 			supportsV2 = true
 		}
 	}
-	if !supportsV1 && !supportsV2 {
+
+	if supportsV2 {
+		return true, nil
+	}
+
+	if !rf.conf.enableCircuitV1 && !supportsV2 {
+		return false, errors.New("doesn't speak circuit v2")
+	}
+	if !maybeSupportsV1 && !supportsV2 {
 		return false, errors.New("doesn't speak circuit v1 or v2")
 	}
-	return supportsV2, nil
+
+	// The node *may* support circuit v1.
+	supportsV1, err := relayv1.CanHop(ctx, rf.host, pi.ID)
+	if err != nil {
+		return false, fmt.Errorf("CanHop failed: %w", err)
+	}
+	if !supportsV1 {
+		return false, errors.New("doesn't speak circuit v1 or v2")
+	}
+	return false, nil
 }
 
 func (rf *relayFinder) handleNewCandidate(ctx context.Context) {
@@ -336,16 +353,6 @@ func (rf *relayFinder) handleNewCandidate(ctx context.Context) {
 					failed = true
 					log.Debugw("failed to reserve slot", "id", id, "error", err)
 				}
-			} else {
-				ok, err := relayv1.CanHop(ctx, rf.host, id)
-				if err != nil {
-					failed = true
-					log.Debugw("error querying relay for v1 hop", "id", id, "error", err)
-				}
-				if !ok {
-					failed = true
-					log.Debugw("relay can't hop", "id", id)
-				}
 			}
 			rf.candidateMx.Lock()
 			if failed {

p2p/protocol/holepunch/holepunch_test.go

@@ -7,6 +7,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/libp2p/go-libp2p/p2p/host/autorelay"
+
 	"github.com/libp2p/go-libp2p"
 
 	"github.com/libp2p/go-libp2p-core/host"
@@ -347,7 +349,7 @@ func mkHostWithStaticAutoRelay(t *testing.T, relay host.Host) host.Host {
 	h, err := libp2p.New(
 		libp2p.ListenAddrs(ma.StringCast("/ip4/127.0.0.1/tcp/0")),
 		libp2p.EnableRelay(),
-		libp2p.EnableAutoRelay(),
+		libp2p.EnableAutoRelay(autorelay.WithCircuitV1Support()),
 		libp2p.ForceReachabilityPrivate(),
 		libp2p.StaticRelays([]peer.AddrInfo{pi}),
 	)