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

@@ -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){
+    this.outputs = processUnspentOutputs(utxo)
-      validateUnspentOutput(uo)
-      var o = unspentOutputToOutput(uo)
-      outputs[o.from] = o
-    })
-
-    this.outputs = outputs
   }
+
   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) {
+function processUnspentOutputs(utxos) {
-  return value == undefined
+  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*/) {

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 = {
+      utxo = {
-        "hash":"6a4062273ac4f9ea4ffca52d9fd102b08f6c32faa0a4d1318e3a7b2e437bb9c7",
-        "outputIndex": 0,
         "address" : "1AZpKpcfCzKDUeTFBQUL4MokQai3m3HMXv",
-        "value": 20000,
+        "hash": fakeTxId(6),
-        "pending": true
+        "index": 0,
+        "pending": true,
+        "value": 20000
       }
-      expectedOutputKey = expectedUtxo.hash + ":" + expectedUtxo.outputIndex
+
+      expectedOutputKey = utxo.hash + ":" + utxo.index
     })
 
-    function addUtxoToOutput(utxo){
+    describe('on construction', function(){
-      var key = utxo.hash + ":" + utxo.outputIndex
+      beforeEach(function(){
-      wallet.outputs[key] = {
+        wallet = new Wallet(seed, networks.bitcoin, [utxo])
-        from: key,
+      })
-        address: utxo.address,
+
-        value: utxo.value,
+      it('matches the expected behaviour', function(){
-        pending: utxo.pending
+        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)
+        var utxo1 = cloneObject(utxo)
-        utxo1.hash = utxo1.hash.replace('7', 'l')
+        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(){
+  // FIXME: remove in 2.x.y
-      var utxo
+  describe('setUnspentOutputs', function(){
-      beforeEach(function(){
+    var utxo
-        utxo = cloneObject([expectedUtxo])
+    var expectedOutputKey
-      })
 
-      it('matches the expected behaviour', function(){
+    beforeEach(function(){
-        wallet.setUnspentOutputs(utxo)
+      utxo = {
-        verifyOutputs()
+        hash: fakeTxId(0),
-      })
+        index: 0,
+        address: '115qa7iPZqn6as57hxLL8E9VUnhmGQxKWi',
+        value: 500000
+      }
 
-      describe('required fields', function(){
+      expectedOutputKey = utxo.hash + ":" + utxo.index
-        ['outputIndex', 'address', 'hash', 'value'].forEach(function(field){
-          it("throws an error when " + field + " is missing", function(){
-            delete utxo[0][field]
 
-            assert.throws(function() {
+      wallet = new Wallet(seed, networks.bitcoin)
-              wallet.setUnspentOutputs(utxo)
+    })
-            }, new RegExp('Invalid unspent output: key ' + field + ' is missing'))
+
+    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 = "n1GyUANZand9Kw6hGSV9837cCC9FFUQzQa"
-      address1 = wallet.generateAddress()
+      address2 = "n2fiWrHqD6GM5GiEqkbWAc6aaZQp3ba93X"
-      address2 = wallet.generateAddress()
 
-      // set up 3 utxo
+      // set up 3 utxos
-      utxo = [
+      var utxos = [
         {
           "hash": fakeTxId(1),
-          "outputIndex": 0,
+          "index": 0,
-          "address" : address1,
+          "address": address1,
           "value": 400000 // not enough for value
         },
         {
           "hash": fakeTxId(2),
-          "outputIndex": 1,
+          "index": 1,
-          "address" : address1,
+          "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 utxo = {
-        var address = wallet.generateAddress()
-        wallet.setUnspentOutputs([{
           hash: fakeTxId(0),
-          outputIndex: 0,
+          index: 0,
-          address: address,
+          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(){
+      it('uses confirmed outputs', function(){
-        utxo.push(
+        var tx2 = new Transaction()
-          {
+        tx2.addInput(fakeTxId(4), 0)
-            "hash": fakeTxId(4),
+        tx2.addOutput(address2, 530000)
-            "outputIndex": 0,
-            "address" : address2,
-            "value": 530000,
-            "pending": true
-          }
-        )
-        wallet.setUnspentOutputs(utxo)
 
+        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 fromValue = 510000
-        var toValue = value / 2
+        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)