Improve block propagation - Closes #541 #623
Conversation
| * @param {Object} block Block object completed | ||
| * @return {Object} Block object reduced | ||
| */ | ||
| __private.deleteBlockProperties = function (block) { |
There was a problem hiding this comment.
totalAmount, totalFee, payloadLength can also be skipped if 0
modules/blocks/verify.js
Outdated
| } | ||
|
|
||
| result.verified = result.errors.length === 0; | ||
| return result; | ||
| if (getBlockId !== block.id) { |
There was a problem hiding this comment.
We change behavior here, previous was:
- overwrite
block.id - validate signature
Now we have:
- validate signature
- validate
block.id
Maybe validation is not needed so we can also skip block.id during broadcast?
There was a problem hiding this comment.
Ok, in this way verifyBlock will validate parameters and at the end generate and add block.id
| @@ -231,6 +268,18 @@ Verify.prototype.processBlock = function (block, broadcast, cb, saveBlock) { | |||
| } | |||
|
|
|||
| async.series({ | |||
| addBlockProperties: function (seriesCb) { | |||
There was a problem hiding this comment.
That should be performed inside objectNormalize
There was a problem hiding this comment.
Block.prototype.create calls to objectNormalize, there are no missed properties, therefore no need to perform addBlockProperties logic. Same for loadBlocksOffset, with data from DB.
receiveBlock will need it, so we can add it to public logic.block and call it from transport and verify to avoid one function with two logics.
test/unit/modules/blocks/verify.js
Outdated
| function createBlock (blocksModule, blockLogic, secret, timestamp, transactions, previousBlock) { | ||
| var keypair = blockLogic.scope.ed.makeKeypair(crypto.createHash('sha256').update(secret, 'utf8').digest()); | ||
| blocksModule.lastBlock.set(previousBlock); | ||
| var newBLock = blockLogic.create({ |
test/unit/modules/blocks/verify.js
Outdated
| } | ||
| blockLogic = __blockLogic; | ||
|
|
||
| modulesLoader.initModules([ |
There was a problem hiding this comment.
It's why I don't like #555 :D
test/unit/modules/blocks/verify.js
Outdated
| done(); | ||
| }); | ||
|
|
||
| it('previous block should fail', function (done) { |
test/unit/modules/blocks/verify.js
Outdated
| done(); | ||
| }); | ||
|
|
||
| it('block timestamp should fail', function (done) { |
There was a problem hiding this comment.
Why is should fail? Mean what's wrong with used timestamp.
test/unit/modules/blocks/verify.js
Outdated
| it('should generate valid block2', function (done) { | ||
| var secret = 'flip relief play educate address plastic doctor fix must frown oppose segment'; | ||
| validBlock2 = createBlock(blocks, blockLogic, secret, 33772862, transactionsValidBlock2, validBlock1); | ||
| expect(validBlock2.version).to.equal(0); |
There was a problem hiding this comment.
Same - check all properties.
test/unit/modules/blocks/verify.js
Outdated
| }); | ||
|
|
||
| it('normalizeBlock should fail (transaction schema: type)', function (done) { | ||
| invalidBlock2.transactions = JSON.parse(JSON.stringify(validBlock2.transactions)); |
There was a problem hiding this comment.
Use fresh copy instead of restoring previously deleted properties.
test/unit/modules/blocks/verify.js
Outdated
|
|
||
| blocksVerify.processBlock(invalidBlock2, false, function (err, result) { | ||
| if (err) { | ||
| expect(err).equal('Failed to validate block schema: Object didn\'t pass validation for format publicKey: invalid-public-key'); |
There was a problem hiding this comment.
It's really a check for fork 3?
LucasIsasmendi
force-pushed
the
541-block-propagation
branch
from
73ec079
to
f4b23c5
Compare
|
The commit with JSDoc was passing, unfortunately the new jenkins implementation is already talking back to github and messed up your test results. I fixed your test results. |
| done(); | ||
| }, false); | ||
| }); | ||
| }); | ||
| }); |
test/unit/modules/blocks/verify.js
Outdated
| delete block.payloadLength; | ||
| } | ||
| delete block.id; | ||
| return; |
There was a problem hiding this comment.
return not used, please remove
test/unit/modules/blocks/verify.js
Outdated
| version:0, | ||
| }; | ||
| var block1; | ||
| var transactionsBlock1 = [ |
karmacoma
added
ready
and removed
pending review
type: bug
type: improvement
type: refactoring
type: test
labels
Closes #541