From b727a65ea0dde78b8806d8e1162e13b1fc03e6e5 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Mon, 18 Aug 2014 15:23:13 +1000 Subject: [PATCH] Wallet: refactor to use Array unspents solely, deprecating unspentsMap --- src/wallet.js | 236 +++++++++++++++++++++++++++++++------------------ test/wallet.js | 87 +++++++++--------- 2 files changed, 197 insertions(+), 126 deletions(-) diff --git a/src/wallet.js b/src/wallet.js index 00a0857..f25f7d8 100644 --- a/src/wallet.js +++ b/src/wallet.js @@ -24,9 +24,12 @@ function Wallet(seed, network) { this.addresses = [] this.changeAddresses = [] this.network = network + this.unspents = [] + + // FIXME: remove in 2.0.0 this.unspentMap = {} - // FIXME: remove in 2.x.y + // FIXME: remove in 2.0.0 var me = this this.newMasterKey = function(seed) { console.warn('newMasterKey is deprecated, please make a new Wallet instance instead') @@ -41,6 +44,7 @@ function Wallet(seed, network) { me.addresses = [] me.changeAddresses = [] + me.unspents = [] me.unspentMap = {} } @@ -50,27 +54,54 @@ function Wallet(seed, network) { this.getInternalAccount = function() { return internalAccount } } -Wallet.prototype.createTx = function(to, value, fixedFee, changeAddress) { +Wallet.prototype.createTransaction = function(to, value, options) { + // FIXME: remove in 2.0.0 + if (typeof options !== 'object') { + if (options !== undefined) { + console.warn('Non options object parameters are deprecated, use options object instead') + + options = { + fixedFee: arguments[2], + changeAddress: arguments[3] + } + } + } + + options = options || {} + assert(value > this.network.dustThreshold, value + ' must be above dust threshold (' + this.network.dustThreshold + ' Satoshis)') - var utxos = getCandidateOutputs(this.unspentMap, value) + var changeAddress = options.changeAddress + var fixedFee = options.fixedFee + var minConf = options.minConf === undefined ? 0 : options.minConf // FIXME: change minConf:1 by default in 2.0.0 + + // filter by minConf, then pending and sort by descending value + var unspents = this.unspents.filter(function(unspent) { + return unspent.confirmations >= minConf + }).filter(function(unspent) { + return !unspent.pending + }).sort(function(o1, o2) { + return o2.value - o1.value + }) + var accum = 0 - var subTotal = value var addresses = [] + var subTotal = value var txb = new TransactionBuilder() txb.addOutput(to, value) - for (var i = 0; i < utxos.length; ++i) { - var utxo = utxos[i] - addresses.push(utxo.address) + for (var i = 0; i < unspents.length; ++i) { + var unspent = unspents[i] + addresses.push(unspent.address) - txb.addInput(utxo.hash, utxo.index) + txb.addInput(unspent.txHash, unspent.index) var fee = fixedFee === undefined ? estimatePaddedFee(txb.buildIncomplete(), this.network) : fixedFee - accum += utxo.value + accum += unspent.value subTotal = value + fee + if (accum >= subTotal) { var change = accum - subTotal @@ -87,15 +118,20 @@ Wallet.prototype.createTx = function(to, value, fixedFee, changeAddress) { return this.signWith(txb, addresses).build() } +// FIXME: remove in 2.0.0 Wallet.prototype.processPendingTx = function(tx){ this.__processTx(tx, true) } +// FIXME: remove in 2.0.0 Wallet.prototype.processConfirmedTx = function(tx){ this.__processTx(tx, false) } +// FIXME: remove in 2.0.0 Wallet.prototype.__processTx = function(tx, isPending) { + console.warn('processTransaction is considered harmful, see issue #260 for more information') + var txId = tx.getId() var txHash = tx.getHash() @@ -110,31 +146,44 @@ Wallet.prototype.__processTx = function(tx, isPending) { var myAddresses = this.addresses.concat(this.changeAddresses) if (myAddresses.indexOf(address) > -1) { - var output = txId + ':' + i + var lookup = txId + ':' + i + if (lookup in this.unspentMap) return - this.unspentMap[output] = { - hash: txHash, - index: i, - value: txOut.value, + // its unique, add it + var unspent = { address: address, + confirmations: 0, // no way to determine this without more information + index: i, + txHash: txHash, + txId: txId, + value: txOut.value, pending: isPending } + + this.unspentMap[lookup] = unspent + this.unspents.push(unspent) } }, this) tx.ins.forEach(function(txIn, i) { // copy and convert to big-endian hex - var txinId = bufferutils.reverse(txIn.hash).toString('hex') - var output = txinId + ':' + txIn.index + var txInId = bufferutils.reverse(txIn.hash).toString('hex') - if (!(output in this.unspentMap)) return + var lookup = txInId + ':' + txIn.index + if (!(lookup in this.unspentMap)) return + + var unspent = this.unspentMap[lookup] if (isPending) { - this.unspentMap[output].pending = true - this.unspentMap[output].spent = true + unspent.pending = true + unspent.spent = true } else { - delete this.unspentMap[output] + delete this.unspentMap[lookup] + + this.unspents = this.unspents.filter(function(unspent2) { + return unspent !== unspent2 + }) } }, this) } @@ -157,9 +206,25 @@ Wallet.prototype.generateChangeAddress = function() { return this.getChangeAddress() } -Wallet.prototype.getBalance = function() { - return this.getUnspentOutputs().reduce(function(accum, output) { - return accum + output.value +Wallet.prototype.getAddress = function() { + if (this.addresses.length === 0) { + this.generateAddress() + } + + return this.addresses[this.addresses.length - 1] +} + +Wallet.prototype.getBalance = function(minConf) { + minConf = minConf || 0 + + return this.unspents.filter(function(unspent) { + return unspent.confirmations >= minConf + + // FIXME: remove spent filter in 2.0.0 + }).filter(function(unspent) { + return !unspent.spent + }).reduce(function(accum, unspent) { + return accum + unspent.value }, 0) } @@ -193,65 +258,77 @@ Wallet.prototype.getPrivateKeyForAddress = function(address) { assert(false, 'Unknown address. Make sure the address is from the keychain and has been generated') } -Wallet.prototype.getReceiveAddress = function() { - if (this.addresses.length === 0) { - this.generateAddress() - } +Wallet.prototype.getUnspentOutputs = function(minConf) { + minConf = minConf || 0 - return this.addresses[this.addresses.length - 1] -} + return this.unspents.filter(function(unspent) { + return unspent.confirmations >= minConf -Wallet.prototype.getUnspentOutputs = function() { - var utxos = [] + // FIXME: remove spent filter in 2.0.0 + }).filter(function(unspent) { + return !unspent.spent + }).map(function(unspent) { + return { + address: unspent.address, + confirmations: unspent.confirmations, + index: unspent.index, + txId: unspent.txId, + value: unspent.value, - for (var key in this.unspentMap) { - var output = this.unspentMap[key] - - // Don't include pending spent outputs - if (!output.spent) { - // hash is little-endian, we want big-endian - var txId = bufferutils.reverse(output.hash) - - utxos.push({ - hash: txId.toString('hex'), - index: output.index, - address: output.address, - value: output.value, - pending: output.pending - }) + // FIXME: remove in 2.0.0 + hash: unspent.txId, + pending: unspent.pending } - } - - return utxos + }) } -Wallet.prototype.setUnspentOutputs = function(utxos) { - utxos.forEach(function(utxo) { - var txId = utxo.hash - assert.equal(typeof txId, 'string', 'Expected txId, got ' + txId) +Wallet.prototype.setUnspentOutputs = function(unspents) { + this.unspentMap = {} + this.unspents = unspents.map(function(unspent) { + // FIXME: remove unspent.hash in 2.0.0 + var txId = unspent.txId || unspent.hash + var index = unspent.index - var hash = bufferutils.reverse(new Buffer(txId, 'hex')) - var index = utxo.index - var address = utxo.address - var value = utxo.value + // FIXME: remove in 2.0.0 + if (unspent.hash !== undefined) { + console.warn('unspent.hash is deprecated, use unspent.txId instead') + } - // FIXME: remove alternative in 2.x.y - if (index === undefined) index = utxo.outputIndex + // FIXME: remove in 2.0.0 + if (index === undefined) { + console.warn('unspent.outputIndex is deprecated, use unspent.index instead') + index = utxo.outputIndex + } - assert.equal(hash.length, 32, 'Expected hash length of 32, got ' + hash.length) + assert.equal(typeof txId, 'string', 'Expected txId, got ' + txId) + assert.equal(txId.length, 64, 'Expected valid txId, got ' + txId) + assert.doesNotThrow(function() { Address.fromBase58Check(unspent.address) }, 'Expected Base58 Address, got ' + unspent.address) assert.equal(typeof index, 'number', 'Expected number index, got ' + index) - assert.doesNotThrow(function() { Address.fromBase58Check(address) }, 'Expected Base58 Address, got ' + address) - assert.equal(typeof value, 'number', 'Expected number value, got ' + value) + assert.equal(typeof unspent.value, 'number', 'Expected number value, got ' + unspent.value) - var output = txId + ':' + index - - this.unspentMap[output] = { - address: address, - hash: hash, - index: index, - pending: utxo.pending, - value: value + // FIXME: remove branch in 2.0.0 + if (unspent.confirmations !== undefined) { + assert.equal(typeof unspent.confirmations, 'number', 'Expected number confirmations, got ' + unspent.confirmations) } + + var txHash = bufferutils.reverse(new Buffer(txId, 'hex')) + + var unspent = { + address: unspent.address, + confirmations: unspent.confirmations || 0, + index: index, + txHash: txHash, + txId: txId, + value: unspent.value, + + // FIXME: remove in 2.0.0 + pending: unspent.pending || false + } + + // FIXME: remove in 2.0.0 + this.unspentMap[txId + ':' + index] = unspent + + return unspent }, this) } @@ -272,21 +349,8 @@ function estimatePaddedFee(tx, network) { return network.estimateFee(tmpTx) } -function getCandidateOutputs(outputs/*, value*/) { - var unspents = [] - - for (var key in outputs) { - var output = outputs[key] - - if (!output.pending) { - unspents.push(output) - } - } - - // sorted by descending value - return unspents.sort(function(o1, o2) { - return o2.value - o1.value - }) -} +// FIXME: 1.0.0 shims, remove in 2.0.0 +Wallet.prototype.getReceiveAddress = Wallet.prototype.getAddress +Wallet.prototype.createTx = Wallet.prototype.createTransaction module.exports = Wallet diff --git a/test/wallet.js b/test/wallet.js index 917c3bf..f73d7be 100644 --- a/test/wallet.js +++ b/test/wallet.js @@ -1,4 +1,5 @@ var assert = require('assert') +var bufferutils = require('../src/bufferutils') var crypto = require('../src/crypto') var networks = require('../src/networks') var sinon = require('sinon') @@ -201,13 +202,12 @@ describe('Wallet', function() { beforeEach(function() { utxo = { "address" : "1AZpKpcfCzKDUeTFBQUL4MokQai3m3HMXv", - "hash": fakeTxId(6), + "confirmations": 1, "index": 0, - "pending": true, - "value": 20000 + "txId": fakeTxId(6), + "value": 20000, + "pending": false } - - expectedOutputKey = utxo.hash + ":" + utxo.index }) describe('on construction', function() { @@ -217,11 +217,10 @@ describe('Wallet', function() { }) it('matches the expected behaviour', function() { - var output = wallet.unspentMap[expectedOutputKey] + var output = wallet.unspents[0] - assert(output) - assert.equal(output.value, utxo.value) assert.equal(output.address, utxo.address) + assert.equal(output.value, utxo.value) }) }) @@ -245,20 +244,32 @@ describe('Wallet', function() { wallet.setUnspentOutputs([utxo]) }) - it('parses wallet outputs to the expected format', function() { - assert.deepEqual(wallet.getUnspentOutputs(), [utxo]) + it('parses wallet unspents to the expected format', function() { + var outputs = wallet.getUnspentOutputs() + var output = outputs[0] + + assert.equal(utxo.address, output.address) + assert.equal(utxo.index, output.index) + assert.equal(utxo.value, output.value) + + // FIXME: remove in 2.0.0 + assert.equal(utxo.txId, output.hash) + assert.equal(utxo.pending, output.pending) + + // new in 2.0.0 + assert.equal(utxo.txId, output.txId) + assert.equal(utxo.confirmations, output.confirmations) }) - it("ignores pending spending outputs (outputs with 'spent' property)", function() { - var output = wallet.unspentMap[expectedOutputKey] - output.pending = true - output.spent = true + it("ignores spent unspents (outputs with 'spent' property)", function() { + var unspent = wallet.unspents[0] + unspent.pending = true + unspent.spent = true assert.deepEqual(wallet.getUnspentOutputs(), []) }) }) }) - // FIXME: remove in 2.x.y describe('setUnspentOutputs', function() { var utxo var expectedOutputKey @@ -271,16 +282,13 @@ describe('Wallet', function() { value: 500000 } - expectedOutputKey = utxo.hash + ":" + utxo.index - wallet = new Wallet(seed, networks.bitcoin) }) it('matches the expected behaviour', function() { wallet.setUnspentOutputs([utxo]) - var output = wallet.unspentMap[expectedOutputKey] - assert(output) + var output = wallet.unspents[0] assert.equal(output.value, utxo.value) assert.equal(output.address, utxo.address) }) @@ -340,13 +348,12 @@ describe('Wallet', function() { Array.prototype.reverse.call(txInId) txInId = txInId.toString('hex') - var key = txInId + ':' + txIn.index - var output = wallet.unspentMap[key] + var unspent = wallet.unspents[0] + assert(!unspent.pending) - assert(!output.pending) wallet.processPendingTx(spendTx) - assert(output.pending) - assert(output.spent, true) + assert(unspent.pending) + assert(unspent.spent, true) }) }) }) @@ -389,7 +396,7 @@ describe('Wallet', function() { } }) - describe("when tx ins outpoint contains a known txhash:i", function() { + describe("when tx ins contains a known txhash:i", function() { var spendTx beforeEach(function() { wallet.addresses = [addresses[0]] // the address fixtureTx2 used as input @@ -403,11 +410,9 @@ describe('Wallet', function() { assert.deepEqual(wallet.unspentMap, {}) }) - it("deletes corresponding 'output'", function() { + it("deletes corresponding 'unspent'", function() { var txIn = spendTx.ins[0] - var txInId = new Buffer(txIn.hash) - Array.prototype.reverse.call(txInId) - txInId = txInId.toString('hex') + var txInId = bufferutils.reverse(txIn.hash).toString('hex') var expected = txInId + ':' + txIn.index assert(expected in wallet.unspentMap) @@ -416,19 +421,20 @@ describe('Wallet', function() { assert(!(expected in wallet.unspentMap)) }) }) - - it("does nothing when none of the involved addresses belong to the wallet", function() { - wallet.processConfirmedTx(tx) - assert.deepEqual(wallet.unspentMap, {}) - }) }) + it("does nothing when none of the involved addresses belong to the wallet", function() { + wallet.processConfirmedTx(tx) + assert.deepEqual(wallet.unspentMap, {}) + }) + + function verifyOutputAdded(index, pending) { var txOut = tx.outs[index] var key = tx.getId() + ":" + index var output = wallet.unspentMap[key] - assert.deepEqual(output.hash, tx.getHash()) + assert.deepEqual(output.txHash, tx.getHash()) assert.equal(output.value, txOut.value) assert.equal(output.pending, pending) @@ -512,13 +518,14 @@ describe('Wallet', function() { }) function getFee(wallet, tx) { - var inputValue = tx.ins.reduce(function(memo, input){ - var id = Array.prototype.reverse.call(input.hash).toString('hex') - return memo + wallet.unspentMap[id + ':' + input.index].value + var inputValue = tx.ins.reduce(function(accum, input) { + var txId = bufferutils.reverse(input.hash).toString('hex') + + return accum + wallet.unspentMap[txId + ':' + input.index].value }, 0) - return tx.outs.reduce(function(memo, output){ - return memo - output.value + return tx.outs.reduce(function(accum, output) { + return accum - output.value }, inputValue) } })