diff --git a/elements/lisk-p2p/src/p2p.ts b/elements/lisk-p2p/src/p2p.ts index 32c298642ee..cf9f1be9105 100644 --- a/elements/lisk-p2p/src/p2p.ts +++ b/elements/lisk-p2p/src/p2p.ts @@ -534,8 +534,10 @@ export class P2P extends EventEmitter { return this._peerPool.getAllConnectedPeerInfos(); } - public getUniqueConnectedPeers(): ReadonlyArray { - return this._peerPool.getUniqueConnectedPeers(); + public getUniqueOutboundConnectedPeers(): ReadonlyArray< + P2PDiscoveredPeerInfo + > { + return this._peerPool.getUniqueOutboundConnectedPeers(); } public getDisconnectedPeers(): ReadonlyArray { diff --git a/elements/lisk-p2p/src/peer_pool.ts b/elements/lisk-p2p/src/peer_pool.ts index 3e53f64d362..56ef60d3286 100644 --- a/elements/lisk-p2p/src/peer_pool.ts +++ b/elements/lisk-p2p/src/peer_pool.ts @@ -534,12 +534,16 @@ export class PeerPool extends EventEmitter { return peers; } - public getUniqueConnectedPeers(): ReadonlyArray { - return getUniquePeersbyIp(this.getAllConnectedPeerInfos()); + public getUniqueOutboundConnectedPeers(): ReadonlyArray< + P2PDiscoveredPeerInfo + > { + return getUniquePeersbyIp(this.getAllConnectedPeerInfos(OutboundPeer)); } - public getAllConnectedPeerInfos(): ReadonlyArray { - return this.getConnectedPeers().map( + public getAllConnectedPeerInfos( + kind?: typeof OutboundPeer | typeof InboundPeer, + ): ReadonlyArray { + return this.getConnectedPeers(kind).map( peer => peer.peerInfo as P2PDiscoveredPeerInfo, ); } diff --git a/elements/lisk-p2p/test/unit/peer_pool.ts b/elements/lisk-p2p/test/unit/peer_pool.ts index d785a6045b3..5d3d4a3f9ed 100644 --- a/elements/lisk-p2p/test/unit/peer_pool.ts +++ b/elements/lisk-p2p/test/unit/peer_pool.ts @@ -248,27 +248,27 @@ describe('peerPool', () => { }); }); - describe('#getUniqueConnectedPeers', () => { + describe('#getUniqueOutboundConnectedPeers', () => { const samplePeers = initializePeerInfoList(); describe('when two peers have same peer infos', () => { - let uniqueConnectedPeers: ReadonlyArray; + let uniqueOutboundConnectedPeers: ReadonlyArray; beforeEach(async () => { const duplicatesList = [...samplePeers, samplePeers[0], samplePeers[1]]; sandbox .stub(peerPool, 'getAllConnectedPeerInfos') .returns(duplicatesList); - uniqueConnectedPeers = peerPool.getUniqueConnectedPeers(); + uniqueOutboundConnectedPeers = peerPool.getUniqueOutboundConnectedPeers(); }); it('should remove the duplicate peers with the same ips', async () => { - expect(uniqueConnectedPeers).eql(samplePeers); + expect(uniqueOutboundConnectedPeers).eql(samplePeers); }); }); describe('when two peers have same IP and different wsPort and height', () => { - let uniqueConnectedPeers: ReadonlyArray; + let uniqueOutboundConnectedPeers: ReadonlyArray; beforeEach(async () => { const peer1 = { @@ -287,16 +287,16 @@ describe('peerPool', () => { sandbox .stub(peerPool, 'getAllConnectedPeerInfos') .returns(duplicatesList); - uniqueConnectedPeers = peerPool.getUniqueConnectedPeers(); + uniqueOutboundConnectedPeers = peerPool.getUniqueOutboundConnectedPeers(); }); it('should remove the duplicate ip and choose the one with higher height', async () => { - expect(uniqueConnectedPeers).eql(samplePeers); + expect(uniqueOutboundConnectedPeers).eql(samplePeers); }); }); describe('when two peers have same IP and different wsPort but same height', () => { - let uniqueConnectedPeers: ReadonlyArray; + let uniqueOutboundConnectedPeers: ReadonlyArray; beforeEach(async () => { const peer1 = { @@ -315,11 +315,11 @@ describe('peerPool', () => { sandbox .stub(peerPool, 'getAllConnectedPeerInfos') .returns(duplicatesList); - uniqueConnectedPeers = peerPool.getUniqueConnectedPeers(); + uniqueOutboundConnectedPeers = peerPool.getUniqueOutboundConnectedPeers(); }); it('should remove the duplicate ip and choose one of the peer in sequence', async () => { - expect(uniqueConnectedPeers).eql(samplePeers); + expect(uniqueOutboundConnectedPeers).eql(samplePeers); }); }); }); diff --git a/framework/src/modules/chain/peers.js b/framework/src/modules/chain/peers.js index 4c04b873b92..2ab315fff2b 100644 --- a/framework/src/modules/chain/peers.js +++ b/framework/src/modules/chain/peers.js @@ -51,14 +51,20 @@ class Peers { const { broadhash } = await this.channel.invoke('app:getApplicationState'); const activeCount = Math.min( - await this.channel.invoke('network:getConnectedPeersCountByFilter', {}), + await this.channel.invoke( + 'network:getUniqueOutboundConnectedPeersCount', + {}, + ), MAX_PEERS, ); const matchedCount = Math.min( - await this.channel.invoke('network:getConnectedPeersCountByFilter', { - broadhash, - }), + await this.channel.invoke( + 'network:getUniqueOutboundConnectedPeersCount', + { + broadhash, + }, + ), MAX_PEERS, ); diff --git a/framework/src/modules/http_api/controllers/node.js b/framework/src/modules/http_api/controllers/node.js index a7677a67cb6..2bb25024e7e 100644 --- a/framework/src/modules/http_api/controllers/node.js +++ b/framework/src/modules/http_api/controllers/node.js @@ -56,8 +56,9 @@ async function _getForgingStatus(publicKey) { * @private */ async function _getNetworkHeight() { - const peers = await library.channel.invoke('network:getConnectedPeers', { + const peers = await library.channel.invoke('network:getPeers', { limit: 100, + state: 2, }); if (!peers || !peers.length) { return 0; diff --git a/framework/src/modules/http_api/controllers/peers.js b/framework/src/modules/http_api/controllers/peers.js index 3884a6ec7ea..2fe7da8841c 100644 --- a/framework/src/modules/http_api/controllers/peers.js +++ b/framework/src/modules/http_api/controllers/peers.js @@ -68,10 +68,7 @@ PeersController.getPeers = async function getPeers(context, next) { try { const peersByFilters = await channel.invoke('network:getPeers', filters); - const peersCount = await channel.invoke( - 'network:getPeersCountByFilter', - filters, - ); + const peersCount = await channel.invoke('network:getPeersCount', filters); return next(null, { data: peersByFilters, diff --git a/framework/src/modules/network/index.js b/framework/src/modules/network/index.js index 1a5848938eb..28e32866aa9 100644 --- a/framework/src/modules/network/index.js +++ b/framework/src/modules/network/index.js @@ -63,15 +63,12 @@ module.exports = class NetworkModule extends BaseModule { getPeers: { handler: action => this.network.actions.getPeers(action), }, - getConnectedPeers: { - handler: action => this.network.actions.getConnectedPeers(action), + getPeersCount: { + handler: action => this.network.actions.getPeersCount(action), }, - getPeersCountByFilter: { - handler: action => this.network.actions.getPeersCountByFilter(action), - }, - getConnectedPeersCountByFilter: { + getUniqueOutboundConnectedPeersCount: { handler: action => - this.network.actions.getConnectedPeersCountByFilter(action), + this.network.actions.getUniqueOutboundConnectedPeersCount(action), }, }; } diff --git a/framework/src/modules/network/lookup_peers_ips.js b/framework/src/modules/network/lookup_peers_ips.js deleted file mode 100644 index 4b005f5d26f..00000000000 --- a/framework/src/modules/network/lookup_peers_ips.js +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright © 2019 Lisk Foundation - * - * See the LICENSE file at the top-level directory of this distribution - * for licensing information. - * - * Unless otherwise agreed in a custom licensing agreement with the Lisk Foundation, - * no part of this software, including this file, may be copied, modified, - * propagated, or distributed except according to the terms contained in the - * LICENSE file. - * - * Removal or modification of this copyright notice is prohibited. - */ - -'use strict'; - -const net = require('net'); -const dns = require('dns'); - -const lookupPromise = (hostname, options) => - new Promise((resolve, reject) => { - dns.lookup(hostname, options, (err, address) => { - if (err) { - return reject(err); - } - - return resolve(address); - }); - }); - -module.exports = async (peersList, enabled) => { - // If peers layer is not enabled there is no need to create the peer's list - if (!enabled) { - return []; - } - - // In case domain names are used, resolve those to IP addresses. - peersList = await Promise.all( - peersList.map(async peer => { - if (net.isIPv4(peer.ip)) { - return peer; - } - - try { - const address = await lookupPromise(peer.ip, { family: 4 }); - return { - ...peer, - ip: address, - }; - } catch (err) { - console.error( - `Failed to resolve peer domain name ${peer.ip} to an IP address`, - ); - return peer; - } - }), - ); - - return peersList; -}; diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index f40c3bad133..51efa6530ba 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -35,14 +35,9 @@ const { EVENT_UNBAN_PEER, } = require('@liskhq/lisk-p2p'); const randomstring = require('randomstring'); -const lookupPeersIPs = require('./lookup_peers_ips'); const { createLoggerComponent } = require('../../components/logger'); const { createStorageComponent } = require('../../components/storage'); -const { - getByFilter, - getCountByFilter, - getConsolidatedPeersList, -} = require('./filter_peers'); +const { filterByParams, consolidatePeers, lookupPeersIPs } = require('./utils'); const { Peer } = require('./components/storage/entities'); const hasNamespaceReg = /:/; @@ -344,34 +339,31 @@ module.exports = class Network { action.params.peerId, ), getPeers: action => { - const peerList = getConsolidatedPeersList({ + const peers = consolidatePeers({ connectedPeers: this.p2p.getConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), }); - return getByFilter(peerList, action.params); + return filterByParams(peers, action.params); }, - getConnectedPeers: action => { - const peerList = getConsolidatedPeersList({ - connectedPeers: this.p2p.getConnectedPeers(), - }); - - return getByFilter(peerList, action.params); - }, - getPeersCountByFilter: action => { - const peerList = getConsolidatedPeersList({ + getPeersCount: action => { + const peers = consolidatePeers({ connectedPeers: this.p2p.getConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), }); - return getCountByFilter(peerList, action.params); + const { limit, offset, ...filterWithoutLimitOffset } = action.params; + + return filterByParams(peers, filterWithoutLimitOffset).length; }, - getConnectedPeersCountByFilter: action => { - const peerList = getConsolidatedPeersList({ - connectedPeers: this.p2p.getUniqueConnectedPeers(), + getUniqueOutboundConnectedPeersCount: action => { + const peers = consolidatePeers({ + connectedPeers: this.p2p.getUniqueOutboundConnectedPeers(), }); - return getCountByFilter(peerList, action.params); + const { limit, offset, ...filterWithoutLimitOffset } = action.params; + + return filterByParams(peers, filterWithoutLimitOffset).length; }, applyPenalty: action => this.p2p.applyPenalty(action.params.peerId, action.params.penalty), diff --git a/framework/src/modules/network/filter_peers.js b/framework/src/modules/network/utils.js similarity index 74% rename from framework/src/modules/network/filter_peers.js rename to framework/src/modules/network/utils.js index 0ed35fdb622..426d7f6811e 100644 --- a/framework/src/modules/network/filter_peers.js +++ b/framework/src/modules/network/utils.js @@ -15,6 +15,52 @@ const { shuffle } = require('lodash'); const PEER_STATE_CONNECTED = 2; const PEER_STATE_DISCONNECTED = 1; + +const net = require('net'); +const dns = require('dns'); + +const lookupPromise = (hostname, options) => + new Promise((resolve, reject) => { + dns.lookup(hostname, options, (err, address) => { + if (err) { + return reject(err); + } + + return resolve(address); + }); + }); + +const lookupPeersIPs = async (peersList, enabled) => { + // If peers layer is not enabled there is no need to create the peer's list + if (!enabled) { + return []; + } + + // In case domain names are used, resolve those to IP addresses. + peersList = await Promise.all( + peersList.map(async peer => { + if (net.isIPv4(peer.ip)) { + return peer; + } + + try { + const address = await lookupPromise(peer.ip, { family: 4 }); + return { + ...peer, + ip: address, + }; + } catch (err) { + console.error( + `Failed to resolve peer domain name ${peer.ip} to an IP address`, + ); + return peer; + } + }), + ); + + return peersList; +}; + /** * Sorts peers. * @@ -73,13 +119,13 @@ const sortPeers = (field, asc) => (a, b) => { }; /** - * Returns peers length by filter but without offset and limit. + * Returns peers by filter. * @param {Array} peers * @param {Object} filter * @returns {int} count * @todo Add description for the params */ -const getByFilter = (peers, filter) => { +const filterByParams = (peers, filters) => { const allowedFields = [ 'ip', 'wsPort', @@ -97,7 +143,7 @@ const getByFilter = (peers, filter) => { offset: filterOffset, sort, ...otherFilters - } = filter; + } = filters; const limit = filterLimit ? Math.abs(filterLimit) : null; const offset = filterOffset ? Math.abs(filterOffset) : 0; @@ -120,8 +166,8 @@ const getByFilter = (peers, filter) => { }, []); // Sorting - if (filter.sort) { - const sortArray = String(filter.sort).split(':'); + if (filters.sort) { + const sortArray = String(filters.sort).split(':'); const auxSortField = allowedFields.includes(sortArray[0]) ? sortArray[0] : null; @@ -148,27 +194,11 @@ const getByFilter = (peers, filter) => { }; /** - * Returns peers length by filter but without offset and limit. - * @param {Array} peers - * @param {Object} filter - * @returns {int} count - * @todo Add description for the params - */ -const getCountByFilter = (peers, filter) => { - const { limit, offset, ...filterWithoutLimitOffset } = filter; - const peersWithoutLimitOffset = getByFilter(peers, filterWithoutLimitOffset); - - return peersWithoutLimitOffset.length; -}; - -/** - * Returns peers length by filter but without offset and limit. - * @param {Array} peers - * @param {Object} filter - * @returns {int} count + * Returns list of consolidated peers + * @param {Object} * @todo Add description for the params */ -const getConsolidatedPeersList = ({ +const consolidatePeers = ({ connectedPeers = [], disconnectedPeers = [], }) => { @@ -192,7 +222,7 @@ const getConsolidatedPeersList = ({ }; module.exports = { - getByFilter, - getCountByFilter, - getConsolidatedPeersList, + filterByParams, + consolidatePeers, + lookupPeersIPs, }; diff --git a/framework/test/mocha/unit/modules/chain/peers.js b/framework/test/mocha/unit/modules/chain/peers.js index 919f5421534..b0fa8ce5cb5 100644 --- a/framework/test/mocha/unit/modules/chain/peers.js +++ b/framework/test/mocha/unit/modules/chain/peers.js @@ -131,10 +131,10 @@ describe('peers', () => { .returns({ broadhash: prefixedPeer.broadhash }); channelMock.invoke - .withArgs('network:getConnectedPeersCountByFilter') + .withArgs('network:getUniqueOutboundConnectedPeersCount') .returns(2); channelMock.invoke - .withArgs('network:getConnectedPeersCountByFilter', { + .withArgs('network:getUniqueOutboundConnectedPeersCount', { broadhash: prefixedPeer.broadhash, }) .returns(2); @@ -143,19 +143,21 @@ describe('peers', () => { it('should call channel invoke thrice', async () => expect(channelMock.invoke.calledThrice).to.be.true); - it('should call channel invoke with action network:getConnectedPeersCountByFilter', async () => + it('should call channel invoke with action network:getUniqueOutboundConnectedPeersCount', async () => expect( channelMock.invoke.calledWithExactly( - 'network:getConnectedPeersCountByFilter', + 'network:getUniqueOutboundConnectedPeersCount', {}, ), ).to.be.true); - it('should call channel invoke with action network:getConnectedPeersCountByFilter and filter broadhash', async () => + it('should call channel invoke with action network:getUniqueOutboundConnectedPeersCount and filter broadhash', async () => expect( channelMock.invoke.calledWithExactly( - 'network:getConnectedPeersCountByFilter', - { broadhash: prefixedPeer.broadhash }, + 'network:getUniqueOutboundConnectedPeersCount', + { + broadhash: prefixedPeer.broadhash, + }, ), ).to.be.true); @@ -169,10 +171,10 @@ describe('peers', () => { describe('when half of connected peers match our broadhash', () => { before(async () => { channelMock.invoke - .withArgs('network:getConnectedPeersCountByFilter') + .withArgs('network:getUniqueOutboundConnectedPeersCount') .returns(2); channelMock.invoke - .withArgs('network:getConnectedPeersCountByFilter', { + .withArgs('network:getUniqueOutboundConnectedPeersCount', { broadhash: prefixedPeer.broadhash, }) .returns(1); diff --git a/framework/test/mocha/unit/modules/http_api/controllers/node.js b/framework/test/mocha/unit/modules/http_api/controllers/node.js index 81052be1c0c..87bc1bd9512 100644 --- a/framework/test/mocha/unit/modules/http_api/controllers/node.js +++ b/framework/test/mocha/unit/modules/http_api/controllers/node.js @@ -178,9 +178,7 @@ describe('node/api', () => { describe('when chain:getNodeStatus answers with all parameters', () => { beforeEach(async () => { channelStub.invoke.withArgs('chain:getNodeStatus').returns(status); - channelStub.invoke - .withArgs('network:getConnectedPeers') - .returns(defaultPeers); + channelStub.invoke.withArgs('network:getPeers').returns(defaultPeers); }); it('should return an object status with all properties', async () => @@ -205,9 +203,7 @@ describe('node/api', () => { channelStub.invoke .withArgs('chain:getNodeStatus') .returns(statusWithoutSomeParameters); - channelStub.invoke - .withArgs('network:getConnectedPeers') - .returns(defaultPeers); + channelStub.invoke.withArgs('network:getPeers').returns(defaultPeers); }); it('should return an object status with some properties to 0', async () => @@ -244,9 +240,7 @@ describe('node/api', () => { height: 438, }, ]; - channelStub.invoke - .withArgs('network:getConnectedPeers') - .returns(defaultPeers); + channelStub.invoke.withArgs('network:getPeers').returns(defaultPeers); networkHeight = await getNetworkHeight(); }); @@ -255,9 +249,9 @@ describe('node/api', () => { }); }); - describe('When network:getConnectedPeers returns a blank list of peers', () => { + describe('When network:getPeers returns a blank list of peers', () => { beforeEach(async () => { - channelStub.invoke.withArgs('network:getConnectedPeers').returns([]); + channelStub.invoke.withArgs('network:getPeers').returns([]); networkHeight = await getNetworkHeight(); }); @@ -266,7 +260,7 @@ describe('node/api', () => { }); }); - describe('When network:getConnectedPeers returns a list of peers with 2 different equal set of peers height', () => { + describe('When network:getPeers returns a list of peers with 2 different equal set of peers height', () => { beforeEach(async () => { const defaultPeers = [ { @@ -288,9 +282,7 @@ describe('node/api', () => { height: MAJORITY_HEIGHT, }, ]; - channelStub.invoke - .withArgs('network:getConnectedPeers') - .returns(defaultPeers); + channelStub.invoke.withArgs('network:getPeers').returns(defaultPeers); networkHeight = await getNetworkHeight(); }); @@ -299,7 +291,7 @@ describe('node/api', () => { }); }); - describe('When network:getConnectedPeers returns a list of peers with 3 different equal set of peers height', () => { + describe('When network:getPeers returns a list of peers with 3 different equal set of peers height', () => { beforeEach(async () => { const defaultPeers = [ { @@ -330,9 +322,7 @@ describe('node/api', () => { height: MAJORITY_HEIGHT, }, ]; - channelStub.invoke - .withArgs('network:getConnectedPeers') - .returns(defaultPeers); + channelStub.invoke.withArgs('network:getPeers').returns(defaultPeers); networkHeight = await getNetworkHeight(); }); @@ -341,7 +331,7 @@ describe('node/api', () => { }); }); - describe('When network:getConnectedPeers returns a list of peers with 2 different unequal set of peers height', () => { + describe('When network:getPeers returns a list of peers with 2 different unequal set of peers height', () => { beforeEach(async () => { const defaultPeers = [ { @@ -363,9 +353,7 @@ describe('node/api', () => { height: MAJORITY_HEIGHT + 1, }, ]; - channelStub.invoke - .withArgs('network:getConnectedPeers') - .returns(defaultPeers); + channelStub.invoke.withArgs('network:getPeers').returns(defaultPeers); networkHeight = await getNetworkHeight(); }); @@ -374,16 +362,14 @@ describe('node/api', () => { }); }); - describe('When network:getConnectedPeers returns only one peer', () => { + describe('When network:getPeers returns only one peer', () => { beforeEach(async () => { const defaultPeers = [ { height: MAJORITY_HEIGHT, }, ]; - channelStub.invoke - .withArgs('network:getConnectedPeers') - .returns(defaultPeers); + channelStub.invoke.withArgs('network:getPeers').returns(defaultPeers); networkHeight = await getNetworkHeight(); }); @@ -392,7 +378,7 @@ describe('node/api', () => { }); }); - describe('When network:getConnectedPeers returns majority of peers with very low height compared to others', () => { + describe('When network:getPeers returns majority of peers with very low height compared to others', () => { beforeEach(async () => { const defaultPeers = [ { @@ -414,9 +400,7 @@ describe('node/api', () => { height: 1, }, ]; - channelStub.invoke - .withArgs('network:getConnectedPeers') - .returns(defaultPeers); + channelStub.invoke.withArgs('network:getPeers').returns(defaultPeers); networkHeight = await getNetworkHeight(); }); diff --git a/framework/test/mocha/unit/modules/network/lookup_peers_ips.js b/framework/test/mocha/unit/modules/network/lookup_peers_ips.js index 9fc630b50c1..afc09310009 100644 --- a/framework/test/mocha/unit/modules/network/lookup_peers_ips.js +++ b/framework/test/mocha/unit/modules/network/lookup_peers_ips.js @@ -14,7 +14,7 @@ 'use strict'; -const lookupPeersIps = require('../../../../../src/modules/network/lookup_peers_ips'); +const { lookupPeersIPs } = require('../../../../../src/modules/network/utils'); const { peers: { list }, } = require('../../../data/app_config.json'); @@ -25,7 +25,7 @@ const ipv4Regex = new RegExp( describe('init_steps/lookup_peers_ips', () => { it('should return empty array if peers are not enabled', async () => { - const result = await lookupPeersIps(list, false); + const result = await lookupPeersIPs(list, false); return expect(result).to.eql([]); }); @@ -39,7 +39,7 @@ describe('init_steps/lookup_peers_ips', () => { }); it('should throw error when failed to resolve hostname', async () => { - await lookupPeersIps([{ ip: 'https://lisk.io/' }], true); + await lookupPeersIPs([{ ip: 'https://lisk.io/' }], true); expect(spyConsoleError).to.be.calledOnce; return expect(spyConsoleError).to.be.calledWith( @@ -48,7 +48,7 @@ describe('init_steps/lookup_peers_ips', () => { }); it('should resolve hostnames to ip address', async () => { - const resolvedIps = await lookupPeersIps(list, true); + const resolvedIps = await lookupPeersIPs(list, true); expect(resolvedIps.length).to.eql(list.length); return resolvedIps.forEach(peer => {