diff --git a/p2p/host/basic/basic_host.go b/p2p/host/basic/basic_host.go index 50ab8dfb08..d2098ad9da 100644 --- a/p2p/host/basic/basic_host.go +++ b/p2p/host/basic/basic_host.go @@ -737,22 +737,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 } diff --git a/p2p/protocol/holepunch/holepunch_test.go b/p2p/protocol/holepunch/holepunch_test.go index 832603a2b8..d4e062efb4 100644 --- a/p2p/protocol/holepunch/holepunch_test.go +++ b/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 } diff --git a/p2p/protocol/identify/id_test.go b/p2p/protocol/identify/id_test.go index 61c7d87acc..ce2b59ae77 100644 --- a/p2p/protocol/identify/id_test.go +++ b/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) } diff --git a/p2p/test/quic/quic_test.go b/p2p/test/quic/quic_test.go index fe52119b89..c88bb581db 100644 --- a/p2p/test/quic/quic_test.go +++ b/p2p/test/quic/quic_test.go @@ -61,6 +61,7 @@ func TestQUICAndWebTransport(t *testing.T) { ) require.NoError(t, err) require.NoError(t, h2.Connect(ctx, peer.AddrInfo{ID: h1.ID(), Addrs: h1.Addrs()})) + require.Eventually(t, func() bool { return len(h1.Network().ConnsToPeer(h2.ID())) > 0 }, time.Second, 25*time.Millisecond) for _, conns := range [][]network.Conn{h2.Network().ConnsToPeer(h1.ID()), h1.Network().ConnsToPeer(h2.ID())} { require.Len(t, conns, 1) if _, err := conns[0].LocalMultiaddr().ValueForProtocol(ma.P_WEBTRANSPORT); err == nil { @@ -78,6 +79,7 @@ func TestQUICAndWebTransport(t *testing.T) { ) require.NoError(t, err) require.NoError(t, h3.Connect(ctx, peer.AddrInfo{ID: h1.ID(), Addrs: h1.Addrs()})) + require.Eventually(t, func() bool { return len(h1.Network().ConnsToPeer(h3.ID())) > 0 }, time.Second, 25*time.Millisecond) for _, conns := range [][]network.Conn{h3.Network().ConnsToPeer(h1.ID()), h1.Network().ConnsToPeer(h3.ID())} { require.Len(t, conns, 1) if _, err := conns[0].LocalMultiaddr().ValueForProtocol(ma.P_WEBTRANSPORT); err != nil {