From 486511e0caa1f8423155ba95fbaafbf0602e1c43 Mon Sep 17 00:00:00 2001 From: MaciejBaj Date: Sun, 21 Jan 2018 16:05:12 +0100 Subject: [PATCH 01/17] Create synchronousTasksSequence and add forging and sync process into --- app.js | 11 +++++++- modules/delegates.js | 16 ++++++----- modules/loader.js | 49 +++++++++++++++++---------------- test/system/synchronousTasks.js | 0 4 files changed, 45 insertions(+), 31 deletions(-) create mode 100644 test/system/synchronousTasks.js diff --git a/app.js b/app.js index 5df230699f3..af0661ed49b 100644 --- a/app.js +++ b/app.js @@ -332,6 +332,15 @@ d.run(function () { cb(null, sequence); }], + synchronousTasksSequence: ['logger', function (scope, cb) { + var sequence = new Sequence({ + onWarning: function (current) { + scope.logger.warn('Synchronous tasks queue', current); + } + }); + cb(null, sequence); + }], + /** * Once config, public, genesisblock, logger, build and network are completed, * adds configuration to `network.app`. @@ -498,7 +507,7 @@ d.run(function () { * at leats will contain the required elements. * @param {nodeStyleCallback} cb - Callback function with resulted load. */ - modules: ['network', 'connect', 'config', 'logger', 'bus', 'sequence', 'dbSequence', 'balancesSequence', 'db', 'logic', 'cache', function (scope, cb) { + modules: ['network', 'connect', 'config', 'logger', 'bus', 'sequence', 'dbSequence', 'balancesSequence', 'synchronousTasksSequence', 'db', 'logic', 'cache', function (scope, cb) { var tasks = {}; diff --git a/modules/delegates.js b/modules/delegates.js index 1178a85f4ea..aa13c1d6f30 100644 --- a/modules/delegates.js +++ b/modules/delegates.js @@ -25,6 +25,7 @@ __private.loaded = false; __private.blockReward = new BlockReward(); __private.keypairs = {}; __private.tmpKeypairs = {}; +__private.forgeAttemptInterval = 1000; /** * Initializes library with scope content and generates a Delegate instance. @@ -46,6 +47,7 @@ function Delegates (cb, scope) { network: scope.network, schema: scope.schema, balancesSequence: scope.balancesSequence, + synchronousTasksSequence: scope.synchronousTasksSequence, logic: { transaction: scope.logic.transaction, }, @@ -598,15 +600,15 @@ Delegates.prototype.onBlockchainReady = function () { library.logger.error('Failed to load delegates', err); } - async.series([ - __private.forge, - modules.transactions.fillPool - ], function () { - return setImmediate(cb); - }); + library.synchronousTasksSequence.add(function (sequenceCb) { + async.series([ + __private.forge, + modules.transactions.fillPool + ], sequenceCb); + }, cb); } - jobsQueue.register('delegatesNextForge', nextForge, 1000); + jobsQueue.register('delegatesNextForge', nextForge, __private.forgeAttemptInterval); }); }; diff --git a/modules/loader.js b/modules/loader.js index 0bb9835f35f..44d6d50de61 100644 --- a/modules/loader.js +++ b/modules/loader.js @@ -44,6 +44,7 @@ function Loader (cb, scope) { bus: scope.bus, genesisblock: scope.genesisblock, balancesSequence: scope.balancesSequence, + synchronousTasksSequence: scope.synchronousTasksSequence, logic: { transaction: scope.logic.transaction, account: scope.logic.account, @@ -609,30 +610,32 @@ __private.sync = function (cb) { __private.isActive = true; __private.syncTrigger(true); - async.series({ - getPeersBefore: function (seriesCb) { - library.logger.debug('Establishing broadhash consensus before sync'); - return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); - }, - loadBlocksFromNetwork: function (seriesCb) { - return __private.loadBlocksFromNetwork(seriesCb); - }, - updateSystem: function (seriesCb) { - return modules.system.update(seriesCb); - }, - getPeersAfter: function (seriesCb) { - library.logger.debug('Establishing broadhash consensus after sync'); - return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); - } - }, function (err) { - __private.isActive = false; - __private.syncTrigger(false); - __private.blocksToSync = 0; + library.synchronousTasksSequence.add(function (sequenceCb) { + async.series({ + getPeersBefore: function (seriesCb) { + library.logger.debug('Establishing broadhash consensus before sync'); + return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); + }, + loadBlocksFromNetwork: function (seriesCb) { + return __private.loadBlocksFromNetwork(seriesCb); + }, + updateSystem: function (seriesCb) { + return modules.system.update(seriesCb); + }, + getPeersAfter: function (seriesCb) { + library.logger.debug('Establishing broadhash consensus after sync'); + return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); + } + }, function (err) { + __private.isActive = false; + __private.syncTrigger(false); + __private.blocksToSync = 0; - library.logger.info('Finished sync'); - library.bus.message('syncFinished'); - return setImmediate(cb, err); - }); + library.logger.info('Finished sync'); + library.bus.message('syncFinished'); + return setImmediate(sequenceCb, err); + }); + }, cb); }; /* diff --git a/test/system/synchronousTasks.js b/test/system/synchronousTasks.js new file mode 100644 index 00000000000..e69de29bb2d From 3a1bdfcd020278d77373d7112e278ffe0133c393 Mon Sep 17 00:00:00 2001 From: MaciejBaj Date: Sun, 21 Jan 2018 16:35:14 +0100 Subject: [PATCH 02/17] Add synchronousTasksSequence to test/common/application --- test/common/application.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/common/application.js b/test/common/application.js index 401df446830..d2849d90c50 100644 --- a/test/common/application.js +++ b/test/common/application.js @@ -160,6 +160,14 @@ function __init (initScope, done) { }); cb(null, sequence); }], + synchronousTasksSequence: ['logger', function (scope, cb) { + var sequence = new Sequence({ + onWarning: function (current) { + scope.logger.warn('Synchronous tasks queue', current); + } + }); + cb(null, sequence); + }], ed: function (cb) { cb(null, require('../../helpers/ed.js')); }, @@ -233,7 +241,7 @@ function __init (initScope, done) { }] }, cb); }], - modules: ['network', 'logger', 'bus', 'sequence', 'dbSequence', 'balancesSequence', 'db', 'logic', function (scope, cb) { + modules: ['network', 'logger', 'bus', 'sequence', 'dbSequence', 'balancesSequence', 'synchronousTasksSequence', 'db', 'logic', function (scope, cb) { var tasks = {}; scope.rewiredModules = {}; From 82d83116e9ebc2bce87c220b6818f1dea0bf6340 Mon Sep 17 00:00:00 2001 From: MaciejBaj Date: Sun, 21 Jan 2018 16:37:58 +0100 Subject: [PATCH 03/17] Add synchronous tasks system tests skeleton --- test/system/synchronousTasks.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/system/synchronousTasks.js b/test/system/synchronousTasks.js index e69de29bb2d..bb6c19efc57 100644 --- a/test/system/synchronousTasks.js +++ b/test/system/synchronousTasks.js @@ -0,0 +1,29 @@ +'use strict'; + +var application = require('./../common/application'); + +describe('synchronousTasks', function () { + + var library; + + before('init sandboxed application', function (done) { + application.init({sandbox: {name: 'lisk_test_synchronous_tasks'}}, function (scope) { + library = scope; + done(); + }); + }); + + after('cleanup sandboxed application', function (done) { + application.cleanup(done); + }); + + describe('when "attempt to forge" synchronous tasks runs every 100 ms and takes 200 ms', function () { + + describe('when "blockchain synchronization" synchronous tasks runs every 100 ms and takes 200 ms', function () { + + it('"attempt to forge" task should never start when "blockchain synchronization" task is running'); + + it('"blockchain synchronization" task should never start when "attempt to forge" task is running'); + }); + }); +}); From 419b5fe7a225f2b9bb6bbc19b9cbbcbcded82da7 Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 14:10:53 +0100 Subject: [PATCH 04/17] Revert delegates file to the state from before #26 changes --- modules/delegates.js | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/modules/delegates.js b/modules/delegates.js index aa13c1d6f30..db7b0b1a890 100644 --- a/modules/delegates.js +++ b/modules/delegates.js @@ -25,7 +25,6 @@ __private.loaded = false; __private.blockReward = new BlockReward(); __private.keypairs = {}; __private.tmpKeypairs = {}; -__private.forgeAttemptInterval = 1000; /** * Initializes library with scope content and generates a Delegate instance. @@ -47,7 +46,6 @@ function Delegates (cb, scope) { network: scope.network, schema: scope.schema, balancesSequence: scope.balancesSequence, - synchronousTasksSequence: scope.synchronousTasksSequence, logic: { transaction: scope.logic.transaction, }, @@ -63,7 +61,7 @@ function Delegates (cb, scope) { self = this; __private.assetTypes[transactionTypes.DELEGATE] = library.logic.transaction.attachAssetType( - transactionTypes.DELEGATE, + transactionTypes.DELEGATE, new Delegate( scope.logger, scope.schema @@ -78,7 +76,7 @@ function Delegates (cb, scope) { * Gets delegate public keys sorted by vote descending. * @private * @param {function} cb - Callback function. - * @returns {setImmediateCallback} + * @returns {setImmediateCallback} */ __private.getKeysSortByVote = function (cb) { modules.accounts.getAccounts({ @@ -99,7 +97,7 @@ __private.getKeysSortByVote = function (cb) { * Gets delegate public keys from previous round, sorted by vote descending. * @private * @param {function} cb - Callback function. - * @returns {setImmediateCallback} + * @returns {setImmediateCallback} */ __private.getDelegatesFromPreviousRound = function (cb) { library.db.query(sql.getDelegatesSnapshot, {limit: slots.delegates}).then(function (rows) { @@ -171,11 +169,11 @@ __private.getBlockSlotData = function (slot, height, cb) { }; /** - * Gets peers, checks consensus and generates new block, once delegates + * Gets peers, checks consensus and generates new block, once delegates * are enabled, client is ready to forge and is the correct slot. * @private * @param {function} cb - Callback function. - * @returns {setImmediateCallback} + * @returns {setImmediateCallback} */ __private.forge = function (cb) { if (!Object.keys(__private.keypairs).length) { @@ -340,7 +338,7 @@ __private.checkDelegates = function (publicKey, votes, state, cb) { * Loads delegates from config and stores in private `keypairs`. * @private * @param {function} cb - Callback function. - * @returns {setImmediateCallback} + * @returns {setImmediateCallback} */ __private.loadDelegates = function (cb) { var secrets; @@ -600,15 +598,15 @@ Delegates.prototype.onBlockchainReady = function () { library.logger.error('Failed to load delegates', err); } - library.synchronousTasksSequence.add(function (sequenceCb) { - async.series([ - __private.forge, - modules.transactions.fillPool - ], sequenceCb); - }, cb); + async.series([ + __private.forge, + modules.transactions.fillPool + ], function () { + return setImmediate(cb); + }); } - jobsQueue.register('delegatesNextForge', nextForge, __private.forgeAttemptInterval); + jobsQueue.register('delegatesNextForge', nextForge, 1000); }); }; From 642ccf4aaf22f1e4879135a01c7faa0865792c74 Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 14:11:31 +0100 Subject: [PATCH 05/17] Remove usages of synchronousTasksSequence --- app.js | 11 +-------- modules/loader.js | 49 ++++++++++++++++++-------------------- test/common/application.js | 10 +------- 3 files changed, 25 insertions(+), 45 deletions(-) diff --git a/app.js b/app.js index af0661ed49b..5df230699f3 100644 --- a/app.js +++ b/app.js @@ -332,15 +332,6 @@ d.run(function () { cb(null, sequence); }], - synchronousTasksSequence: ['logger', function (scope, cb) { - var sequence = new Sequence({ - onWarning: function (current) { - scope.logger.warn('Synchronous tasks queue', current); - } - }); - cb(null, sequence); - }], - /** * Once config, public, genesisblock, logger, build and network are completed, * adds configuration to `network.app`. @@ -507,7 +498,7 @@ d.run(function () { * at leats will contain the required elements. * @param {nodeStyleCallback} cb - Callback function with resulted load. */ - modules: ['network', 'connect', 'config', 'logger', 'bus', 'sequence', 'dbSequence', 'balancesSequence', 'synchronousTasksSequence', 'db', 'logic', 'cache', function (scope, cb) { + modules: ['network', 'connect', 'config', 'logger', 'bus', 'sequence', 'dbSequence', 'balancesSequence', 'db', 'logic', 'cache', function (scope, cb) { var tasks = {}; diff --git a/modules/loader.js b/modules/loader.js index 44d6d50de61..0bb9835f35f 100644 --- a/modules/loader.js +++ b/modules/loader.js @@ -44,7 +44,6 @@ function Loader (cb, scope) { bus: scope.bus, genesisblock: scope.genesisblock, balancesSequence: scope.balancesSequence, - synchronousTasksSequence: scope.synchronousTasksSequence, logic: { transaction: scope.logic.transaction, account: scope.logic.account, @@ -610,32 +609,30 @@ __private.sync = function (cb) { __private.isActive = true; __private.syncTrigger(true); - library.synchronousTasksSequence.add(function (sequenceCb) { - async.series({ - getPeersBefore: function (seriesCb) { - library.logger.debug('Establishing broadhash consensus before sync'); - return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); - }, - loadBlocksFromNetwork: function (seriesCb) { - return __private.loadBlocksFromNetwork(seriesCb); - }, - updateSystem: function (seriesCb) { - return modules.system.update(seriesCb); - }, - getPeersAfter: function (seriesCb) { - library.logger.debug('Establishing broadhash consensus after sync'); - return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); - } - }, function (err) { - __private.isActive = false; - __private.syncTrigger(false); - __private.blocksToSync = 0; + async.series({ + getPeersBefore: function (seriesCb) { + library.logger.debug('Establishing broadhash consensus before sync'); + return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); + }, + loadBlocksFromNetwork: function (seriesCb) { + return __private.loadBlocksFromNetwork(seriesCb); + }, + updateSystem: function (seriesCb) { + return modules.system.update(seriesCb); + }, + getPeersAfter: function (seriesCb) { + library.logger.debug('Establishing broadhash consensus after sync'); + return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); + } + }, function (err) { + __private.isActive = false; + __private.syncTrigger(false); + __private.blocksToSync = 0; - library.logger.info('Finished sync'); - library.bus.message('syncFinished'); - return setImmediate(sequenceCb, err); - }); - }, cb); + library.logger.info('Finished sync'); + library.bus.message('syncFinished'); + return setImmediate(cb, err); + }); }; /* diff --git a/test/common/application.js b/test/common/application.js index d2849d90c50..401df446830 100644 --- a/test/common/application.js +++ b/test/common/application.js @@ -160,14 +160,6 @@ function __init (initScope, done) { }); cb(null, sequence); }], - synchronousTasksSequence: ['logger', function (scope, cb) { - var sequence = new Sequence({ - onWarning: function (current) { - scope.logger.warn('Synchronous tasks queue', current); - } - }); - cb(null, sequence); - }], ed: function (cb) { cb(null, require('../../helpers/ed.js')); }, @@ -241,7 +233,7 @@ function __init (initScope, done) { }] }, cb); }], - modules: ['network', 'logger', 'bus', 'sequence', 'dbSequence', 'balancesSequence', 'synchronousTasksSequence', 'db', 'logic', function (scope, cb) { + modules: ['network', 'logger', 'bus', 'sequence', 'dbSequence', 'balancesSequence', 'db', 'logic', function (scope, cb) { var tasks = {}; scope.rewiredModules = {}; From f9cc957def75ea66213b623f8b0702c56813e3b6 Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 14:13:18 +0100 Subject: [PATCH 06/17] Assign forge every 1000ms interval as variable --- modules/delegates.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/delegates.js b/modules/delegates.js index db7b0b1a890..12a7fb708cb 100644 --- a/modules/delegates.js +++ b/modules/delegates.js @@ -25,6 +25,7 @@ __private.loaded = false; __private.blockReward = new BlockReward(); __private.keypairs = {}; __private.tmpKeypairs = {}; +__private.forgeAttemptInterval = 1000; /** * Initializes library with scope content and generates a Delegate instance. @@ -606,7 +607,7 @@ Delegates.prototype.onBlockchainReady = function () { }); } - jobsQueue.register('delegatesNextForge', nextForge, 1000); + jobsQueue.register('delegatesNextForge', nextForge, __private.forgeAttemptInterval); }); }; From ab70f8f5cf6207d0393deda4a0cef870c2195eab Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 14:49:45 +0100 Subject: [PATCH 07/17] Move delegates.onBlockchainReady logic into functions --- modules/delegates.js | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/modules/delegates.js b/modules/delegates.js index 12a7fb708cb..4e63262bb38 100644 --- a/modules/delegates.js +++ b/modules/delegates.js @@ -585,29 +585,30 @@ Delegates.prototype.onBind = function (scope) { ); }; +__private.nextForge = function (cb) { + async.series([ + __private.forge, + modules.transactions.fillPool + ], function () { + return setImmediate(cb); + }); +}; + +__private.logLoadDelegatesError = function (err) { + library.logger.error('Failed to load delegates', err); +}; + /** * Loads delegates. * @implements module:transactions#Transactions~fillPool */ Delegates.prototype.onBlockchainReady = function () { __private.loaded = true; - __private.loadDelegates(function (err) { - - function nextForge (cb) { - if (err) { - library.logger.error('Failed to load delegates', err); - } - - async.series([ - __private.forge, - modules.transactions.fillPool - ], function () { - return setImmediate(cb); - }); + if (err) { + jobsQueue.register('logLoadDelegatesError', __private.logLoadDelegatesError.bind(null, err), __private.forgeAttemptInterval); } - - jobsQueue.register('delegatesNextForge', nextForge, __private.forgeAttemptInterval); + jobsQueue.register('delegatesNextForge', __private.nextForge, __private.forgeAttemptInterval); }); }; From dda267ad23543934f4b90eecf0a3612f414db69c Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 14:54:55 +0100 Subject: [PATCH 08/17] Add delegates.__private.nextForge to global sequence --- modules/delegates.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/modules/delegates.js b/modules/delegates.js index 4e63262bb38..be85f01e685 100644 --- a/modules/delegates.js +++ b/modules/delegates.js @@ -585,13 +585,11 @@ Delegates.prototype.onBind = function (scope) { ); }; -__private.nextForge = function (cb) { +__private.nextForge = function (sequenceCb) { async.series([ __private.forge, modules.transactions.fillPool - ], function () { - return setImmediate(cb); - }); + ], sequenceCb); }; __private.logLoadDelegatesError = function (err) { @@ -608,7 +606,7 @@ Delegates.prototype.onBlockchainReady = function () { if (err) { jobsQueue.register('logLoadDelegatesError', __private.logLoadDelegatesError.bind(null, err), __private.forgeAttemptInterval); } - jobsQueue.register('delegatesNextForge', __private.nextForge, __private.forgeAttemptInterval); + jobsQueue.register('delegatesNextForge', library.sequence.add.bind(null, __private.nextForge), __private.forgeAttemptInterval); }); }; From d6012a4ae1ecfa3844e442202c0db13a3f499513 Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 15:36:12 +0100 Subject: [PATCH 09/17] Remove redundant sequence from delegates.__private.forge --- modules/delegates.js | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/modules/delegates.js b/modules/delegates.js index be85f01e685..888eff00d9c 100644 --- a/modules/delegates.js +++ b/modules/delegates.js @@ -208,28 +208,23 @@ __private.forge = function (cb) { return setImmediate(cb); } - library.sequence.add(function (cb) { - async.series({ - getPeers: function (seriesCb) { - return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); - }, - checkBroadhash: function (seriesCb) { - if (modules.transport.poorConsensus()) { - return setImmediate(seriesCb, ['Inadequate broadhash consensus', modules.transport.consensus(), '%'].join(' ')); - } else { - return setImmediate(seriesCb); - } - } - }, function (err) { - if (err) { - library.logger.warn(err); - return setImmediate(cb, err); + async.series({ + getPeers: function (seriesCb) { + return modules.transport.getPeers({limit: constants.maxPeers}, seriesCb); + }, + checkBroadhash: function (seriesCb) { + if (modules.transport.poorConsensus()) { + return setImmediate(seriesCb, ['Inadequate broadhash consensus', modules.transport.consensus(), '%'].join(' ')); } else { - return modules.blocks.process.generateBlock(currentBlockData.keypair, currentBlockData.time, cb); + return setImmediate(seriesCb); } - }); + }, + generateBlock: function (seriesCb) { + return modules.blocks.process.generateBlock(currentBlockData.keypair, currentBlockData.time, seriesCb); + } }, function (err) { if (err) { + library.logger.warn(err); library.logger.error('Failed to generate block within delegate slot', err); } else { var forgedBlock = modules.blocks.lastBlock.get(); From 77f9711497808cc86aa69368ec1049fba383ea21 Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 15:50:46 +0100 Subject: [PATCH 10/17] Handle jobsQueue callbacks properly in delegates.onBlockchainReady --- modules/delegates.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/modules/delegates.js b/modules/delegates.js index 888eff00d9c..09235a7b465 100644 --- a/modules/delegates.js +++ b/modules/delegates.js @@ -587,8 +587,9 @@ __private.nextForge = function (sequenceCb) { ], sequenceCb); }; -__private.logLoadDelegatesError = function (err) { +__private.logLoadDelegatesError = function (err, cb) { library.logger.error('Failed to load delegates', err); + return setImmediate(cb); }; /** @@ -599,9 +600,13 @@ Delegates.prototype.onBlockchainReady = function () { __private.loaded = true; __private.loadDelegates(function (err) { if (err) { - jobsQueue.register('logLoadDelegatesError', __private.logLoadDelegatesError.bind(null, err), __private.forgeAttemptInterval); + jobsQueue.register('logLoadDelegatesError', function (jobsQueueCb) { + __private.logLoadDelegatesError(err, jobsQueueCb); + }, __private.forgeAttemptInterval); } - jobsQueue.register('delegatesNextForge', library.sequence.add.bind(null, __private.nextForge), __private.forgeAttemptInterval); + jobsQueue.register('delegatesNextForge', function (jobsQueueCb) { + library.sequence.add(__private.nextForge, jobsQueueCb); + }, __private.forgeAttemptInterval); }); }; From 2eda4575111d183707e7d2aae27c4d2f531a1b48 Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 17:25:35 +0100 Subject: [PATCH 11/17] Add tests to check forge and sync process exclusiveness --- test/system/synchronousTasks.js | 73 +++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 4 deletions(-) diff --git a/test/system/synchronousTasks.js b/test/system/synchronousTasks.js index bb6c19efc57..e3ca4afe862 100644 --- a/test/system/synchronousTasks.js +++ b/test/system/synchronousTasks.js @@ -1,5 +1,7 @@ 'use strict'; +var expect = require('chai').expect; +var Rx = require('rx'); var application = require('./../common/application'); describe('synchronousTasks', function () { @@ -17,13 +19,76 @@ describe('synchronousTasks', function () { application.cleanup(done); }); - describe('when "attempt to forge" synchronous tasks runs every 100 ms and takes 200 ms', function () { + describe('when events are emitted after any of synchronous task starts', function () { - describe('when "blockchain synchronization" synchronous tasks runs every 100 ms and takes 200 ms', function () { + var attemptToForgeRunningSubject = new Rx.BehaviorSubject(); + var synchronizeBlockchainRunningSubject = new Rx.BehaviorSubject(); - it('"attempt to forge" task should never start when "blockchain synchronization" task is running'); + describe('when "attempt to forge" synchronous tasks runs every 100 ms and takes 101 ms', function () { - it('"blockchain synchronization" task should never start when "attempt to forge" task is running'); + var intervalMs = 100; + var durationMs = intervalMs + 1; + + before(function () { + library.modules.delegates.onBlockchainReady = library.rewiredModules.delegates.prototype.onBlockchainReady; + library.rewiredModules.delegates.__set__('__private.forgeAttemptInterval', intervalMs); + library.rewiredModules.delegates.__set__('__private.nextForge', function (nextForgeCb) { + attemptToForgeRunningSubject.onNext(true); + setTimeout(function () { + attemptToForgeRunningSubject.onNext(false); + nextForgeCb(); + }, durationMs); + }); + library.modules.delegates.onBlockchainReady(); + }); + + describe('when "blockchain synchronization" synchronous tasks runs every 100 ms and takes 101 ms', function () { + + before(function () { + library.rewiredModules.loader.__set__('__private.syncInterval', intervalMs); + library.rewiredModules.loader.__set__('__private.sync', function (nextForgeCb) { + synchronizeBlockchainRunningSubject.onNext(true); + setTimeout(function () { + synchronizeBlockchainRunningSubject.onNext(false); + nextForgeCb(); + }, durationMs); + }); + var jobsQueue = require('../../helpers/jobsQueue'); + var originalLoaderSyncTimerJob = jobsQueue.jobs['loaderSyncTimer']; + clearTimeout(originalLoaderSyncTimerJob); // Terminate original job + jobsQueue.jobs['loaderSyncTimer'] = null; // Remove original job + library.modules.loader.onPeersReady(); // Execute the mocked blockchain synchronization process + }); + + describe('Within 5000 ms', function () { + + beforeEach(function () { + setTimeout(function () { + attemptToForgeRunningSubject.onCompleted(); + synchronizeBlockchainRunningSubject.onCompleted(); + attemptToForgeRunningSubject = new Rx.BehaviorSubject(); + synchronizeBlockchainRunningSubject = new Rx.BehaviorSubject(); + }, 5000); + }); + + after(function () { + attemptToForgeRunningSubject.dispose(); + synchronizeBlockchainRunningSubject.dispose(); + }); + + it('"attempt to forge" task should never start when "blockchain synchronization" task is running', function (done) { + attemptToForgeRunningSubject + .filter(function (isStarting) {return isStarting;}) + .subscribe(function () {expect(synchronizeBlockchainRunningSubject.getValue()).to.be.false;}, done, done); + }); + + it('"blockchain synchronization" task should never start when "attempt to forge" task is running', function (done) { + synchronizeBlockchainRunningSubject + .filter(function (isStarting) {return isStarting;}) + .subscribe(function () {expect(attemptToForgeRunningSubject.getValue()).to.be.false;}, done, done); + }); + }); + }); }); }); }); From 00de60118c017f54fbf4ae2581a0a31d2c85ba25 Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 17:39:36 +0100 Subject: [PATCH 12/17] Remove redundant check if sync process runs in fillPool --- logic/transactionPool.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/logic/transactionPool.js b/logic/transactionPool.js index dc3255e09a2..7cd9fbbbb91 100644 --- a/logic/transactionPool.js +++ b/logic/transactionPool.js @@ -583,8 +583,6 @@ TransactionPool.prototype.expireTransactions = function (cb) { * @returns {setImmediateCallback|applyUnconfirmedList} */ TransactionPool.prototype.fillPool = function (cb) { - if (modules.loader.syncing()) { return setImmediate(cb); } - var unconfirmedCount = self.countUnconfirmed(); library.logger.debug('Transaction pool size: ' + unconfirmedCount); From 5a675d61c65e86a8a2ea853b9ee3e67281ba4b73 Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Mon, 22 Jan 2018 17:42:34 +0100 Subject: [PATCH 13/17] Add rx-node as dev dependency --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 76911281c23..778317ac553 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,6 @@ "z-schema": "=3.18.2" }, "devDependencies": { - "rewire": "=2.5.2", "bitcore-mnemonic": "=1.1.1", "bluebird": "=3.5.0", "browserify-bignum": "=1.3.0-2", @@ -77,6 +76,8 @@ "lisk-js": "=0.5.0", "mocha": "=3.2.0", "moment": "=2.17.1", + "rewire": "=2.5.2", + "rx-node": "=1.0.2", "sinon": "=1.17.7", "supertest": "=3.0.0" } From d1fddaf0d965cd24fd0a7df83089058250c649fe Mon Sep 17 00:00:00 2001 From: Oliver Beddows Date: Tue, 23 Jan 2018 12:39:34 +0100 Subject: [PATCH 14/17] Apply standards --- test/system/synchronousTasks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/system/synchronousTasks.js b/test/system/synchronousTasks.js index e3ca4afe862..fb34a2aa69e 100644 --- a/test/system/synchronousTasks.js +++ b/test/system/synchronousTasks.js @@ -60,7 +60,7 @@ describe('synchronousTasks', function () { library.modules.loader.onPeersReady(); // Execute the mocked blockchain synchronization process }); - describe('Within 5000 ms', function () { + describe('within 5000 ms', function () { beforeEach(function () { setTimeout(function () { From 7051c568d3a77e37059d2c7a5302300cb87a375c Mon Sep 17 00:00:00 2001 From: Maciej Baj Date: Tue, 23 Jan 2018 12:40:41 +0100 Subject: [PATCH 15/17] Refactor synchronousTasks tests so it reuses the same task mock --- test/system/synchronousTasks.js | 53 +++++++++++++++++---------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/test/system/synchronousTasks.js b/test/system/synchronousTasks.js index fb34a2aa69e..f872d03b890 100644 --- a/test/system/synchronousTasks.js +++ b/test/system/synchronousTasks.js @@ -21,38 +21,46 @@ describe('synchronousTasks', function () { describe('when events are emitted after any of synchronous task starts', function () { - var attemptToForgeRunningSubject = new Rx.BehaviorSubject(); - var synchronizeBlockchainRunningSubject = new Rx.BehaviorSubject(); + var intervalMs; + var durationMs; + var attemptToForgeRunningSubject; + var synchronizeBlockchainRunningSubject; + + var synchronousTaskMock = function (isTaskRunningSubject, nextCb) { + isTaskRunningSubject.onNext(true); + setTimeout(function () { + isTaskRunningSubject.onNext(false); + nextCb(); + }, durationMs); + }; + + before(function () { + attemptToForgeRunningSubject = new Rx.BehaviorSubject(); + synchronizeBlockchainRunningSubject = new Rx.BehaviorSubject(); + }); + + after(function () { + attemptToForgeRunningSubject.dispose(); + synchronizeBlockchainRunningSubject.dispose(); + }); - describe('when "attempt to forge" synchronous tasks runs every 100 ms and takes 101 ms', function () { + describe('when "attempt to forge" synchronous task runs every 100 ms and takes 101 ms', function () { - var intervalMs = 100; - var durationMs = intervalMs + 1; + intervalMs = 100; + durationMs = intervalMs + 1; before(function () { library.modules.delegates.onBlockchainReady = library.rewiredModules.delegates.prototype.onBlockchainReady; library.rewiredModules.delegates.__set__('__private.forgeAttemptInterval', intervalMs); - library.rewiredModules.delegates.__set__('__private.nextForge', function (nextForgeCb) { - attemptToForgeRunningSubject.onNext(true); - setTimeout(function () { - attemptToForgeRunningSubject.onNext(false); - nextForgeCb(); - }, durationMs); - }); + library.rewiredModules.delegates.__set__('__private.nextForge', synchronousTaskMock.bind(null, attemptToForgeRunningSubject)); library.modules.delegates.onBlockchainReady(); }); - describe('when "blockchain synchronization" synchronous tasks runs every 100 ms and takes 101 ms', function () { + describe('when "blockchain synchronization" synchronous task runs every 100 ms and takes 101 ms', function () { before(function () { library.rewiredModules.loader.__set__('__private.syncInterval', intervalMs); - library.rewiredModules.loader.__set__('__private.sync', function (nextForgeCb) { - synchronizeBlockchainRunningSubject.onNext(true); - setTimeout(function () { - synchronizeBlockchainRunningSubject.onNext(false); - nextForgeCb(); - }, durationMs); - }); + library.rewiredModules.loader.__set__('__private.sync', synchronousTaskMock.bind(null, synchronizeBlockchainRunningSubject)); var jobsQueue = require('../../helpers/jobsQueue'); var originalLoaderSyncTimerJob = jobsQueue.jobs['loaderSyncTimer']; clearTimeout(originalLoaderSyncTimerJob); // Terminate original job @@ -71,11 +79,6 @@ describe('synchronousTasks', function () { }, 5000); }); - after(function () { - attemptToForgeRunningSubject.dispose(); - synchronizeBlockchainRunningSubject.dispose(); - }); - it('"attempt to forge" task should never start when "blockchain synchronization" task is running', function (done) { attemptToForgeRunningSubject .filter(function (isStarting) {return isStarting;}) From 830ff84af4ccd10aacb1413b4978358a522ad73c Mon Sep 17 00:00:00 2001 From: Oliver Beddows Date: Tue, 23 Jan 2018 12:42:27 +0100 Subject: [PATCH 16/17] Rename forgeAttemptInterval to forgeInterval --- modules/delegates.js | 6 +++--- test/system/synchronousTasks.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/delegates.js b/modules/delegates.js index 09235a7b465..6cd8241b9aa 100644 --- a/modules/delegates.js +++ b/modules/delegates.js @@ -25,7 +25,7 @@ __private.loaded = false; __private.blockReward = new BlockReward(); __private.keypairs = {}; __private.tmpKeypairs = {}; -__private.forgeAttemptInterval = 1000; +__private.forgeInterval = 1000; /** * Initializes library with scope content and generates a Delegate instance. @@ -602,11 +602,11 @@ Delegates.prototype.onBlockchainReady = function () { if (err) { jobsQueue.register('logLoadDelegatesError', function (jobsQueueCb) { __private.logLoadDelegatesError(err, jobsQueueCb); - }, __private.forgeAttemptInterval); + }, __private.forgeInterval); } jobsQueue.register('delegatesNextForge', function (jobsQueueCb) { library.sequence.add(__private.nextForge, jobsQueueCb); - }, __private.forgeAttemptInterval); + }, __private.forgeInterval); }); }; diff --git a/test/system/synchronousTasks.js b/test/system/synchronousTasks.js index f872d03b890..bca434dcc72 100644 --- a/test/system/synchronousTasks.js +++ b/test/system/synchronousTasks.js @@ -51,7 +51,7 @@ describe('synchronousTasks', function () { before(function () { library.modules.delegates.onBlockchainReady = library.rewiredModules.delegates.prototype.onBlockchainReady; - library.rewiredModules.delegates.__set__('__private.forgeAttemptInterval', intervalMs); + library.rewiredModules.delegates.__set__('__private.forgeInterval', intervalMs); library.rewiredModules.delegates.__set__('__private.nextForge', synchronousTaskMock.bind(null, attemptToForgeRunningSubject)); library.modules.delegates.onBlockchainReady(); }); From 957e514c75ded86fb0a3917dab8017f1c401b316 Mon Sep 17 00:00:00 2001 From: Oliver Beddows Date: Tue, 23 Jan 2018 12:57:29 +0100 Subject: [PATCH 17/17] Remove already logged error --- modules/delegates.js | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/delegates.js b/modules/delegates.js index 6cd8241b9aa..422ce07bdd8 100644 --- a/modules/delegates.js +++ b/modules/delegates.js @@ -224,7 +224,6 @@ __private.forge = function (cb) { } }, function (err) { if (err) { - library.logger.warn(err); library.logger.error('Failed to generate block within delegate slot', err); } else { var forgedBlock = modules.blocks.lastBlock.get();