Wallet: cleanup createTx control flow

Unknowingly this also revealed a subtle bug in the previous
implementation which allowed the creation of transactions even
when no UTXOs existed.

src/wallet.js

@@ -179,34 +179,40 @@ function Wallet(seed, options) {
   }
 
   this.createTx = function(to, value, fixedFee, changeAddress) {
-    checkDust(value)
+    if (isDust(value)) throw new Error("Value must be above dust threshold")
+
+    var utxos = getCandidateOutputs(value)
+    var accum = 0
+    var subTotal = value
 
     var tx = new Transaction()
     tx.addOutput(to, value)
 
-    var utxo = getCandidateOutputs(value)
-    var totalInValue = 0
-    for(var i=0; i<utxo.length; i++){
-      var output = utxo[i]
-      tx.addInput(output.receive)
+    for (var i = 0; i < utxos.length; ++i) {
+      var utxo = utxos[i]
 
-      totalInValue += output.value
-      if(totalInValue < value) continue
+      tx.addInput(utxo.receive)
+      accum += utxo.value
 
       var fee = fixedFee == undefined ? estimateFeePadChangeOutput(tx) : fixedFee
-      if(totalInValue < value + fee) continue
 
-      var change = totalInValue - value - fee
-      if(change > 0 && !isDust(change)) {
-        tx.addOutput(changeAddress || getChangeAddress(), change)
+      subTotal = value + fee
+      if (accum >= subTotal) {
+        var change = accum - subTotal
+
+        if (!isDust(change)) {
+          tx.addOutput(changeAddress || getChangeAddress(), change)
+        }
+
+        break
       }
-      break
     }
 
-    checkInsufficientFund(totalInValue, value, fee)
+    if (accum < subTotal) {
+      throw new Error('Not enough funds: ' + accum + ' < ' + subTotal)
+    }
 
     this.sign(tx)
-
     return tx
   }
 
@@ -232,12 +238,6 @@ function Wallet(seed, options) {
     return amount <= me.dustThreshold
   }
 
-  function checkDust(value){
-    if (isNullOrUndefined(value) || isDust(value)) {
-      throw new Error("Value must be above dust threshold")
-    }
-  }
-
   function getCandidateOutputs(value){
     var unspent = []
     for (var key in me.outputs){
@@ -263,13 +263,6 @@ function Wallet(seed, options) {
     return me.changeAddresses[me.changeAddresses.length - 1]
   }
 
-  function checkInsufficientFund(totalInValue, value, fee) {
-    if(totalInValue < value + fee) {
-      throw new Error('Not enough money to send funds including transaction fee. Have: ' +
-                      totalInValue + ', needed: ' + (value + fee))
-    }
-  }
-
   this.sign = function(tx) {
     tx.ins.forEach(function(inp,i) {
       var output = me.outputs[inp.outpoint.hash + ':' + inp.outpoint.index]

test/wallet.js

@@ -492,13 +492,23 @@ describe('Wallet', function() {
 
     describe('testnet', function(){
       it('should create transaction', function(){
-        var to = 'mt7MyTVVEWnbwpF5hBn6fgnJcv95Syk2ue'
         var wallet = new Wallet(seed, {network: 'testnet'})
-        var tx = wallet.createTx(to, value)
+        var address = wallet.generateAddress()
 
+        wallet.setUnspentOutputs([{
+          hash: fakeTxHash(0),
+          outputIndex: 0,
+          address: address,
+          value: value
+        }])
+
+        var to = 'mt7MyTVVEWnbwpF5hBn6fgnJcv95Syk2ue'
+        var toValue = value - 20000
+
+        var tx = wallet.createTx(to, toValue)
         assert.equal(tx.outs.length, 1)
         assert.equal(tx.outs[0].address.toString(), to)
-        assert.equal(tx.outs[0].value, value)
+        assert.equal(tx.outs[0].value, toValue)
       })
     })
 
@@ -605,7 +615,7 @@ describe('Wallet', function() {
 
         assert.throws(function() {
           wallet.createTx(to, value)
-        }, /Not enough money to send funds including transaction fee. Have: 1420000, needed: 1420001/)
+        }, /Not enough funds: 1420000 < 1420001/)
       })
     })
   })