Skip to content

Commit 6ac9125

Browse files
braydonfbraydonf
committed
bitcoind: _tryAll -> _tryAllClients
Fixes a timing bug with not all clients being tried
1 parent e87f628 commit 6ac9125

File tree

2 files changed

+68
-46
lines changed

2 files changed

+68
-46
lines changed

‎lib/services/bitcoind.js

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -428,8 +428,15 @@ Bitcoin.prototype._resetCaches = function() {
428428
this.blockOverviewCache.reset();
429429
};
430430

431-
Bitcoin.prototype._tryAll = function(func, callback) {
432-
async.retry({times: this.nodes.length, interval: this.tryAllInterval || 1000}, func, callback);
431+
Bitcoin.prototype._tryAllClients = function(func, callback) {
432+
var self = this;
433+
var nodesIndex = 0;
434+
var retry = function(done) {
435+
var client = self.nodes[nodesIndex].client;
436+
nodesIndex++;
437+
func(client, done);
438+
};
439+
async.retry({times: this.nodes.length, interval: this.tryAllInterval || 1000}, retry, callback);
433440
};
434441

435442
Bitcoin.prototype._wrapRPCError = function(errObj) {
@@ -1537,8 +1544,8 @@ Bitcoin.prototype.getAddressSummary = function(addressArg, options, callback) {
15371544
Bitcoin.prototype._maybeGetBlockHash = function(blockArg, callback) {
15381545
var self = this;
15391546
if (_.isNumber(blockArg) || (blockArg.length < 40 && /^[0-9]+$/.test(blockArg))) {
1540-
self._tryAll(function(done) {
1541-
self.client.getBlockHash(blockArg, function(err, response) {
1547+
self._tryAllClients(function(client, done) {
1548+
client.getBlockHash(blockArg, function(err, response) {
15421549
if (err) {
15431550
return done(self._wrapRPCError(err));
15441551
}
@@ -1563,7 +1570,7 @@ Bitcoin.prototype.getRawBlock = function(blockArg, callback) {
15631570
if (err) {
15641571
return callback(err);
15651572
}
1566-
self._tryAll(function(done) {
1573+
self._tryAllClients(function(client, done) {
15671574
self.client.getBlock(blockhash, false, function(err, response) {
15681575
if (err) {
15691576
return done(self._wrapRPCError(err));
@@ -1603,8 +1610,8 @@ Bitcoin.prototype.getBlockOverview = function(blockArg, callback) {
16031610
callback(null, cachedBlock);
16041611
});
16051612
} else {
1606-
self._tryAll(function(done) {
1607-
self.client.getBlock(blockhash, true, function(err, response) {
1613+
self._tryAllClients(function(client, done) {
1614+
client.getBlock(blockhash, true, function(err, response) {
16081615
if (err) {
16091616
return done(self._wrapRPCError(err));
16101617
}
@@ -1654,8 +1661,8 @@ Bitcoin.prototype.getBlock = function(blockArg, callback) {
16541661
callback(null, cachedBlock);
16551662
});
16561663
} else {
1657-
self._tryAll(function(done) {
1658-
self.client.getBlock(blockhash, false, function(err, response) {
1664+
self._tryAllClients(function(client, done) {
1665+
client.getBlock(blockhash, false, function(err, response) {
16591666
if (err) {
16601667
return done(self._wrapRPCError(err));
16611668
}
@@ -1713,8 +1720,8 @@ Bitcoin.prototype.getBlockHeader = function(blockArg, callback) {
17131720
if (err) {
17141721
return callback(err);
17151722
}
1716-
self._tryAll(function(done) {
1717-
self.client.getBlockHeader(blockhash, function(err, response) {
1723+
self._tryAllClients(function(client, done) {
1724+
client.getBlockHeader(blockhash, function(err, response) {
17181725
if (err) {
17191726
return done(self._wrapRPCError(err));
17201727
}
@@ -1795,8 +1802,8 @@ Bitcoin.prototype.getRawTransaction = function(txid, callback) {
17951802
callback(null, tx);
17961803
});
17971804
} else {
1798-
self._tryAll(function(done) {
1799-
self.client.getRawTransaction(txid, function(err, response) {
1805+
self._tryAllClients(function(client, done) {
1806+
client.getRawTransaction(txid, function(err, response) {
18001807
if (err) {
18011808
return done(self._wrapRPCError(err));
18021809
}
@@ -1822,8 +1829,8 @@ Bitcoin.prototype.getTransaction = function(txid, callback) {
18221829
callback(null, tx);
18231830
});
18241831
} else {
1825-
self._tryAll(function(done) {
1826-
self.client.getRawTransaction(txid, function(err, response) {
1832+
self._tryAllClients(function(client, done) {
1833+
client.getRawTransaction(txid, function(err, response) {
18271834
if (err) {
18281835
return done(self._wrapRPCError(err));
18291836
}
@@ -1937,8 +1944,8 @@ Bitcoin.prototype.getDetailedTransaction = function(txid, callback) {
19371944
callback(null, tx);
19381945
});
19391946
} else {
1940-
self._tryAll(function(done) {
1941-
self.client.getRawTransaction(txid, 1, function(err, response) {
1947+
self._tryAllClients(function(client, done) {
1948+
client.getRawTransaction(txid, 1, function(err, response) {
19421949
if (err) {
19431950
return done(self._wrapRPCError(err));
19441951
}

‎test/services/bitcoind.unit.js

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -525,46 +525,61 @@ describe('Bitcoin Service', function() {
525525
});
526526
});
527527

528-
describe('#_tryAll', function() {
529-
it('will retry the number of bitcoind nodes', function(done) {
528+
describe('#_tryAllClients', function() {
529+
it('will retry for each node client', function(done) {
530530
var bitcoind = new BitcoinService(baseConfig);
531531
bitcoind.tryAllInterval = 1;
532-
bitcoind.nodes.push({});
533-
bitcoind.nodes.push({});
534-
bitcoind.nodes.push({});
535-
var count = 0;
536-
var func = function(callback) {
537-
count++;
538-
if (count <= 2) {
539-
callback(new Error('test'));
540-
} else {
541-
callback();
532+
bitcoind.nodes.push({
533+
client: {
534+
getInfo: sinon.stub().callsArgWith(0, new Error('test'))
542535
}
543-
};
544-
bitcoind._tryAll(function(next) {
545-
func(next);
546-
}, function() {
547-
count.should.equal(3);
536+
});
537+
bitcoind.nodes.push({
538+
client: {
539+
getInfo: sinon.stub().callsArgWith(0, new Error('test'))
540+
}
541+
});
542+
bitcoind.nodes.push({
543+
client: {
544+
getInfo: sinon.stub().callsArg(0)
545+
}
546+
});
547+
bitcoind._tryAllClients(function(client, next) {
548+
client.getInfo(next);
549+
}, function(err) {
550+
if (err) {
551+
return done(err);
552+
}
553+
bitcoind.nodes[0].client.getInfo.callCount.should.equal(1);
554+
bitcoind.nodes[1].client.getInfo.callCount.should.equal(1);
555+
bitcoind.nodes[2].client.getInfo.callCount.should.equal(1);
548556
done();
549557
});
550558
});
551-
it('will get error if all fail', function(done) {
559+
it('will get error if all clients fail', function(done) {
552560
var bitcoind = new BitcoinService(baseConfig);
553561
bitcoind.tryAllInterval = 1;
554-
bitcoind.nodes.push({});
555-
bitcoind.nodes.push({});
556-
bitcoind.nodes.push({});
557-
var count = 0;
558-
var func = function(callback) {
559-
count++;
560-
callback(new Error('test'));
561-
};
562-
bitcoind._tryAll(function(next) {
563-
func(next);
562+
bitcoind.nodes.push({
563+
client: {
564+
getInfo: sinon.stub().callsArgWith(0, new Error('test'))
565+
}
566+
});
567+
bitcoind.nodes.push({
568+
client: {
569+
getInfo: sinon.stub().callsArgWith(0, new Error('test'))
570+
}
571+
});
572+
bitcoind.nodes.push({
573+
client: {
574+
getInfo: sinon.stub().callsArgWith(0, new Error('test'))
575+
}
576+
});
577+
bitcoind._tryAllClients(function(client, next) {
578+
client.getInfo(next);
564579
}, function(err) {
565580
should.exist(err);
581+
err.should.be.instanceOf(Error);
566582
err.message.should.equal('test');
567-
count.should.equal(3);
568583
done();
569584
});
570585
});

0 commit comments

Comments
 (0)