fix: do not return self on peerstore.peers

This commit is contained in:
Vasco Santos 2020-07-15 14:34:51 +02:00 committed by Jacob Heun
parent 5bce4cbbc6
commit e1b8edcfa3
No known key found for this signature in database
GPG Key ID: CA5A94C15809879F
6 changed files with 44 additions and 42 deletions

View File

@ -184,13 +184,14 @@ class IdentifyService {
try { try {
const envelope = await Envelope.openAndCertify(signedPeerRecord, PeerRecord.DOMAIN) const envelope = await Envelope.openAndCertify(signedPeerRecord, PeerRecord.DOMAIN)
if (this.peerStore.addressBook.consumePeerRecord(envelope)) { if (this.peerStore.addressBook.consumePeerRecord(envelope)) {
this.peerStore.protoBook.set(id, protocols)
return return
} }
} catch (err) { } catch (err) {
log('received invalid envelope, discard it and fallback to listenAddrs is available', 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 { try {
this.peerStore.addressBook.set(id, listenAddrs.map((addr) => multiaddr(addr))) this.peerStore.addressBook.set(id, listenAddrs.map((addr) => multiaddr(addr)))
} catch (err) { } catch (err) {
@ -290,19 +291,14 @@ class IdentifyService {
try { try {
const envelope = await Envelope.openAndCertify(message.signedPeerRecord, PeerRecord.DOMAIN) const envelope = await Envelope.openAndCertify(message.signedPeerRecord, PeerRecord.DOMAIN)
if (this.peerStore.addressBook.consumePeerRecord(envelope)) { if (this.peerStore.addressBook.consumePeerRecord(envelope)) {
this.peerStore.protoBook.set(id, message.protocols)
return return
} }
} catch (err) { } catch (err) {
<<<<<<< HEAD
log('received invalid envelope, discard it and fallback to listenAddrs is available', err) 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 { try {
this.peerStore.addressBook.set(id, message.listenAddrs.map((addr) => multiaddr(addr))) this.peerStore.addressBook.set(id, message.listenAddrs.map((addr) => multiaddr(addr)))
} catch (err) { } catch (err) {
@ -317,7 +313,7 @@ class IdentifyService {
* Get self signed peer record raw envelope. * Get self signed peer record raw envelope.
* @return {Buffer} * @return {Buffer}
*/ */
async _getSelfPeerRecord() { async _getSelfPeerRecord () {
const selfSignedPeerRecord = this.peerStore.addressBook.getRawEnvelope(this.peerId) const selfSignedPeerRecord = this.peerStore.addressBook.getRawEnvelope(this.peerId)
// TODO: support invalidation when dynamic multiaddrs are supported // TODO: support invalidation when dynamic multiaddrs are supported

View File

@ -51,10 +51,11 @@ class Libp2p extends EventEmitter {
this.peerStore = (this.datastore && this._options.peerStore.persistence) this.peerStore = (this.datastore && this._options.peerStore.persistence)
? new PersistentPeerStore({ ? new PersistentPeerStore({
peerId: this.peerId,
datastore: this.datastore, datastore: this.datastore,
...this._options.peerStore ...this._options.peerStore
}) })
: new PeerStore() : new PeerStore({ peerId: this.peerId })
// Addresses {listen, announce, noAnnounce} // Addresses {listen, announce, noAnnounce}
this.addresses = this._options.addresses this.addresses = this._options.addresses

View File

@ -37,9 +37,11 @@ class PeerStore extends EventEmitter {
/** /**
* @constructor * @constructor
*/ */
constructor () { constructor ({ peerId } = {}) {
super() super()
this._peerId = peerId
/** /**
* AddressBook containing a map of peerIdStr to Address. * AddressBook containing a map of peerIdStr to Address.
*/ */
@ -72,7 +74,7 @@ class PeerStore extends EventEmitter {
stop () {} stop () {}
/** /**
* Get all the stored information of every peer. * Get all the stored information of every peer known.
* @returns {Map<string, Peer>} * @returns {Map<string, Peer>}
*/ */
get peers () { get peers () {
@ -83,6 +85,9 @@ class PeerStore extends EventEmitter {
...this.metadataBook.data.keys() ...this.metadataBook.data.keys()
]) ])
// Remove self peer if present
this._peerId && storedPeers.delete(this._peerId.toB58String())
const peersData = new Map() const peersData = new Map()
storedPeers.forEach((idStr) => { storedPeers.forEach((idStr) => {
peersData.set(idStr, this.get(PeerId.createFromCID(idStr))) peersData.set(idStr, this.get(PeerId.createFromCID(idStr)))

View File

@ -28,11 +28,12 @@ class PersistentPeerStore extends PeerStore {
/** /**
* @constructor * @constructor
* @param {Object} properties * @param {Object} properties
* @param {PeerId} properties.peerId
* @param {Datastore} properties.datastore Datastore to persist data. * @param {Datastore} properties.datastore Datastore to persist data.
* @param {number} [properties.threshold = 5] Number of dirty peers allowed before commit data. * @param {number} [properties.threshold = 5] Number of dirty peers allowed before commit data.
*/ */
constructor ({ datastore, threshold = 5 }) { constructor ({ peerId, datastore, threshold = 5 }) {
super() super({ peerId })
/** /**
* Backend datastore used to persist data. * Backend datastore used to persist data.

View File

@ -74,7 +74,7 @@ describe('Identify', () => {
const [local, remote] = duplexPair() const [local, remote] = duplexPair()
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY }) 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') sinon.spy(localIdentify.peerStore.protoBook, 'set')
// Run identify // Run identify
@ -87,15 +87,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) expect(localIdentify.peerStore.protoBook.set.callCount).to.equal(1)
// Validate the remote peer gets updated in the peer store // Validate the remote peer gets updated in the peer store
const call = localIdentify.peerStore.addressBook.set.firstCall const addresses = localIdentify.peerStore.addressBook.get(remotePeer)
expect(call.args[0].id.bytes).to.equal(remotePeer.bytes) expect(addresses).to.exist()
expect(call.args[1]).to.exist() expect(addresses).have.lengthOf(listenMaddrs.length)
expect(call.args[1]).have.lengthOf(listenMaddrs.length) expect(addresses.map((a) => a.multiaddr)[0].equals(listenMaddrs[0]))
expect(call.args[1][0].equals(listenMaddrs[0])) expect(addresses.map((a) => a.isCertified)[0]).to.eql(true)
}) })
// LEGACY // LEGACY
@ -128,7 +128,7 @@ describe('Identify', () => {
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY }) sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY })
sinon.stub(Envelope, 'openAndCertify').throws() sinon.stub(Envelope, 'openAndCertify').throws()
sinon.spy(localIdentify.peerStore.addressBook, 'consumePeerRecord') sinon.spy(localIdentify.peerStore.addressBook, 'set')
sinon.spy(localIdentify.peerStore.protoBook, 'set') sinon.spy(localIdentify.peerStore.protoBook, 'set')
sinon.spy(localIdentify.peerStore.metadataBook, 'set') sinon.spy(localIdentify.peerStore.metadataBook, 'set')
@ -142,7 +142,7 @@ 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) expect(localIdentify.peerStore.protoBook.set.callCount).to.equal(1)
const metadataArgs = localIdentify.peerStore.metadataBook.set.firstCall.args const metadataArgs = localIdentify.peerStore.metadataBook.set.firstCall.args
@ -151,11 +151,11 @@ describe('Identify', () => {
expect(metadataArgs[2].toString()).to.equal(`js-libp2p/${pkg.version}`) expect(metadataArgs[2].toString()).to.equal(`js-libp2p/${pkg.version}`)
// Validate the remote peer gets updated in the peer store // Validate the remote peer gets updated in the peer store
const addresses = localIdentify.peerStore.addressBook.get(remotePeer) const call = localIdentify.peerStore.addressBook.set.firstCall
expect(addresses).to.exist() expect(call.args[0].id.bytes).to.equal(remotePeer.bytes)
expect(addresses).have.lengthOf(listenMaddrs.length) expect(call.args[1]).to.exist()
expect(addresses.map((a) => a.multiaddr)[0].equals(listenMaddrs[0])) expect(call.args[1]).have.lengthOf(listenMaddrs.length)
expect(addresses.map((a) => a.isCertified)[0]).to.eql(true) expect(call.args[1][0].equals(listenMaddrs[0]))
}) })
it('should throw if identified peer is the wrong peer', async () => { it('should throw if identified peer is the wrong peer', async () => {
@ -297,7 +297,7 @@ describe('Identify', () => {
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_PUSH }) sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_PUSH })
sinon.stub(Envelope, 'openAndCertify').throws() sinon.stub(Envelope, 'openAndCertify').throws()
sinon.spy(remoteIdentify.peerStore.addressBook, 'consumePeerRecord') sinon.spy(remoteIdentify.peerStore.addressBook, 'set')
sinon.spy(remoteIdentify.peerStore.protoBook, 'set') sinon.spy(remoteIdentify.peerStore.protoBook, 'set')
// Run identify // Run identify
@ -310,13 +310,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) expect(remoteIdentify.peerStore.protoBook.set.callCount).to.equal(1)
const addresses = localIdentify.peerStore.addressBook.get(localPeer) const [peerId, multiaddrs] = remoteIdentify.peerStore.addressBook.set.firstCall.args
expect(addresses).to.exist() expect(peerId.bytes).to.eql(localPeer.bytes)
expect(addresses).have.lengthOf(listenMaddrs.length) expect(multiaddrs).to.eql(listenMaddrs)
expect(addresses.map((a) => a.multiaddr)).to.eql(listenMaddrs)
const [peerId2, protocols] = remoteIdentify.peerStore.protoBook.set.firstCall.args const [peerId2, protocols] = remoteIdentify.peerStore.protoBook.set.firstCall.args
expect(peerId2.bytes).to.eql(localPeer.bytes) expect(peerId2.bytes).to.eql(localPeer.bytes)

View File

@ -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 () => { 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 commitSpy = sinon.spy(libp2p.peerStore, '_commitData')
const peers = await peerUtils.createPeerId({ number: 2 }) const peers = await peerUtils.createPeerId({ number: 3 })
const multiaddrs = [ const multiaddrs = [
multiaddr('/ip4/156.10.1.22/tcp/1000'), multiaddr('/ip4/156.10.1.22/tcp/1000'),
multiaddr('/ip4/156.10.1.23/tcp/1000') multiaddr('/ip4/156.10.1.23/tcp/1000')
@ -543,15 +543,15 @@ describe('libp2p.peerStore (Persisted)', () => {
await libp2p.start() await libp2p.start()
// AddressBook // AddressBook
libp2p.peerStore.addressBook.set(peers[0], [multiaddrs[0]]) libp2p.peerStore.addressBook.set(peers[1], [multiaddrs[0]])
libp2p.peerStore.addressBook.set(peers[1], [multiaddrs[1]]) libp2p.peerStore.addressBook.set(peers[2], [multiaddrs[1]])
// let batch commit complete // let batch commit complete
await Promise.all(commitSpy.returnValues) await Promise.all(commitSpy.returnValues)
// ProtoBook // ProtoBook
libp2p.peerStore.protoBook.set(peers[0], protocols)
libp2p.peerStore.protoBook.set(peers[1], protocols) libp2p.peerStore.protoBook.set(peers[1], protocols)
libp2p.peerStore.protoBook.set(peers[2], protocols)
// let batch commit complete // let batch commit complete
await Promise.all(commitSpy.returnValues) await Promise.all(commitSpy.returnValues)
@ -582,13 +582,13 @@ describe('libp2p.peerStore (Persisted)', () => {
expect(newNode.peerStore.peers.size).to.equal(2) expect(newNode.peerStore.peers.size).to.equal(2)
// Validate data // Validate data
const peer0 = newNode.peerStore.get(peers[0]) const peer0 = newNode.peerStore.get(peers[1])
expect(peer0.id.toB58String()).to.eql(peers[0].toB58String()) expect(peer0.id.toB58String()).to.eql(peers[1].toB58String())
expect(peer0.protocols).to.have.members(protocols) expect(peer0.protocols).to.have.members(protocols)
expect(peer0.addresses.map((a) => a.multiaddr.toString())).to.have.members([multiaddrs[0].toString()]) expect(peer0.addresses.map((a) => a.multiaddr.toString())).to.have.members([multiaddrs[0].toString()])
const peer1 = newNode.peerStore.get(peers[1]) const peer1 = newNode.peerStore.get(peers[2])
expect(peer1.id.toB58String()).to.eql(peers[1].toB58String()) expect(peer1.id.toB58String()).to.eql(peers[2].toB58String())
expect(peer1.protocols).to.have.members(protocols) expect(peer1.protocols).to.have.members(protocols)
expect(peer1.addresses.map((a) => a.multiaddr.toString())).to.have.members([multiaddrs[1].toString()]) expect(peer1.addresses.map((a) => a.multiaddr.toString())).to.have.members([multiaddrs[1].toString()])