Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Initial documentation for blocks module - Closes #542 #553

Merged
merged 2 commits into from
May 11, 2017

Conversation

4miners
Copy link
Contributor

@4miners 4miners commented Apr 12, 2017

Addresses modules/blocks.js in #542.

Copy link
Contributor

@Isabello Isabello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammatical changes/spelling changes. The rest looks great :)

@@ -252,14 +329,17 @@ __private.getIdSequence = function (height, cb) {
}
}

// Add last block at the beginning if set not cointains it already
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Add last block at the beginning if set not cointains it already

Add last block at the beginning if the set does not contain it already

library.db.query(sql.getIdSequence(), {height: height, limit: 5, delegates: constants.activeDelegates}).then(function (rows) {
if (rows.length === 0) {
return setImmediate(cb, 'Failed to get id sequence for height: ' + height);
}

var ids = [];

// Add genesis block at the end if set not cointains it already
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add genesis block at the end if the set does not contain it already

__private.saveGenesisBlock = function (cb) {
// Check if there is already block with genesis block ID in database
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if genesis block ID already exists in the database

transaction.blockId = block.id;
// Create bytea fileds (buffers), and returns pseudo-row promise-like object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fields

Blocks.prototype.getLastBlock = function () {
if (__private.lastBlock) {
var epoch = constants.epochTime / 1000;
var lastBlockTime = epoch + __private.lastBlock.timestamp;
var currentTime = new Date().getTime() / 1000;

//FIXME: That function modify global last block object - not good, for what we need those properties?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function modifies global last block object.

We should find a different way, as this can produce inconsistencies.

blocks = __private.readDbRows(rows);
}

return setImmediate(cb, err, blocks);
});
};

/**
* Loads full blocks from database, used when rebuilding blockchain, snapshoting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snapshotting

* @method loadBlocksOffset
* @param {number} limit Limit amount of blocks
* @param {number} offset Offset to start at
* @param {boolean} verify Indicator that block need to be verified
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs

library.dbSequence.add(function (cb) {
// Loads full blocks from database
// FIXME: Weird logic in that SQL query, also ordering used can be performance bootleneck - to rewrite
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bottleneck

return setImmediate(cb, check.errors[0]);
}
}
if (block.id === genesisblock.block.id) {
__private.applyGenesisBlock(block, cb);
} else {
// Apply block - broadcast: false, saveBlock: false
// FIXME: Looks like we missing some validations here, because applyBlock is different than processBlock used elesewhere
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are missing

return setImmediate(cb, check.errors[0]);
}
}
if (block.id === genesisblock.block.id) {
__private.applyGenesisBlock(block, cb);
} else {
// Apply block - broadcast: false, saveBlock: false
// FIXME: Looks like we missing some validations here, because applyBlock is different than processBlock used elesewhere
// - that need to be checked and adjusted to be consistent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistency should be verified and fixed

@karmacoma karmacoma self-assigned this Apr 12, 2017
@karmacoma karmacoma changed the title Initial documentation for blocks module Initial documentation for blocks module - #542 Apr 12, 2017
* @param {Function} cb Callback function
* @return {Function} cb Callback function from params (through setImmediate)
* @return {Object} cb.err Error if occurred
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this method returns a call to afterSave. It should be something like returns {Error|afterSave} if SQL transaction is ok, returns safterSave execution, if not returns error

* @param {Object} block Full normalized block
* @param {Object} blockPromises Not used
* @return {Object} t SQL connection object filled with inserts
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@throws Will throw 'Invalid promise' when no promise, promise.values or promise.table

* @param {boolean} saveBlock Indicator that block needs to be saved to database
* @return {Function} cb Callback function from params (through setImmediate)
* @return {Object} cb.err Error if occurred
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @emits SIGTERM

* @method sandboxApi
* @param {string} call Name of the function to be called
* @param {Object} args Arguments
* @return {Function} cb Callback function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace last comment @return with @param

* @method onReceiveBlock
* @listens module:transport~event:receiveBlock
* @param {block} block New block
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed return parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that function doesn't return.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returns empty when client not ready to receive block

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it should be @return {undefined}? When I ommit @return tag compiler should assume that the function returns undefined.

@4miners
Copy link
Contributor Author

4miners commented Apr 22, 2017

Fixed issues from review, thanks.

@karmacoma karmacoma changed the title Initial documentation for blocks module - #542 Initial documentation for blocks module - Closes #542 May 11, 2017
@karmacoma karmacoma assigned 4miners and unassigned karmacoma May 11, 2017
@4miners 4miners merged commit 928d1cc into LiskArchive:542-jsdoc May 11, 2017
@4miners 4miners deleted the 542-jsdoc branch May 11, 2017 10:53
@karmacoma karmacoma added ready and removed ready labels Jun 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants