From d9b2a0969bc46873a7f6ea6788122ae587809a56 Mon Sep 17 00:00:00 2001 From: Braydon Fuller Date: Mon, 12 Oct 2015 09:29:49 -0400 Subject: [PATCH] Fixed bug with balance There was a bug when getting unspent outputs that would include an output that was spent in the mempool in addition to the new output with the change address. This lead to a balance having an output counted twice towards the end balance. The solution is to have the isSpent method for the address service to also include if the output was spent in the mempool, as the isSpent method exposed from bitcoind only includes if the output was spent in a block. --- lib/services/address/index.js | 32 +++++++++--- test/services/address/index.unit.js | 79 ++++++++++++++++++++++++----- 2 files changed, 90 insertions(+), 21 deletions(-) diff --git a/lib/services/address/index.js b/lib/services/address/index.js index df2a593c..873e272c 100644 --- a/lib/services/address/index.js +++ b/lib/services/address/index.js @@ -1048,8 +1048,12 @@ AddressService.prototype.getUnspentOutputsForAddress = function(address, queryMe return callback(new errors.NoOutputs('Address ' + address + ' has no outputs'), []); } + var opts = { + queryMempool: queryMempool + }; + var isUnspent = function(output, callback) { - self.isUnspent(output, callback); + self.isUnspent(output, opts, callback); }; async.filter(outputs, isUnspent, function(results) { @@ -1061,25 +1065,37 @@ AddressService.prototype.getUnspentOutputsForAddress = function(address, queryMe /** * Will give the inverse of isSpent * @param {Object} output + * @param {Object} options + * @param {Boolean} options.queryMempool - Include mempool in results * @param {Function} callback */ -AddressService.prototype.isUnspent = function(output, callback) { - this.isSpent(output, function(spent) { +AddressService.prototype.isUnspent = function(output, options, callback) { + $.checkArgument(_.isFunction(callback)); + this.isSpent(output, options, function(spent) { callback(!spent); }); }; /** - * Will determine if an output is spent, results do not include the mempool. + * Will determine if an output is spent. * @param {Object} output - An output as returned from getOutputs + * @param {Object} options + * @param {Boolean} options.queryMempool - Include mempool in results * @param {Function} callback */ -AddressService.prototype.isSpent = function(output, callback) { +AddressService.prototype.isSpent = function(output, options, callback) { + $.checkArgument(_.isFunction(callback)); + var queryMempool = _.isUndefined(options.queryMempool) ? true : options.queryMempool; var self = this; var txid = output.prevTxId ? output.prevTxId.toString('hex') : output.txid; - + var spent = self.node.services.bitcoind.isSpent(txid, output.outputIndex); + if (!spent && queryMempool) { + var spentIndexKey = [txid, output.outputIndex].join('-'); + spent = self.mempoolSpentIndex[spentIndexKey] ? true : false; + } setImmediate(function() { - callback(self.node.services.bitcoind.isSpent(txid, output.outputIndex)); + // TODO error should be the first argument? + callback(spent); }); }; @@ -1179,7 +1195,7 @@ AddressService.prototype.getAddressSummary = function(address, options, callback var txids = []; for(var i = 0; i < outputs.length; i++) { - // Bitcoind's isSpent at the moment only works for confirmed transactions + // Bitcoind's isSpent only works for confirmed transactions var spentDB = self.node.services.bitcoind.isSpent(outputs[i].txid, outputs[i].outputIndex); var spentIndexKey = [outputs[i].txid, outputs[i].outputIndex].join('-'); var spentMempool = self.mempoolSpentIndex[spentIndexKey]; diff --git a/test/services/address/index.unit.js b/test/services/address/index.unit.js index 17655b49..5add2cce 100644 --- a/test/services/address/index.unit.js +++ b/test/services/address/index.unit.js @@ -872,7 +872,7 @@ describe('Address Service', function() { var am = new AddressService({node: mocknode}); am.getOutputs = sinon.stub().callsArgWith(2, null, outputs); - am.isUnspent = function(output, callback) { + am.isUnspent = function(output, options, callback) { callback(!outputs[i].spent); i++; }; @@ -914,24 +914,24 @@ describe('Address Service', function() { }); it('should give true when isSpent() gives false', function(done) { - am.isSpent = sinon.stub().callsArgWith(1, false); - am.isUnspent('1KiW1A4dx1oRgLHtDtBjcunUGkYtFgZ1W', function(unspent) { + am.isSpent = sinon.stub().callsArgWith(2, false); + am.isUnspent('1KiW1A4dx1oRgLHtDtBjcunUGkYtFgZ1W', {}, function(unspent) { unspent.should.equal(true); done(); }); }); it('should give false when isSpent() gives true', function(done) { - am.isSpent = sinon.stub().callsArgWith(1, true); - am.isUnspent('1KiW1A4dx1oRgLHtDtBjcunUGkYtFgZ1W', function(unspent) { + am.isSpent = sinon.stub().callsArgWith(2, true); + am.isUnspent('1KiW1A4dx1oRgLHtDtBjcunUGkYtFgZ1W', {},function(unspent) { unspent.should.equal(false); done(); }); }); it('should give false when isSpent() returns an error', function(done) { - am.isSpent = sinon.stub().callsArgWith(1, new Error('error')); - am.isUnspent('1KiW1A4dx1oRgLHtDtBjcunUGkYtFgZ1W', function(unspent) { + am.isSpent = sinon.stub().callsArgWith(2, new Error('error')); + am.isUnspent('1KiW1A4dx1oRgLHtDtBjcunUGkYtFgZ1W', {}, function(unspent) { unspent.should.equal(false); done(); }); @@ -939,7 +939,6 @@ describe('Address Service', function() { }); describe('#isSpent', function() { - var am; var db = {}; var testnode = { db: db, @@ -949,16 +948,70 @@ describe('Address Service', function() { } } }; - before(function() { - am = new AddressService({node: testnode}); + it('should give true if bitcoind.isSpent gives true', function(done) { + var am = new AddressService({node: testnode}); am.node.services.bitcoind = { isSpent: sinon.stub().returns(true), on: sinon.stub() }; + am.isSpent({}, {}, function(spent) { + spent.should.equal(true); + done(); + }); }); - - it('should give true if bitcoind.isSpent gives true', function(done) { - am.isSpent('output', function(spent) { + it('should give true if bitcoind.isSpent is false and mempoolSpentIndex is true', function(done) { + var am = new AddressService({node: testnode}); + am.node.services.bitcoind = { + isSpent: sinon.stub().returns(false), + on: sinon.stub() + }; + var txid = '3b6bc2939d1a70ce04bc4f619ee32608fbff5e565c1f9b02e4eaa97959c59ae7'; + var outputIndex = 0; + var output = { + prevTxId: new Buffer(txid, 'hex'), + outputIndex: outputIndex + }; + var spentKey = [txid, outputIndex].join('-'); + am.mempoolSpentIndex[spentKey] = new Buffer(5); + am.isSpent(output, {queryMempool: true}, function(spent) { + spent.should.equal(true); + done(); + }); + }); + it('should give false if spent in mempool with queryMempool set to false', function(done) { + var am = new AddressService({node: testnode}); + am.node.services.bitcoind = { + isSpent: sinon.stub().returns(false), + on: sinon.stub() + }; + var txid = '3b6bc2939d1a70ce04bc4f619ee32608fbff5e565c1f9b02e4eaa97959c59ae7'; + var outputIndex = 0; + var output = { + prevTxId: new Buffer(txid, 'hex'), + outputIndex: outputIndex + }; + var spentKey = [txid, outputIndex].join('-'); + am.mempoolSpentIndex[spentKey] = new Buffer(5); + am.isSpent(output, {queryMempool: false}, function(spent) { + spent.should.equal(false); + done(); + }); + }); + it('default to querying the mempool', function(done) { + var am = new AddressService({node: testnode}); + am.node.services.bitcoind = { + isSpent: sinon.stub().returns(false), + on: sinon.stub() + }; + var txid = '3b6bc2939d1a70ce04bc4f619ee32608fbff5e565c1f9b02e4eaa97959c59ae7'; + var outputIndex = 0; + var output = { + prevTxId: new Buffer(txid, 'hex'), + outputIndex: outputIndex + }; + var spentKey = [txid, outputIndex].join('-'); + am.mempoolSpentIndex[spentKey] = new Buffer(5); + am.isSpent(output, {}, function(spent) { spent.should.equal(true); done(); });