From 2569edf90163c9acbb42458eb0c1cef24b93d898 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Thu, 22 Aug 2019 16:16:33 +0200 Subject: [PATCH 01/14] Remove redundant byFilter tag from network actions --- framework/src/modules/chain/peers.js | 4 ++-- .../src/modules/http_api/controllers/peers.js | 5 +---- framework/src/modules/network/index.js | 9 ++++----- framework/src/modules/network/network.js | 4 ++-- framework/test/mocha/unit/modules/chain/peers.js | 16 ++++++++-------- 5 files changed, 17 insertions(+), 21 deletions(-) diff --git a/framework/src/modules/chain/peers.js b/framework/src/modules/chain/peers.js index 4c04b873b92..254565f843d 100644 --- a/framework/src/modules/chain/peers.js +++ b/framework/src/modules/chain/peers.js @@ -51,12 +51,12 @@ 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:getConnectedPeersCount', {}), MAX_PEERS, ); const matchedCount = Math.min( - await this.channel.invoke('network:getConnectedPeersCountByFilter', { + await this.channel.invoke('network:getConnectedPeersCount', { broadhash, }), MAX_PEERS, 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..c9f0d1c3201 100644 --- a/framework/src/modules/network/index.js +++ b/framework/src/modules/network/index.js @@ -66,12 +66,11 @@ module.exports = class NetworkModule extends BaseModule { getConnectedPeers: { handler: action => this.network.actions.getConnectedPeers(action), }, - getPeersCountByFilter: { - handler: action => this.network.actions.getPeersCountByFilter(action), + getPeersCount: { + handler: action => this.network.actions.getPeersCount(action), }, - getConnectedPeersCountByFilter: { - handler: action => - this.network.actions.getConnectedPeersCountByFilter(action), + getConnectedPeersCount: { + handler: action => this.network.actions.getConnectedPeersCount(action), }, }; } diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index f40c3bad133..6586cb39120 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -358,7 +358,7 @@ module.exports = class Network { return getByFilter(peerList, action.params); }, - getPeersCountByFilter: action => { + getPeersCount: action => { const peerList = getConsolidatedPeersList({ connectedPeers: this.p2p.getConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), @@ -366,7 +366,7 @@ module.exports = class Network { return getCountByFilter(peerList, action.params); }, - getConnectedPeersCountByFilter: action => { + getConnectedPeersCount: action => { const peerList = getConsolidatedPeersList({ connectedPeers: this.p2p.getUniqueConnectedPeers(), }); diff --git a/framework/test/mocha/unit/modules/chain/peers.js b/framework/test/mocha/unit/modules/chain/peers.js index 919f5421534..58b9bfa4c68 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:getConnectedPeersCount') .returns(2); channelMock.invoke - .withArgs('network:getConnectedPeersCountByFilter', { + .withArgs('network:getConnectedPeersCount', { broadhash: prefixedPeer.broadhash, }) .returns(2); @@ -143,18 +143,18 @@ 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:getConnectedPeersCount', async () => expect( channelMock.invoke.calledWithExactly( - 'network:getConnectedPeersCountByFilter', + 'network:getConnectedPeersCount', {}, ), ).to.be.true); - it('should call channel invoke with action network:getConnectedPeersCountByFilter and filter broadhash', async () => + it('should call channel invoke with action network:getConnectedPeersCount and filter broadhash', async () => expect( channelMock.invoke.calledWithExactly( - 'network:getConnectedPeersCountByFilter', + 'network:getConnectedPeersCount', { broadhash: prefixedPeer.broadhash }, ), ).to.be.true); @@ -169,10 +169,10 @@ describe('peers', () => { describe('when half of connected peers match our broadhash', () => { before(async () => { channelMock.invoke - .withArgs('network:getConnectedPeersCountByFilter') + .withArgs('network:getConnectedPeersCount') .returns(2); channelMock.invoke - .withArgs('network:getConnectedPeersCountByFilter', { + .withArgs('network:getConnectedPeersCount', { broadhash: prefixedPeer.broadhash, }) .returns(1); From e3f3a82c948d0b5940d4b54d25cb6257f353b46d Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Thu, 22 Aug 2019 16:19:45 +0200 Subject: [PATCH 02/14] Remove unneeded list tag from network action --- framework/src/modules/network/filter_peers.js | 4 ++-- framework/src/modules/network/network.js | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/framework/src/modules/network/filter_peers.js b/framework/src/modules/network/filter_peers.js index 0ed35fdb622..e861ef5b9d6 100644 --- a/framework/src/modules/network/filter_peers.js +++ b/framework/src/modules/network/filter_peers.js @@ -168,7 +168,7 @@ const getCountByFilter = (peers, filter) => { * @returns {int} count * @todo Add description for the params */ -const getConsolidatedPeersList = ({ +const getConsolidatedPeers = ({ connectedPeers = [], disconnectedPeers = [], }) => { @@ -194,5 +194,5 @@ const getConsolidatedPeersList = ({ module.exports = { getByFilter, getCountByFilter, - getConsolidatedPeersList, + getConsolidatedPeers, }; diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index 6586cb39120..dd3ed0708d9 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -41,7 +41,7 @@ const { createStorageComponent } = require('../../components/storage'); const { getByFilter, getCountByFilter, - getConsolidatedPeersList, + getConsolidatedPeers, } = require('./filter_peers'); const { Peer } = require('./components/storage/entities'); @@ -344,34 +344,34 @@ module.exports = class Network { action.params.peerId, ), getPeers: action => { - const peerList = getConsolidatedPeersList({ + const peers = getConsolidatedPeers({ connectedPeers: this.p2p.getConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), }); - return getByFilter(peerList, action.params); + return getByFilter(peers, action.params); }, getConnectedPeers: action => { - const peerList = getConsolidatedPeersList({ + const peers = getConsolidatedPeers({ connectedPeers: this.p2p.getConnectedPeers(), }); - return getByFilter(peerList, action.params); + return getByFilter(peers, action.params); }, getPeersCount: action => { - const peerList = getConsolidatedPeersList({ + const peers = getConsolidatedPeers({ connectedPeers: this.p2p.getConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), }); - return getCountByFilter(peerList, action.params); + return getCountByFilter(peers, action.params); }, getConnectedPeersCount: action => { - const peerList = getConsolidatedPeersList({ + const peers = getConsolidatedPeers({ connectedPeers: this.p2p.getUniqueConnectedPeers(), }); - return getCountByFilter(peerList, action.params); + return getCountByFilter(peers, action.params); }, applyPenalty: action => this.p2p.applyPenalty(action.params.peerId, action.params.penalty), From 3de161d3292d60203c4472aa1286054e4151f93f Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Thu, 22 Aug 2019 16:58:18 +0200 Subject: [PATCH 03/14] Remove redundant actions from network module --- framework/src/modules/chain/peers.js | 5 +- .../src/modules/http_api/controllers/node.js | 3 +- framework/src/modules/network/index.js | 6 --- framework/src/modules/network/network.js | 16 +------ .../test/mocha/unit/modules/chain/peers.js | 33 ++++++------- .../unit/modules/http_api/controllers/node.js | 46 ++++++------------- 6 files changed, 36 insertions(+), 73 deletions(-) diff --git a/framework/src/modules/chain/peers.js b/framework/src/modules/chain/peers.js index 254565f843d..0ca8778eb56 100644 --- a/framework/src/modules/chain/peers.js +++ b/framework/src/modules/chain/peers.js @@ -51,13 +51,14 @@ class Peers { const { broadhash } = await this.channel.invoke('app:getApplicationState'); const activeCount = Math.min( - await this.channel.invoke('network:getConnectedPeersCount', {}), + await this.channel.invoke('network:getPeersCount', { state: 2 }), MAX_PEERS, ); const matchedCount = Math.min( - await this.channel.invoke('network:getConnectedPeersCount', { + await this.channel.invoke('network:getPeersCount', { broadhash, + state: 2, }), 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/network/index.js b/framework/src/modules/network/index.js index c9f0d1c3201..e27e27ec397 100644 --- a/framework/src/modules/network/index.js +++ b/framework/src/modules/network/index.js @@ -63,15 +63,9 @@ 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), }, - getConnectedPeersCount: { - handler: action => this.network.actions.getConnectedPeersCount(action), - }, }; } diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index dd3ed0708d9..52ec9139146 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -351,24 +351,10 @@ module.exports = class Network { return getByFilter(peers, action.params); }, - getConnectedPeers: action => { - const peers = getConsolidatedPeers({ - connectedPeers: this.p2p.getConnectedPeers(), - }); - - return getByFilter(peers, action.params); - }, getPeersCount: action => { - const peers = getConsolidatedPeers({ - connectedPeers: this.p2p.getConnectedPeers(), - disconnectedPeers: this.p2p.getDisconnectedPeers(), - }); - - return getCountByFilter(peers, action.params); - }, - getConnectedPeersCount: action => { const peers = getConsolidatedPeers({ connectedPeers: this.p2p.getUniqueConnectedPeers(), + disconnectedPeers: this.p2p.getDisconnectedPeers(), }); return getCountByFilter(peers, action.params); diff --git a/framework/test/mocha/unit/modules/chain/peers.js b/framework/test/mocha/unit/modules/chain/peers.js index 58b9bfa4c68..cbb6314cfdb 100644 --- a/framework/test/mocha/unit/modules/chain/peers.js +++ b/framework/test/mocha/unit/modules/chain/peers.js @@ -130,12 +130,11 @@ describe('peers', () => { .withArgs('app:getApplicationState') .returns({ broadhash: prefixedPeer.broadhash }); + channelMock.invoke.withArgs('network:getPeersCount').returns(2); channelMock.invoke - .withArgs('network:getConnectedPeersCount') - .returns(2); - channelMock.invoke - .withArgs('network:getConnectedPeersCount', { + .withArgs('network:getPeersCount', { broadhash: prefixedPeer.broadhash, + state: 2, }) .returns(2); }); @@ -143,20 +142,19 @@ describe('peers', () => { it('should call channel invoke thrice', async () => expect(channelMock.invoke.calledThrice).to.be.true); - it('should call channel invoke with action network:getConnectedPeersCount', async () => + it('should call channel invoke with action network:getPeersCount', async () => expect( - channelMock.invoke.calledWithExactly( - 'network:getConnectedPeersCount', - {}, - ), + channelMock.invoke.calledWithExactly('network:getPeersCount', { + state: 2, + }), ).to.be.true); - it('should call channel invoke with action network:getConnectedPeersCount and filter broadhash', async () => + it('should call channel invoke with action network:getPeersCount and filter broadhash', async () => expect( - channelMock.invoke.calledWithExactly( - 'network:getConnectedPeersCount', - { broadhash: prefixedPeer.broadhash }, - ), + channelMock.invoke.calledWithExactly('network:getPeersCount', { + broadhash: prefixedPeer.broadhash, + state: 2, + }), ).to.be.true); it('should return consensus as a number', async () => @@ -168,12 +166,11 @@ describe('peers', () => { describe('when half of connected peers match our broadhash', () => { before(async () => { + channelMock.invoke.withArgs('network:getPeersCount').returns(2); channelMock.invoke - .withArgs('network:getConnectedPeersCount') - .returns(2); - channelMock.invoke - .withArgs('network:getConnectedPeersCount', { + .withArgs('network:getPeersCount', { broadhash: prefixedPeer.broadhash, + state: 2, }) .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(); }); From 5bf26461e0832dc6102eb980d604579875d3211a Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Thu, 22 Aug 2019 17:05:13 +0200 Subject: [PATCH 04/14] Remove misleading get tag from network util function --- framework/src/modules/network/filter_peers.js | 4 ++-- framework/src/modules/network/network.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/framework/src/modules/network/filter_peers.js b/framework/src/modules/network/filter_peers.js index e861ef5b9d6..c5d6f2b2d1d 100644 --- a/framework/src/modules/network/filter_peers.js +++ b/framework/src/modules/network/filter_peers.js @@ -168,7 +168,7 @@ const getCountByFilter = (peers, filter) => { * @returns {int} count * @todo Add description for the params */ -const getConsolidatedPeers = ({ +const consolidatedPeers = ({ connectedPeers = [], disconnectedPeers = [], }) => { @@ -194,5 +194,5 @@ const getConsolidatedPeers = ({ module.exports = { getByFilter, getCountByFilter, - getConsolidatedPeers, + consolidatedPeers, }; diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index 52ec9139146..9b1aaa20941 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -41,7 +41,7 @@ const { createStorageComponent } = require('../../components/storage'); const { getByFilter, getCountByFilter, - getConsolidatedPeers, + consolidatedPeers, } = require('./filter_peers'); const { Peer } = require('./components/storage/entities'); @@ -344,7 +344,7 @@ module.exports = class Network { action.params.peerId, ), getPeers: action => { - const peers = getConsolidatedPeers({ + const peers = consolidatedPeers({ connectedPeers: this.p2p.getConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), }); @@ -352,7 +352,7 @@ module.exports = class Network { return getByFilter(peers, action.params); }, getPeersCount: action => { - const peers = getConsolidatedPeers({ + const peers = consolidatedPeers({ connectedPeers: this.p2p.getUniqueConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), }); From 2a63774e849bf385a136958556ac6eb98a64e2b3 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Thu, 22 Aug 2019 17:07:20 +0200 Subject: [PATCH 05/14] Rename network utils file --- framework/src/modules/network/network.js | 6 +----- framework/src/modules/network/{filter_peers.js => utils.js} | 0 2 files changed, 1 insertion(+), 5 deletions(-) rename framework/src/modules/network/{filter_peers.js => utils.js} (100%) diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index 9b1aaa20941..75509e92759 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -38,11 +38,7 @@ const randomstring = require('randomstring'); const lookupPeersIPs = require('./lookup_peers_ips'); const { createLoggerComponent } = require('../../components/logger'); const { createStorageComponent } = require('../../components/storage'); -const { - getByFilter, - getCountByFilter, - consolidatedPeers, -} = require('./filter_peers'); +const { getByFilter, getCountByFilter, consolidatedPeers } = require('./utils'); const { Peer } = require('./components/storage/entities'); const hasNamespaceReg = /:/; diff --git a/framework/src/modules/network/filter_peers.js b/framework/src/modules/network/utils.js similarity index 100% rename from framework/src/modules/network/filter_peers.js rename to framework/src/modules/network/utils.js From 99797e973d624f3ef42249049ac28e2c62e40905 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Thu, 22 Aug 2019 17:11:51 +0200 Subject: [PATCH 06/14] Merge lookupPeersIPs into network utils --- .../src/modules/network/lookup_peers_ips.js | 60 ------------------- framework/src/modules/network/network.js | 8 ++- framework/src/modules/network/utils.js | 47 +++++++++++++++ .../unit/modules/network/lookup_peers_ips.js | 2 +- 4 files changed, 54 insertions(+), 63 deletions(-) delete mode 100644 framework/src/modules/network/lookup_peers_ips.js 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 75509e92759..07110b4407b 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -35,10 +35,14 @@ 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, consolidatedPeers } = require('./utils'); +const { + getByFilter, + getCountByFilter, + consolidatedPeers, + lookupPeersIPs, +} = require('./utils'); const { Peer } = require('./components/storage/entities'); const hasNamespaceReg = /:/; diff --git a/framework/src/modules/network/utils.js b/framework/src/modules/network/utils.js index c5d6f2b2d1d..7a0f6370566 100644 --- a/framework/src/modules/network/utils.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. * @@ -195,4 +241,5 @@ module.exports = { getByFilter, getCountByFilter, consolidatedPeers, + lookupPeersIPs, }; 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..5368fc17832 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'); From 2798b7ec7e3fd329c29269b46739adefc60760e0 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Thu, 22 Aug 2019 17:16:31 +0200 Subject: [PATCH 07/14] Remove unnecessary getCountByFilter network util --- framework/src/modules/network/network.js | 11 ++++------- framework/src/modules/network/utils.js | 15 --------------- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index 07110b4407b..aaeed6d1537 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -37,12 +37,7 @@ const { const randomstring = require('randomstring'); const { createLoggerComponent } = require('../../components/logger'); const { createStorageComponent } = require('../../components/storage'); -const { - getByFilter, - getCountByFilter, - consolidatedPeers, - lookupPeersIPs, -} = require('./utils'); +const { getByFilter, consolidatedPeers, lookupPeersIPs } = require('./utils'); const { Peer } = require('./components/storage/entities'); const hasNamespaceReg = /:/; @@ -357,7 +352,9 @@ module.exports = class Network { disconnectedPeers: this.p2p.getDisconnectedPeers(), }); - return getCountByFilter(peers, action.params); + const { limit, offset, ...filterWithoutLimitOffset } = action.params; + + return getByFilter(peers, filterWithoutLimitOffset).length; }, applyPenalty: action => this.p2p.applyPenalty(action.params.peerId, action.params.penalty), diff --git a/framework/src/modules/network/utils.js b/framework/src/modules/network/utils.js index 7a0f6370566..8c9af989613 100644 --- a/framework/src/modules/network/utils.js +++ b/framework/src/modules/network/utils.js @@ -193,20 +193,6 @@ const getByFilter = (peers, filter) => { return filteredPeers; }; -/** - * 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 @@ -239,7 +225,6 @@ const consolidatedPeers = ({ module.exports = { getByFilter, - getCountByFilter, consolidatedPeers, lookupPeersIPs, }; From 3827e6d1428a7773699c7358ae37b7705a1fe03d Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Thu, 22 Aug 2019 17:19:04 +0200 Subject: [PATCH 08/14] Rename getByFilter network util to filterByParams --- framework/src/modules/network/network.js | 10 +++++++--- framework/src/modules/network/utils.js | 18 ++++++++---------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index aaeed6d1537..bb6a36a6289 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -37,7 +37,11 @@ const { const randomstring = require('randomstring'); const { createLoggerComponent } = require('../../components/logger'); const { createStorageComponent } = require('../../components/storage'); -const { getByFilter, consolidatedPeers, lookupPeersIPs } = require('./utils'); +const { + filterByParams, + consolidatedPeers, + lookupPeersIPs, +} = require('./utils'); const { Peer } = require('./components/storage/entities'); const hasNamespaceReg = /:/; @@ -344,7 +348,7 @@ module.exports = class Network { disconnectedPeers: this.p2p.getDisconnectedPeers(), }); - return getByFilter(peers, action.params); + return filterByParams(peers, action.params); }, getPeersCount: action => { const peers = consolidatedPeers({ @@ -354,7 +358,7 @@ module.exports = class Network { const { limit, offset, ...filterWithoutLimitOffset } = action.params; - return getByFilter(peers, filterWithoutLimitOffset).length; + return filterByParams(peers, filterWithoutLimitOffset).length; }, applyPenalty: action => this.p2p.applyPenalty(action.params.peerId, action.params.penalty), diff --git a/framework/src/modules/network/utils.js b/framework/src/modules/network/utils.js index 8c9af989613..cc430df8088 100644 --- a/framework/src/modules/network/utils.js +++ b/framework/src/modules/network/utils.js @@ -119,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', @@ -143,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; @@ -166,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; @@ -194,10 +194,8 @@ const getByFilter = (peers, filter) => { }; /** - * 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} {connectedPeers , disconnectedPeers} * @todo Add description for the params */ const consolidatedPeers = ({ @@ -224,7 +222,7 @@ const consolidatedPeers = ({ }; module.exports = { - getByFilter, + filterByParams, consolidatedPeers, lookupPeersIPs, }; From bd4451d5fb0228221bfa57ccfb0c4d7f48a0e2e7 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Thu, 22 Aug 2019 17:20:43 +0200 Subject: [PATCH 09/14] Fix typo in network util name --- framework/src/modules/network/network.js | 10 +++------- framework/src/modules/network/utils.js | 6 +++--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index bb6a36a6289..e68e357bd5a 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -37,11 +37,7 @@ const { const randomstring = require('randomstring'); const { createLoggerComponent } = require('../../components/logger'); const { createStorageComponent } = require('../../components/storage'); -const { - filterByParams, - consolidatedPeers, - lookupPeersIPs, -} = require('./utils'); +const { filterByParams, consolidatePeers, lookupPeersIPs } = require('./utils'); const { Peer } = require('./components/storage/entities'); const hasNamespaceReg = /:/; @@ -343,7 +339,7 @@ module.exports = class Network { action.params.peerId, ), getPeers: action => { - const peers = consolidatedPeers({ + const peers = consolidatePeers({ connectedPeers: this.p2p.getConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), }); @@ -351,7 +347,7 @@ module.exports = class Network { return filterByParams(peers, action.params); }, getPeersCount: action => { - const peers = consolidatedPeers({ + const peers = consolidatePeers({ connectedPeers: this.p2p.getUniqueConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), }); diff --git a/framework/src/modules/network/utils.js b/framework/src/modules/network/utils.js index cc430df8088..426d7f6811e 100644 --- a/framework/src/modules/network/utils.js +++ b/framework/src/modules/network/utils.js @@ -195,10 +195,10 @@ const filterByParams = (peers, filters) => { /** * Returns list of consolidated peers - * @param {Object} {connectedPeers , disconnectedPeers} + * @param {Object} * @todo Add description for the params */ -const consolidatedPeers = ({ +const consolidatePeers = ({ connectedPeers = [], disconnectedPeers = [], }) => { @@ -223,6 +223,6 @@ const consolidatedPeers = ({ module.exports = { filterByParams, - consolidatedPeers, + consolidatePeers, lookupPeersIPs, }; From a063860383af14c9b994d2c9599e572fe3843c12 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Fri, 23 Aug 2019 12:13:19 +0200 Subject: [PATCH 10/14] Keep getUniqueConnectedPeersCount() network action to calculate consensus --- framework/src/modules/chain/peers.js | 5 ++- framework/src/modules/network/network.js | 11 +++++- .../test/mocha/unit/modules/chain/peers.js | 34 +++++++++++-------- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/framework/src/modules/chain/peers.js b/framework/src/modules/chain/peers.js index 0ca8778eb56..ca29f6f1b75 100644 --- a/framework/src/modules/chain/peers.js +++ b/framework/src/modules/chain/peers.js @@ -51,14 +51,13 @@ class Peers { const { broadhash } = await this.channel.invoke('app:getApplicationState'); const activeCount = Math.min( - await this.channel.invoke('network:getPeersCount', { state: 2 }), + await this.channel.invoke('network:getUniqueConnectedPeersCount'), MAX_PEERS, ); const matchedCount = Math.min( - await this.channel.invoke('network:getPeersCount', { + await this.channel.invoke('network:getUniqueConnectedPeersCount', { broadhash, - state: 2, }), MAX_PEERS, ); diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index e68e357bd5a..0d534d7d3b0 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -348,7 +348,7 @@ module.exports = class Network { }, getPeersCount: action => { const peers = consolidatePeers({ - connectedPeers: this.p2p.getUniqueConnectedPeers(), + connectedPeers: this.p2p.getConnectedPeers(), disconnectedPeers: this.p2p.getDisconnectedPeers(), }); @@ -356,6 +356,15 @@ module.exports = class Network { return filterByParams(peers, filterWithoutLimitOffset).length; }, + getUniqueConnectedPeersCount: action => { + const peers = consolidatePeers({ + connectedPeers: this.p2p.getUniqueConnectedPeers(), + }); + + 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/test/mocha/unit/modules/chain/peers.js b/framework/test/mocha/unit/modules/chain/peers.js index cbb6314cfdb..33154166a51 100644 --- a/framework/test/mocha/unit/modules/chain/peers.js +++ b/framework/test/mocha/unit/modules/chain/peers.js @@ -130,11 +130,12 @@ describe('peers', () => { .withArgs('app:getApplicationState') .returns({ broadhash: prefixedPeer.broadhash }); - channelMock.invoke.withArgs('network:getPeersCount').returns(2); channelMock.invoke - .withArgs('network:getPeersCount', { + .withArgs('network:getUniqueConnectedPeersCount') + .returns(2); + channelMock.invoke + .withArgs('network:getUniqueConnectedPeersCount', { broadhash: prefixedPeer.broadhash, - state: 2, }) .returns(2); }); @@ -142,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:getPeersCount', async () => + it('should call channel invoke with action network:getUniqueConnectedPeersCount', async () => expect( - channelMock.invoke.calledWithExactly('network:getPeersCount', { - state: 2, - }), + channelMock.invoke.calledWithExactly( + 'network:getUniqueConnectedPeersCount', + ), ).to.be.true); - it('should call channel invoke with action network:getPeersCount and filter broadhash', async () => + it('should call channel invoke with action network:getUniqueConnectedPeersCount and filter broadhash', async () => expect( - channelMock.invoke.calledWithExactly('network:getPeersCount', { - broadhash: prefixedPeer.broadhash, - state: 2, - }), + channelMock.invoke.calledWithExactly( + 'network:getUniqueConnectedPeersCount', + { + broadhash: prefixedPeer.broadhash, + }, + ), ).to.be.true); it('should return consensus as a number', async () => @@ -166,11 +169,12 @@ describe('peers', () => { describe('when half of connected peers match our broadhash', () => { before(async () => { - channelMock.invoke.withArgs('network:getPeersCount').returns(2); channelMock.invoke - .withArgs('network:getPeersCount', { + .withArgs('network:getUniqueConnectedPeersCount') + .returns(2); + channelMock.invoke + .withArgs('network:getUniqueConnectedPeersCount', { broadhash: prefixedPeer.broadhash, - state: 2, }) .returns(1); }); From 88cdbacf50f253b95f7f5a32a136b6e6076f2768 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Fri, 23 Aug 2019 13:10:14 +0200 Subject: [PATCH 11/14] Calculate broadhash consensus only from Outbound peers --- elements/lisk-p2p/src/p2p.ts | 6 ++++-- elements/lisk-p2p/src/peer_pool.ts | 12 ++++++++---- elements/lisk-p2p/test/unit/peer_pool.ts | 18 +++++++++--------- framework/src/modules/chain/peers.js | 11 +++++++---- framework/src/modules/network/network.js | 4 ++-- .../test/mocha/unit/modules/chain/peers.js | 16 ++++++++-------- 6 files changed, 38 insertions(+), 29 deletions(-) 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 2f3baf79a52..dff44e60c19 100644 --- a/elements/lisk-p2p/src/peer_pool.ts +++ b/elements/lisk-p2p/src/peer_pool.ts @@ -527,12 +527,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..88961b6120a 100644 --- a/elements/lisk-p2p/test/unit/peer_pool.ts +++ b/elements/lisk-p2p/test/unit/peer_pool.ts @@ -252,23 +252,23 @@ describe('peerPool', () => { 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 ca29f6f1b75..d2192ff6041 100644 --- a/framework/src/modules/chain/peers.js +++ b/framework/src/modules/chain/peers.js @@ -51,14 +51,17 @@ class Peers { const { broadhash } = await this.channel.invoke('app:getApplicationState'); const activeCount = Math.min( - await this.channel.invoke('network:getUniqueConnectedPeersCount'), + await this.channel.invoke('network:getUniqueOutboundConnectedPeersCount'), MAX_PEERS, ); const matchedCount = Math.min( - await this.channel.invoke('network:getUniqueConnectedPeersCount', { - broadhash, - }), + await this.channel.invoke( + 'network:getUniqueOutboundConnectedPeersCount', + { + broadhash, + }, + ), MAX_PEERS, ); diff --git a/framework/src/modules/network/network.js b/framework/src/modules/network/network.js index 0d534d7d3b0..51efa6530ba 100644 --- a/framework/src/modules/network/network.js +++ b/framework/src/modules/network/network.js @@ -356,9 +356,9 @@ module.exports = class Network { return filterByParams(peers, filterWithoutLimitOffset).length; }, - getUniqueConnectedPeersCount: action => { + getUniqueOutboundConnectedPeersCount: action => { const peers = consolidatePeers({ - connectedPeers: this.p2p.getUniqueConnectedPeers(), + connectedPeers: this.p2p.getUniqueOutboundConnectedPeers(), }); const { limit, offset, ...filterWithoutLimitOffset } = action.params; diff --git a/framework/test/mocha/unit/modules/chain/peers.js b/framework/test/mocha/unit/modules/chain/peers.js index 33154166a51..c5aa5523aa2 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:getUniqueConnectedPeersCount') + .withArgs('network:getUniqueOutboundConnectedPeersCount') .returns(2); channelMock.invoke - .withArgs('network:getUniqueConnectedPeersCount', { + .withArgs('network:getUniqueOutboundConnectedPeersCount', { broadhash: prefixedPeer.broadhash, }) .returns(2); @@ -143,17 +143,17 @@ describe('peers', () => { it('should call channel invoke thrice', async () => expect(channelMock.invoke.calledThrice).to.be.true); - it('should call channel invoke with action network:getUniqueConnectedPeersCount', async () => + it('should call channel invoke with action network:getUniqueOutboundConnectedPeersCount', async () => expect( channelMock.invoke.calledWithExactly( - 'network:getUniqueConnectedPeersCount', + 'network:getUniqueOutboundConnectedPeersCount', ), ).to.be.true); - it('should call channel invoke with action network:getUniqueConnectedPeersCount and filter broadhash', async () => + it('should call channel invoke with action network:getUniqueOutboundConnectedPeersCount and filter broadhash', async () => expect( channelMock.invoke.calledWithExactly( - 'network:getUniqueConnectedPeersCount', + 'network:getUniqueOutboundConnectedPeersCount', { broadhash: prefixedPeer.broadhash, }, @@ -170,10 +170,10 @@ describe('peers', () => { describe('when half of connected peers match our broadhash', () => { before(async () => { channelMock.invoke - .withArgs('network:getUniqueConnectedPeersCount') + .withArgs('network:getUniqueOutboundConnectedPeersCount') .returns(2); channelMock.invoke - .withArgs('network:getUniqueConnectedPeersCount', { + .withArgs('network:getUniqueOutboundConnectedPeersCount', { broadhash: prefixedPeer.broadhash, }) .returns(1); From 90e747e7bd72267dba73f99d0ee78b0db525e9a0 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Mon, 26 Aug 2019 11:45:12 +0200 Subject: [PATCH 12/14] Register getUniqueOutboundConnectedPeersCount network action to the bus --- framework/src/modules/chain/peers.js | 5 ++++- framework/src/modules/network/index.js | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/framework/src/modules/chain/peers.js b/framework/src/modules/chain/peers.js index d2192ff6041..2ab315fff2b 100644 --- a/framework/src/modules/chain/peers.js +++ b/framework/src/modules/chain/peers.js @@ -51,7 +51,10 @@ class Peers { const { broadhash } = await this.channel.invoke('app:getApplicationState'); const activeCount = Math.min( - await this.channel.invoke('network:getUniqueOutboundConnectedPeersCount'), + await this.channel.invoke( + 'network:getUniqueOutboundConnectedPeersCount', + {}, + ), MAX_PEERS, ); diff --git a/framework/src/modules/network/index.js b/framework/src/modules/network/index.js index e27e27ec397..28e32866aa9 100644 --- a/framework/src/modules/network/index.js +++ b/framework/src/modules/network/index.js @@ -66,6 +66,10 @@ module.exports = class NetworkModule extends BaseModule { getPeersCount: { handler: action => this.network.actions.getPeersCount(action), }, + getUniqueOutboundConnectedPeersCount: { + handler: action => + this.network.actions.getUniqueOutboundConnectedPeersCount(action), + }, }; } From 07d7bb2dc97a5465c81b80aad2a93efa4cbe1afb Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Mon, 26 Aug 2019 12:13:39 +0200 Subject: [PATCH 13/14] Small fix on framework unit tests --- framework/test/mocha/unit/modules/chain/peers.js | 1 + .../test/mocha/unit/modules/network/lookup_peers_ips.js | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/framework/test/mocha/unit/modules/chain/peers.js b/framework/test/mocha/unit/modules/chain/peers.js index c5aa5523aa2..b0fa8ce5cb5 100644 --- a/framework/test/mocha/unit/modules/chain/peers.js +++ b/framework/test/mocha/unit/modules/chain/peers.js @@ -147,6 +147,7 @@ describe('peers', () => { expect( channelMock.invoke.calledWithExactly( 'network:getUniqueOutboundConnectedPeersCount', + {}, ), ).to.be.true); 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 5368fc17832..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/utils'); +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 => { From dbae9474a91dcfa44da06817676ce929044bf09a Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Mon, 26 Aug 2019 12:19:06 +0200 Subject: [PATCH 14/14] Correct description from P2P unit test --- elements/lisk-p2p/test/unit/peer_pool.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/lisk-p2p/test/unit/peer_pool.ts b/elements/lisk-p2p/test/unit/peer_pool.ts index 88961b6120a..5d3d4a3f9ed 100644 --- a/elements/lisk-p2p/test/unit/peer_pool.ts +++ b/elements/lisk-p2p/test/unit/peer_pool.ts @@ -248,7 +248,7 @@ describe('peerPool', () => { }); }); - describe('#getUniqueConnectedPeers', () => { + describe('#getUniqueOutboundConnectedPeers', () => { const samplePeers = initializePeerInfoList(); describe('when two peers have same peer infos', () => {