From 2f00c9ab353a5fac49c882d65433a6a737d3534b Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Thu, 14 Aug 2014 11:03:54 +1000 Subject: [PATCH] Wallet: rework unspents to primarily work on initialization The RegExp for the UTXO validation was removed as the errors are now more verbose and specific to each case. --- src/wallet.js | 85 +++++++--------- test/wallet.js | 263 ++++++++++++++++++++++++++----------------------- 2 files changed, 177 insertions(+), 171 deletions(-) diff --git a/src/wallet.js b/src/wallet.js index f377377..8dd2ca2 100644 --- a/src/wallet.js +++ b/src/wallet.js @@ -7,7 +7,7 @@ var HDNode = require('./hdnode') var Transaction = require('./transaction') var Script = require('./script') -function Wallet(seed, network) { +function Wallet(seed, network, unspents) { seed = seed || crypto.randomBytes(32) network = network || networks.bitcoin @@ -26,7 +26,7 @@ function Wallet(seed, network) { this.changeAddresses = [] // Transaction output data - this.outputs = {} + this.outputs = unspents ? processUnspentOutputs(unspents) : {} this.generateAddress = function() { var key = externalAccount.derive(this.addresses.length) @@ -77,16 +77,11 @@ function Wallet(seed, network) { } this.setUnspentOutputs = function(utxo) { - var outputs = {} + console.warn('setUnspentOutputs is deprecated, please use the constructor option instead') - utxo.forEach(function(uo){ - validateUnspentOutput(uo) - var o = unspentOutputToOutput(uo) - outputs[o.from] = o - }) - - this.outputs = outputs + this.outputs = processUnspentOutputs(utxo) } + this.processPendingTx = function(tx){ processTx(tx, true) } @@ -95,6 +90,8 @@ function Wallet(seed, network) { processTx(tx, false) } + var me = this + function processTx(tx, isPending) { var txid = tx.getId() @@ -241,49 +238,13 @@ function outputToUnspentOutput(output){ return { hash: hashAndIndex[0], - outputIndex: parseInt(hashAndIndex[1]), + index: parseInt(hashAndIndex[1]), address: output.address, value: output.value, pending: output.pending } } -function unspentOutputToOutput(o) { - var hash = o.hash - var key = hash + ":" + o.outputIndex - return { - from: key, - address: o.address, - value: o.value, - pending: o.pending - } -} - -function validateUnspentOutput(uo) { - var missingField - - if (isNullOrUndefined(uo.hash)) { - missingField = "hash" - } - - var requiredKeys = ['outputIndex', 'address', 'value'] - requiredKeys.forEach(function (key) { - if (isNullOrUndefined(uo[key])){ - missingField = key - } - }) - - if (missingField) { - var message = [ - 'Invalid unspent output: key', missingField, 'is missing.', - 'A valid unspent output must contain' - ] - message.push(requiredKeys.join(', ')) - message.push("and hash") - throw new Error(message.join(' ')) - } -} - function estimatePaddedFee(tx, network) { var tmpTx = tx.clone() tmpTx.addOutput(Script.EMPTY, network.dustSoftThreshold || 0) @@ -291,8 +252,34 @@ function estimatePaddedFee(tx, network) { return network.estimateFee(tmpTx) } -function isNullOrUndefined(value) { - return value == undefined +function processUnspentOutputs(utxos) { + var outputs = {} + + utxos.forEach(function(utxo){ + var hash = new Buffer(utxo.hash, 'hex') + var index = utxo.index + var address = utxo.address + var value = utxo.value + + // FIXME: remove alternative in 2.x.y + if (index === undefined) index = utxo.outputIndex + + assert.equal(hash.length, 32, 'Expected hash length of 32, got ' + hash.length) + 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) + + var key = utxo.hash + ':' + utxo.index + + outputs[key] = { + from: key, + address: address, + value: value, + pending: utxo.pending + } + }) + + return outputs } function getCandidateOutputs(outputs/*, value*/) { diff --git a/test/wallet.js b/test/wallet.js index 456775d..c08ae2f 100644 --- a/test/wallet.js +++ b/test/wallet.js @@ -26,13 +26,17 @@ function fakeTxId(i) { } describe('Wallet', function() { - var seed, wallet + var seed beforeEach(function(){ seed = crypto.sha256("don't use a string seed like this in real life") - wallet = new Wallet(seed) }) describe('constructor', function() { + var wallet + beforeEach(function(){ + wallet = new Wallet(seed) + }) + it('defaults to Bitcoin network', function() { assert.equal(wallet.getMasterKey().network, networks.bitcoin) }) @@ -116,6 +120,11 @@ describe('Wallet', function() { }) describe('generateChangeAddress', function(){ + var wallet + beforeEach(function(){ + wallet = new Wallet(seed) + }) + it('generates change addresses', function(){ var wallet = new Wallet(seed, networks.testnet) var expectedAddresses = ["mnXiDR4MKsFxcKJEZjx4353oXvo55iuptn"] @@ -126,6 +135,11 @@ describe('Wallet', function() { }) describe('getPrivateKey', function(){ + var wallet + beforeEach(function(){ + wallet = new Wallet(seed) + }) + it('returns the private key at the given index of external account', function(){ var wallet = new Wallet(seed, networks.testnet) @@ -135,6 +149,11 @@ describe('Wallet', function() { }) describe('getInternalPrivateKey', function(){ + var wallet + beforeEach(function(){ + wallet = new Wallet(seed) + }) + it('returns the private key at the given index of internal account', function(){ var wallet = new Wallet(seed, networks.testnet) @@ -144,6 +163,11 @@ describe('Wallet', function() { }) describe('getPrivateKeyForAddress', function(){ + var wallet + beforeEach(function(){ + wallet = new Wallet(seed) + }) + it('returns the private key for the given address', function(){ var wallet = new Wallet(seed, networks.testnet) wallet.generateChangeAddress() @@ -162,6 +186,7 @@ describe('Wallet', function() { it('raises an error when address is not found', function(){ var wallet = new Wallet(seed, networks.testnet) + assert.throws(function() { wallet.getPrivateKeyForAddress("n2fiWrHqD6GM5GiEqkbWAc6aaZQp3ba93X") }, /Unknown address. Make sure the address is from the keychain and has been generated/) @@ -169,51 +194,55 @@ describe('Wallet', function() { }) describe('Unspent Outputs', function(){ - var expectedUtxo, expectedOutputKey + var utxo, expectedOutputKey + var wallet + beforeEach(function(){ - expectedUtxo = { - "hash":"6a4062273ac4f9ea4ffca52d9fd102b08f6c32faa0a4d1318e3a7b2e437bb9c7", - "outputIndex": 0, + utxo = { "address" : "1AZpKpcfCzKDUeTFBQUL4MokQai3m3HMXv", - "value": 20000, - "pending": true + "hash": fakeTxId(6), + "index": 0, + "pending": true, + "value": 20000 } - expectedOutputKey = expectedUtxo.hash + ":" + expectedUtxo.outputIndex + + expectedOutputKey = utxo.hash + ":" + utxo.index }) - function addUtxoToOutput(utxo){ - var key = utxo.hash + ":" + utxo.outputIndex - wallet.outputs[key] = { - from: key, - address: utxo.address, - value: utxo.value, - pending: utxo.pending - } - } + describe('on construction', function(){ + beforeEach(function(){ + wallet = new Wallet(seed, networks.bitcoin, [utxo]) + }) + + it('matches the expected behaviour', function(){ + var output = wallet.outputs[expectedOutputKey] + + assert(output) + assert.equal(output.value, utxo.value) + assert.equal(output.address, utxo.address) + }) + }) describe('getBalance', function(){ - var utxo1 - beforeEach(function(){ - utxo1 = cloneObject(expectedUtxo) - utxo1.hash = utxo1.hash.replace('7', 'l') + var utxo1 = cloneObject(utxo) + utxo1.hash = fakeTxId(5) + + wallet = new Wallet(seed, networks.bitcoin, [utxo, utxo1]) }) it('sums over utxo values', function(){ - addUtxoToOutput(expectedUtxo) - addUtxoToOutput(utxo1) - assert.equal(wallet.getBalance(), 40000) }) }) describe('getUnspentOutputs', function(){ beforeEach(function(){ - addUtxoToOutput(expectedUtxo) + wallet = new Wallet(seed, networks.bitcoin, [utxo]) }) it('parses wallet outputs to the expect format', function(){ - assert.deepEqual(wallet.getUnspentOutputs(), [expectedUtxo]) + assert.deepEqual(wallet.getUnspentOutputs(), [utxo]) }) it("ignores pending spending outputs (outputs with 'to' property)", function(){ @@ -223,40 +252,54 @@ describe('Wallet', function() { assert.deepEqual(wallet.getUnspentOutputs(), []) }) }) + }) - describe('setUnspentOutputs', function(){ - var utxo - beforeEach(function(){ - utxo = cloneObject([expectedUtxo]) - }) + // FIXME: remove in 2.x.y + describe('setUnspentOutputs', function(){ + var utxo + var expectedOutputKey - it('matches the expected behaviour', function(){ - wallet.setUnspentOutputs(utxo) - verifyOutputs() - }) + beforeEach(function(){ + utxo = { + hash: fakeTxId(0), + index: 0, + address: '115qa7iPZqn6as57hxLL8E9VUnhmGQxKWi', + value: 500000 + } - describe('required fields', function(){ - ['outputIndex', 'address', 'hash', 'value'].forEach(function(field){ - it("throws an error when " + field + " is missing", function(){ - delete utxo[0][field] + expectedOutputKey = utxo.hash + ":" + utxo.index - assert.throws(function() { - wallet.setUnspentOutputs(utxo) - }, new RegExp('Invalid unspent output: key ' + field + ' is missing')) + wallet = new Wallet(seed, networks.bitcoin) + }) + + it('matches the expected behaviour', function(){ + wallet.setUnspentOutputs([utxo]) + + var output = wallet.outputs[expectedOutputKey] + assert(output) + assert.equal(output.value, utxo.value) + assert.equal(output.address, utxo.address) + }) + + describe('required fields', function(){ + ['index', 'address', 'hash', 'value'].forEach(function(field){ + it("throws an error when " + field + " is missing", function(){ + delete utxo[field] + + assert.throws(function() { + wallet.setUnspentOutputs([utxo]) }) }) }) - - function verifyOutputs() { - var output = wallet.outputs[expectedOutputKey] - assert(output) - assert.equal(output.value, utxo[0].value) - assert.equal(output.address, utxo[0].address) - } }) }) describe('Process transaction', function(){ + var wallet + beforeEach(function(){ + wallet = new Wallet(seed) + }) + var addresses var tx @@ -389,39 +432,42 @@ describe('Wallet', function() { }) describe('createTx', function(){ - var to, value + var wallet var address1, address2 + var to, value beforeEach(function(){ - to = '15mMHKL96tWAUtqF3tbVf99Z8arcmnJrr3' + to = 'mt7MyTVVEWnbwpF5hBn6fgnJcv95Syk2ue' value = 500000 - // generate 2 addresses - address1 = wallet.generateAddress() - address2 = wallet.generateAddress() + address1 = "n1GyUANZand9Kw6hGSV9837cCC9FFUQzQa" + address2 = "n2fiWrHqD6GM5GiEqkbWAc6aaZQp3ba93X" - // set up 3 utxo - utxo = [ + // set up 3 utxos + var utxos = [ { "hash": fakeTxId(1), - "outputIndex": 0, - "address" : address1, + "index": 0, + "address": address1, "value": 400000 // not enough for value }, { "hash": fakeTxId(2), - "outputIndex": 1, - "address" : address1, + "index": 1, + "address": address1, "value": 500000 // enough for only value }, { "hash": fakeTxId(3), - "outputIndex": 0, + "index": 0, "address" : address2, "value": 510000 // enough for value and fee } ] - wallet.setUnspentOutputs(utxo) + + wallet = new Wallet(seed, networks.testnet, utxos) + wallet.generateAddress() + wallet.generateAddress() }) describe('transaction fee', function(){ @@ -441,17 +487,18 @@ describe('Wallet', function() { }) it('does not overestimate fees when network has dustSoftThreshold', function(){ - var wallet = new Wallet(seed, networks.litecoin) - var address = wallet.generateAddress() - wallet.setUnspentOutputs([{ + var utxo = { hash: fakeTxId(0), - outputIndex: 0, - address: address, + index: 0, + address: "LeyySKbQrRRwodKEj1W4a8y3YQupPLw5os", value: 500000 - }]) + } + + var wallet = new Wallet(seed, networks.litecoin, [utxo]) + wallet.generateAddress() value = 200000 - var tx = wallet.createTx(address, value) + var tx = wallet.createTx(utxo.address, value) assert.equal(getFee(wallet, tx), 100000) }) @@ -477,18 +524,25 @@ describe('Wallet', function() { assert.equal(tx.ins[0].index, 0) }) - it('ignores pending outputs', function(){ - utxo.push( - { - "hash": fakeTxId(4), - "outputIndex": 0, - "address" : address2, - "value": 530000, - "pending": true - } - ) - wallet.setUnspentOutputs(utxo) + it('uses confirmed outputs', function(){ + var tx2 = new Transaction() + tx2.addInput(fakeTxId(4), 0) + tx2.addOutput(address2, 530000) + wallet.processConfirmedTx(tx2) + var tx = wallet.createTx(to, value) + + assert.equal(tx.ins.length, 1) + assert.deepEqual(tx.ins[0].hash, tx2.getHash()) + assert.equal(tx.ins[0].index, 0) + }) + + it('ignores pending outputs', function(){ + var tx2 = new Transaction() + tx2.addInput(fakeTxId(4), 0) + tx2.addOutput(address2, 530000) + + wallet.processPendingTx(tx2) var tx = wallet.createTx(to, value) assert.equal(tx.ins.length, 1) @@ -497,46 +551,11 @@ describe('Wallet', function() { }) }) - describe('works for testnet', function(){ - it('should create transaction', function(){ - var wallet = new Wallet(seed, networks.testnet) - var address = wallet.generateAddress() - - wallet.setUnspentOutputs([{ - hash: fakeTxId(0), - outputIndex: 0, - address: address, - value: value - }]) - - var to = 'mt7MyTVVEWnbwpF5hBn6fgnJcv95Syk2ue' - var toValue = value - 10000 - - var tx = wallet.createTx(to, toValue) - assert.equal(tx.outs.length, 1) - - var outAddress = Address.fromOutputScript(tx.outs[0].script, networks.testnet) - assert.equal(outAddress.toString(), to) - assert.equal(tx.outs[0].value, toValue) - }) - }) - describe('changeAddress', function(){ it('should allow custom changeAddress', function(){ - var wallet = new Wallet(seed, networks.testnet) - var address = wallet.generateAddress() - - wallet.setUnspentOutputs([{ - hash: fakeTxId(0), - outputIndex: 0, - address: address, - value: value - }]) - assert.equal(wallet.getBalance(), value) - var changeAddress = 'mfrFjnKZUvTcvdAK2fUX5D8v1Epu5H8JCk' - var to = 'mt7MyTVVEWnbwpF5hBn6fgnJcv95Syk2ue' - var toValue = value / 2 + var fromValue = 510000 + var toValue = fromValue / 2 var fee = 1e3 var tx = wallet.createTx(to, toValue, fee, changeAddress) @@ -549,7 +568,7 @@ describe('Wallet', function() { assert.equal(tx.outs[0].value, toValue) assert.equal(outAddress1.toString(), changeAddress) - assert.equal(tx.outs[1].value, value - (toValue + fee)) + assert.equal(tx.outs[1].value, fromValue - (toValue + fee)) }) }) @@ -559,7 +578,7 @@ describe('Wallet', function() { assert.equal(tx.outs.length, 1) var out = tx.outs[0] - var outAddress = Address.fromOutputScript(out.script) + var outAddress = Address.fromOutputScript(out.script, networks.testnet) assert.equal(outAddress.toString(), to) assert.equal(out.value, value) @@ -574,7 +593,7 @@ describe('Wallet', function() { assert.equal(tx.outs.length, 2) var out = tx.outs[1] - var outAddress = Address.fromOutputScript(out.script) + var outAddress = Address.fromOutputScript(out.script, networks.testnet) assert.equal(outAddress.toString(), wallet.changeAddresses[1]) assert.equal(out.value, 10000) @@ -588,7 +607,7 @@ describe('Wallet', function() { assert.equal(wallet.changeAddresses.length, 1) var out = tx.outs[1] - var outAddress = Address.fromOutputScript(out.script) + var outAddress = Address.fromOutputScript(out.script, networks.testnet) assert.equal(outAddress.toString(), wallet.changeAddresses[0]) assert.equal(out.value, 10000)