Merge pull request #1214 from dskloet/fix/fee

Don't ignore the fee when it's explicitly specified.

lib/errors/spec.js

@@ -86,7 +86,17 @@ module.exports = [{
     message: 'Output satoshis are invalid',
   }, {
     name: 'FeeError',
-    message: 'Fees are not correctly set {0}',
+    message: 'Internal Error on Fee {0}',
+    errors: [{
+      name: 'TooSmall',
+      message: 'Fee is too small: {0}',
+    }, {
+      name: 'TooLarge',
+      message: 'Fee is too large: {0}',
+    }, {
+      name: 'Different',
+      message: 'Unspent value is different from specified fee: {0}',
+    }]
   }, {
     name: 'ChangeAddressMissing',
     message: 'Change address is missing'

lib/transaction/transaction.js

@@ -196,6 +196,11 @@ Transaction.prototype.getSerializationError = function(opts) {
     return new errors.Transaction.InvalidSatoshis();
   }
 
+  var feeIsDifferent = this._isFeeDifferent();
+  if (feeIsDifferent) {
+    return new errors.Transaction.FeeError.Different(feeIsDifferent);
+  }
+
   var missingChange = this._missingChange();
   var feeIsTooLarge = this._isFeeTooLarge();
   var feeIsTooSmall = this._isFeeTooSmall();
@@ -205,10 +210,10 @@ Transaction.prototype.getSerializationError = function(opts) {
     if (missingChange) {
       return new errors.Transaction.ChangeAddressMissing('Fee is too large and no change address was provided');
     }
-    return new errors.Transaction.FeeError(feeIsTooLarge);
+    return new errors.Transaction.FeeError.TooLarge(feeIsTooLarge);
   }
   if (!opts.disableSmallFees && feeIsTooSmall) {
-    return new errors.Transaction.FeeError(feeIsTooSmall);
+    return new errors.Transaction.FeeError.TooSmall(feeIsTooSmall);
   }
   if (!opts.disableDustOutputs && this._hasDustOutputs()) {
     return new errors.Transaction.DustOutputs();
@@ -221,11 +226,21 @@ Transaction.prototype.getSerializationError = function(opts) {
   }
 };
 
+Transaction.prototype._isFeeDifferent = function() {
+  if (!_.isUndefined(this._fee)) {
+    var fee = this._fee;
+    var unspent = this._getUnspentValue();
+    if (fee !== unspent) {
+      return 'Unspent value is ' + unspent + ' but specified fee is ' + fee;
+    }
+  }
+};
+
 Transaction.prototype._isFeeTooLarge = function() {
   var fee = this._getUnspentValue();
   var maximumFee = Math.floor(Transaction.FEE_SECURITY_MARGIN * this._estimateFee());
   if (fee > maximumFee) {
-    return 'Fee is too large: expected less than ' + maximumFee + ' but got ' + fee;
+    return 'expected less than ' + maximumFee + ' but got ' + fee;
   }
 };
 
@@ -233,7 +248,7 @@ Transaction.prototype._isFeeTooSmall = function() {
   var fee = this._getUnspentValue();
   var minimumFee = Math.ceil(this._estimateFee() / Transaction.FEE_SECURITY_MARGIN);
   if (fee < minimumFee) {
-    return 'Fee is too small: expected more than ' + minimumFee + ' but got ' + fee;
+    return 'expected more than ' + minimumFee + ' but got ' + fee;
   }
 };
 
@@ -766,6 +781,8 @@ Transaction.prototype._updateChangeOutput = function() {
 /**
  * Calculates the fee of the transaction.
  *
+ * If there's a fixed fee set, return that.
+ *
  * If there is no change output set, the fee is the
  * total value of the outputs minus inputs. Note that
  * a serialized transaction only specifies the value
@@ -774,17 +791,20 @@ Transaction.prototype._updateChangeOutput = function() {
  * This method therefore raises a "MissingPreviousOutput"
  * error when called on a serialized transaction.
  *
- * If there's a fixed fee set, return that.
- * If there's no fee set, estimate it based on size.
+ * If there's no fee set and no change address,
+ * estimate the fee based on size.
  *
  * @return {Number} fee of this transaction in satoshis
  */
 Transaction.prototype.getFee = function() {
+  if (!_.isUndefined(this._fee)) {
+    return this._fee;
+  }
   // if no change output is set, fees should equal all the unspent amount
   if (!this._changeScript) {
     return this._getUnspentValue();
   }
-  return _.isUndefined(this._fee) ? this._estimateFee() : this._fee;
+  return this._estimateFee();
 };
 
 /**

test/transaction/transaction.js

@@ -266,7 +266,7 @@ describe('Transaction', function() {
         .sign(privateKey);
       expect(function() {
         return transaction.serialize();
-      }).to.throw(errors.Transaction.FeeError);
+      }).to.throw(errors.Transaction.FeeError.TooSmall);
     });
     it('on second call to sign, change is not recalculated', function() {
       var transaction = new Transaction()
@@ -332,7 +332,7 @@ describe('Transaction', function() {
         .to(toAddress, 40000000);
       expect(function() {
         return transaction.serialize();
-      }).to.throw(errors.Transaction.FeeError);
+      }).to.throw(errors.Transaction.FeeError.TooLarge);
     });
     it('fails if a dust output is created', function() {
       var transaction = new Transaction()
@@ -364,6 +364,16 @@ describe('Transaction', function() {
         return transaction.serialize();
       }).to.not.throw(errors.Transaction.DustOutputs);
     });
+    it('fails when outputs and fee don\'t add to total input', function() {
+      var transaction = new Transaction()
+        .from(simpleUtxoWith1BTC)
+        .to(toAddress, 99900000)
+        .fee(99999)
+        .sign(privateKey);
+      expect(function() {
+        return transaction.serialize();
+      }).to.throw(errors.Transaction.FeeError.Different);
+    });
     describe('skipping checks', function() {
       var buildSkipTest = function(builder, check) {
         return function() {