Modify transaction interface

* Add checks when serializing
* Add default _estimateSize to generic inputs
* Fix multisig size estimation
* Change _addOutput to addOutput
* Add addInput and using that internally
* Split `getFee` out from `_updateChangeOutput`

lib/errors/spec.js

@@ -34,6 +34,18 @@ module.exports = [{
   }, {
     name: 'InvalidArgumentType',
     message: format('Invalid Argument for {2}, expected {1} but got ') + '+ typeof arguments[0]',
+  }, {
+    name: 'Transaction',
+    message: format('Internal Error on Transaction {0}'),
+    errors: [
+      {
+        name: 'FeeError',
+        message: format('Fees are not correctly set {0}'),
+      }, {
+        name: 'ChangeAddressMissing',
+        message: format('Change address is missing')
+      }
+    ]
   }, {
     name: 'Script',
     message: format('Internal Error on Script {0}'),

lib/transaction/input/input.js

@@ -156,4 +156,10 @@ Input.prototype.isNull = function() {
     this.outputIndex === 0xffffffff;
 };
 
+Input.prototype._estimateSize = function() {
+  var bufferWriter = new BufferWriter();
+  this.toBufferWriter(bufferWriter);
+  return bufferWriter.toBuffer().length;
+};
+
 module.exports = Input;

lib/transaction/input/multisigscripthash.js

@@ -1,7 +1,6 @@
 'use strict';
 
 var _ = require('lodash');
-var JSUtil = require('../../util/js');
 var inherits = require('inherits');
 var Input = require('./input');
 var Output = require('../output');
@@ -123,11 +122,14 @@ MultiSigScriptHashInput.prototype.isValidSignature = function(transaction, signa
   );
 };
 
-MultiSigScriptHashInput.OPCODES_SIZE = 10;
-MultiSigScriptHashInput.SIGNATURE_SIZE = 36;
+MultiSigScriptHashInput.OPCODES_SIZE = 7; // serialized size (<=3) + 0 .. N .. M OP_CHECKMULTISIG
+MultiSigScriptHashInput.SIGNATURE_SIZE = 74; // size (1) + DER (<=72) + sighash (1)
+MultiSigScriptHashInput.PUBKEY_SIZE = 34; // size (1) + DER (<=33)
 
 MultiSigScriptHashInput.prototype._estimateSize = function() {
-  return MultiSigScriptHashInput.OPCODES_SIZE + this.threshold * MultiSigScriptHashInput.SIGNATURE_SIZE;
+  return MultiSigScriptHashInput.OPCODES_SIZE +
+    this.threshold * MultiSigScriptHashInput.SIGNATURE_SIZE +
+    this.publicKeys.length * MultiSigScriptHashInput.PUBKEY_SIZE;
 };
 
 module.exports = MultiSigScriptHashInput;

lib/transaction/input/publickeyhash.js

@@ -85,10 +85,10 @@ PublicKeyHashInput.prototype.isFullySigned = function() {
   return this.script.isPublicKeyHashIn();
 };
 
-PublicKeyHashInput.FIXED_SIZE = 32 + 4 + 2;
-PublicKeyHashInput.SCRIPT_MAX_SIZE = 34 + 20;
+PublicKeyHashInput.SCRIPT_MAX_SIZE = 73 + 34; // sigsize (1 + 72) + pubkey (1 + 33)
+
 PublicKeyHashInput.prototype._estimateSize = function() {
-  return PublicKeyHashInput.FIXED_SIZE + PublicKeyHashInput.SCRIPT_MAX_SIZE;
+  return PublicKeyHashInput.SCRIPT_MAX_SIZE;
 };
 
 module.exports = PublicKeyHashInput;

lib/transaction/transaction.js

@@ -3,8 +3,8 @@
 var _ = require('lodash');
 var $ = require('../util/preconditions');
 var buffer = require('buffer');
-var assert = require('assert');
 
+var errors = require('../errors');
 var util = require('../util/js');
 var bufferUtil = require('../util/buffer');
 var JSUtil = require('../util/js');
@@ -98,12 +98,46 @@ Transaction.prototype._getHash = function() {
  * Retrieve a hexa string that can be used with bitcoind's CLI interface
  * (decoderawtransaction, sendrawtransaction)
  *
+ * @param {boolean=} unsafe if true, skip testing for fees that are too high
  * @return {string}
  */
-Transaction.prototype.serialize = Transaction.prototype.toString = function() {
+Transaction.prototype.serialize = function(unsafe) {
+  if (unsafe) {
+    return this.uncheckedSerialize();
+  } else {
+    return this.checkedSerialize();
+  }
+};
+
+Transaction.prototype.uncheckedSerialize = Transaction.prototype.toString = function() {
   return this.toBuffer().toString('hex');
 };
 
+Transaction.prototype.checkedSerialize = Transaction.prototype.toString = function() {
+  var feeError = this._validateFees();
+  if (feeError) {
+    var changeError = this._validateChange();
+    if (changeError) {
+      throw new errors.Transaction.ChangeAddressMissing();
+    } else {
+      throw new errors.Transaction.FeeError(feeError);
+    }
+  }
+  return this.uncheckedSerialize();
+};
+
+Transaction.prototype._validateFees = function() {
+  if (this.getFee() > Transaction.FEE_SECURITY_MARGIN * this._estimateFee()) {
+    return 'Fee is more than ' + Transaction.FEE_SECURITY_MARGIN + ' times the suggested amount';
+  }
+};
+
+Transaction.prototype._validateChange = function() {
+  if (!this._change) {
+    return 'Missing change address';
+  }
+};
+
 Transaction.prototype.inspect = function() {
   return '<Transaction: ' + this.toString() + '>';
 };
@@ -327,10 +361,9 @@ Transaction.prototype._fromMultisigOldUtxo = function(utxo, pubkeys, threshold)
 };
 
 Transaction.prototype._fromMultisigNewUtxo = function(utxo, pubkeys, threshold) {
-  this._changeSetup = false;
   utxo.address = utxo.address && new Address(utxo.address);
   utxo.script = new Script(util.isHexa(utxo.script) ? new buffer.Buffer(utxo.script, 'hex') : utxo.script);
-  this.inputs.push(new MultiSigScriptHashInput({
+  this.addInput(new MultiSigScriptHashInput({
     output: new Output({
       script: utxo.script,
       satoshis: utxo.satoshis
@@ -340,7 +373,35 @@ Transaction.prototype._fromMultisigNewUtxo = function(utxo, pubkeys, threshold)
     sequenceNumber: DEFAULT_SEQNUMBER,
     script: Script.empty()
   }, pubkeys, threshold));
-  this._inputAmount += utxo.satoshis;
+};
+
+/**
+ * Add an input to this transaction. The input must be an instance of the `Input` class.
+ * It should have information about the Output that it's spending, but if it's not already
+ * set, two additional parameters, `outputScript` and `satoshis` can be provided.
+ *
+ * @param {Input} input
+ * @param {String|Script} outputScript
+ * @param {number} satoshis
+ * @return Transaction this, for chaining
+ */
+Transaction.prototype.addInput = function(input, outputScript, satoshis) {
+  $.checkArgumentType(input, Input, 'input');
+  if (!input.output || !(input.output instanceof Output) && !outputScript && !satoshis) {
+    throw new Transaction.NeedMoreInfo('Need information about the UTXO script and satoshis');
+  }
+  if (!input.output && outputScript && satoshis) {
+    outputScript = outputScript instanceof Script ? outputScript : new Script(outputScript);
+    $.checkArgumentType(satoshis, 'number', 'satoshis');
+    input.output = new Output({
+      script: outputScript,
+      satoshis: satoshis
+    });
+  }
+  this._changeSetup = false;
+  this.inputs.push(input);
+  this._inputAmount += input.output.satoshis;
+  return this;
 };
 
 /**
@@ -396,7 +457,7 @@ Transaction.prototype.change = function(address) {
  * @return {Transaction} this, for chaining
  */
 Transaction.prototype.to = function(address, amount) {
-  this._addOutput(new Output({
+  this.addOutput(new Output({
     script: Script(new Address(address)),
     satoshis: amount
   }));
@@ -414,14 +475,15 @@ Transaction.prototype.to = function(address, amount) {
  * @return {Transaction} this, for chaining
  */
 Transaction.prototype.addData = function(value) {
-  this._addOutput(new Output({
+  this.addOutput(new Output({
     script: Script.buildDataOut(value),
     satoshis: 0
   }));
   return this;
 };
 
-Transaction.prototype._addOutput = function(output) {
+Transaction.prototype.addOutput = function(output) {
+  $.checkArgumentType(output, Output, 'output');
   this.outputs.push(output);
   this._changeSetup = false;
   this._outputAmount += output.satoshis;
@@ -440,12 +502,11 @@ Transaction.prototype._updateChangeOutput = function() {
   if (!_.isUndefined(this._changeOutput)) {
     this.removeOutput(this._changeOutput);
   }
-  var estimatedSize = this._estimateSize();
-  var available = this._inputAmount - this._outputAmount;
-  var fee = this._fee || Transaction._estimateFee(estimatedSize, available);
+  var available = this._getUnspentValue();
+  var fee = this.getFee();
   if (available - fee > 0) {
     this._changeOutput = this.outputs.length;
-    this._addOutput(new Output({
+    this.addOutput(new Output({
       script: Script.fromAddress(this._change),
       satoshis: available - fee
     }));
@@ -455,6 +516,20 @@ Transaction.prototype._updateChangeOutput = function() {
   this._changeSetup = true;
 };
 
+Transaction.prototype.getFee = function() {
+  return this._fee || this._estimateFee();
+};
+
+Transaction.prototype._estimateFee = function() {
+  var estimatedSize = this._estimateSize();
+  var available = this._getUnspentValue();
+  return Transaction._estimateFee(estimatedSize, available);
+};
+
+Transaction.prototype._getUnspentValue = function() {
+  return this._inputAmount - this._outputAmount;
+};
+
 Transaction.prototype._clearSignatures = function() {
   _.each(this.inputs, function(input) {
     input.clearSignatures();

test/script/interpreter.js

@@ -208,7 +208,7 @@ describe('Interpreter', function() {
       sequenceNumber: 0xffffffff,
       script: Script('OP_0 OP_0')
     }));
-    credtx._addOutput(new Transaction.Output({
+    credtx.addOutput(new Transaction.Output({
       script: scriptPubkey,
       satoshis: 0
     }));
@@ -221,7 +221,7 @@ describe('Interpreter', function() {
       sequenceNumber: 0xffffffff,
       script: scriptSig
     }));
-    spendtx._addOutput(new Transaction.Output({
+    spendtx.addOutput(new Transaction.Output({
       script: Script(),
       satoshis: 0
     }));