Skip to content

Commit

Permalink
chore: address review
Browse files Browse the repository at this point in the history
  • Loading branch information
vasco-santos committed Jun 29, 2020
1 parent abf9869 commit 1b3bd46
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 64 deletions.
4 changes: 2 additions & 2 deletions src/identify/consts.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ module.exports.MULTICODEC_IDENTIFY = '/p2p/id/1.1.0'
module.exports.MULTICODEC_IDENTIFY_PUSH = '/p2p/id/push/1.1.0'

// Legacy
module.exports.MULTICODEC_IDENTIFY_LEGACY = '/ipfs/id/1.0.0'
module.exports.MULTICODEC_IDENTIFY_PUSH_LEGACY = '/ipfs/id/push/1.0.0'
module.exports.MULTICODEC_IDENTIFY_1_0_0 = '/ipfs/id/1.0.0'
module.exports.MULTICODEC_IDENTIFY_PUSH_1_0_0 = '/ipfs/id/push/1.0.0'
86 changes: 35 additions & 51 deletions src/identify/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ const PeerRecord = require('../record/peer-record')

const {
MULTICODEC_IDENTIFY,
MULTICODEC_IDENTIFY_LEGACY,
MULTICODEC_IDENTIFY_1_0_0,
MULTICODEC_IDENTIFY_PUSH,
MULTICODEC_IDENTIFY_PUSH_LEGACY,
MULTICODEC_IDENTIFY_PUSH_1_0_0,
AGENT_VERSION,
PROTOCOL_VERSION
} = require('./consts')
Expand Down Expand Up @@ -97,10 +97,10 @@ class IdentifyService {
push (connections) {
const pushes = connections.map(async connection => {
try {
const { protocol, stream } = await connection.newStream([MULTICODEC_IDENTIFY_PUSH, MULTICODEC_IDENTIFY_PUSH_LEGACY])
const { protocol, stream } = await connection.newStream([MULTICODEC_IDENTIFY_PUSH, MULTICODEC_IDENTIFY_PUSH_1_0_0])

// Handle Legacy
if (protocol === MULTICODEC_IDENTIFY_PUSH_LEGACY) {
if (protocol === MULTICODEC_IDENTIFY_PUSH_1_0_0) {
return pipe(
[{
listenAddrs: this._libp2p.multiaddrs.map((ma) => ma.buffer),
Expand All @@ -112,11 +112,11 @@ class IdentifyService {
)
}

const envelope = await this._getSelfPeerRecord()
const signedPeerRecord = envelope.marshal()
const signedPeerRecord = await this._getSelfPeerRecord()

await pipe(
[{
listenAddrs: this._libp2p.multiaddrs.map((ma) => ma.buffer),
signedPeerRecord,
protocols: Array.from(this._protocols.keys())
}],
Expand Down Expand Up @@ -159,7 +159,7 @@ class IdentifyService {
* @returns {Promise<void>}
*/
async identify (connection) {
const { protocol, stream } = await connection.newStream([MULTICODEC_IDENTIFY, MULTICODEC_IDENTIFY_LEGACY])
const { protocol, stream } = await connection.newStream([MULTICODEC_IDENTIFY, MULTICODEC_IDENTIFY_1_0_0])
const [data] = await pipe(
[],
stream,
Expand Down Expand Up @@ -198,7 +198,7 @@ class IdentifyService {
observedAddr = IdentifyService.getCleanMultiaddr(observedAddr)

// LEGACY: differentiate message with SignedPeerRecord
if (protocol === MULTICODEC_IDENTIFY_LEGACY) {
if (protocol === MULTICODEC_IDENTIFY_1_0_0) {
// Update peers data in PeerStore
this.peerStore.addressBook.set(id, listenAddrs.map((addr) => multiaddr(addr)))
this.peerStore.protoBook.set(id, protocols)
Expand Down Expand Up @@ -249,13 +249,11 @@ class IdentifyService {
handleMessage ({ connection, stream, protocol }) {
switch (protocol) {
case MULTICODEC_IDENTIFY:
case MULTICODEC_IDENTIFY_1_0_0:
return this._handleIdentify({ connection, stream })
case MULTICODEC_IDENTIFY_LEGACY:
return this._handleIdentifyLegacy({ connection, stream })
case MULTICODEC_IDENTIFY_PUSH:
case MULTICODEC_IDENTIFY_PUSH_1_0_0:
return this._handlePush({ connection, stream })
case MULTICODEC_IDENTIFY_PUSH_LEGACY:
return this._handlePushLegacy({ connection, stream })
default:
log.error('cannot handle unknown protocol %s', protocol)
}
Expand All @@ -275,45 +273,14 @@ class IdentifyService {
publicKey = this.peerId.pubKey.bytes
}

const envelope = await this._getSelfPeerRecord()
const signedPeerRecord = envelope.marshal()

const message = Message.encode({
protocolVersion: PROTOCOL_VERSION,
agentVersion: AGENT_VERSION,
publicKey,
signedPeerRecord,
observedAddr: connection.remoteAddr.buffer,
protocols: Array.from(this._protocols.keys())
})

pipe(
[message],
lp.encode(),
stream,
consume
)
}

/**
* Sends the `Identify` response with listen addresses (LEGACY)
* to the requesting peer over the given `connection`
* @private
* @param {object} options
* @param {*} options.stream
* @param {Connection} options.connection
*/
_handleIdentifyLegacy ({ connection, stream }) {
let publicKey = Buffer.alloc(0)
if (this.peerId.pubKey) {
publicKey = this.peerId.pubKey.bytes
}
const signedPeerRecord = await this._getSelfPeerRecord()

const message = Message.encode({
protocolVersion: PROTOCOL_VERSION,
agentVersion: AGENT_VERSION,
publicKey,
listenAddrs: this._libp2p.multiaddrs.map((ma) => ma.buffer),
signedPeerRecord,
observedAddr: connection.remoteAddr.buffer,
protocols: Array.from(this._protocols.keys())
})
Expand Down Expand Up @@ -350,6 +317,22 @@ class IdentifyService {
return log.error('received invalid message', err)
}

const id = connection.remotePeer

// Legacy
if (!message.signedPeerRecord) {
try {
this.peerStore.addressBook.set(id, message.listenAddrs.map((addr) => multiaddr(addr)))
} catch (err) {
return log.error('received invalid listen addrs', err)
}

// Update the protocols
this.peerStore.protoBook.set(id, message.protocols)

return
}

// Open envelope and verify if is authenticated
let envelope
try {
Expand All @@ -369,7 +352,6 @@ class IdentifyService {
}

// Update peers data in PeerStore
const id = connection.remotePeer
try {
// TODO: Store as certified record

Expand Down Expand Up @@ -420,8 +402,8 @@ class IdentifyService {
}

/**
* Get self signed peer record envelope.
* @return {Envelope}
* Get self signed peer record raw envelope.
* @return {Buffer}
*/
async _getSelfPeerRecord () {
// TODO: Verify if updated
Expand All @@ -433,7 +415,9 @@ class IdentifyService {
peerId: this.peerId,
multiaddrs: this._libp2p.multiaddrs
})
this._selfRecord = await Envelope.seal(peerRecord, this.peerId)
const envelope = await Envelope.seal(peerRecord, this.peerId)

this._selfRecord = envelope.marshal()

return this._selfRecord
}
Expand All @@ -446,8 +430,8 @@ module.exports.IdentifyService = IdentifyService
*/
module.exports.multicodecs = {
IDENTIFY: MULTICODEC_IDENTIFY,
IDENTIFY_LEGACY: MULTICODEC_IDENTIFY_LEGACY,
IDENTIFY_1_0_0: MULTICODEC_IDENTIFY_1_0_0,
IDENTIFY_PUSH: MULTICODEC_IDENTIFY_PUSH,
IDENTIFY_PUSH_LEGACY: MULTICODEC_IDENTIFY_PUSH_LEGACY
IDENTIFY_PUSH_1_0_0: MULTICODEC_IDENTIFY_PUSH_1_0_0
}
module.exports.Message = Message
22 changes: 11 additions & 11 deletions test/identify/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ const protocols = new Map([
])

const protocolsLegacy = new Map([
[multicodecs.IDENTIFY_LEGACY, () => { }],
[multicodecs.IDENTIFY_PUSH_LEGACY, () => { }]
[multicodecs.IDENTIFY_1_0_0, () => { }],
[multicodecs.IDENTIFY_PUSH_1_0_0, () => { }]
])

describe('Identify', () => {
Expand All @@ -49,7 +49,7 @@ describe('Identify', () => {
sinon.restore()
})

it('should be able to identify another peer with legacy protocol', async () => {
it('should be able to identify another peer with 1.0.0 legacy protocol', async () => {
const localIdentify = new IdentifyService({
libp2p: {
peerId: localPeer,
Expand Down Expand Up @@ -81,7 +81,7 @@ describe('Identify', () => {
const remoteConnectionMock = { remoteAddr: observedAddr }

const [local, remote] = duplexPair()
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_LEGACY })
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_1_0_0 })

sinon.spy(localIdentify.peerStore.addressBook, 'set')
sinon.spy(localIdentify.peerStore.protoBook, 'set')
Expand All @@ -92,7 +92,7 @@ describe('Identify', () => {
remoteIdentify.handleMessage({
connection: remoteConnectionMock,
stream: remote,
protocol: multicodecs.IDENTIFY_LEGACY
protocol: multicodecs.IDENTIFY_1_0_0
})
])

Expand Down Expand Up @@ -214,7 +214,7 @@ describe('Identify', () => {
})

describe('push', () => {
it('should be able to push identify updates to another peer with legacy protocol', async () => {
it('should be able to push identify updates to another peer with 1.0.0 legacy protocols', async () => {
const connectionManager = new EventEmitter()
connectionManager.getConnection = () => {}

Expand All @@ -225,8 +225,8 @@ describe('Identify', () => {
multiaddrs: listenMaddrs
},
protocols: new Map([
[multicodecs.IDENTIFY_LEGACY],
[multicodecs.IDENTIFY_PUSH_LEGACY],
[multicodecs.IDENTIFY_1_0_0],
[multicodecs.IDENTIFY_PUSH_1_0_0],
['/echo/1.0.0']
])
})
Expand All @@ -247,12 +247,12 @@ describe('Identify', () => {
})

// Setup peer protocols and multiaddrs
const localProtocols = new Set([multicodecs.IDENTIFY_LEGACY, multicodecs.IDENTIFY_PUSH_LEGACY, '/echo/1.0.0'])
const localProtocols = new Set([multicodecs.IDENTIFY_1_0_0, multicodecs.IDENTIFY_PUSH_1_0_0, '/echo/1.0.0'])
const localConnectionMock = { newStream: () => {} }
const remoteConnectionMock = { remotePeer: localPeer }

const [local, remote] = duplexPair()
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_PUSH_LEGACY })
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_PUSH_1_0_0 })

sinon.spy(remoteIdentify.peerStore.addressBook, 'set')
sinon.spy(remoteIdentify.peerStore.protoBook, 'set')
Expand All @@ -263,7 +263,7 @@ describe('Identify', () => {
remoteIdentify.handleMessage({
connection: remoteConnectionMock,
stream: remote,
protocol: multicodecs.IDENTIFY_PUSH_LEGACY
protocol: multicodecs.IDENTIFY_PUSH_1_0_0
})
])

Expand Down

0 comments on commit 1b3bd46

Please sign in to comment.