Move resolve test to swarm_dial_test

p2p/host/basic/basic_host.go

@@ -709,13 +709,6 @@ func (h *BasicHost) Connect(ctx context.Context, pi peer.AddrInfo) error {
 		}
 	}
 
-	// TODO remove this
-	// resolved, err := h.resolveAddrs(ctx, h.Peerstore().PeerInfo(pi.ID))
-	// if err != nil {
-	// 	return err
-	// }
-	// h.Peerstore().AddAddrs(pi.ID, resolved, peerstore.TempAddrTTL)
-
 	return h.dialPeer(ctx, pi.ID)
 }
 

p2p/host/basic/basic_host_test.go

@@ -17,14 +17,12 @@ import (
 	"github.com/libp2p/go-libp2p/core/peerstore"
 	"github.com/libp2p/go-libp2p/core/protocol"
 	"github.com/libp2p/go-libp2p/core/record"
-	"github.com/libp2p/go-libp2p/core/test"
 	"github.com/libp2p/go-libp2p/p2p/host/autonat"
 	"github.com/libp2p/go-libp2p/p2p/host/eventbus"
 	swarmt "github.com/libp2p/go-libp2p/p2p/net/swarm/testing"
 	"github.com/libp2p/go-libp2p/p2p/protocol/identify"
 
 	ma "github.com/multiformats/go-multiaddr"
-	madns "github.com/multiformats/go-multiaddr-dns"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -526,111 +524,6 @@ func TestProtoDowngrade(t *testing.T) {
 	assertWait(t, connectedOn, "/testing")
 }
 
-func TestAddrResolution(t *testing.T) {
-	ctx := context.Background()
-
-	p1 := test.RandPeerIDFatal(t)
-	p2 := test.RandPeerIDFatal(t)
-	addr1 := ma.StringCast("/dnsaddr/example.com")
-	addr2 := ma.StringCast("/ip4/192.0.2.1/tcp/123")
-	p2paddr1 := ma.StringCast("/dnsaddr/example.com/p2p/" + p1.Pretty())
-	p2paddr2 := ma.StringCast("/ip4/192.0.2.1/tcp/123/p2p/" + p1.Pretty())
-	p2paddr3 := ma.StringCast("/ip4/192.0.2.1/tcp/123/p2p/" + p2.Pretty())
-
-	backend := &madns.MockResolver{
-		TXT: map[string][]string{"_dnsaddr.example.com": {
-			"dnsaddr=" + p2paddr2.String(), "dnsaddr=" + p2paddr3.String(),
-		}},
-	}
-	resolver, err := madns.NewResolver(madns.WithDefaultResolver(backend))
-	require.NoError(t, err)
-
-	h, err := NewHost(swarmt.GenSwarm(t), &HostOpts{MultiaddrResolver: resolver})
-	require.NoError(t, err)
-	defer h.Close()
-
-	pi, err := peer.AddrInfoFromP2pAddr(p2paddr1)
-	require.NoError(t, err)
-
-	tctx, cancel := context.WithTimeout(ctx, time.Millisecond*100)
-	defer cancel()
-	_ = h.Connect(tctx, *pi)
-
-	addrs := h.Peerstore().Addrs(pi.ID)
-
-	require.Len(t, addrs, 2)
-	require.Contains(t, addrs, addr1)
-	require.Contains(t, addrs, addr2)
-}
-
-func TestAddrResolutionRecursive(t *testing.T) {
-	ctx := context.Background()
-
-	p1, err := test.RandPeerID()
-	if err != nil {
-		t.Error(err)
-	}
-	p2, err := test.RandPeerID()
-	if err != nil {
-		t.Error(err)
-	}
-	addr1 := ma.StringCast("/dnsaddr/example.com")
-	addr2 := ma.StringCast("/ip4/192.0.2.1/tcp/123")
-	p2paddr1 := ma.StringCast("/dnsaddr/example.com/p2p/" + p1.Pretty())
-	p2paddr2 := ma.StringCast("/dnsaddr/example.com/p2p/" + p2.Pretty())
-	p2paddr1i := ma.StringCast("/dnsaddr/foo.example.com/p2p/" + p1.Pretty())
-	p2paddr2i := ma.StringCast("/dnsaddr/bar.example.com/p2p/" + p2.Pretty())
-	p2paddr1f := ma.StringCast("/ip4/192.0.2.1/tcp/123/p2p/" + p1.Pretty())
-
-	backend := &madns.MockResolver{
-		TXT: map[string][]string{
-			"_dnsaddr.example.com": {
-				"dnsaddr=" + p2paddr1i.String(),
-				"dnsaddr=" + p2paddr2i.String(),
-			},
-			"_dnsaddr.foo.example.com": {
-				"dnsaddr=" + p2paddr1f.String(),
-			},
-			"_dnsaddr.bar.example.com": {
-				"dnsaddr=" + p2paddr2i.String(),
-			},
-		},
-	}
-	resolver, err := madns.NewResolver(madns.WithDefaultResolver(backend))
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	h, err := NewHost(swarmt.GenSwarm(t), &HostOpts{MultiaddrResolver: resolver})
-	require.NoError(t, err)
-	defer h.Close()
-
-	pi1, err := peer.AddrInfoFromP2pAddr(p2paddr1)
-	if err != nil {
-		t.Error(err)
-	}
-
-	tctx, cancel := context.WithTimeout(ctx, time.Millisecond*100)
-	defer cancel()
-	_ = h.Connect(tctx, *pi1)
-
-	addrs1 := h.Peerstore().Addrs(pi1.ID)
-	require.Len(t, addrs1, 2)
-	require.Contains(t, addrs1, addr1)
-	require.Contains(t, addrs1, addr2)
-
-	pi2, err := peer.AddrInfoFromP2pAddr(p2paddr2)
-	if err != nil {
-		t.Error(err)
-	}
-
-	_ = h.Connect(tctx, *pi2)
-
-	addrs2 := h.Peerstore().Addrs(pi2.ID)
-	require.Len(t, addrs2, 1)
-	require.Contains(t, addrs2, addr1)
-}
-
 func TestAddrChangeImmediatelyIfAddressNonEmpty(t *testing.T) {
 	ctx := context.Background()
 	taddrs := []ma.Multiaddr{ma.StringCast("/ip4/1.2.3.4/tcp/1234")}

p2p/net/swarm/dial_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/libp2p/go-libp2p/core/peerstore"
 	testutil "github.com/libp2p/go-libp2p/core/test"
 	"github.com/libp2p/go-libp2p/p2p/net/swarm"
-	. "github.com/libp2p/go-libp2p/p2p/net/swarm"
 	swarmt "github.com/libp2p/go-libp2p/p2p/net/swarm/testing"
 
 	"github.com/libp2p/go-libp2p-testing/ci"
@@ -23,7 +22,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func closeSwarms(swarms []*Swarm) {
+func closeSwarms(swarms []*swarm.Swarm) {
 	for _, s := range swarms {
 		s.Close()
 	}
@@ -130,7 +129,7 @@ func TestSimultDials(t *testing.T) {
 	{
 		var wg sync.WaitGroup
 		errs := make(chan error, 20) // 2 connect calls in each of the 10 for-loop iterations
-		connect := func(s *Swarm, dst peer.ID, addr ma.Multiaddr) {
+		connect := func(s *swarm.Swarm, dst peer.ID, addr ma.Multiaddr) {
 			// copy for other peer
 			log.Debugf("TestSimultOpen: connecting: %s --> %s (%s)", s.LocalPeer(), dst, addr)
 			s.Peerstore().AddAddr(dst, addr, peerstore.TempAddrTTL)
@@ -216,11 +215,11 @@ func TestDialWait(t *testing.T) {
 	}
 	duration := time.Since(before)
 
-	if duration < dialTimeout*DialAttempts {
-		t.Error("< dialTimeout * DialAttempts not being respected", duration, dialTimeout*DialAttempts)
+	if duration < dialTimeout*swarm.DialAttempts {
+		t.Error("< dialTimeout * DialAttempts not being respected", duration, dialTimeout*swarm.DialAttempts)
 	}
-	if duration > 2*dialTimeout*DialAttempts {
-		t.Error("> 2*dialTimeout * DialAttempts not being respected", duration, 2*dialTimeout*DialAttempts)
+	if duration > 2*dialTimeout*swarm.DialAttempts {
+		t.Error("> 2*dialTimeout * DialAttempts not being respected", duration, 2*dialTimeout*swarm.DialAttempts)
 	}
 
 	if !s1.Backoff().Backoff(s2p, s2addr) {
@@ -458,11 +457,11 @@ func TestDialBackoffClears(t *testing.T) {
 	require.Error(t, err, "dialing to broken addr worked...")
 	duration := time.Since(before)
 
-	if duration < dialTimeout*DialAttempts {
-		t.Error("< dialTimeout * DialAttempts not being respected", duration, dialTimeout*DialAttempts)
+	if duration < dialTimeout*swarm.DialAttempts {
+		t.Error("< dialTimeout * DialAttempts not being respected", duration, dialTimeout*swarm.DialAttempts)
 	}
-	if duration > 2*dialTimeout*DialAttempts {
-		t.Error("> 2*dialTimeout * DialAttempts not being respected", duration, 2*dialTimeout*DialAttempts)
+	if duration > 2*dialTimeout*swarm.DialAttempts {
+		t.Error("> 2*dialTimeout * DialAttempts not being respected", duration, 2*dialTimeout*swarm.DialAttempts)
 	}
 	require.True(t, s1.Backoff().Backoff(s2.LocalPeer(), s2bad), "s2 should now be on backoff")
 
@@ -502,7 +501,7 @@ func TestDialPeerFailed(t *testing.T) {
 	//   * [/ip4/127.0.0.1/tcp/34881] failed to negotiate security protocol: context deadline exceeded
 	// ...
 
-	dialErr, ok := err.(*DialError)
+	dialErr, ok := err.(*swarm.DialError)
 	if !ok {
 		t.Fatalf("expected *DialError, got %T", err)
 	}
@@ -657,5 +656,5 @@ func TestDialSelf(t *testing.T) {
 	s1 := swarms[0]
 
 	_, err := s1.DialPeer(context.Background(), s1.LocalPeer())
-	require.ErrorIs(t, err, ErrDialToSelf, "expected error from self dial")
+	require.ErrorIs(t, err, swarm.ErrDialToSelf, "expected error from self dial")
 }

p2p/net/swarm/swarm_dial.go

@@ -10,6 +10,7 @@ import (
 	"github.com/libp2p/go-libp2p/core/canonicallog"
 	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/libp2p/go-libp2p/core/peerstore"
 	"github.com/libp2p/go-libp2p/core/transport"
 
 	ma "github.com/multiformats/go-multiaddr"
@@ -331,8 +332,7 @@ func (s *Swarm) addrsForDial(ctx context.Context, p peer.ID) ([]ma.Multiaddr, er
 		return nil, ErrNoGoodAddresses
 	}
 
-	// Do we store the addrs?
-	// h.Peerstore().AddAddrs(pi.ID, goodAddrs, peerstore.TempAddrTTL)
+	s.peers.AddAddrs(p, goodAddrs, peerstore.TempAddrTTL)
 
 	return resolved, nil
 }

p2p/net/swarm/swarm_dial_test.go

@@ -10,10 +10,13 @@ import (
 	"github.com/libp2p/go-libp2p/core/crypto"
 	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/libp2p/go-libp2p/core/peerstore"
 	"github.com/libp2p/go-libp2p/core/test"
 	"github.com/libp2p/go-libp2p/p2p/host/peerstore/pstoremem"
+	"github.com/libp2p/go-libp2p/p2p/transport/tcp"
 	"github.com/libp2p/go-libp2p/p2p/transport/websocket"
 	"github.com/multiformats/go-multiaddr"
+	ma "github.com/multiformats/go-multiaddr"
 	madns "github.com/multiformats/go-multiaddr-dns"
 	"github.com/stretchr/testify/require"
 )
@@ -46,6 +49,7 @@ func TestAddrsForDial(t *testing.T) {
 	require.NoError(t, err)
 	s, err := NewSwarm(id, ps, WithMultiaddrResolver(resolver))
 	require.NoError(t, err)
+	defer s.Close()
 	err = s.AddTransport(tpt)
 	require.NoError(t, err)
 
@@ -59,3 +63,136 @@ func TestAddrsForDial(t *testing.T) {
 
 	require.NotZero(t, len(mas))
 }
+
+func newTestSwarmWithResolver(t *testing.T, resolver *madns.Resolver) *Swarm {
+	priv, _, err := crypto.GenerateEd25519Key(rand.Reader)
+	require.NoError(t, err)
+	id, err := peer.IDFromPrivateKey(priv)
+	require.NoError(t, err)
+	ps, err := pstoremem.NewPeerstore()
+	require.NoError(t, err)
+	ps.AddPubKey(id, priv.GetPublic())
+	ps.AddPrivKey(id, priv)
+	t.Cleanup(func() { ps.Close() })
+	s, err := NewSwarm(id, ps, WithMultiaddrResolver(resolver))
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		s.Close()
+	})
+
+	// Add a tcp transport so that we know we can dial a tcp multiaddr and we don't filter it out.
+	tpt, err := tcp.NewTCPTransport(nil, network.NullResourceManager)
+	require.NoError(t, err)
+	err = s.AddTransport(tpt)
+	require.NoError(t, err)
+
+	return s
+}
+
+func TestAddrResolution(t *testing.T) {
+	ctx := context.Background()
+
+	p1 := test.RandPeerIDFatal(t)
+	p2 := test.RandPeerIDFatal(t)
+	addr1 := ma.StringCast("/dnsaddr/example.com")
+	addr2 := ma.StringCast("/ip4/192.0.2.1/tcp/123")
+
+	p2paddr2 := ma.StringCast("/ip4/192.0.2.1/tcp/123/p2p/" + p1.Pretty())
+	p2paddr3 := ma.StringCast("/ip4/192.0.2.1/tcp/123/p2p/" + p2.Pretty())
+
+	backend := &madns.MockResolver{
+		TXT: map[string][]string{"_dnsaddr.example.com": {
+			"dnsaddr=" + p2paddr2.String(), "dnsaddr=" + p2paddr3.String(),
+		}},
+	}
+	resolver, err := madns.NewResolver(madns.WithDefaultResolver(backend))
+	require.NoError(t, err)
+
+	s := newTestSwarmWithResolver(t, resolver)
+
+	s.peers.AddAddr(p1, addr1, time.Hour)
+
+	tctx, cancel := context.WithTimeout(ctx, time.Millisecond*100)
+	defer cancel()
+	mas, err := s.addrsForDial(tctx, p1)
+	require.NoError(t, err)
+
+	require.Len(t, mas, 1)
+	require.Contains(t, mas, addr2)
+
+	addrs := s.peers.Addrs(p1)
+	require.Len(t, addrs, 2)
+	require.Contains(t, addrs, addr1)
+	require.Contains(t, addrs, addr2)
+}
+
+func TestAddrResolutionRecursive(t *testing.T) {
+	ctx := context.Background()
+
+	p1, err := test.RandPeerID()
+	if err != nil {
+		t.Error(err)
+	}
+	p2, err := test.RandPeerID()
+	if err != nil {
+		t.Error(err)
+	}
+	addr1 := ma.StringCast("/dnsaddr/example.com")
+	addr2 := ma.StringCast("/ip4/192.0.2.1/tcp/123")
+	p2paddr1 := ma.StringCast("/dnsaddr/example.com/p2p/" + p1.Pretty())
+	p2paddr2 := ma.StringCast("/dnsaddr/example.com/p2p/" + p2.Pretty())
+	p2paddr1i := ma.StringCast("/dnsaddr/foo.example.com/p2p/" + p1.Pretty())
+	p2paddr2i := ma.StringCast("/dnsaddr/bar.example.com/p2p/" + p2.Pretty())
+	p2paddr1f := ma.StringCast("/ip4/192.0.2.1/tcp/123/p2p/" + p1.Pretty())
+
+	backend := &madns.MockResolver{
+		TXT: map[string][]string{
+			"_dnsaddr.example.com": {
+				"dnsaddr=" + p2paddr1i.String(),
+				"dnsaddr=" + p2paddr2i.String(),
+			},
+			"_dnsaddr.foo.example.com": {
+				"dnsaddr=" + p2paddr1f.String(),
+			},
+			"_dnsaddr.bar.example.com": {
+				"dnsaddr=" + p2paddr2i.String(),
+			},
+		},
+	}
+	resolver, err := madns.NewResolver(madns.WithDefaultResolver(backend))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// h, err := NewHost(swarmt.GenSwarm(t), &HostOpts{MultiaddrResolver: resolver})
+	// require.NoError(t, err)
+	// defer h.Close()
+
+	s := newTestSwarmWithResolver(t, resolver)
+
+	pi1, err := peer.AddrInfoFromP2pAddr(p2paddr1)
+	require.NoError(t, err)
+
+	tctx, cancel := context.WithTimeout(ctx, time.Millisecond*100)
+	defer cancel()
+	s.Peerstore().AddAddrs(pi1.ID, pi1.Addrs, peerstore.TempAddrTTL)
+	_, err = s.addrsForDial(tctx, p1)
+	require.NoError(t, err)
+
+	addrs1 := s.Peerstore().Addrs(pi1.ID)
+	require.Len(t, addrs1, 2)
+	require.Contains(t, addrs1, addr1)
+	require.Contains(t, addrs1, addr2)
+
+	pi2, err := peer.AddrInfoFromP2pAddr(p2paddr2)
+	require.NoError(t, err)
+
+	s.Peerstore().AddAddrs(pi2.ID, pi2.Addrs, peerstore.TempAddrTTL)
+	_, err = s.addrsForDial(tctx, p2)
+	// This never resolves to a good address
+	require.Equal(t, ErrNoGoodAddresses, err)
+
+	addrs2 := s.Peerstore().Addrs(pi2.ID)
+	require.Len(t, addrs2, 1)
+	require.Contains(t, addrs2, addr1)
+}