-
Notifications
You must be signed in to change notification settings - Fork 458
Improve block propagation - Closes #541 #623
Changes from 14 commits
8f9c3cf
f134cae
9a0131d
53462e6
4c949b5
f836fad
c9f5e0f
df5c320
2d3e959
e069ea4
084c0b5
b1e0f52
eee4927
fe4c74e
5fc2ee2
e30c968
1cddd52
23cba63
d168c0c
8d7c4a8
b17114e
ca3130c
f4b23c5
059834b
9015e51
ac70b4e
7fc68ae
4adfa0d
e16422b
6c60167
1b96bd7
e925954
ad6124d
78567cb
c2885a7
360949f
4dee044
268c9c1
2277051
fbdda75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -327,13 +327,12 @@ __private.applyTransaction = function (block, transaction, sender, cb) { | |
* @method applyBlock | ||
* @emits SIGTERM | ||
* @param {Object} block Full normalized block | ||
* @param {boolean} broadcast Indicator that block needs to be broadcasted | ||
* @param {Function} cb Callback function | ||
* @param {boolean} saveBlock Indicator that block needs to be saved to database | ||
* @param {Function} cb Callback function | ||
* @return {Function} cb Callback function from params (through setImmediate) | ||
* @return {Object} cb.err Error if occurred | ||
*/ | ||
Chain.prototype.applyBlock = function (block, broadcast, cb, saveBlock) { | ||
Chain.prototype.applyBlock = function (block, saveBlock, cb) { | ||
// Prevent shutdown during database writes. | ||
modules.blocks.isActive.set(true); | ||
|
||
|
@@ -458,13 +457,11 @@ Chain.prototype.applyBlock = function (block, broadcast, cb, saveBlock) { | |
} | ||
|
||
library.logger.debug('Block applied correctly with ' + block.transactions.length + ' transactions'); | ||
library.bus.message('newBlock', block, broadcast); | ||
|
||
// DATABASE write. Update delegates accounts | ||
modules.rounds.tick(block, seriesCb); | ||
}); | ||
} else { | ||
library.bus.message('newBlock', block, broadcast); | ||
|
||
// DATABASE write. Update delegates accounts | ||
modules.rounds.tick(block, seriesCb); | ||
|
@@ -497,6 +494,20 @@ Chain.prototype.applyBlock = function (block, broadcast, cb, saveBlock) { | |
}); | ||
}; | ||
|
||
/** | ||
* Broadcast reduced block to increase network performance. | ||
* @param {Object} reducedBlock reduced block | ||
* @param {boolean} broadcast Indicator that block needs to be broadcasted | ||
* @throws {string} Error description | ||
*/ | ||
Chain.prototype.broadcastReducedBlock = function (reducedBlock, broadcast) { | ||
try { | ||
library.bus.message('newBlock', reducedBlock, broadcast); | ||
} catch (e) { | ||
throw 'reduced newBlock broadcast message error: ' + e; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aren't we loosing stack here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also bus.message function throw an error even? |
||
} | ||
library.logger.debug(['reducedBlock', reducedBlock.id, 'broadcasted correctly'].join(' ')); | ||
}; | ||
|
||
/** | ||
* Deletes last block, undo transactions, recalculate round | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,9 @@ var crypto = require('crypto'); | |
var slots = require('../../helpers/slots.js'); | ||
var sql = require('../../sql/blocks.js'); | ||
var exceptions = require('../../helpers/exceptions.js'); | ||
var BSON = require('bson'); | ||
|
||
var bson = new BSON(); | ||
|
||
var modules, library, self, __private = {}; | ||
|
||
|
@@ -87,9 +90,47 @@ __private.checkTransaction = function (block, transaction, cb) { | |
}); | ||
}; | ||
|
||
/** | ||
* Adds default properties to block. | ||
* @private | ||
* @param {Object} block Block object reduced | ||
* @return {Object} Block object completed | ||
*/ | ||
__private.addBlockProperties = function (block) { | ||
if (block.version === undefined) { | ||
block.version = 0; | ||
} | ||
if (block.numberOfTransactions === undefined) { | ||
if (block.transactions === undefined) { | ||
block.numberOfTransactions = 0; | ||
} else { | ||
block.numberOfTransactions = block.transactions.length; | ||
} | ||
} | ||
|
||
return block; | ||
}; | ||
|
||
/** | ||
* Deletes default properties from block. | ||
* @private | ||
* @param {Object} block Block object completed | ||
* @return {Object} Block object reduced | ||
*/ | ||
__private.deleteBlockProperties = function (block) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
var reducedBlock = JSON.parse(JSON.stringify(block)); | ||
if (reducedBlock.version === 0) { | ||
delete reducedBlock.version; | ||
} | ||
// verifyBlock ensures numberOfTransactions is transactions.length | ||
if (reducedBlock.numberOfTransactions) { | ||
delete reducedBlock.numberOfTransactions; | ||
} | ||
return reducedBlock; | ||
}; | ||
|
||
/** | ||
* Verify block and return all possible errors related to block | ||
* | ||
* @public | ||
* @method verifyBlock | ||
* @param {Object} block Full block | ||
|
@@ -158,7 +199,7 @@ Verify.prototype.verifyBlock = function (block) { | |
totalAmount += transaction.amount; | ||
totalFee += transaction.fee; | ||
} | ||
|
||
if (payloadHash.digest().toString('hex') !== block.payloadHash) { | ||
return 'Invalid payload hash'; | ||
} | ||
|
@@ -227,6 +268,18 @@ Verify.prototype.processBlock = function (block, broadcast, cb, saveBlock) { | |
} | ||
|
||
async.series({ | ||
addBlockProperties: function (seriesCb) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That should be performed inside There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if (!broadcast) { | ||
try { | ||
// set default properties | ||
block = __private.addBlockProperties(block); | ||
} catch (err) { | ||
return setImmediate(seriesCb, err); | ||
} | ||
} | ||
|
||
return setImmediate(seriesCb); | ||
}, | ||
normalizeBlock: function (seriesCb) { | ||
try { | ||
block = library.logic.block.objectNormalize(block); | ||
|
@@ -236,6 +289,20 @@ Verify.prototype.processBlock = function (block, broadcast, cb, saveBlock) { | |
|
||
return setImmediate(seriesCb); | ||
}, | ||
deleteBlockProperties: function (seriesCb) { | ||
if (broadcast) { | ||
try { | ||
// delete default properties | ||
var blockReduced = __private.deleteBlockProperties(block); | ||
var serializedBlockReduced = bson.serialize(blockReduced); | ||
modules.blocks.chain.broadcastReducedBlock(serializedBlockReduced, broadcast); | ||
} catch (err) { | ||
return setImmediate(seriesCb, err); | ||
} | ||
} | ||
|
||
return setImmediate(seriesCb); | ||
}, | ||
verifyBlock: function (seriesCb) { | ||
// Sanity check of the block, if values are coherent. | ||
// No access to database | ||
|
@@ -289,7 +356,7 @@ Verify.prototype.processBlock = function (block, broadcast, cb, saveBlock) { | |
// * Block and transactions have valid values (signatures, block slots, etc...) | ||
// * The check against database state passed (for instance sender has enough LSK, votes are under 101, etc...) | ||
// We thus update the database with the transactions values, save the block and tick it. | ||
modules.blocks.chain.applyBlock(block, broadcast, cb, saveBlock); | ||
modules.blocks.chain.applyBlock(block, saveBlock, cb); | ||
} | ||
}); | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need try-catch block here?
library.bus.message
not throws.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, at the beginning bus.message was not in the test scope and try/catch warned about that. There is no reason to keep the warning because bus will be loaded from start.