From 0a4e0dd9fd8f166075a31ab180735c6dd4ccb286 Mon Sep 17 00:00:00 2001 From: Chris Kleeschulte Date: Thu, 26 Oct 2017 15:35:01 -0400 Subject: [PATCH] Fixed tests and repaired reorg logic. --- bitcore-node.json.sample | 26 ++++++++++++++++++------- lib/services/block/index.js | 30 ++++++++++++++++++++++------- lib/services/header/index.js | 30 ++++++++++++++++++++++++----- test/services/block/index.unit.js | 14 ++++++-------- test/services/header/index.unit.js | 2 +- test/services/mempool/index.unit.js | 5 +++-- 6 files changed, 77 insertions(+), 30 deletions(-) diff --git a/bitcore-node.json.sample b/bitcore-node.json.sample index 153f6e45..6fba4aac 100644 --- a/bitcore-node.json.sample +++ b/bitcore-node.json.sample @@ -1,5 +1,5 @@ { - "network": "livenet", + "network": "testnet", "port": 3001, "datadir": "/tmp", "services": [ @@ -7,16 +7,28 @@ "db", "header", "block", + "mempool", + "address", "transaction", "timestamp", - "mempool", - "address" + "fee", + "insight-api", + "web" ], "servicesConfig": { - "p2p": { - "peers": [ - { "ip": { "v4": "" } } - ] + "insight-api": { + "routePrefix": "api", + "disableRateLimiter": true, + "enableCache": true + }, + "fee": { + "rpc": { + "user": "local", + "pass": "local", + "host": "localhost", + "protocol": "http", + "port": 18332 + } } } } diff --git a/lib/services/block/index.js b/lib/services/block/index.js index 945b8b8f..ff98f78b 100644 --- a/lib/services/block/index.js +++ b/lib/services/block/index.js @@ -421,6 +421,9 @@ BlockService.prototype.syncPercentage = function(callback) { }; BlockService.prototype._detectReorg = function(block) { + // a block that is regarded as a "reorging block" could be one that was + // mined using a previously-orphaned block as its previous block. + // in this case, we want to completely ignore this block and move on return bcoin.util.revHex(block.prevBlock) !== this._tip.hash; }; @@ -617,7 +620,7 @@ BlockService.prototype._findLatestValidBlockHeader = function(callback) { async.until(function() { - return iterCount++ >= self._recentBlockHashes.length || header; + return iterCount++ > self._recentBlockHashes.length || header; }, function(next) { @@ -627,15 +630,24 @@ BlockService.prototype._findLatestValidBlockHeader = function(callback) { return next(err); } + var hash = blockServiceHash; + var height = blockServiceHeight; + + blockServiceHeight--; + blockServiceHash = self._recentBlockHashes.get(hash); + if (!_header) { + // try again with the previous hash of the current hash return next(); } - if (_header.hash === blockServiceHash && _header.height === blockServiceHeight--) { + // if there was no reorg (the header service just received an orphan block, we should + // get the header of our tip here. + if (_header.hash === hash && _header.height === height) { header = _header; + return next(); } - blockServiceHash = self._recentBlockHashes.get(blockServiceHash); next(); }); @@ -648,9 +660,8 @@ BlockService.prototype._findLatestValidBlockHeader = function(callback) { // the header could be undefined // this means that the header service has no record of // any of our recent block hashes in its indexes. - // this means we've reorged deeper than recentBlockHashesCount number - // of blocks or the header service connected to the wrong chain. - // either way, we can't continue + // if some joker mines a block using an orphan block as its prev block, then the effect of this will be + // us detecting a reorg, but not actually reorging anything assert(header, 'Block Service: we could not locate any of our recent block hashes in the header service ' + 'index. Perhaps our header service sync\'ed to the wrong chain?'); @@ -718,7 +729,7 @@ BlockService.prototype._handleReorg = function(callback) { var self = this; - // we want to ensure that we can reask for previously delievered inventory + // we want to ensure that we can re-ask for previously delievered inventory self._p2p.clearInventoryCache(); var commonAncestorHeader; @@ -734,6 +745,11 @@ BlockService.prototype._handleReorg = function(callback) { return next(err); } + // nothing to do, skip and proceed + if (_commonAncestorHeader.hash === self._tip.hash) { + return callback(); + } + commonAncestorHeader = _commonAncestorHeader; next(); diff --git a/lib/services/header/index.js b/lib/services/header/index.js index 740775ab..e0f7ef3c 100644 --- a/lib/services/header/index.js +++ b/lib/services/header/index.js @@ -195,13 +195,33 @@ HeaderService.prototype.start = function(callback) { self._adjustTipBackToCheckpoint(); - if (self._tip.height === 0) { - return self._setGenesisBlock(next); - } + async.waterfall([ - self._adjustHeadersForCheckPointTip(next); + function(next) { + + if (self._tip.height === 0) { + return self._setGenesisBlock(next); + } + + next(); + + }, + + function(next) { + self._adjustHeadersForCheckPointTip(next); + } + + ], function(err) { + + if (err) { + return next(err); + } + + next(); + + }); + } - }, ], function(err) { if (err) { diff --git a/test/services/block/index.unit.js b/test/services/block/index.unit.js index ea0ddaa0..b2da9ae5 100644 --- a/test/services/block/index.unit.js +++ b/test/services/block/index.unit.js @@ -44,22 +44,20 @@ describe('Block Service', function() { }); }); - describe('#_findCommonAncestorAndBlockHashesToRemove', function() { + describe('#_findLatestValidBlockHeader', function() { - it('should find the common ancestor and hashes between the current chain and the new chain', function(done) { + it('should find the latest valid block header whose hash is also in our block index', function(done) { - sandbox.stub(blockService, '_getBlock').callsArgWith(1, null, block2); - blockService._tip = { hash: 'aa' }; - var headers = new utils.SimpleMap(); + blockService._tip = { hash: 'aa', height: 2 }; - blockService._header = { getBlockHeader: sandbox.stub().callsArgWith(1, null, { hash: 'bb' }) }; - blockService._findCommonAncestorAndBlockHashesToRemove(function(err, header, hashes) { + blockService._header = { getBlockHeader: sandbox.stub().callsArgWith(1, null, { hash: 'aa', height: 2 }) }; + blockService._findLatestValidBlockHeader(function(err, header) { if(err) { return done(err); } - expect(header).to.deep.equal({ hash: 'bb' }); + expect(header).to.deep.equal({ hash: 'aa', height: 2 }); done(); }); diff --git a/test/services/header/index.unit.js b/test/services/header/index.unit.js index b3084a0e..5a02a193 100644 --- a/test/services/header/index.unit.js +++ b/test/services/header/index.unit.js @@ -52,7 +52,7 @@ describe('Header Service', function() { headerService.start(function() { expect(setGenesisBlock.calledOnce).to.be.true; - expect(adjustHeadersForCheckPointTip.calledOnce).to.be.false; + expect(adjustHeadersForCheckPointTip.calledOnce).to.be.true; expect(setListeners.calledOnce).to.be.true; expect(headerService._tip).to.be.deep.equal({ height: 0, hash: '00' }); expect(headerService._encoding).to.be.instanceOf(Encoding); diff --git a/test/services/mempool/index.unit.js b/test/services/mempool/index.unit.js index f969c890..0c83bda6 100644 --- a/test/services/mempool/index.unit.js +++ b/test/services/mempool/index.unit.js @@ -32,12 +32,10 @@ describe('Mempool Service', function() { it('should get the db prefix', function(done) { var getPrefix = sandbox.stub().callsArgWith(1, null, new Buffer('0001', 'hex')); - var startSubs = sandbox.stub(mempoolService, '_startSubscriptions'); mempoolService._db = { getPrefix: getPrefix }; mempoolService.start(function() { expect(getPrefix.calledOnce).to.be.true; - expect(startSubs.calledOnce).to.be.true; done(); }); }); @@ -82,6 +80,9 @@ describe('Mempool Service', function() { describe('#_onBlock', function() { it('should remove block\'s txs from database', function(done) { + mempoolService.node = { openBus: sinon.stub() }; + mempoolService._p2p = { getMempool: sinon.stub() }; + sandbox.stub(mempoolService, '_startSubscriptions'); mempoolService.enable(); mempoolService.onBlock(block, function(err, ops) { expect(ops[0].type).to.deep.equal('del');