fix(routing/http/contentrouter): additional FindPeer checks (#616)

* fix(routing/http/contentrouter): return error when no responses are found in FindPeer
* fix(routing/http/contentrouter): fix findpeer iteration log message
* fix(routing/http/contentrouter): only return information for FindPeer that matches the requested peerID
* fix(routing/http/contentrouter): only return information for FindPeer if there are actual addresses
* fix(routing/http/contentrouter): switch logging from Warnw -> Warnf where there aren't key pairs
* docs: update CHANGELOG.md

---------

Co-authored-by: Marcin Rataj <lidel@lidel.org>

CHANGELOG.md

@@ -24,6 +24,8 @@ The following emojis are used to highlight certain changes:
 
 ### Fixed
 
+- `routing/http/contentrouter` The `FindPeer` now returns `routing.ErrNotFound` when no addresses are found
+
 ### Security
 
 ## [v0.20.0]

routing/http/contentrouter/contentrouter.go

@@ -119,7 +119,7 @@ func readProviderResponses(ctx context.Context, iter iter.ResultIter[types.Recor
 	for iter.Next() {
 		res := iter.Val()
 		if res.Err != nil {
-			logger.Warnw("error iterating provider responses: %s", res.Err)
+			logger.Warnf("error iterating provider responses: %s", res.Err)
 			continue
 		}
 		v := res.Val
@@ -200,21 +200,32 @@ func (c *contentRouter) FindPeer(ctx context.Context, pid peer.ID) (peer.AddrInf
 	for iter.Next() {
 		res := iter.Val()
 		if res.Err != nil {
-			logger.Warnw("error iterating provider responses: %s", res.Err)
+			logger.Warnf("error iterating peer responses: %s", res.Err)
 			continue
 		}
+
+		if *res.Val.ID != pid {
+			logger.Warnf("searched for peerID %s, got response for %s:", pid, *res.Val.ID)
+			continue
+		}
+
 		var addrs []multiaddr.Multiaddr
 		for _, a := range res.Val.Addrs {
 			addrs = append(addrs, a.Multiaddr)
 		}
 
+		// If there are no addresses there's nothing of value to return
+		if len(addrs) == 0 {
+			continue
+		}
+
 		return peer.AddrInfo{
-			ID:    *res.Val.ID,
+			ID:    pid,
 			Addrs: addrs,
 		}, nil
 	}
 
-	return peer.AddrInfo{}, err
+	return peer.AddrInfo{}, routing.ErrNotFound
 }
 
 func (c *contentRouter) PutValue(ctx context.Context, key string, data []byte, opts ...routing.Option) error {

routing/http/contentrouter/contentrouter_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/libp2p/go-libp2p/core/crypto"
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/libp2p/go-libp2p/core/routing"
+	"github.com/multiformats/go-multiaddr"
 	"github.com/multiformats/go-multihash"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -187,16 +188,73 @@ func TestFindPeer(t *testing.T) {
 		{
 			Schema:    types.SchemaPeer,
 			ID:        &p1,
+			Addrs:     []types.Multiaddr{{Multiaddr: multiaddr.StringCast("/ip4/1.2.3.4/tcp/1234")}},
 			Protocols: []string{"transport-bitswap"},
 		},
 	}
 	aisIter := iter.ToResultIter[*types.PeerRecord](iter.FromSlice(ais))
 
 	client.On("FindPeers", ctx, p1).Return(aisIter, nil)
 
-	peer, err := crc.FindPeer(ctx, p1)
+	p, err := crc.FindPeer(ctx, p1)
 	require.NoError(t, err)
-	require.Equal(t, peer.ID, p1)
+	require.Equal(t, p.ID, p1)
+}
+
+func TestFindPeerNoAddresses(t *testing.T) {
+	ctx := context.Background()
+	client := &mockClient{}
+	crc := NewContentRoutingClient(client)
+
+	p1 := peer.ID("peer1")
+	ais := []*types.PeerRecord{
+		{
+			Schema:    types.SchemaPeer,
+			ID:        &p1,
+			Protocols: []string{"transport-bitswap"},
+		},
+	}
+	aisIter := iter.ToResultIter[*types.PeerRecord](iter.FromSlice(ais))
+
+	client.On("FindPeers", ctx, p1).Return(aisIter, nil)
+
+	_, err := crc.FindPeer(ctx, p1)
+	require.ErrorIs(t, err, routing.ErrNotFound)
+}
+
+func TestFindPeerWrongPeer(t *testing.T) {
+	ctx := context.Background()
+	client := &mockClient{}
+	crc := NewContentRoutingClient(client)
+
+	p1 := peer.ID("peer1")
+	p2 := peer.ID("peer2")
+	ais := []*types.PeerRecord{
+		{
+			Schema: types.SchemaPeer,
+			ID:     &p2,
+		},
+	}
+	aisIter := iter.ToResultIter[*types.PeerRecord](iter.FromSlice(ais))
+
+	client.On("FindPeers", ctx, p1).Return(aisIter, nil)
+
+	_, err := crc.FindPeer(ctx, p1)
+	require.ErrorIs(t, err, routing.ErrNotFound)
+}
+
+func TestFindPeerNoPeer(t *testing.T) {
+	ctx := context.Background()
+	client := &mockClient{}
+	crc := NewContentRoutingClient(client)
+
+	p1 := peer.ID("peer1")
+	aisIter := iter.ToResultIter[*types.PeerRecord](iter.FromSlice([]*types.PeerRecord{}))
+
+	client.On("FindPeers", ctx, p1).Return(aisIter, nil)
+
+	_, err := crc.FindPeer(ctx, p1)
+	require.ErrorIs(t, err, routing.ErrNotFound)
 }
 
 func makeName(t *testing.T) (crypto.PrivKey, ipns.Name) {