mtx: subtract fee evenly from all outputs.

lib/primitives/mtx.js

@@ -1223,33 +1223,69 @@ MTX.prototype.selectCoins = function selectCoins(coins, options) {
 };
 
 /**
- * Attempt to subtract a fee from outputs.
+ * Attempt to subtract a fee from a single output.
+ * @param {Number} index
  * @param {Amount} fee
- * @param {Number?} index
  */
 
-MTX.prototype.subtractFee = function subtractFee(fee, index) {
+MTX.prototype.subtractFee = function subtractFee(index, fee) {
-  if (typeof index === 'number') {
+  assert(typeof index === 'number');
-    const output = this.outputs[index];
+  assert(typeof fee === 'number');
 
-    if (!output)
+  const output = this.outputs[index];
-      throw new Error('Subtraction index does not exist.');
 
-    const min = fee + output.getDustThreshold();
+  if (!output)
+    throw new Error('Subtraction index does not exist.');
 
-    if (output.value < min)
+  if (output.value < fee + output.getDustThreshold())
-      throw new Error('Could not subtract fee.');
+    throw new Error('Could not subtract fee.');
 
-    output.value -= fee;
+  output.value -= fee;
+};
 
-    return;
+/**
-  }
+ * Attempt to subtract a fee from all outputs evenly.
+ * @param {Amount} fee
+ */
+
+MTX.prototype.subtractFee = function subtractFee(fee) {
+  assert(typeof fee === 'number');
+
+  let outputs = 0;
 
   for (const output of this.outputs) {
-    const min = fee + output.getDustThreshold();
+    // Ignore nulldatas and
+    // other OP_RETURN scripts.
+    if (output.script.isUnspendable())
+      continue;
+    outputs += 1;
+  }
 
-    if (output.value >= min) {
+  if (outputs === 0)
-      output.value -= fee;
+    throw new Error('Could not subtract fee.');
+
+  const left = fee % outputs;
+  const share = (fee - left) / outputs;
+
+  // First pass, remove even shares.
+  for (const output of this.outputs) {
+    if (output.script.isUnspendable())
+      continue;
+
+    if (output.value < share + output.getDustThreshold())
+      throw new Error('Could not subtract fee.');
+
+    output.value -= share;
+  }
+
+  // Second pass, remove the remainder
+  // for the one unlucky output.
+  for (const output of this.outputs) {
+    if (output.script.isUnspendable())
+      continue;
+
+    if (output.value >= left + output.getDustThreshold()) {
+      output.value -= left;
       return;
     }
   }
@@ -1277,20 +1313,25 @@ MTX.prototype.fund = async function fund(coins, options) {
     this.addCoin(coin);
 
   // Attempt to subtract fee.
-  if (select.shouldSubtract)
+  if (select.subtractFee) {
-    this.subtractFee(select.fee, select.subtractFee);
+    const index = select.subtractIndex;
+    if (index !== -1)
+      this.subtractIndex(index, select.fee);
+    else
+      this.subtractFee(select.fee);
+  }
 
   // Add a change output.
-  const change = new Output();
+  const output = new Output();
-  change.value = select.change;
+  output.value = select.change;
-  change.script.fromAddress(select.changeAddress);
+  output.script.fromAddress(select.changeAddress);
 
-  if (change.isDust(policy.MIN_RELAY)) {
+  if (output.isDust(policy.MIN_RELAY)) {
     // Do nothing. Change is added to fee.
     this.changeIndex = -1;
     assert.strictEqual(this.getFee(), select.fee + select.change);
   } else {
-    this.outputs.push(change);
+    this.outputs.push(output);
     this.changeIndex = this.outputs.length - 1;
     assert.strictEqual(this.getFee(), select.fee);
   }
@@ -1511,8 +1552,8 @@ function CoinSelector(tx, options) {
   this.fee = CoinSelector.MIN_FEE;
 
   this.selection = 'value';
-  this.shouldSubtract = false;
+  this.subtractFee = false;
-  this.subtractFee = null;
+  this.subtractIndex = -1;
   this.height = -1;
   this.depth = -1;
   this.hardFee = -1;
@@ -1569,16 +1610,23 @@ CoinSelector.prototype.fromOptions = function fromOptions(options) {
 
   if (options.subtractFee != null) {
     if (typeof options.subtractFee === 'number') {
-      assert(util.isU32(options.subtractFee));
+      assert(util.isInt(options.subtractFee));
-      this.subtractFee = options.subtractFee;
+      assert(options.subtractFee >= -1);
-      this.shouldSubtract = true;
+      this.subtractIndex = options.subtractFee;
+      this.subtractFee = this.subtractIndex !== -1;
     } else {
       assert(typeof options.subtractFee === 'boolean');
       this.subtractFee = options.subtractFee;
-      this.shouldSubtract = options.subtractFee;
     }
   }
 
+  if (options.subtractIndex != null) {
+    assert(util.isInt(options.subtractIndex));
+    assert(options.subtractIndex >= -1);
+    this.subtractIndex = options.subtractIndex;
+    this.subtractFee = this.subtractIndex !== -1;
+  }
+
   if (options.height != null) {
     assert(util.isInt(options.height));
     assert(options.height >= -1);
@@ -1673,7 +1721,7 @@ CoinSelector.prototype.init = function init(coins) {
  */
 
 CoinSelector.prototype.total = function total() {
-  if (this.shouldSubtract)
+  if (this.subtractFee)
     return this.outputValue;
   return this.outputValue + this.fee;
 };
@@ -1727,17 +1775,15 @@ CoinSelector.prototype.isSpendable = function isSpendable(coin) {
  */
 
 CoinSelector.prototype.getFee = function getFee(size) {
-  let fee;
+  // This is mostly here for testing.
-
+  // i.e. A fee rounded to the nearest
+  // kb is easier to predict ahead of time.
   if (this.round) {
-    // This is mostly here for testing.
+    const fee = policy.getRoundFee(size, this.rate);
-    // i.e. A fee rounded to the nearest
+    return Math.min(fee, CoinSelector.MAX_FEE);
-    // kb is easier to predict ahead of time.
-    fee = policy.getRoundFee(size, this.rate);
-  } else {
-    fee = policy.getMinFee(size, this.rate);
   }
 
+  const fee = policy.getMinFee(size, this.rate);
   return Math.min(fee, CoinSelector.MAX_FEE);
 };
 

lib/wallet/http.js

@@ -389,6 +389,7 @@ HTTPServer.prototype.initRouter = function initRouter() {
       selection: valid.str('selection'),
       smart: valid.bool('smart'),
       subtractFee: valid.bool('subtractFee'),
+      subtractIndex: valid.i32('subtractIndex'),
       depth: valid.u32(['confirmations', 'depth']),
       outputs: []
     };
@@ -427,6 +428,7 @@ HTTPServer.prototype.initRouter = function initRouter() {
       selection: valid.str('selection'),
       smart: valid.bool('smart'),
       subtractFee: valid.bool('subtractFee'),
+      subtractIndex: valid.i32('subtractIndex'),
       depth: valid.u32(['confirmations', 'depth']),
       outputs: []
     };

lib/wallet/rpc.js

@@ -1261,7 +1261,6 @@ RPC.prototype.sendFrom = async function sendFrom(args, help) {
 
   const options = {
     account: name,
-    subtractFee: false,
     rate: this.feeRate,
     depth: minconf,
     outputs: [{
@@ -1287,7 +1286,7 @@ RPC.prototype.sendMany = async function sendMany(args, help) {
   let name = valid.str(0);
   const sendTo = valid.obj(1);
   const minconf = valid.u32(2, 1);
-  const subtractFee = valid.bool(4, false);
+  const subtract = valid.bool(4, false);
 
   if (name === '')
     name = 'default';
@@ -1320,7 +1319,7 @@ RPC.prototype.sendMany = async function sendMany(args, help) {
 
   const options = {
     outputs: outputs,
-    subtractFee: subtractFee,
+    subtractFee: subtract,
     account: name,
     depth: minconf
   };
@@ -1341,7 +1340,7 @@ RPC.prototype.sendToAddress = async function sendToAddress(args, help) {
   const valid = new Validator([args]);
   const str = valid.str(0);
   const value = valid.ufixed(1, 8);
-  const subtractFee = valid.bool(4, false);
+  const subtract = valid.bool(4, false);
 
   const addr = parseAddress(str, this.network);
 
@@ -1349,7 +1348,7 @@ RPC.prototype.sendToAddress = async function sendToAddress(args, help) {
     throw new RPCError(errs.TYPE_ERROR, 'Invalid parameter.');
 
   const options = {
-    subtractFee: subtractFee,
+    subtractFee: subtract,
     rate: this.feeRate,
     outputs: [{
       address: addr,

lib/wallet/wallet.js

@@ -1396,11 +1396,12 @@ Wallet.prototype._fund = async function _fund(mtx, options) {
     depth: options.depth,
     hardFee: options.hardFee,
     subtractFee: options.subtractFee,
+    subtractIndex: options.subtractIndex,
     changeAddress: account.change.getAddress(),
     height: this.db.state.height,
     rate: rate,
     maxFee: options.maxFee,
-    estimate: this.estimateSize.bind(this)
+    estimate: prev => this.estimateSize(prev)
   });
 
   assert(mtx.getFee() <= MTX.Selector.MAX_FEE, 'TX exceeds MAX_FEE.');