From 780947614751736b1619b47b8ec8183dde964f06 Mon Sep 17 00:00:00 2001 From: Chris Kleeschulte Date: Tue, 8 Aug 2017 17:53:07 -0400 Subject: [PATCH] Fixed off by one error when gathering blocks. --- lib/services/block/index.js | 54 ++++++++++++++++--------- lib/services/header/index.js | 31 ++++++-------- lib/services/p2p/index.js | 1 + lib/services/timestamp/index.js | 4 -- lib/services/transaction/index.js | 7 ++-- test/services/block/index.unit.js | 7 +++- test/services/header/index.unit.js | 2 +- test/services/transaction/index.unit.js | 4 +- 8 files changed, 61 insertions(+), 49 deletions(-) diff --git a/lib/services/block/index.js b/lib/services/block/index.js index aba6c6b3..343126b2 100644 --- a/lib/services/block/index.js +++ b/lib/services/block/index.js @@ -33,22 +33,20 @@ inherits(BlockService, BaseService); BlockService.dependencies = [ 'p2p', 'db', 'header' ]; -BlockService.MAX_BLOCKS = 1; - // --- public prototype functions BlockService.prototype.getAPIMethods = function() { var methods = [ ['getBlock', this, this.getBlock, 1], ['getRawBlock', this, this.getRawBlock, 1], ['getBlockOverview', this, this.getBlockOverview, 1], - ['getBestBlockHash', this, this.getBestBlockHash, 0] + ['getBestBlockHash', this, this.getBestBlockHash, 0], + ['syncPercentage', this, this.syncPercentage, 0] ]; return methods; }; -BlockService.prototype.getBestBlockHash = function() { - var headers = this._header.getAllHeaders(); - return headers[headers.length - 1].hash; +BlockService.prototype.getBestBlockHash = function(callback) { + callback(this._header.getAllHeaders().getLastIndex().hash); }; BlockService.prototype.getBlock = function(arg, callback) { @@ -165,10 +163,13 @@ BlockService.prototype.subscribe = function(name, emitter) { }; +BlockService.prototype._syncPercentage = function() { + var ratio = this._tip.height/this._header.getAllHeaders().size; + return (ratio*100).toFixed(2); +}; + BlockService.prototype.syncPercentage = function(callback) { - var p2pHeight = this._p2p.getBestHeight(); - var percentage = ((p2pHeight / (this._tip.height || p2pHeight)) * 100).toFixed(2); - callback(null, percentage); + callback(null, this._syncPercentage()); }; BlockService.prototype.unsubscribe = function(name, emitter) { @@ -460,7 +461,7 @@ BlockService.prototype.onBlock = function(block, callback) { }); }; -BlockService.prototype._onBlock = function(block) { +BlockService.prototype._onBlock = function(block, header) { if (this.node.stopping || this._reorging) { return; @@ -468,6 +469,12 @@ BlockService.prototype._onBlock = function(block) { log.debug('Block Service: new block: ' + block.rhash()); + // if this block's height is not what we expect, do not process. + if (header.height !== this._tip.height + 1) { + log.debug('Block Service: New block appears to be a newer block than we can handle, skipping.'); + return; + } + var reorg = this._detectReorg(block); if (reorg) { this._handleReorg(block, this._header.getAllHeaders()); @@ -494,7 +501,7 @@ BlockService.prototype._setTip = function(tip) { BlockService.prototype._startSync = function() { - this._numNeeded = this._bestHeight - this._tip.height; + this._numNeeded = this._header.getAllHeaders().size - this._tip.height; if (this._numNeeded <= 0) { return; } @@ -525,22 +532,33 @@ BlockService.prototype._sync = function() { } var headers = this._header.getAllHeaders(); - var size = headers.size - 1; + var lastHeaderIndex = headers.size - 1; - if (this._tip.height < size) { + if (this._tip.height < lastHeaderIndex) { - if (this._tip.height % 100 === 0) { - log.info('Block Service: Blocks download progress: ' + this._tip.height + '/' + - this._bestHeight + ' (' + (this._tip.height / this._bestHeight*100).toFixed(2) + '%)'); + if (this._tip.height % 144 === 0) { + log.info('Block Service: Blocks download progress: ' + + this._tip.height + '/' + headers.size + + ' (' + this._syncPercentage() + '%)'); } - var end = headers.getIndex(Math.min(this._tip.height + BlockService.MAX_BLOCKS + 1, size)); - var endHash = end ? end.hash : null; + // the end should be our current tip height + 2 blocks, but if we only + // have one block to go, then the end should be 0, so we just get the last block + var endHash; + if (this._tip.height === lastHeaderIndex - 1) { + endHash = 0; + } else { + var end = headers.getIndex(this._tip.height + 2); + endHash = end ? end.hash : null; + } this._p2p.getBlocks({ startHash: this._tip.hash, endHash: endHash }); return; } + log.info('Block Service: The best block hash is: ' + this._tip.hash + + ' at height: ' + this._tip.height); + }; module.exports = BlockService; diff --git a/lib/services/header/index.js b/lib/services/header/index.js index d1469007..3040437b 100644 --- a/lib/services/header/index.js +++ b/lib/services/header/index.js @@ -181,23 +181,17 @@ HeaderService.prototype._onBlock = function(block) { log.debug('Header Service: new block: ' + hash); - // this is case where a block was requested by referencing a - // hash from our own headers set. - if (this._headers.get(hash)) { - - for (var i = 0; i < this.subscriptions.block.length; i++) { - this.subscriptions.block[i].emit('header/block', block); - } - - return; - + var header = this._headers.get(hash); + if (!header) { + header = block.toHeaders().toJSON(); + header.timestamp = header.ts; + header.prevHash = header.prevBlock; + this._onHeaders([header]); } - var header = block.toHeaders().toJSON(); - header.timestamp = header.ts; - header.prevHash = header.prevBlock; - this._onHeaders([header]); - + for (var i = 0; i < this.subscriptions.block.length; i++) { + this.subscriptions.block[i].emit('header/block', block); + } }; HeaderService.prototype._onHeaders = function(headers) { @@ -240,7 +234,8 @@ HeaderService.prototype._onHeaders = function(headers) { return; } - log.debug('Header Service: download complete.'); + log.info('Header Service: ' + self._headers.size - 1 + ' headers downloaded and persisted.'); + log.info('Header Service: ' + self._headers.getLastIndex().hash + ' is the best block hash.'); // at this point, we can check our header list to see if our starting tip diverged from the tip // that we have now @@ -300,7 +295,7 @@ HeaderService.prototype._startSync = function() { HeaderService.prototype._sync = function() { log.debug('Header Service: download progress: ' + this._tip.height + '/' + - this._bestHeight + ' (' + (this._tip.height / this._bestHeight*100).toFixed(2) + '%)'); + this._bestHeight + ' (' + (this._tip.height / this._bestHeight*100.00).toFixed(2) + '%)'); this._p2p.getHeaders({ startHash: this._tip.hash }); @@ -323,7 +318,7 @@ HeaderService.prototype._getPersistedHeaders = function(callback) { var start = self._encoding.encodeHeaderKey(0); var end = self._encoding.encodeHeaderKey(startingHeight + 1); - log.debug('Getting persisted headers from genesis block to block ' + (self._tip.height + 1)); + log.debug('Getting persisted headers from genesis block to block ' + self._tip.height); var criteria = { gte: start, diff --git a/lib/services/p2p/index.js b/lib/services/p2p/index.js index 7f43ba5e..68a50bcf 100644 --- a/lib/services/p2p/index.js +++ b/lib/services/p2p/index.js @@ -176,6 +176,7 @@ P2P.prototype._connect = function() { }; P2P.prototype._getBestHeight = function() { + if (this._peers === 0) { return 0; } diff --git a/lib/services/timestamp/index.js b/lib/services/timestamp/index.js index b53d34b4..36a99441 100644 --- a/lib/services/timestamp/index.js +++ b/lib/services/timestamp/index.js @@ -29,10 +29,6 @@ TimestampService.prototype.getAPIMethods = function() { ]; }; -TimestampService.prototype.syncPercentage = function(callback) { - return callback(null, ((this._tip.height / this._block.getBestBlockHeight()) * 100).toFixed(2) + '%'); -}; - TimestampService.prototype.getBlockHashesByTimestamp = function(low, high, callback) { assert(_.isNumber(low) && _.isNumber(high) && low < high, diff --git a/lib/services/transaction/index.js b/lib/services/transaction/index.js index 33b1f8eb..8b55f68e 100644 --- a/lib/services/transaction/index.js +++ b/lib/services/transaction/index.js @@ -37,7 +37,6 @@ TransactionService.prototype.getAPIMethods = function() { ['getRawTransaction', this, this.getRawTransaction, 1], ['getTransaction', this, this.getTransaction, 1], ['getDetailedTransaction', this, this.getDetailedTransaction, 1], - ['syncPercentage', this, this.syncPercentage, 0], ['getInputValues', this, this._getInputValues, 1] ]; }; @@ -90,13 +89,13 @@ TransactionService.prototype.getTransaction = function(txid, options, callback) } // TODO: this could cause crazy amounts of recursion if input values are missing from the entire chain of txs - self._addMissingInputValues(_tx, callback); + self._addMissingInputValues(_tx, options, callback); }); }; -TransactionService.prototype._addMissingInputValues = function(tx, callback) { +TransactionService.prototype._addMissingInputValues = function(tx, options, callback) { // if we have cache misses from when we populated input values, // then we must go and find them after the fact (lazy-load). @@ -110,7 +109,7 @@ TransactionService.prototype._addMissingInputValues = function(tx, callback) { } var outputIndex = input.prevout.index; - self.getTransaction(input.prevout.txid(), function(err, _tx) { + self.getTransaction(input.prevout.txid(), options, function(err, _tx) { if (err || !_tx) { return next(err || new Error('tx not found for tx id: ' + input.prevout.txid())); diff --git a/test/services/block/index.unit.js b/test/services/block/index.unit.js index 819a89de..aaed646d 100644 --- a/test/services/block/index.unit.js +++ b/test/services/block/index.unit.js @@ -101,7 +101,8 @@ describe('Block Service', function() { var processBlock = sandbox.stub(blockService, '_processBlock'); blockService._unprocessedBlocks = []; blockService._header = { getAllHeaders: getAllHeaders }; - blockService._onBlock(block); + blockService._tip = { height: 123 }; + blockService._onBlock(block, { height: 124 }); expect(detectReorg.callCount).to.equal(1); expect(processBlock.callCount).to.equal(1); }); @@ -115,7 +116,8 @@ describe('Block Service', function() { var handleReorg = sandbox.stub(blockService, '_handleReorg'); blockService._unprocessedBlocks = []; blockService._header = { getAllHeaders: getAllHeaders }; - blockService._onBlock(block); + blockService._tip = { height: 123 }; + blockService._onBlock(block, { height: 124 }); expect(handleReorg.callCount).to.equal(1); expect(detectReorg.callCount).to.equal(1); expect(processBlock.callCount).to.equal(0); @@ -166,6 +168,7 @@ describe('Block Service', function() { var sync = sandbox.stub(blockService, '_sync'); blockService._bestHeight = 100; blockService._tip = { height: 99 }; + blockService._header = { getAllHeaders: sinon.stub().returns({ size: 100 }) }; blockService._startSync(); expect(sync.calledOnce).to.be.true; expect(blockService._numNeeded).to.equal(1); diff --git a/test/services/header/index.unit.js b/test/services/header/index.unit.js index 02951e83..bc603c4f 100644 --- a/test/services/header/index.unit.js +++ b/test/services/header/index.unit.js @@ -111,7 +111,7 @@ describe('Header Service', function() { headerService._bestHeight = { height: 1 }; headerService._headers = { getIndex: function() { return { hash: header.hash }; }, - getLastIndex: sinon.stub(), + getLastIndex: sinon.stub().returns({ hash: 'aa' }), set: sinon.stub() }; var getChainwork = sandbox.stub(headerService, '_getChainwork').returns(new BN(1)); diff --git a/test/services/transaction/index.unit.js b/test/services/transaction/index.unit.js index 3f69d00e..cca24e2b 100644 --- a/test/services/transaction/index.unit.js +++ b/test/services/transaction/index.unit.js @@ -124,9 +124,9 @@ describe('Transaction Service', function() { describe('#_addMissingInputValues', function() { it('should add missing input values on a tx', function(done) { - sandbox.stub(txService, 'getTransaction').callsArgWith(1, null, tx); + sandbox.stub(txService, 'getTransaction').callsArgWith(2, null, tx); tx.__inputValues = []; - txService._addMissingInputValues(tx, function(err, tx) { + txService._addMissingInputValues(tx, {}, function(err, tx) { if (err) { return done(err); }