basichost: don't wait for Identify in Host.Connect

p2p/host/basic/basic_host.go

@@ -735,22 +735,10 @@ func (h *BasicHost) Connect(ctx context.Context, pi peer.AddrInfo) error {
 // the connection once it has been opened.
 func (h *BasicHost) dialPeer(ctx context.Context, p peer.ID) error {
 	log.Debugf("host %s dialing %s", h.ID(), p)
-	c, err := h.Network().DialPeer(ctx, p)
-	if err != nil {
+	if _, err := h.Network().DialPeer(ctx, p); err != nil {
 		return fmt.Errorf("failed to dial: %w", err)
 	}
 
-	// TODO: Consider removing this? On one hand, it's nice because we can
-	// assume that things like the agent version are usually set when this
-	// returns. On the other hand, we don't _really_ need to wait for this.
-	//
-	// This is mostly here to preserve existing behavior.
-	select {
-	case <-h.ids.IdentifyWait(c):
-	case <-ctx.Done():
-		return fmt.Errorf("identify failed to complete: %w", ctx.Err())
-	}
-
 	log.Debugf("host %s finished dialing %s", h.ID(), p)
 	return nil
 }

p2p/protocol/holepunch/holepunch_test.go

@@ -123,7 +123,8 @@ func TestDirectDialWorks(t *testing.T) {
 	require.Len(t, h1.Network().ConnsToPeer(h2.ID()), 0)
 	require.NoError(t, h1ps.DirectConnect(h2.ID()))
 	require.GreaterOrEqual(t, len(h1.Network().ConnsToPeer(h2.ID())), 1)
-	require.GreaterOrEqual(t, len(h2.Network().ConnsToPeer(h1.ID())), 1)
+	// h1 might finish the handshake first, but h2 should see the connection shortly after
+	require.Eventually(t, func() bool { return len(h2.Network().ConnsToPeer(h1.ID())) > 0 }, time.Second, 25*time.Millisecond)
 	events := tr.getEvents()
 	require.Len(t, events, 1)
 	require.Equal(t, events[0].Type, holepunch.DirectDialEvtT)
@@ -490,6 +491,7 @@ func makeRelayedHosts(t *testing.T, h1opt, h2opt []holepunch.Option, addHolePunc
 		ID:    h2.ID(),
 		Addrs: []ma.Multiaddr{raddr},
 	}))
+	require.Eventually(t, func() bool { return len(h2.Network().ConnsToPeer(h1.ID())) > 0 }, time.Second, 50*time.Millisecond)
 	return
 }
 

p2p/protocol/identify/id_test.go

@@ -473,25 +473,25 @@ func TestUserAgent(t *testing.T) {
 	defer cancel()
 
 	h1, err := libp2p.New(libp2p.UserAgent("foo"), libp2p.ListenAddrStrings("/ip4/127.0.0.1/tcp/0"))
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	defer h1.Close()
 
 	h2, err := libp2p.New(libp2p.UserAgent("bar"), libp2p.ListenAddrStrings("/ip4/127.0.0.1/tcp/0"))
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	defer h2.Close()
 
-	err = h1.Connect(ctx, peer.AddrInfo{ID: h2.ID(), Addrs: h2.Addrs()})
-	if err != nil {
-		t.Fatal(err)
+	sub, err := h1.EventBus().Subscribe(&event.EvtPeerIdentificationCompleted{})
+	require.NoError(t, err)
+	defer sub.Close()
+
+	require.NoError(t, h1.Connect(ctx, peer.AddrInfo{ID: h2.ID(), Addrs: h2.Addrs()}))
+	select {
+	case <-sub.Out():
+	case <-time.After(time.Second):
+		t.Fatal("timeout")
 	}
 	av, err := h1.Peerstore().Get(h2.ID(), "AgentVersion")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	if ver, ok := av.(string); !ok || ver != "bar" {
 		t.Errorf("expected agent version %q, got %q", "bar", av)
 	}