Repository: libp2p/go-libp2p
Pull request #770 (2020-01-14 19:45:16)

identify: emit events; filter out loopback addresses
_**Merge strategy: squash and merge.**_

With this PR, go-libp2p:

1. starts emitting identification complete/failed events when identify runs on a peer.
2. filters loopback addresses from identify.

I'm not 100% sold on the latter change, and I'm seeking validation/input. See original discussion on #657; I only merged that patch to stabilize, but now that it's percolating up to master, it's worthy of reconsideration.


Patch:
diff --git a/go.mod b/go.mod
index 7cb953c7..a215127b 100644
--- a/go.mod
+++ b/go.mod
@@ -13,7 +13,7 @@ require (
 	github.com/libp2p/go-libp2p-autonat v0.1.1
 	github.com/libp2p/go-libp2p-blankhost v0.1.4
 	github.com/libp2p/go-libp2p-circuit v0.1.4
-	github.com/libp2p/go-libp2p-core v0.3.0
+	github.com/libp2p/go-libp2p-core v0.3.1
 	github.com/libp2p/go-libp2p-discovery v0.2.0
 	github.com/libp2p/go-libp2p-loggables v0.1.0
 	github.com/libp2p/go-libp2p-mplex v0.2.1
diff --git a/go.sum b/go.sum
index 09d410c9..3cfebf64 100644
--- a/go.sum
+++ b/go.sum
@@ -153,10 +153,15 @@ github.com/libp2p/go-libp2p-core v0.2.0 h1:ycFtuNwtZBAJSxzaHbyv6NjG3Yj5Nmra1csHa
 github.com/libp2p/go-libp2p-core v0.2.0/go.mod h1:X0eyB0Gy93v0DZtSYbEM7RnMChm9Uv3j7yRXjO77xSI=
 github.com/libp2p/go-libp2p-core v0.2.2 h1:Sv1ggdoMx9c7v7FOFkR7agraHCnAgqYsXrU1ARSRUMs=
 github.com/libp2p/go-libp2p-core v0.2.2/go.mod h1:8fcwTbsG2B+lTgRJ1ICZtiM5GWCWZVoVrLaDRvIRng0=
-github.com/libp2p/go-libp2p-core v0.2.4 h1:Et6ykkTwI6PU44tr8qUF9k43vP0aduMNniShAbUJJw8=
 github.com/libp2p/go-libp2p-core v0.2.4/go.mod h1:STh4fdfa5vDYr0/SzYYeqnt+E6KfEV5VxfIrm0bcI0g=
 github.com/libp2p/go-libp2p-core v0.3.0 h1:F7PqduvrztDtFsAa/bcheQ3azmNo+Nq7m8hQY5GiUW8=
 github.com/libp2p/go-libp2p-core v0.3.0/go.mod h1:ACp3DmS3/N64c2jDzcV429ukDpicbL6+TrrxANBjPGw=
+github.com/libp2p/go-libp2p-core v0.3.1-0.20191230184106-204a57d1afe1 h1:3WfZnKXp/5B9EIwbV9ts0W681Q+K1/ZQdlPH2dBXAHg=
+github.com/libp2p/go-libp2p-core v0.3.1-0.20191230184106-204a57d1afe1/go.mod h1:LRlbRHNBEfe8yypXy9+tfroNCfmGmYGjFGe5mU7bmWY=
+github.com/libp2p/go-libp2p-core v0.3.1 h1:hEnSDjScfjYvPHoTgZhC4F62M8K1x1Oco/BY0RZ1N3s=
+github.com/libp2p/go-libp2p-core v0.3.1/go.mod h1:thvWy0hvaSBhnVBaW37BvzgVV68OUhgJJLAa6almrII=
+github.com/libp2p/go-libp2p-crypto v0.1.0 h1:k9MFy+o2zGDNGsaoZl0MA3iZ75qXxr9OOoAZF+sD5OQ=
+github.com/libp2p/go-libp2p-crypto v0.1.0 h1:k9MFy+o2zGDNGsaoZl0MA3iZ75qXxr9OOoAZF+sD5OQ=
 github.com/libp2p/go-libp2p-crypto v0.1.0/go.mod h1:sPUokVISZiy+nNuTTH/TY+leRSxnFj/2GLjtOTW90hI=
 github.com/libp2p/go-libp2p-discovery v0.2.0 h1:1p3YSOq7VsgaL+xVHPi8XAmtGyas6D2J6rWBEfz/aiY=
 github.com/libp2p/go-libp2p-discovery v0.2.0/go.mod h1:s4VGaxYMbw4+4+tsoQTqh7wfxg97AEdo4GYBt6BadWg=
@@ -249,7 +254,6 @@ github.com/minio/sha256-simd v0.0.0-20190131020904-2d45a736cd16/go.mod h1:2FMWW+
 github.com/minio/sha256-simd v0.0.0-20190328051042-05b4dd3047e5/go.mod h1:2FMWW+8GMoPweT6+pI63m9YE3Lmw4J71hV56Chs1E/U=
 github.com/minio/sha256-simd v0.1.0 h1:U41/2erhAKcmSI14xh/ZTUdBPOzDOIfS93ibzUSl8KM=
 github.com/minio/sha256-simd v0.1.0/go.mod h1:2FMWW+8GMoPweT6+pI63m9YE3Lmw4J71hV56Chs1E/U=
-github.com/minio/sha256-simd v0.1.1-0.20190913151208-6de447530771 h1:MHkK1uRtFbVqvAgvWxafZe54+5uBxLluGylDiKgdhwo=
 github.com/minio/sha256-simd v0.1.1-0.20190913151208-6de447530771/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=
 github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKUJU=
 github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=
@@ -337,11 +341,13 @@ github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkU
 github.com/spf13/cobra v0.0.5/go.mod h1:3K3wKZymM7VvHMDS9+Akkh4K60UwM26emMESw8tLCHU=
 github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo=
 github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
+github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
 github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s=
 github.com/src-d/envconfig v1.0.0/go.mod h1:Q9YQZ7BKITldTBnoxsE5gOeB5y66RyPXeue/R4aaNBc=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
+github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
@@ -422,7 +428,6 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm
 golang.org/x/tools v0.0.0-20181030221726-6c7e314b6563/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20181130052023-1c3d964395ce/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
-golang.org/x/tools v0.0.0-20190226205152-f727befe758c h1:vamGzbGri8IKo20MQncCuljcQ5uAO6kaCeawQPVblAI=
 golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=
 golang.org/x/tools v0.0.0-20190311212946-11955173bddd h1:/e+gpKk9r3dJobndpTytxS2gOy6m5uvpg+ISQoEcusQ=
 golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
diff --git a/libp2p.go b/libp2p.go
index 32fddbdb..dbdd09cf 100644
--- a/libp2p.go
+++ b/libp2p.go
@@ -46,7 +46,7 @@ func ChainOptions(opts ...Option) Option {
 // - If no security transport is provided, the host uses the go-libp2p's secio
 // encrypted transport to encrypt all traffic;
 //
-// - If no peer identity is provided, it generates a random RSA 2048 key-par
+// - If no peer identity is provided, it generates a random RSA 2048 key-pair
 // and derives a new identity from it;
 //
 // - If no peerstore is provided, the host is initialized with an empty
diff --git a/p2p/protocol/identify/id.go b/p2p/protocol/identify/id.go
index 35e2e57a..a0e1ae25 100644
--- a/p2p/protocol/identify/id.go
+++ b/p2p/protocol/identify/id.go
@@ -23,6 +23,7 @@ import (
 	logging "github.com/ipfs/go-log"
 
 	ma "github.com/multiformats/go-multiaddr"
+	manet "github.com/multiformats/go-multiaddr-net"
 	msmux "github.com/multiformats/go-multistream"
 )
 
@@ -85,7 +86,9 @@ type IDService struct {
 
 	subscription event.Subscription
 	emitters     struct {
-		evtPeerProtocolsUpdated event.Emitter
+		evtPeerProtocolsUpdated        event.Emitter
+		evtPeerIdentificationCompleted event.Emitter
+		evtPeerIdentificationFailed    event.Emitter
 	}
 }
 
@@ -124,6 +127,14 @@ func NewIDService(ctx context.Context, h host.Host, opts ...Option) *IDService {
 	if err != nil {
 		log.Warningf("identify service not emitting peer protocol updates; err: %s", err)
 	}
+	s.emitters.evtPeerIdentificationCompleted, err = h.EventBus().Emitter(&event.EvtPeerIdentificationCompleted{})
+	if err != nil {
+		log.Warningf("identify service not emitting identification completed events; err: %s", err)
+	}
+	s.emitters.evtPeerIdentificationFailed, err = h.EventBus().Emitter(&event.EvtPeerIdentificationFailed{})
+	if err != nil {
+		log.Warningf("identify service not emitting identification failed events; err: %s", err)
+	}
 
 	h.SetStreamHandler(ID, s.requestHandler)
 	h.SetStreamHandler(IDPush, s.pushHandler)
@@ -164,6 +175,11 @@ func (ids *IDService) ObservedAddrsFor(local ma.Multiaddr) []ma.Multiaddr {
 }
 
 func (ids *IDService) IdentifyConn(c network.Conn) {
+	var (
+		s   network.Stream
+		err error
+	)
+
 	ids.currmu.Lock()
 	if wait, found := ids.currid[c]; found {
 		ids.currmu.Unlock()
@@ -180,9 +196,16 @@ func (ids *IDService) IdentifyConn(c network.Conn) {
 		ids.currmu.Lock()
 		delete(ids.currid, c)
 		ids.currmu.Unlock()
+
+		// emit the appropriate event.
+		if p := c.RemotePeer(); err == nil {
+			ids.emitters.evtPeerIdentificationCompleted.Emit(event.EvtPeerIdentificationCompleted{Peer: p})
+		} else {
+			ids.emitters.evtPeerIdentificationFailed.Emit(event.EvtPeerIdentificationFailed{Peer: p, Reason: err})
+		}
 	}()
 
-	s, err := c.NewStream()
+	s, err = c.NewStream()
 	if err != nil {
 		log.Debugf("error opening initial stream for %s: %s", ID, err)
 		log.Event(context.TODO(), "IdentifyOpenFailed", c.RemotePeer())
@@ -193,7 +216,7 @@ func (ids *IDService) IdentifyConn(c network.Conn) {
 	s.SetProtocol(ID)
 
 	// ok give the response to our handler.
-	if err := msmux.SelectProtoOrFail(ID, s); err != nil {
+	if err = msmux.SelectProtoOrFail(ID, s); err != nil {
 		log.Event(context.TODO(), "IdentifyOpenFailed", c.RemotePeer(), logging.Metadata{"error": err})
 		s.Reset()
 		return
@@ -310,9 +333,14 @@ func (ids *IDService) populateMessage(mes *pb.Identify, c network.Conn) {
 
 	// set listen addrs, get our latest addrs from Host.
 	laddrs := ids.Host.Addrs()
-	mes.ListenAddrs = make([][]byte, len(laddrs))
-	for i, addr := range laddrs {
-		mes.ListenAddrs[i] = addr.Bytes()
+	// Note: LocalMultiaddr is sometimes 0.0.0.0
+	viaLoopback := manet.IsIPLoopback(c.LocalMultiaddr()) || manet.IsIPLoopback(c.RemoteMultiaddr())
+	mes.ListenAddrs = make([][]byte, 0, len(laddrs))
+	for _, addr := range laddrs {
+		if !viaLoopback && manet.IsIPLoopback(addr) {
+			continue
+		}
+		mes.ListenAddrs = append(mes.ListenAddrs, addr.Bytes())
 	}
 	log.Debugf("%s sent listen addrs to %s: %s", c.LocalPeer(), c.RemotePeer(), laddrs)
 


Test patch:
diff --git a/p2p/protocol/identify/id_test.go b/p2p/protocol/identify/id_test.go
index 5c422a90..234dbe48 100644
--- a/p2p/protocol/identify/id_test.go
+++ b/p2p/protocol/identify/id_test.go
@@ -17,11 +17,14 @@ import (
 	"github.com/libp2p/go-libp2p-core/peer"
 	"github.com/libp2p/go-libp2p-core/peerstore"
 	"github.com/libp2p/go-libp2p-core/protocol"
+	coretest "github.com/libp2p/go-libp2p-core/test"
 
 	blhost "github.com/libp2p/go-libp2p-blankhost"
 	swarmt "github.com/libp2p/go-libp2p-swarm/testing"
 	"github.com/libp2p/go-libp2p/p2p/protocol/identify"
 
+	"github.com/libp2p/go-libp2p-peerstore/pstoremem"
+	mocknet "github.com/libp2p/go-libp2p/p2p/net/mock"
 	ma "github.com/multiformats/go-multiaddr"
 )
 
@@ -56,6 +59,11 @@ func subtestIDService(t *testing.T) {
 		t.Fatal("should have a conn here")
 	}
 
+	sub, err := ids1.Host.EventBus().Subscribe(new(event.EvtPeerIdentificationCompleted), eventbus.BufSize(16))
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	ids1.IdentifyConn(h1t2c[0])
 
 	// the IDService should be opened automatically, by the network.
@@ -102,6 +110,13 @@ func subtestIDService(t *testing.T) {
 	// Forget the rest.
 	testKnowsAddrs(t, h1, h2p, []ma.Multiaddr{})
 	testKnowsAddrs(t, h2, h1p, []ma.Multiaddr{})
+
+	// test that we received the "identify completed" event.
+	select {
+	case <-sub.Out():
+	case <-time.After(5 * time.Second):
+		t.Fatalf("expected EvtPeerIdentificationCompleted event within 5 seconds; none received")
+	}
 }
 
 func testKnowsAddrs(t *testing.T, h host.Host, p peer.ID, expected []ma.Multiaddr) {
@@ -191,6 +206,80 @@ func TestProtoMatching(t *testing.T) {
 	}
 }
 
+func TestLocalhostAddrFiltering(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	mn := mocknet.New(ctx)
+	id1 := coretest.RandPeerIDFatal(t)
+	ps1 := pstoremem.NewPeerstore()
+	p1addr1, _ := ma.NewMultiaddr("/ip4/1.2.3.4/tcp/1234")
+	p1addr2, _ := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/2345")
+	ps1.AddAddrs(id1, []ma.Multiaddr{p1addr1, p1addr2}, peerstore.PermanentAddrTTL)
+	p1, err := mn.AddPeerWithPeerstore(id1, ps1)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	id2 := coretest.RandPeerIDFatal(t)
+	ps2 := pstoremem.NewPeerstore()
+	p2addr1, _ := ma.NewMultiaddr("/ip4/1.2.3.5/tcp/1234")
+	p2addr2, _ := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/3456")
+	p2addrs := []ma.Multiaddr{p2addr1, p2addr2}
+	ps2.AddAddrs(id2, p2addrs, peerstore.PermanentAddrTTL)
+	p2, err := mn.AddPeerWithPeerstore(id2, ps2)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	id3 := coretest.RandPeerIDFatal(t)
+	ps3 := pstoremem.NewPeerstore()
+	p3addr1, _ := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/4567")
+	ps3.AddAddrs(id3, []ma.Multiaddr{p3addr1}, peerstore.PermanentAddrTTL)
+	p3, err := mn.AddPeerWithPeerstore(id3, ps3)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = mn.LinkAll()
+	if err != nil {
+		t.Fatal(err)
+	}
+	p1.Connect(ctx, peer.AddrInfo{
+		ID:    id2,
+		Addrs: p2addrs[0:1],
+	})
+	p3.Connect(ctx, peer.AddrInfo{
+		ID:    id2,
+		Addrs: p2addrs[1:],
+	})
+
+	_ = identify.NewIDService(ctx, p1)
+	ids2 := identify.NewIDService(ctx, p2)
+	ids3 := identify.NewIDService(ctx, p3)
+
+	conns := p2.Network().ConnsToPeer(id1)
+	if len(conns) == 0 {
+		t.Fatal("no conns")
+	}
+	conn := conns[0]
+	ids2.IdentifyConn(conn)
+	addrs := p2.Peerstore().Addrs(id1)
+	if len(addrs) != 1 {
+		t.Fatalf("expected one addr, found %s", addrs)
+	}
+
+	conns = p3.Network().ConnsToPeer(id2)
+	if len(conns) == 0 {
+		t.Fatal("no conns")
+	}
+	conn = conns[0]
+	ids3.IdentifyConn(conn)
+	addrs = p3.Peerstore().Addrs(id2)
+	if len(addrs) != 2 {
+		t.Fatalf("expected 2 addrs for %s, found %d: %s", id2, len(addrs), addrs)
+	}
+}
+
 func TestIdentifyDeltaOnProtocolChange(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()