Skip to content

Commit

Permalink
chore: apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-Authored-By: Jacob Heun <[email protected]>
  • Loading branch information
vasco-santos and jacobheun committed Apr 24, 2020
1 parent 0f6e878 commit 984d933
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 21 deletions.
2 changes: 1 addition & 1 deletion doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ Once a content router succeeds, the iteration will stop. If the DHT is enabled,
```js
// Iterate over the providers found for the given cid
for await (const provider of libp2p.contentRouting.findProviders(cid)) {
console.log(provider.id, provider.addrs)
console.log(provider.id, provider.multiaddrs)
}
```

Expand Down
13 changes: 9 additions & 4 deletions src/dialer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const log = debug('libp2p:dialer')
log.error = debug('libp2p:dialer:error')

const { DialRequest } = require('./dial-request')
const getPeerId = require('../get-peer-id')
const getPeer = require('../get-peer')

const { codes } = require('../errors')
const {
Expand Down Expand Up @@ -106,8 +106,13 @@ class Dialer {
* @returns {DialTarget}
*/
_createDialTarget (peer) {
const peerId = getPeerId(peer, this.peerStore)
let addrs = this.peerStore.addressBook.getMultiaddrsForPeer(peerId)
const { id, multiaddrs } = getPeer(peer)

if (multiaddrs) {
this.peerStore.addressBook.add(id, multiaddrs)
}

let addrs = this.peerStore.addressBook.getMultiaddrsForPeer(id)

// If received a multiaddr to dial, it should be the first to use
// But, if we know other multiaddrs for the peer, we should try them too.
Expand All @@ -117,7 +122,7 @@ class Dialer {
}

return {
id: peerId.toB58String(),
id: id.toB58String(),
addrs
}
}
Expand Down
15 changes: 7 additions & 8 deletions src/get-peer-id.js → src/get-peer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ const errCode = require('err-code')
const { codes } = require('./errors')

/**
* Converts the given `peer` to a `PeerId` instance.
* Converts the given `peer` to a `Peer` object.
* If a multiaddr is received, the addressBook is updated.
* @param {PeerId|Multiaddr|string} peer
* @param {PeerStore} peerStore
* @returns {PeerId}
* @returns {{ id: PeerId, multiaddrs: Array<Multiaddr> }}
*/
function getPeerId (peer, peerStore) {
function getPeer (peer) {
if (typeof peer === 'string') {
peer = multiaddr(peer)
}
Expand All @@ -31,11 +31,10 @@ function getPeerId (peer, peerStore) {
}
}

if (addr && peerStore) {
peerStore.addressBook.add(peer, [addr])
return {
id: peer,
multiaddrs: addr ? [addr] : undefined
}

return peer
}

module.exports = getPeerId
module.exports = getPeer
2 changes: 1 addition & 1 deletion src/identify/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class IdentifyService {
* @param {Registrar} options.registrar
* @param {Map<string, handler>} options.protocols A reference to the protocols we support
* @param {PeerId} options.peerId The peer running the identify service
* @param {{ listen: Array<Multiaddr>}} options.addresses The peer aaddresses
* @param {{ listen: Array<Multiaddr>}} options.addresses The peer addresses
*/
constructor (options) {
/**
Expand Down
16 changes: 9 additions & 7 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const PeerId = require('peer-id')
const peerRouting = require('./peer-routing')
const contentRouting = require('./content-routing')
const pubsub = require('./pubsub')
const getPeerId = require('./get-peer-id')
const getPeer = require('./get-peer')
const { validate: validateConfig } = require('./config')
const { codes } = require('./errors')

Expand Down Expand Up @@ -290,11 +290,13 @@ class Libp2p extends EventEmitter {
* @returns {Promise<Connection|*>}
*/
async dialProtocol (peer, protocols, options) {
const peerId = getPeerId(peer, this.peerStore)
let connection = this.registrar.getConnection(peerId)
const { id, multiaddrs } = getPeer(peer, this.peerStore)
let connection = this.registrar.getConnection(id)

if (!connection) {
connection = await this.dialer.connectToPeer(peer, options)
} else {
this.peerStore.addressBook.add(id, multiaddrs)
}

// If a protocol was provided, create a new stream
Expand All @@ -311,10 +313,10 @@ class Libp2p extends EventEmitter {
* @returns {Promise<void>}
*/
hangUp (peer) {
const peerId = getPeerId(peer)
const { id } = getPeer(peer)

return Promise.all(
this.registrar.connections.get(peerId.toB58String()).map(connection => {
this.registrar.connections.get(id.toB58String()).map(connection => {
return connection.close()
})
)
Expand All @@ -326,9 +328,9 @@ class Libp2p extends EventEmitter {
* @returns {Promise<number>}
*/
ping (peer) {
const peerId = getPeerId(peer)
const { id } = getPeer(peer)

return ping(this, peerId)
return ping(this, id)
}

/**
Expand Down

0 comments on commit 984d933

Please sign in to comment.