diff --git a/network/p2pNetwork.go b/network/p2pNetwork.go index cc37961d51..c2656d2b41 100644 --- a/network/p2pNetwork.go +++ b/network/p2pNetwork.go @@ -35,6 +35,7 @@ import ( pubsub "github.com/libp2p/go-libp2p-pubsub" "github.com/libp2p/go-libp2p/core/network" "github.com/libp2p/go-libp2p/core/peer" + manet "github.com/multiformats/go-multiaddr/net" ) // P2PNetwork implements the GossipNode interface @@ -182,19 +183,29 @@ func (n *P2PNetwork) GetGenesisID() string { // Address returns a string and whether that is a 'final' address or guessed. func (n *P2PNetwork) Address() (string, bool) { addrInfo := n.service.AddrInfo() + if len(addrInfo.Addrs) == 0 { + return "", false + } addrs, err := peer.AddrInfoToP2pAddrs(&addrInfo) if err != nil { n.log.Warnf("Failed to generate valid multiaddr: %v", err) return "", false } - if len(addrs) == 0 { - return "", false + // loop through and see if we have a non loopback address available + for _, addr := range addrs { + if !manet.IsIPLoopback(addr) && !manet.IsIPUnspecified(addr) { + return addr.String(), true + + } } - if len(addrs) > 1 { - n.log.Infof("Multiple addresses found, using first one from %v", addrs) + // We don't have a non loopback address, so just return the first one if it contains an ip4 address or port + addr := addrs[0].String() + if strings.Contains(addr, "/ip4/") && strings.Contains(addr, "/tcp/") { + return addr, true + } + return "", false - return addrs[0].String(), true } // Broadcast sends a message. diff --git a/network/p2pNetwork_test.go b/network/p2pNetwork_test.go index c3c0522a7e..2768deae03 100644 --- a/network/p2pNetwork_test.go +++ b/network/p2pNetwork_test.go @@ -28,7 +28,12 @@ import ( "github.com/algorand/go-algorand/network/p2p" "github.com/algorand/go-algorand/protocol" "github.com/algorand/go-algorand/test/partitiontest" + + pubsub "github.com/libp2p/go-libp2p-pubsub" + "github.com/libp2p/go-libp2p/core/network" + "github.com/libp2p/go-libp2p/core/peer" peerstore "github.com/libp2p/go-libp2p/core/peer" + ma "github.com/multiformats/go-multiaddr" "github.com/stretchr/testify/require" ) @@ -175,3 +180,125 @@ func TestP2PSubmitWS(t *testing.T) { 50*time.Millisecond, ) } + +type mockService struct { + id peer.ID + addrs []ma.Multiaddr + peers map[peer.ID]peer.AddrInfo +} + +func (s *mockService) Close() error { + return nil +} + +func (s *mockService) ID() peer.ID { + return s.id +} + +func (s *mockService) AddrInfo() peer.AddrInfo { + return peer.AddrInfo{ + ID: s.id, + Addrs: s.addrs, + } +} + +func (s *mockService) DialNode(ctx context.Context, peer *peer.AddrInfo) error { + s.peers[peer.ID] = *peer + return nil +} + +func (s *mockService) DialPeersUntilTargetCount(targetConnCount int) { +} + +func (s *mockService) ClosePeer(peer peer.ID) error { + if _, ok := s.peers[peer]; ok { + delete(s.peers, peer) + } + return nil +} + +func (s *mockService) Conns() []network.Conn { + return nil +} + +func (s *mockService) ListPeersForTopic(topic string) []peer.ID { + return nil +} + +func (s *mockService) Subscribe(topic string, val pubsub.ValidatorEx) (*pubsub.Subscription, error) { + return nil, nil +} +func (s *mockService) Publish(ctx context.Context, topic string, data []byte) error { + return nil +} + +func (s *mockService) setAddrs(addrs []ma.Multiaddr) { + s.addrs = addrs +} + +func makeMockService(id peer.ID, addrs []ma.Multiaddr) *mockService { + return &mockService{ + id: id, + addrs: addrs, + } +} + +func TestP2PNetworkAddress(t *testing.T) { + partitiontest.PartitionTest(t) + + cfg := config.GetDefaultLocal() + log := logging.TestingLog(t) + netA, err := NewP2PNetwork(log, cfg, "", nil, genesisID, config.Devtestnet) + defer netA.Stop() + require.NoError(t, err) + addrInfo := netA.service.AddrInfo() + // close the real service since we will substitute a mock one + netA.service.Close() + + // define some multiaddrs we will use in the test + loopbackAddr, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/1234") + require.NoError(t, err) + unspecifiedAddr, err := ma.NewMultiaddr("/ip4/0.0.0.0/tcp/0") + require.NoError(t, err) + publicAddr, err := ma.NewMultiaddr("/ip4/12.86.192.5/tcp/5678") + require.NoError(t, err) + publicAddr2, err := ma.NewMultiaddr("/ip4/23.97.191.6/tcp/1564") + require.NoError(t, err) + + // first two are invalid so third one should be returned as the first public address + addrsA := []ma.Multiaddr{ + loopbackAddr, + unspecifiedAddr, + publicAddr, + publicAddr2, + } + mockService := makeMockService(addrInfo.ID, addrsA) + netA.service = mockService + + retAddr, ok := netA.Address() + require.True(t, ok) + // using Contains since the return of Address also includes the public peerID + require.Contains(t, retAddr, publicAddr.String()) + + // don't have a public address so return the first one + addrsB := []ma.Multiaddr{ + loopbackAddr, + unspecifiedAddr, + } + mockService.addrs = addrsB + retAddr, ok = netA.Address() + require.True(t, ok) + require.Contains(t, retAddr, loopbackAddr.String()) + + // confirm that we don't return an address if none is supplied + mockService.addrs = nil + retAddr, ok = netA.Address() + require.False(t, ok) + require.Empty(t, retAddr) + + mockService.addrs = addrsA // these are still valid addresses + mockService.id = "invalid peer ID" // this won't parse and encode properly + retAddr, ok = netA.Address() + require.False(t, ok) + require.Empty(t, retAddr) +}