fix: address pull review comments

Author: janos

pkg/p2p/libp2p/internal/handshake/handshake.go

@@ -5,6 +5,7 @@
 package handshake
 
 import (
+	"cmp"
 	"context"
 	"errors"
 	"fmt"
@@ -200,13 +201,7 @@ func (s *Service) Handshake(ctx context.Context, stream p2p.Stream, peerMultiadd
 
 	// sort to remove potential duplicates
 	slices.SortFunc(advertisableUnderlays, func(a, b ma.Multiaddr) int {
-		if a.Equal(b) {
-			return 0
-		}
-		if a.String() < b.String() {
-			return -1
-		}
-		return 1
+		return cmp.Compare(a.String(), b.String())
 	})
 	// remove duplicates
 	advertisableUnderlays = slices.CompactFunc(advertisableUnderlays, func(a, b ma.Multiaddr) bool {
@@ -304,13 +299,7 @@ func (s *Service) Handle(ctx context.Context, stream p2p.Stream, peerMultiaddrs
 
 	// sort to remove potential duplicates
 	slices.SortFunc(advertisableUnderlays, func(a, b ma.Multiaddr) int {
-		if a.Equal(b) {
-			return 0
-		}
-		if a.String() < b.String() {
-			return -1
-		}
-		return 1
+		return cmp.Compare(a.String(), b.String())
 	})
 	// remove duplicates
 	advertisableUnderlays = slices.CompactFunc(advertisableUnderlays, func(a, b ma.Multiaddr) bool {

pkg/p2p/libp2p/libp2p.go

@@ -413,10 +413,7 @@ func (s *Service) handleIncoming(stream network.Stream) {
 	peerID := stream.Conn().RemotePeer()
 	handshakeStream := newStream(stream, s.metrics)
 
-	waitPeersCtx, cancel := context.WithTimeout(s.ctx, peerstoreWaitAddrsTimeout)
-	defer cancel()
-
-	peerMultiaddrs, err := buildFullMAs(waitPeerAddrs(waitPeersCtx, s.host.Peerstore(), peerID), peerID)
+	peerMultiaddrs, err := s.peerMultiaddrs(s.ctx, stream.Conn(), peerID)
 	if err != nil {
 		s.logger.Debug("stream handler: handshake: build remote multiaddrs", "peer_id", peerID, "error", err)
 		s.logger.Error(nil, "stream handler: handshake: build remote multiaddrs", "peer_id", peerID)
@@ -425,18 +422,6 @@ func (s *Service) handleIncoming(stream network.Stream) {
 		return
 	}
 
-	if len(peerMultiaddrs) == 0 {
-		fullRemoteAddress, err := buildFullMA(stream.Conn().RemoteMultiaddr(), peerID)
-		if err != nil {
-			s.logger.Debug("stream handler: handshake: build full remote peer multi address", "peer_id", peerID, "error", err)
-			s.logger.Error(nil, "stream handler: handshake: build full remote peer multi address", "peer_id", peerID)
-			_ = handshakeStream.Reset()
-			_ = s.host.Network().ClosePeer(peerID)
-			return
-		}
-		peerMultiaddrs = append(peerMultiaddrs, fullRemoteAddress)
-	}
-
 	i, err := s.handshakeService.Handle(
 		s.ctx,
 		handshakeStream,
@@ -829,24 +814,13 @@ func (s *Service) Connect(ctx context.Context, addrs []ma.Multiaddr) (address *b
 
 	handshakeStream := newStream(stream, s.metrics)
 
-	waitPeersCtx, cancel := context.WithTimeout(ctx, peerstoreWaitAddrsTimeout)
-	defer cancel()
-
-	peerMultiaddrs, err := buildFullMAs(waitPeerAddrs(waitPeersCtx, s.host.Peerstore(), peerID), peerID)
+	peerMultiaddrs, err := s.peerMultiaddrs(ctx, stream.Conn(), peerID)
 	if err != nil {
 		_ = handshakeStream.Reset()
 		_ = s.host.Network().ClosePeer(peerID)
 		return nil, fmt.Errorf("build peer multiaddrs: %w", err)
 	}
 
-	if len(peerMultiaddrs) == 0 {
-		fullRemoteAddress, err := buildFullMA(stream.Conn().RemoteMultiaddr(), peerID)
-		if err != nil {
-			return nil, fmt.Errorf("build full remote peer multi address: %w", err)
-		}
-		peerMultiaddrs = append(peerMultiaddrs, fullRemoteAddress)
-	}
-
 	i, err := s.handshakeService.Handshake(
 		s.ctx,
 		handshakeStream,
@@ -1217,6 +1191,29 @@ func (s *Service) determineCurrentNetworkStatus(err error) error {
 	return err
 }
 
+// peerMultiaddrs builds full multiaddresses for a peer given information from
+// libp2p host peerstore and falling back to the remote address from the
+// connection.
+func (s *Service) peerMultiaddrs(ctx context.Context, conn network.Conn, peerID libp2ppeer.ID) ([]ma.Multiaddr, error) {
+	waitPeersCtx, cancel := context.WithTimeout(ctx, peerstoreWaitAddrsTimeout)
+	defer cancel()
+
+	peerMultiaddrs, err := buildFullMAs(waitPeerAddrs(waitPeersCtx, s.host.Peerstore(), peerID), peerID)
+	if err != nil {
+		return nil, fmt.Errorf("build peer multiaddrs: %w", err)
+	}
+
+	if len(peerMultiaddrs) == 0 {
+		fullRemoteAddress, err := buildFullMA(conn.RemoteMultiaddr(), peerID)
+		if err != nil {
+			return nil, fmt.Errorf("build full remote peer multi address: %w", err)
+		}
+		peerMultiaddrs = append(peerMultiaddrs, fullRemoteAddress)
+	}
+
+	return peerMultiaddrs, nil
+}
+
 var version270 = *semver.Must(semver.NewVersion("2.7.0"))
 
 func (s *Service) bee260BackwardCompatibility(peerID libp2ppeer.ID) bool {