Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

chore: use new peer store api #179

Merged
merged 1 commit into from
Apr 16, 2020
Merged

chore: use new peer store api #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@
"delay": "^4.3.0",
"dirty-chai": "^2.0.1",
"it-pair": "^1.0.0",
"libp2p": "libp2p/js-libp2p#chore/deprecate-old-peer-store-api",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jacobheun I am not sure if mocking the peerStore is the best thing to do here, or depend on libp2p as a dev dependency again. What do you think? It could be worth having a mocks repo for this kind of stuff, but we would need to make sure it was properly updated with new changes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vasco-santos something I think could be a better setup is to have libp2p as a dev dependency of the modules, and then have the libp2p test suite just try to run these tests as part of CI. I know we'd need to overcome some hurdles of what happens during breaking change releases, but this might be an opportunity to trial that flow with the 0.28 release and maybe just do the DHT to start with. If we're able to make that a smooth process, we could do that for the other modules.

This gets us a step in that direction, so I am good with having it as a dev dep.

"lodash": "^4.17.11",
"lodash.random": "^3.2.0",
"lodash.range": "^3.2.0",
"p-defer": "^3.0.0",
"p-each-series": "^2.1.0",
"p-map-series": "^2.1.0",
"p-retry": "^4.2.0",
"peer-book": "~0.9.2",
"sinon": "^9.0.0"
},
"contributors": [
Expand Down
9 changes: 2 additions & 7 deletions src/content-routing/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,7 @@ module.exports = (dht) => {
const provs = await dht.providers.getProviders(key)

provs.forEach((id) => {
let info
if (dht.peerStore.has(id)) {
info = dht.peerStore.get(id)
} else {
info = dht.peerStore.put(new PeerInfo(id))
}
const info = dht.peerStore.get(id) || new PeerInfo(id)
out.push(info)
})

Expand Down Expand Up @@ -113,7 +108,7 @@ module.exports = (dht) => {
dht._log('(%s) found %s provider entries', dht.peerInfo.id.toB58String(), provs.length)

provs.forEach((prov) => {
pathProviders.push(dht.peerStore.put(prov))
pathProviders.push(prov)
})

// hooray we have all that we want
Expand Down
12 changes: 8 additions & 4 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,14 +321,18 @@ class KadDHT extends EventEmitter {
*/
async _nearestPeersToQuery (msg) {
const key = await utils.convertBuffer(msg.key)

const ids = this.routingTable.closestPeers(key, this.kBucketSize)

return ids.map((p) => {
if (this.peerStore.has(p)) {
return this.peerStore.get(p)
const peer = this.peerStore.get(p)
const peerInfo = new PeerInfo(p)

if (peer) {
peer.protocols.forEach((p) => peerInfo.protocols.add(p))
peer.multiaddrInfos.forEach((mi) => peerInfo.multiaddrs.add(mi.multiaddr))
}
return this.peerStore.put(new PeerInfo(p))

return peerInfo
})
}

Expand Down
42 changes: 20 additions & 22 deletions src/peer-routing/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const errcode = require('err-code')
const pTimeout = require('p-timeout')

const PeerId = require('peer-id')
const PeerInfo = require('peer-info')
const crypto = require('libp2p-crypto')

const c = require('../constants')
Expand All @@ -24,11 +23,7 @@ module.exports = (dht) => {
dht._log('findPeerLocal %s', peer.toB58String())
const p = await dht.routingTable.find(peer)

if (!p || !dht.peerStore.has(p)) {
return
}

return dht.peerStore.get(p)
return p && dht.peerStore.get(p)
}

/**
Expand Down Expand Up @@ -57,7 +52,12 @@ module.exports = (dht) => {

return msg.closerPeers
.filter((pInfo) => !dht._isSelf(pInfo.id))
.map((pInfo) => dht.peerStore.put(pInfo))
.map((pInfo) => {
// Add known address to peer store
dht.peerStore.addressBook.add(pInfo.id, pInfo.multiaddrs.toArray())

return pInfo
})
}

/**
Expand Down Expand Up @@ -128,9 +128,13 @@ module.exports = (dht) => {

// sanity check
const match = peers.find((p) => p.isEqual(id))
if (match && dht.peerStore.has(id)) {
dht._log('found in peerStore')
return dht.peerStore.get(id)
if (match) {
const peer = dht.peerStore.get(id)

if (peer) {
dht._log('found in peerStore')
return peer
}
}

// query the network
Expand Down Expand Up @@ -169,7 +173,7 @@ module.exports = (dht) => {
result.paths.forEach((result) => {
if (result.success) {
success = true
dht.peerStore.put(result.peer)
dht.peerStore.addressBook.add(result.peer.id, result.peer.multiaddrs.toArray())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that this is now a discrete action. It's much clearer what's happening than a put imo.

}
})
dht._log('findPeer %s: %s', id.toB58String(), success)
Expand Down Expand Up @@ -228,16 +232,10 @@ module.exports = (dht) => {
dht._log('getPublicKey %s', peer.toB58String())

// local check
let info
if (dht.peerStore.has(peer)) {
info = dht.peerStore.get(peer)

if (info && info.id.pubKey) {
dht._log('getPublicKey: found local copy')
return info.id.pubKey
}
} else {
info = dht.peerStore.put(new PeerInfo(peer))
const info = dht.peerStore.get(peer)
if (info && info.id.pubKey) {
dht._log('getPublicKey: found local copy')
return info.id.pubKey
}

// try the node directly
Expand All @@ -252,7 +250,7 @@ module.exports = (dht) => {
}

info.id = new PeerId(peer.id, null, pk)
dht.peerStore.put(info)
dht.peerStore.addressBook.add(info.id, info.multiaddrs.toArray())

return pk
}
Expand Down
1 change: 0 additions & 1 deletion src/query/workerQueue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ class WorkerQueue {
if (this.dht._isSelf(closer.id)) {
return
}
closer = this.dht.peerStore.put(closer)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new information of the peer at this point

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it's unnecessary as the emit below will cause libp2p to update the peer store

this.dht._peerDiscovered(closer)
await this.path.addPeerToQuery(closer.id)
}))
Expand Down
3 changes: 2 additions & 1 deletion src/rpc/handlers/add-provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ module.exports = (dht) => {
log('received provider %s for %s (addrs %s)', peer.id.toB58String(), cid.toBaseEncodedString(), pi.multiaddrs.toArray().map((m) => m.toString()))

if (!dht._isSelf(pi.id)) {
dht.peerStore.put(pi)
// Add known address to peer store
dht.peerStore.addressBook.add(pi.id, pi.multiaddrs.toArray())
return dht.providers.addProvider(cid, pi.id)
}
})
Expand Down
8 changes: 1 addition & 7 deletions src/rpc/handlers/get-providers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,7 @@ module.exports = (dht) => {
dht._betterPeersToQuery(msg, peer)
])

const providers = peers.map((p) => {
if (dht.peerStore.has(p)) {
return dht.peerStore.get(p)
}

return dht.peerStore.put(new PeerInfo(p))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already known providers, no need to add to the peerStore. Moreover, the peerInfo is just created here, so there is nothing relevant to add

})
const providers = peers.map((p) => new PeerInfo(p))

if (has) {
providers.push(dht.peerInfo)
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/handlers/get-value.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module.exports = (dht) => {

if (dht._isSelf(id)) {
info = dht.peerInfo
} else if (dht.peerStore.has(id)) {
} else {
info = dht.peerStore.get(id)
}

Expand Down
12 changes: 6 additions & 6 deletions test/kad-dht.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ describe('KadDHT', () => {
const dhts = await tdht.spawn(2)

const ids = dhts.map((d) => d.peerInfo.id)
dhts[0].peerStore.put(dhts[1].peerInfo)
dhts[0].peerStore.addressBook.add(dhts[1].peerInfo.id, dhts[1].peerInfo.multiaddrs.toArray())

const key = await dhts[0].getPublicKey(ids[1])
expect(key).to.eql(dhts[1].peerInfo.id.pubKey)
Expand All @@ -671,7 +671,7 @@ describe('KadDHT', () => {
await tdht.connect(dhts[0], dhts[1])

// remove the pub key to be sure it is fetched
dhts[0].peerStore.put(dhts[1].peerInfo, true)
dhts[0].peerStore.addressBook.add(dhts[1].peerInfo.id, dhts[1].peerInfo.multiaddrs.toArray())

const key = await dhts[0].getPublicKey(ids[1])
expect(key.equals(dhts[1].peerInfo.id.pubKey)).to.eql(true)
Expand All @@ -694,7 +694,7 @@ describe('KadDHT', () => {
it('_nearestPeersToQuery', async () => {
const [dht] = await tdht.spawn(1)

dht.peerStore.put(peerInfos[1])
dht.peerStore.addressBook.add(peerInfos[1].id, peerInfos[1].multiaddrs.toArray())
await dht._add(peerInfos[1])
const res = await dht._nearestPeersToQuery({ key: 'hello' })
expect(res).to.be.eql([peerInfos[1]])
Expand All @@ -703,8 +703,8 @@ describe('KadDHT', () => {
it('_betterPeersToQuery', async () => {
const [dht] = await tdht.spawn(1)

dht.peerStore.put(peerInfos[1])
dht.peerStore.put(peerInfos[2])
dht.peerStore.addressBook.add(peerInfos[1].id, peerInfos[1].multiaddrs.toArray())
dht.peerStore.addressBook.add(peerInfos[2].id, peerInfos[2].multiaddrs.toArray())

await dht._add(peerInfos[1])
await dht._add(peerInfos[2])
Expand Down Expand Up @@ -769,7 +769,7 @@ describe('KadDHT', () => {
it('_verifyRecordLocally', async () => {
const [dht] = await tdht.spawn(1)

dht.peerStore.put(peerInfos[1])
dht.peerStore.addressBook.add(peerInfos[1].id, peerInfos[1].multiaddrs.toArray())

const record = new Record(
Buffer.from('hello'),
Expand Down
4 changes: 2 additions & 2 deletions test/query/index.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ chai.use(require('chai-checkmark'))
const expect = chai.expect
const sinon = require('sinon')
const delay = require('delay')
const PeerBook = require('peer-book')
const PeerStore = require('libp2p/src/peer-store')

const Query = require('../../src/query')
const Path = require('../../src/query/path')
Expand Down Expand Up @@ -46,7 +46,7 @@ describe('Query', () => {
})

before('create a dht', () => {
const peerStore = new PeerBook()
const peerStore = new PeerStore()
dht = new DHT({
dialer: {},
peerStore,
Expand Down
4 changes: 2 additions & 2 deletions test/rpc/handlers/add-provider.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('rpc - handlers - AddProvider', () => {
const bookEntry = dht.peerStore.get(provider.id)

// Favour peerInfo from payload over peerInfo from sender
expect(bookEntry.multiaddrs.toArray()).to.eql(
expect(bookEntry.multiaddrInfos.map((mi) => mi.multiaddr)).to.eql(
provider.multiaddrs.toArray()
)
})
Expand All @@ -100,7 +100,7 @@ describe('rpc - handlers - AddProvider', () => {

const provs = await dht.providers.getProviders(cid)

expect(dht.peerStore.has(provider.id)).to.equal(false)
expect(dht.peerStore.get(provider.id)).to.equal(undefined)
expect(provs).to.have.length(1)
expect(provs[0].id).to.eql(provider.id.id)
})
Expand Down
2 changes: 1 addition & 1 deletion test/rpc/handlers/get-value.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe('rpc - handlers - GetValue', () => {

const msg = new Message(T, key, 0)

dht.peerStore.put(other)
dht.peerStore.addressBook.add(other.id, other.multiaddrs.toArray())
await dht._add(other)
const response = await handler(dht)(peers[0], msg)
expect(response.record).to.exist()
Expand Down
2 changes: 1 addition & 1 deletion test/rpc/index.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('rpc', () => {
const defer = pDefer()
const [dht] = await tdht.spawn(1)

dht.peerStore.put(peerInfos[1])
dht.peerStore.addressBook.set(peerInfos[1].id, peerInfos[1].multiaddrs.toArray())

const msg = new Message(Message.TYPES.GET_VALUE, Buffer.from('hello'), 5)

Expand Down
4 changes: 2 additions & 2 deletions test/simulation/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
/* eslint-disable no-console */

'use strict'
const PeerBook = require('peer-book')
const PeerStore = require('libp2p/src/peer-store')
const PeerId = require('peer-id')
const PeerInfo = require('peer-info')
const multihashes = require('multihashes')
Expand Down Expand Up @@ -85,7 +85,7 @@ async function setup () {
async function GetClosestPeersSimulation () {
const dht = new DHT({
_peerInfo: ourPeerInfo,
_peerBook: new PeerBook(),
_peerBook: new PeerStore(),
handle: () => {},
on: () => {}
}, {
Expand Down
4 changes: 2 additions & 2 deletions test/utils/test-dht.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const PeerBook = require('peer-book')
const PeerStore = require('libp2p/src/peer-store')
const pRetry = require('p-retry')
const delay = require('delay')

Expand All @@ -27,7 +27,7 @@ class TestDHT {

async _spawnOne (index, options = {}) {
const regRecord = {}
const peerStore = new PeerBook()
const peerStore = new PeerStore()

// Disable random walk by default for more controlled testing
options = {
Expand Down