Merge pull request #1184 from braydonf/bug/NaN-output

Added precondition to transaction.to for a positive integer as an amount. Closes #1183

lib/transaction/output.js

@@ -7,6 +7,7 @@ var bufferUtil = require('../util/buffer');
 var JSUtil = require('../util/js');
 var BufferWriter = require('../encoding/bufferwriter');
 var Script = require('../script');
+var $ = require('../util/preconditions');
 
 function Output(params) {
   if (!(this instanceof Output)) {
@@ -50,6 +51,10 @@ Object.defineProperty(Output.prototype, 'satoshis', {
       this._satoshisBN = BN.fromNumber(num);
       this._satoshis = num;
     }
+    $.checkState(
+      JSUtil.isPositiveInteger(this._satoshis),
+      'Output satoshis is not a positive integer'
+    );
   }
 });
 

lib/transaction/transaction.js

@@ -73,7 +73,7 @@ Transaction.MAX_MONEY = 21000000 * 1e8;
 // nlocktime limit to be considered block height rather than a timestamp
 Transaction.NLOCKTIME_BLOCKHEIGHT_LIMIT = 5e8;
 
-// Max value for an unsigned 32 bit value 
+// Max value for an unsigned 32 bit value
 Transaction.NLOCKTIME_MAX_VALUE = 4294967295;
 
 // Value used for fee estimation (satoshis per kilobyte)
@@ -639,6 +639,10 @@ Transaction.prototype.getChangeOutput = function() {
  * @return {Transaction} this, for chaining
  */
 Transaction.prototype.to = function(address, amount) {
+  $.checkArgument(
+    JSUtil.isPositiveInteger(amount),
+    'Amount is expected to be a positive integer'
+  );
   this.addOutput(new Output({
     script: Script(new Address(address)),
     satoshis: amount
@@ -687,7 +691,7 @@ Transaction.prototype._addOutput = function(output) {
 /**
  * Calculates or gets the total output amount in satoshis
  *
- * @return {Number} the transaction total output amount 
+ * @return {Number} the transaction total output amount
  */
 Transaction.prototype._getOutputAmount = function() {
   if (_.isUndefined(this._outputAmount)) {
@@ -704,7 +708,7 @@ Transaction.prototype._getOutputAmount = function() {
 /**
  * Calculates or gets the total input amount in satoshis
  *
- * @return {Number} the transaction total input amount 
+ * @return {Number} the transaction total input amount
  */
 Transaction.prototype._getInputAmount = function() {
   if (_.isUndefined(this._inputAmount)) {

lib/util/js.js

@@ -59,7 +59,7 @@ module.exports = {
    * @param {Object} values - An object of properties
    * @return {Object} The target object
    */
-  defineImmutable: function defineImmutable(target, values){
+  defineImmutable: function defineImmutable(target, values) {
     Object.keys(values).forEach(function(key){
       Object.defineProperty(target, key, {
         configurable: false,
@@ -68,5 +68,17 @@ module.exports = {
       });
     });
     return target;
+  },
+  /**
+   * Checks that a value is a positive integer
+   *
+   * @param {*} value
+   * @return {Boolean}
+   */
+  isPositiveInteger: function isPositiveInteger(value) {
+    return typeof value === 'number' &&
+      isFinite(value) &&
+      Math.floor(value) === value &&
+      value >= 0;
   }
 };

test/transaction/output.js

@@ -30,6 +30,52 @@ describe('Output', function() {
     newOutput.satoshis.should.equal(100);
   });
 
+  it('can be assigned a satoshi amount with a string', function() {
+    var newOutput = new Output({
+      satoshis: '100',
+      script: Script.empty()
+    });
+    newOutput.satoshis.should.equal(100);
+  });
+
+  describe('will error if output is not a positive integer', function() {
+    it('-100', function() {
+      (function() {
+        var newOutput = new Output({
+          satoshis: -100,
+          script: Script.empty()
+        });
+      }).should.throw('Output satoshis is not a positive integer');
+    });
+
+    it('1.1', function() {
+      (function() {
+        var newOutput = new Output({
+          satoshis: 1.1,
+          script: Script.empty()
+        });
+      }).should.throw('Output satoshis is not a positive integer');
+    });
+
+    it('NaN', function() {
+      (function() {
+        var newOutput = new Output({
+          satoshis: NaN,
+          script: Script.empty()
+        });
+      }).should.throw('Output satoshis is not a positive integer');
+    });
+
+    it('Infinity', function() {
+      (function() {
+        var newOutput = new Output({
+          satoshis: Infinity,
+          script: Script.empty()
+        });
+      }).should.throw('Output satoshis is not a positive integer');
+    });
+  });
+
   var expectEqualOutputs = function(a, b) {
     a.satoshis.should.equal(b.satoshis);
     a.script.toString().should.equal(b.script.toString());

test/transaction/transaction.js

@@ -50,9 +50,10 @@ describe('Transaction', function() {
     object.outputs[0].satoshis.should.equal(testAmount - 10000);
   });
 
-  it('can take a string argument as an amount', function() {
-    var stringTx = new Transaction().to('mrU9pEmAx26HcbKVrABvgL7AwA5fjNFoDc', '10000');
-    (stringTx.outputAmount).should.equal(10000);
+  it('will not accept NaN as an amount', function() {
+    (function() {
+      var stringTx = new Transaction().to('mrU9pEmAx26HcbKVrABvgL7AwA5fjNFoDc', NaN);
+    }).should.throw('Amount is expected to be a positive integer');
   });
 
   it('returns the fee correctly', function() {

test/util/js.js

@@ -10,7 +10,7 @@ var JSUtil = bitcore.util.js;
 describe('js utils', function() {
 
   describe('isValidJSON', function() {
-    
+
     var hexa = '8080808080808080808080808080808080808080808080808080808080808080';
     var json = '{"key": ["value", "value2"]}';
     var json2 = '["value", "value2", {"key": "value"}]';
@@ -32,4 +32,47 @@ describe('js utils', function() {
 
   });
 
+  describe('isPositiveInteger', function() {
+    it('false for float', function() {
+      var a = JSUtil.isPositiveInteger(0.1);
+      a.should.equal(false);
+    });
+
+    it('false for string float', function() {
+      var a = JSUtil.isPositiveInteger('0.1');
+      a.should.equal(false);
+    });
+
+    it('false for string integer', function() {
+      var a = JSUtil.isPositiveInteger('1');
+      a.should.equal(false);
+    });
+
+    it('false for negative integer', function() {
+      var a = JSUtil.isPositiveInteger(-1);
+      a.should.equal(false);
+    });
+
+    it('false for negative integer string', function() {
+      var a = JSUtil.isPositiveInteger('-1');
+      a.should.equal(false);
+    });
+
+    it('false for infinity', function() {
+      var a = JSUtil.isPositiveInteger(Infinity);
+      a.should.equal(false);
+    });
+
+    it('false for NaN', function() {
+      var a = JSUtil.isPositiveInteger(NaN);
+      a.should.equal(false);
+    });
+
+    it('true for positive integer', function() {
+      var a = JSUtil.isPositiveInteger(1000);
+      a.should.equal(true);
+    });
+
+  });
+
 });