Skip to content

Commit

Permalink
fix: do not return self on peerstore.peers
Browse files Browse the repository at this point in the history
  • Loading branch information
vasco-santos committed Jul 15, 2020
1 parent c34881b commit 864d153
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 42 deletions.
14 changes: 5 additions & 9 deletions src/identify/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,14 @@ class IdentifyService {
try {
const envelope = await Envelope.openAndCertify(signedPeerRecord, PeerRecord.DOMAIN)
if (this.peerStore.addressBook.consumePeerRecord(envelope)) {
this.peerStore.protoBook.set(id, protocols)
return
}
} catch (err) {
log('received invalid envelope, discard it and fallback to listenAddrs is available', err)
}

// Update peers data in PeerStore
// LEGACY: Update peers data in PeerStore
try {
this.peerStore.addressBook.set(id, listenAddrs.map((addr) => multiaddr(addr)))
} catch (err) {
Expand Down Expand Up @@ -289,19 +290,14 @@ class IdentifyService {
try {
const envelope = await Envelope.openAndCertify(message.signedPeerRecord, PeerRecord.DOMAIN)
if (this.peerStore.addressBook.consumePeerRecord(envelope)) {
this.peerStore.protoBook.set(id, message.protocols)
return
}
} catch (err) {
<<<<<<< HEAD
log('received invalid envelope, discard it and fallback to listenAddrs is available', err)
// Try Legacy
addresses = message.listenAddrs
=======
log('received invalid envelope, discard it and fallback to listenAddrs is available')
>>>>>>> chore: add certified peer records to persisted peer store
}

// Update peers data in PeerStore
// LEGACY: Update peers data in PeerStore
try {
this.peerStore.addressBook.set(id, message.listenAddrs.map((addr) => multiaddr(addr)))
} catch (err) {
Expand All @@ -316,7 +312,7 @@ class IdentifyService {
* Get self signed peer record raw envelope.
* @return {Buffer}
*/
async _getSelfPeerRecord() {
async _getSelfPeerRecord () {
const selfSignedPeerRecord = this.peerStore.addressBook.getRawEnvelope(this.peerId)

// TODO: support invalidation when dynamic multiaddrs are supported
Expand Down
3 changes: 2 additions & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ class Libp2p extends EventEmitter {

this.peerStore = (this.datastore && this._options.peerStore.persistence)
? new PersistentPeerStore({
peerId: this.peerId,
datastore: this.datastore,
...this._options.peerStore
})
: new PeerStore()
: new PeerStore({ peerId: this.peerId })

// Addresses {listen, announce, noAnnounce}
this.addresses = this._options.addresses
Expand Down
9 changes: 7 additions & 2 deletions src/peer-store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ class PeerStore extends EventEmitter {
/**
* @constructor
*/
constructor () {
constructor ({ peerId } = {}) {
super()

this._peerId = peerId

/**
* AddressBook containing a map of peerIdStr to Address.
*/
Expand Down Expand Up @@ -72,7 +74,7 @@ class PeerStore extends EventEmitter {
stop () {}

/**
* Get all the stored information of every peer.
* Get all the stored information of every peer known.
* @returns {Map<string, Peer>}
*/
get peers () {
Expand All @@ -83,6 +85,9 @@ class PeerStore extends EventEmitter {
...this.metadataBook.data.keys()
])

// Remove self peer if present
this._peerId && storedPeers.delete(this._peerId.toB58String())

const peersData = new Map()
storedPeers.forEach((idStr) => {
peersData.set(idStr, this.get(PeerId.createFromCID(idStr)))
Expand Down
5 changes: 3 additions & 2 deletions src/peer-store/persistent/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ class PersistentPeerStore extends PeerStore {
/**
* @constructor
* @param {Object} properties
* @param {PeerId} properties.peerId
* @param {Datastore} properties.datastore Datastore to persist data.
* @param {number} [properties.threshold = 5] Number of dirty peers allowed before commit data.
*/
constructor ({ datastore, threshold = 5 }) {
super()
constructor ({ peerId, datastore, threshold = 5 }) {
super({ peerId })

/**
* Backend datastore used to persist data.
Expand Down
39 changes: 19 additions & 20 deletions test/identify/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('Identify', () => {
const [local, remote] = duplexPair()
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY })

sinon.spy(localIdentify.peerStore.addressBook, 'set')
sinon.spy(localIdentify.peerStore.addressBook, 'consumePeerRecord')
sinon.spy(localIdentify.peerStore.protoBook, 'set')

// Run identify
Expand All @@ -86,15 +86,15 @@ describe('Identify', () => {
})
])

expect(localIdentify.peerStore.addressBook.set.callCount).to.equal(1)
expect(localIdentify.peerStore.addressBook.consumePeerRecord.callCount).to.equal(1)
expect(localIdentify.peerStore.protoBook.set.callCount).to.equal(1)

// Validate the remote peer gets updated in the peer store
const call = localIdentify.peerStore.addressBook.set.firstCall
expect(call.args[0].id.bytes).to.equal(remotePeer.bytes)
expect(call.args[1]).to.exist()
expect(call.args[1]).have.lengthOf(listenMaddrs.length)
expect(call.args[1][0].equals(listenMaddrs[0]))
const addresses = localIdentify.peerStore.addressBook.get(remotePeer)
expect(addresses).to.exist()
expect(addresses).have.lengthOf(listenMaddrs.length)
expect(addresses.map((a) => a.multiaddr)[0].equals(listenMaddrs[0]))
expect(addresses.map((a) => a.isCertified)[0]).to.eql(true)
})

// LEGACY
Expand Down Expand Up @@ -127,7 +127,7 @@ describe('Identify', () => {
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY })
sinon.stub(Envelope, 'openAndCertify').throws()

sinon.spy(localIdentify.peerStore.addressBook, 'consumePeerRecord')
sinon.spy(localIdentify.peerStore.addressBook, 'set')
sinon.spy(localIdentify.peerStore.protoBook, 'set')

// Run identify
Expand All @@ -140,15 +140,15 @@ describe('Identify', () => {
})
])

expect(localIdentify.peerStore.addressBook.consumePeerRecord.callCount).to.equal(1)
expect(localIdentify.peerStore.addressBook.set.callCount).to.equal(1)
expect(localIdentify.peerStore.protoBook.set.callCount).to.equal(1)

// Validate the remote peer gets updated in the peer store
const addresses = localIdentify.peerStore.addressBook.get(remotePeer)
expect(addresses).to.exist()
expect(addresses).have.lengthOf(listenMaddrs.length)
expect(addresses.map((a) => a.multiaddr)[0].equals(listenMaddrs[0]))
expect(addresses.map((a) => a.isCertified)[0]).to.eql(true)
const call = localIdentify.peerStore.addressBook.set.firstCall
expect(call.args[0].id.bytes).to.equal(remotePeer.bytes)
expect(call.args[1]).to.exist()
expect(call.args[1]).have.lengthOf(listenMaddrs.length)
expect(call.args[1][0].equals(listenMaddrs[0]))
})

it('should throw if identified peer is the wrong peer', async () => {
Expand Down Expand Up @@ -290,7 +290,7 @@ describe('Identify', () => {
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_PUSH })
sinon.stub(Envelope, 'openAndCertify').throws()

sinon.spy(remoteIdentify.peerStore.addressBook, 'consumePeerRecord')
sinon.spy(remoteIdentify.peerStore.addressBook, 'set')
sinon.spy(remoteIdentify.peerStore.protoBook, 'set')

// Run identify
Expand All @@ -303,13 +303,12 @@ describe('Identify', () => {
})
])

expect(remoteIdentify.peerStore.addressBook.consumePeerRecord.callCount).to.equal(1)
expect(remoteIdentify.peerStore.addressBook.set.callCount).to.equal(1)
expect(remoteIdentify.peerStore.protoBook.set.callCount).to.equal(1)

const addresses = localIdentify.peerStore.addressBook.get(localPeer)
expect(addresses).to.exist()
expect(addresses).have.lengthOf(listenMaddrs.length)
expect(addresses.map((a) => a.multiaddr)).to.eql(listenMaddrs)
const [peerId, multiaddrs] = remoteIdentify.peerStore.addressBook.set.firstCall.args
expect(peerId.bytes).to.eql(localPeer.bytes)
expect(multiaddrs).to.eql(listenMaddrs)

const [peerId2, protocols] = remoteIdentify.peerStore.protoBook.set.firstCall.args
expect(peerId2.bytes).to.eql(localPeer.bytes)
Expand Down
16 changes: 8 additions & 8 deletions test/peer-store/persisted-peer-store.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ describe('libp2p.peerStore (Persisted)', () => {

it('should load content to the peerStore when a new node is started with the same datastore', async () => {
const commitSpy = sinon.spy(libp2p.peerStore, '_commitData')
const peers = await peerUtils.createPeerId({ number: 2 })
const peers = await peerUtils.createPeerId({ number: 3 })
const multiaddrs = [
multiaddr('/ip4/156.10.1.22/tcp/1000'),
multiaddr('/ip4/156.10.1.23/tcp/1000')
Expand All @@ -543,15 +543,15 @@ describe('libp2p.peerStore (Persisted)', () => {
await libp2p.start()

// AddressBook
libp2p.peerStore.addressBook.set(peers[0], [multiaddrs[0]])
libp2p.peerStore.addressBook.set(peers[1], [multiaddrs[1]])
libp2p.peerStore.addressBook.set(peers[1], [multiaddrs[0]])
libp2p.peerStore.addressBook.set(peers[2], [multiaddrs[1]])

// let batch commit complete
await Promise.all(commitSpy.returnValues)

// ProtoBook
libp2p.peerStore.protoBook.set(peers[0], protocols)
libp2p.peerStore.protoBook.set(peers[1], protocols)
libp2p.peerStore.protoBook.set(peers[2], protocols)

// let batch commit complete
await Promise.all(commitSpy.returnValues)
Expand Down Expand Up @@ -582,13 +582,13 @@ describe('libp2p.peerStore (Persisted)', () => {
expect(newNode.peerStore.peers.size).to.equal(2)

// Validate data
const peer0 = newNode.peerStore.get(peers[0])
expect(peer0.id.toB58String()).to.eql(peers[0].toB58String())
const peer0 = newNode.peerStore.get(peers[1])
expect(peer0.id.toB58String()).to.eql(peers[1].toB58String())
expect(peer0.protocols).to.have.members(protocols)
expect(peer0.addresses.map((a) => a.multiaddr.toString())).to.have.members([multiaddrs[0].toString()])

const peer1 = newNode.peerStore.get(peers[1])
expect(peer1.id.toB58String()).to.eql(peers[1].toB58String())
const peer1 = newNode.peerStore.get(peers[2])
expect(peer1.id.toB58String()).to.eql(peers[2].toB58String())
expect(peer1.protocols).to.have.members(protocols)
expect(peer1.addresses.map((a) => a.multiaddr.toString())).to.have.members([multiaddrs[1].toString()])

Expand Down

0 comments on commit 864d153

Please sign in to comment.