From 73737c32f1523f9fbb47207f74708fbf28aa75c6 Mon Sep 17 00:00:00 2001 From: michielmulders Date: Mon, 4 Mar 2019 14:08:57 +0100 Subject: [PATCH 1/5] Remove storage read channel invoke call from http to chain --- framework/src/modules/chain/logic/block.js | 5 +- .../modules/http_api/controllers/blocks.js | 66 +++++++++++++++++-- 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/framework/src/modules/chain/logic/block.js b/framework/src/modules/chain/logic/block.js index 3166fa8e2e9..9f67ffdb592 100644 --- a/framework/src/modules/chain/logic/block.js +++ b/framework/src/modules/chain/logic/block.js @@ -517,11 +517,10 @@ Block.prototype.dbRead = function(raw) { }; /** - * Creates block object based on raw data. + * Creates block object based on raw database block data. * - * @param {Object} raw + * @param {Object} raw Raw database data block object * @returns {null|block} Block object - * @todo Add description for the params */ Block.prototype.storageRead = function(raw) { if (!raw.id) { diff --git a/framework/src/modules/http_api/controllers/blocks.js b/framework/src/modules/http_api/controllers/blocks.js index d92a78f1bde..0e4516f09ac 100644 --- a/framework/src/modules/http_api/controllers/blocks.js +++ b/framework/src/modules/http_api/controllers/blocks.js @@ -15,9 +15,12 @@ 'use strict'; const _ = require('lodash'); +const crypto = require('crypto'); const apiCodes = require('../helpers/api_codes.js'); const ApiError = require('../helpers/api_error.js'); +const Bignum = require('../helpers/bignum.js'); +const __private = {}; let library; let sortFields; @@ -108,6 +111,61 @@ BlocksController.getBlocks = function(context, next) { }); }; +/** + * Gets address by public. + * + * @private + * @param {publicKey} publicKey Public key + * @returns {address} address + * @todo Replace by Lisk Elements once integrated. + */ +__private.getAddressByPublicKey = function(publicKey) { + const publicKeyHash = crypto + .createHash('sha256') + .update(publicKey, 'hex') + .digest(); + const temp = Buffer.alloc(8); + + for (let i = 0; i < 8; i++) { + temp[i] = publicKeyHash[7 - i]; + } + + const address = `${Bignum.fromBuffer(temp).toString()}L`; + return address; +}; + +function parseBlockFromDatabase(raw) { + if (!raw.id) { + return null; + } + + const block = { + id: raw.id, + version: parseInt(raw.version), + timestamp: parseInt(raw.timestamp), + height: parseInt(raw.height), + previousBlock: raw.previousBlockId, + numberOfTransactions: parseInt(raw.numberOfTransactions), + totalAmount: new Bignum(raw.totalAmount), + totalFee: new Bignum(raw.totalFee), + reward: new Bignum(raw.reward), + payloadLength: parseInt(raw.payloadLength), + payloadHash: raw.payloadHash, + generatorPublicKey: raw.generatorPublicKey, + generatorId: __private.getAddressByPublicKey(raw.generatorPublicKey), + blockSignature: raw.blockSignature, + confirmations: parseInt(raw.confirmations), + }; + + if (raw.transactions) { + block.transactions = raw.transactions; + } + + block.totalForged = block.totalFee.plus(block.reward).toString(); + + return block; +} + /** * Get filtered list of blocks (without transactions - BasicBlock). * @@ -180,13 +238,7 @@ function _list(params, cb) { library.storage.entities.Block.get(filters, options) // FIXME: Can have poor performance because it performs SHA256 hash calculation for each block .then(async rows => - setImmediate( - cb, - null, - await Promise.all( - rows.map(row => library.channel.invoke('chain:storageRead', [row])) - ) - ) + setImmediate(cb, null, rows.map(parseBlockFromDatabase)) ) .catch(err => { library.logger.error(err.stack); From 4aa1e7d5df01e2e73d9c90ba52401ee1beb99835 Mon Sep 17 00:00:00 2001 From: michielmulders Date: Mon, 4 Mar 2019 14:22:08 +0100 Subject: [PATCH 2/5] Add comments for function --- framework/src/modules/http_api/controllers/blocks.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/framework/src/modules/http_api/controllers/blocks.js b/framework/src/modules/http_api/controllers/blocks.js index 0e4516f09ac..2c0bc7e7b7c 100644 --- a/framework/src/modules/http_api/controllers/blocks.js +++ b/framework/src/modules/http_api/controllers/blocks.js @@ -134,6 +134,12 @@ __private.getAddressByPublicKey = function(publicKey) { return address; }; +/** + * Parse raw block data from database into expected API response type for blocks + * + * @param {Object} raw Raw block data from database + * @return {block} Block formatted according to API specification + */ function parseBlockFromDatabase(raw) { if (!raw.id) { return null; From a8d97709b5fc8c162045d8d425a65b6b4622ba8b Mon Sep 17 00:00:00 2001 From: michielmulders Date: Tue, 5 Mar 2019 16:27:50 +0100 Subject: [PATCH 3/5] Add tests, add better formatting JSON, remove private function --- .../modules/http_api/controllers/blocks.js | 16 +++--- .../modules/http_api/controllers/blocks.js | 53 +++++++++++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/framework/src/modules/http_api/controllers/blocks.js b/framework/src/modules/http_api/controllers/blocks.js index 2c0bc7e7b7c..fea553bffc7 100644 --- a/framework/src/modules/http_api/controllers/blocks.js +++ b/framework/src/modules/http_api/controllers/blocks.js @@ -20,7 +20,6 @@ const apiCodes = require('../helpers/api_codes.js'); const ApiError = require('../helpers/api_error.js'); const Bignum = require('../helpers/bignum.js'); -const __private = {}; let library; let sortFields; @@ -119,7 +118,7 @@ BlocksController.getBlocks = function(context, next) { * @returns {address} address * @todo Replace by Lisk Elements once integrated. */ -__private.getAddressByPublicKey = function(publicKey) { +function getAddressByPublicKey(publicKey) { const publicKeyHash = crypto .createHash('sha256') .update(publicKey, 'hex') @@ -132,7 +131,7 @@ __private.getAddressByPublicKey = function(publicKey) { const address = `${Bignum.fromBuffer(temp).toString()}L`; return address; -}; +} /** * Parse raw block data from database into expected API response type for blocks @@ -152,23 +151,22 @@ function parseBlockFromDatabase(raw) { height: parseInt(raw.height), previousBlock: raw.previousBlockId, numberOfTransactions: parseInt(raw.numberOfTransactions), - totalAmount: new Bignum(raw.totalAmount), - totalFee: new Bignum(raw.totalFee), - reward: new Bignum(raw.reward), + totalAmount: new Bignum(raw.totalAmount).toString(), + totalFee: new Bignum(raw.totalFee).toString(), + reward: new Bignum(raw.reward).toString(), payloadLength: parseInt(raw.payloadLength), payloadHash: raw.payloadHash, generatorPublicKey: raw.generatorPublicKey, - generatorId: __private.getAddressByPublicKey(raw.generatorPublicKey), + generatorId: getAddressByPublicKey(raw.generatorPublicKey), blockSignature: raw.blockSignature, confirmations: parseInt(raw.confirmations), + totalForged: new Bignum(raw.totalFee).plus(raw.reward).toString(), }; if (raw.transactions) { block.transactions = raw.transactions; } - block.totalForged = block.totalFee.plus(block.reward).toString(); - return block; } diff --git a/framework/test/mocha/unit/modules/http_api/controllers/blocks.js b/framework/test/mocha/unit/modules/http_api/controllers/blocks.js index 30b95ffc94a..2381906d480 100644 --- a/framework/test/mocha/unit/modules/http_api/controllers/blocks.js +++ b/framework/test/mocha/unit/modules/http_api/controllers/blocks.js @@ -22,16 +22,39 @@ const BlocksController = rewire( describe('blocks/api', () => { let _list; + let parseBlockFromDatabase; let library; let loggerSpy; let storageStub; let channelStub; + let rawBlock; beforeEach(done => { + rawBlock = { + id: '8999789716699660339', + payloadHash: + 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', + generatorPublicKey: + '68680ca0bcd4676489976837edeac305c34f652e970386013ef26e67589a2516', + blockSignature: + '51356c69d94762ef355a95a960002aabc80d331da136cc07082bf98856e57ea9d13d2c74dd923c412221b36f4c3e276b2d83d9019521ddf003d0b39698d4ae0a', + height: 1860, + totalFee: '0', + reward: '0', + payloadLength: 0, + previousBlockId: '14888522644597494628', + numberOfTransactions: 0, + totalAmount: '0', + timestamp: 87670360, + version: '1', + confirmations: 37, + }; + storageStub = { entities: { Block: { get: sinonSandbox.stub().resolves([]), + getRaw: sinonSandbox.stub().resolves(rawBlock), }, }, }; @@ -52,6 +75,7 @@ describe('blocks/api', () => { }); library = BlocksController.__get__('library'); _list = BlocksController.__get__('_list'); + parseBlockFromDatabase = BlocksController.__get__('parseBlockFromDatabase'); done(); }); @@ -69,6 +93,35 @@ describe('blocks/api', () => { }); }); + describe('parseBlockFromDatabase', () => { + let formattedBlock; + + beforeEach(async () => { + formattedBlock = parseBlockFromDatabase(rawBlock); + }); + + it('should return a block properly formatted when transactions is undefined', async () => { + const rawBlockWithTransactions = _.cloneDeep(rawBlock); + formattedBlock = parseBlockFromDatabase(rawBlockWithTransactions); + expect(formattedBlock).deep.equal(formattedBlock); + expect(formattedBlock).to.not.have.property('transactions'); + }); + + it('should return a block properly formatted when transactions is not undefined', async () => { + const rawBlockWithTransactions = _.cloneDeep(rawBlock); + rawBlockWithTransactions.transactions = []; + formattedBlock = parseBlockFromDatabase(rawBlockWithTransactions); + expect(formattedBlock).to.have.property('transactions'); + }); + + it('should return null when no id is present for raw data', async () => { + const rawBlockWithoutID = _.cloneDeep(rawBlock); + delete rawBlockWithoutID.id; + formattedBlock = parseBlockFromDatabase(rawBlockWithoutID); + expect(formattedBlock).to.be.null; + }); + }); + describe('_list', () => { describe('list', () => { afterEach(done => { From 8c850268bff393d89c44be6c58ea44e05b3f1748 Mon Sep 17 00:00:00 2001 From: michielmulders Date: Tue, 5 Mar 2019 16:50:33 +0100 Subject: [PATCH 4/5] Remove unused storageStub method --- framework/test/mocha/unit/modules/http_api/controllers/blocks.js | 1 - 1 file changed, 1 deletion(-) diff --git a/framework/test/mocha/unit/modules/http_api/controllers/blocks.js b/framework/test/mocha/unit/modules/http_api/controllers/blocks.js index 2381906d480..d6b05ddae91 100644 --- a/framework/test/mocha/unit/modules/http_api/controllers/blocks.js +++ b/framework/test/mocha/unit/modules/http_api/controllers/blocks.js @@ -54,7 +54,6 @@ describe('blocks/api', () => { entities: { Block: { get: sinonSandbox.stub().resolves([]), - getRaw: sinonSandbox.stub().resolves(rawBlock), }, }, }; From e0372f3c400d04aa58c3132b2b6ef5bc9cfb2f34 Mon Sep 17 00:00:00 2001 From: michielmulders Date: Wed, 6 Mar 2019 14:44:11 +0100 Subject: [PATCH 5/5] Remove chain:storageRead action from Chain module --- framework/src/modules/chain/chain.js | 2 -- framework/src/modules/chain/index.js | 1 - 2 files changed, 3 deletions(-) diff --git a/framework/src/modules/chain/chain.js b/framework/src/modules/chain/chain.js index 127917fad19..6daf9adeb43 100644 --- a/framework/src/modules/chain/chain.js +++ b/framework/src/modules/chain/chain.js @@ -233,8 +233,6 @@ module.exports = class Chain { action.params[0], action.params[1] ), - storageRead: async action => - this.scope.logic.block.storageRead(action.params[0]), getLastConsensus: async () => this.scope.modules.peers.getLastConsensus(), loaderLoaded: async () => this.scope.modules.loader.loaded(), loaderSyncing: async () => this.scope.modules.loader.syncing(), diff --git a/framework/src/modules/chain/index.js b/framework/src/modules/chain/index.js index d547391ebcb..9959662a523 100644 --- a/framework/src/modules/chain/index.js +++ b/framework/src/modules/chain/index.js @@ -65,7 +65,6 @@ module.exports = class ChainModule extends BaseModule { getPeersCountByFilter: async action => this.chain.actions.getPeersCountByFilter(action), postSignature: async action => this.chain.actions.postSignature(action), - storageRead: async action => this.chain.actions.storageRead(action), getLastConsensus: async () => this.chain.actions.getLastConsensus(), loaderLoaded: async () => this.chain.actions.loaderLoaded(), loaderSyncing: async () => this.chain.actions.loaderSyncing(),