peerRouting.findPeer() does not accept strings, only PeerId instances

[a1300]

• Version: v0.29.3
• Platform: Linux
• Subsystem:

Type: Bug

Severity: Low

Description: peerRouting.findPeer() does not accept string representations for PeerId, it only accepts PeerId instances

The API.md#peerroutingfindpeer says findPeer() only accepts PeerId instances

On the other hand the JSDoc type documentation findPeer() only accepts string

js-libp2p/src/peer-routing.js

Lines 14 to 23 in e9e4b73

	return {
	/**
	* Iterates over all peer routers in series to find the given peer.
	*
	* @param {string} id - The id of the peer to find
	* @param {object} [options]
	* @param {number} [options.timeout] - How long the query should run
	* @returns {Promise<{ id: PeerId, multiaddrs: Multiaddr[] }>}
	*/
	findPeer: async (id, options) => { // eslint-disable-line require-await

If I understand the following test correctly, it uses findPeer() also with a string

js-libp2p/test/peer-routing/peer-routing.node.js

Lines 115 to 129 in e9e4b73

	it('should be able to find a peer', async () => {
	const peerKey = 'QmTp9VkYvnHyrqKQuFPiuZkiX9gPcqj6x5LJ1rmWuSySnL'
	const mockApi = nock('http://0.0.0.0:60197')
	.post('/api/v0/dht/findpeer')
	.query(true)
	.reply(200, `{"Extra":"","ID":"some other id","Responses":null,"Type":0}\n{"Extra":"","ID":"","Responses":[{"Addrs":["/ip4/127.0.0.1/tcp/4001"],"ID":"${peerKey}"}],"Type":2}\n`, [
	'Content-Type', 'application/json',
	'X-Chunked-Output', '1'
	])
	const peer = await node.peerRouting.findPeer(peerKey)
	expect(peer.id).to.equal(peerKey)
	expect(mockApi.isDone()).to.equal(true)
	})

Steps to reproduce the error:

git clone https://gist.github.com/a1300/79357f742a6e459a9a801e60bd2840ef libp2p-peerRouting-no-string
cd libp2p-peerRouting-no-string
npm install
node index

[vasco-santos]

Per https://github.com/libp2p/js-libp2p-interfaces/tree/master/src/peer-routing#api this should definitely be PeerId and not string. Both the DHT and delegated peer routing also expect PeerId.

The test mocks the response as the "real" tests for the delegate are in the delegate repo. As a result, what is provided in the parameter is not even used. Anyway, we should turn it into a PeerId with PeerId.createFromCID. Want to PR these changes?

[vasco-santos]

Also, FYI I am working on #802 . Once we get all the modules to use this, these type of documentation errors should be caught in CI

[vasco-santos]

Just shipped libp2p rc for 0.30 where this is fixed. Type checking will now run.