address review comments

p2p/host/autonat/autonat.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	host "github.com/libp2p/go-libp2p-host"
+	inet "github.com/libp2p/go-libp2p-net"
 	peer "github.com/libp2p/go-libp2p-peer"
 	ma "github.com/multiformats/go-multiaddr"
 )
@@ -29,8 +30,7 @@ const (
 var (
 	AutoNATBootDelay       = 15 * time.Second
 	AutoNATRefreshInterval = 15 * time.Minute
-
-	AutoNATRequestTimeout = 60 * time.Second
+	AutoNATRequestTimeout  = 60 * time.Second
 )
 
 // AutoNAT is the interface for ambient NAT autodiscovery
@@ -86,7 +86,12 @@ func (as *AmbientAutoNAT) PublicAddr() (ma.Multiaddr, error) {
 func (as *AmbientAutoNAT) background() {
 	// wait a bit for the node to come online and establish some connections
 	// before starting autodetection
-	time.Sleep(AutoNATBootDelay)
+	select {
+	case <-time.After(AutoNATBootDelay):
+	case <-as.ctx.Done():
+		return
+	}
+
 	for {
 		as.autodetect()
 		select {
@@ -109,7 +114,7 @@ func (as *AmbientAutoNAT) autodetect() {
 
 	for _, p := range peers {
 		ctx, cancel := context.WithTimeout(as.ctx, AutoNATRequestTimeout)
-		a, err := cli.Dial(ctx, p)
+		a, err := cli.DialBack(ctx, p)
 		cancel()
 
 		switch {
@@ -148,7 +153,7 @@ func (as *AmbientAutoNAT) getPeers() []peer.ID {
 
 	peers := make([]peer.ID, 0, len(as.peers))
 	for p := range as.peers {
-		if len(as.host.Network().ConnsToPeer(p)) > 0 {
+		if as.host.Network().Connectedness(p) == inet.Connected {
 			peers = append(peers, p)
 		}
 	}

p2p/host/autonat/client.go

@@ -16,8 +16,9 @@ import (
 
 // AutoNATClient is a stateless client interface to AutoNAT peers
 type AutoNATClient interface {
-	// Dial requests from a peer providing AutoNAT services to test dial back
-	Dial(ctx context.Context, p peer.ID) (ma.Multiaddr, error)
+	// DialBack requests from a peer providing AutoNAT services to test dial back
+	// and report the address on a successful connection.
+	DialBack(ctx context.Context, p peer.ID) (ma.Multiaddr, error)
 }
 
 // AutoNATError is the class of errors signalled by AutoNAT services
@@ -35,7 +36,7 @@ type client struct {
 	h host.Host
 }
 
-func (c *client) Dial(ctx context.Context, p peer.ID) (ma.Multiaddr, error) {
+func (c *client) DialBack(ctx context.Context, p peer.ID) (ma.Multiaddr, error) {
 	s, err := c.h.NewStream(ctx, p, AutoNATProto)
 	if err != nil {
 		return nil, err

p2p/host/autonat/svc.go

@@ -166,7 +166,7 @@ func (as *AutoNATService) doDial(pi pstore.PeerInfo) *pb.Message_DialResponse {
 	}
 
 	ra := conns[0].RemoteMultiaddr()
-	conns[0].Close()
+	as.dialer.Network().ClosePeer(pi.ID)
 	return newDialResponseOK(ra)
 }
 

p2p/host/autonat/svc_test.go

@@ -55,7 +55,7 @@ func TestAutoNATServiceDialError(t *testing.T) {
 	hc, ac := makeAutoNATClient(ctx, t)
 	connect(t, hs, hc)
 
-	_, err := ac.Dial(ctx, hs.ID())
+	_, err := ac.DialBack(ctx, hs.ID())
 	if err == nil {
 		t.Fatal("Dial back succeeded unexpectedly!")
 	}
@@ -78,7 +78,7 @@ func TestAutoNATServiceDialSuccess(t *testing.T) {
 	hc, ac := makeAutoNATClient(ctx, t)
 	connect(t, hs, hc)
 
-	_, err := ac.Dial(ctx, hs.ID())
+	_, err := ac.DialBack(ctx, hs.ID())
 	if err != nil {
 		t.Fatalf("Dial back failed: %s", err.Error())
 	}
@@ -101,12 +101,12 @@ func TestAutoNATServiceDialRateLimiter(t *testing.T) {
 	hc, ac := makeAutoNATClient(ctx, t)
 	connect(t, hs, hc)
 
-	_, err := ac.Dial(ctx, hs.ID())
+	_, err := ac.DialBack(ctx, hs.ID())
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	_, err = ac.Dial(ctx, hs.ID())
+	_, err = ac.DialBack(ctx, hs.ID())
 	if err == nil {
 		t.Fatal("Dial back succeeded unexpectedly!")
 	}
@@ -117,7 +117,7 @@ func TestAutoNATServiceDialRateLimiter(t *testing.T) {
 
 	time.Sleep(2 * time.Second)
 
-	_, err = ac.Dial(ctx, hs.ID())
+	_, err = ac.DialBack(ctx, hs.ID())
 	if err != nil {
 		t.Fatal(err)
 	}