handle remove for block better.

lib/bcoin/mempool.js

@@ -559,18 +559,12 @@ Mempool.prototype.getTXByAddress = function getTXByAddress(addresses, callback)
 Mempool.prototype.fillHistory = function fillHistory(tx, callback) {
   var self = this;
 
-  if (Array.isArray(tx)) {
+  if (tx.isCoinbase()) {
-    return utils.forEachSerial(tx, function(tx, next) {
+    callback = utils.asyncify(callback);
-      self.fillHistory(tx, next);
+    return callback(null, tx);
-    }, callback);
   }
 
-  callback = utils.asyncify(callback);
+  utils.forEachSerial(tx.inputs, function(input, next) {
-
-  if (tx.isCoinbase())
-    return callback(null, tx);
-
-  utils.forEach(tx.inputs, function(input, next) {
     if (input.coin)
       return next();
 
@@ -600,18 +594,12 @@ Mempool.prototype.fillHistory = function fillHistory(tx, callback) {
 Mempool.prototype.fillCoins = function fillCoins(tx, callback) {
   var self = this;
 
-  if (Array.isArray(tx)) {
+  if (tx.isCoinbase()) {
-    return utils.forEachSerial(tx, function(tx, next) {
+    callback = utils.asyncify(callback);
-      self.fillCoins(tx, next);
+    return callback(null, tx);
-    }, callback);
   }
 
-  callback = utils.asyncify(callback);
+  utils.forEachSerial(tx.inputs, function(input, next) {
-
-  if (tx.isCoinbase())
-    return callback(null, tx);
-
-  utils.forEach(tx.inputs, function(input, next) {
     if (input.coin)
       return next();
 
@@ -842,7 +830,7 @@ Mempool.prototype.addTX = function addTX(tx, callback, force) {
  * and may lend itself to race conditions if used unwisely.
  * This function will also resolve orphans if possible (the
  * resolved orphans _will_ be validated).
- * @param {TX} tx
+ * @param {MempoolEntry} entry
  * @param {Function} callback - Returns [{@link VerifyError}].
  */
 
@@ -903,7 +891,7 @@ Mempool.prototype.addUnchecked = function addUnchecked(entry, callback, force) {
 /**
  * Remove a transaction from the mempool. Generally
  * only called when a new block is added to the main chain.
- * @param {TX} tx
+ * @param {MempoolEntry} entry
  * @param {Function} callback
  */
 
@@ -1425,7 +1413,7 @@ Mempool.prototype.fillAllCoins = function fillAllCoins(tx, callback) {
     if (tx.hasCoins())
       return callback(null, tx);
 
-    utils.forEach(tx.inputs, function(input, next) {
+    utils.forEachSerial(tx.inputs, function(input, next) {
       var hash = input.prevout.hash;
       var index = input.prevout.index;
 
@@ -1587,7 +1575,7 @@ Mempool.prototype.getConfidence = function getConfidence(hash, callback) {
 /**
  * Add a transaction to the mempool database.
  * @private
- * @param {TX} tx
+ * @param {MempoolEntry} entry
  * @param {Function} callback
  */
 
@@ -1616,14 +1604,14 @@ Mempool.prototype._addUnchecked = function _addUnchecked(entry, callback) {
 
     assert(input.coin);
 
-    batch.del('c/' + key);
-    batch.put('s/' + key, tx.hash());
-
     if (this.options.indexAddress) {
       address = input.getHash('hex');
       if (address)
         batch.del('C/' + address + '/' + key);
     }
+
+    batch.del('c/' + key);
+    batch.put('s/' + key, tx.hash());
   }
 
   for (i = 0; i < tx.outputs.length; i++) {
@@ -1633,15 +1621,15 @@ Mempool.prototype._addUnchecked = function _addUnchecked(entry, callback) {
     if (output.script.isUnspendable())
       continue;
 
-    coin = bcoin.coin.fromTX(tx, i).toRaw();
-
-    batch.put('c/' + key, coin);
-
     if (this.options.indexAddress) {
       address = output.getHash('hex');
       if (address)
         batch.put('C/' + address + '/' + key, DUMMY);
     }
+
+    coin = bcoin.coin.fromTX(tx, i).toRaw();
+
+    batch.put('c/' + key, coin);
   }
 
   return batch.write(callback);
@@ -1650,28 +1638,37 @@ Mempool.prototype._addUnchecked = function _addUnchecked(entry, callback) {
 /**
  * Remove a transaction from the database. Note
  * that this _may_ not disconnect the inputs.
+ * Transactions get removed for 2 reasons:
+ * Either they are included in a block,
+ * or they are limited.
+ *
+ * - If they are limited, we want to disconnect
+ *   the inputs and also remove all spender
+ *   transactions along with their outputs/coins.
+ *
+ * - If they are included in a block, we do not
+ *   disconnect the inputs (the coins have already
+ *   been used on the blockchain-layer). We also
+ *   do not remove spenders, since they are still
+ *   spending valid coins that exist on the blockchain.
+ *
  * @private
- * @param {Hash} hash
+ * @param {MempoolEntry} entry
+ * @param {Boolean} limit
  * @param {Function} callback
  */
 
-Mempool.prototype._removeUnchecked = function _removeUnchecked(hash, limit, callback) {
+Mempool.prototype._removeUnchecked = function _removeUnchecked(entry, limit, callback) {
   var self = this;
-  var batch, i, addresses, tx;
+  var tx = entry.tx;
+  var hash = tx.hash('hex');
+  var batch = this.db.batch();
+  var i, addresses, input, output, key, address;
 
-  if (hash.tx)
+  this._removeSpenders(entry, limit, function(err) {
-    hash = hash.tx.hash('hex');
-
-  this.getEntry(hash, function(err, entry) {
     if (err)
       return callback(err);
 
-    if (!entry)
-      return callback();
-
-    tx = entry.tx;
-    batch = self.db.batch();
-
     batch.del('t/' + hash);
     batch.del('m/' + pad32(entry.ts) + '/' + hash);
 
@@ -1681,79 +1678,98 @@ Mempool.prototype._removeUnchecked = function _removeUnchecked(hash, limit, call
         batch.del('T/' + addresses[i] + '/' + hash);
     }
 
-    utils.forEachSerial(tx.inputs, function(input, next) {
+    for (i = 0; i < tx.inputs.length; i++) {
-      var key = input.prevout.hash + '/' + input.prevout.index;
+      input = tx.inputs[i];
-      var address;
+      key = input.prevout.hash + '/' + input.prevout.index;
 
       if (tx.isCoinbase())
-        return next();
+        break;
-
-      if (!input.coin)
-        return next();
 
       batch.del('s/' + key);
 
-      self.hasTX(input.prevout.hash, function(err, result) {
+      // We only disconnect inputs if this
+      // is a limited transaction. For block
+      // transactions, the coins are still
+      // spent. They were spent on the
+      // blockchain.
+      if (!limit)
+        continue;
+
+      assert(input.coin);
+
+      if (input.coin.height !== -1)
+        continue;
+
+      if (self.options.indexAddress) {
+        address = input.getHash('hex');
+        if (address)
+          batch.put('C/' + address + '/' + key, DUMMY);
+      }
+
+      batch.put('c/' + key, input.coin.toRaw());
+    }
+
+    for (i = 0; i < tx.outputs.length; i++) {
+      output = tx.outputs[i];
+      key = hash + '/' + i;
+
+      if (output.script.isUnspendable())
+        continue;
+
+      if (self.options.indexAddress) {
+        address = output.getHash('hex');
+        if (address)
+          batch.del('C/' + address + '/' + key);
+      }
+
+      batch.del('c/' + key);
+    }
+
+    return batch.write(callback);
+  });
+};
+
+/**
+ * Recursively remove spenders of a transaction.
+ * @private
+ * @param {MempoolEntry} entry
+ * @param {Boolean} limit
+ * @param {Function} callback
+ */
+
+Mempool.prototype._removeSpenders = function _removeSpenders(entry, limit, callback) {
+  var self = this;
+  var tx = entry.tx;
+  var hash;
+
+  // We do not remove spenders if this is
+  // being removed for a block. The spenders
+  // are still spending valid coins (which
+  // now exist on the blockchain).
+  if (!limit)
+    return callback();
+
+  hash = tx.hash('hex');
+
+  utils.forEachSerial(tx.outputs, function(output, next, i) {
+    self.isSpent(hash, i, function(err, spender) {
+      if (err)
+        return next(err);
+
+      if (!spender)
+        return next();
+
+      self.getEntry(spender, function(err, entry) {
         if (err)
           return next(err);
 
-        if (result) {
+        if (!entry)
-          batch.put('c/' + key, input.coin.toRaw());
-          if (self.options.indexAddress) {
-            address = input.getHash('hex');
-            if (address)
-              batch.put('C/' + address + '/' + key, DUMMY);
-          }
-        } else {
-          batch.del('c/' + key);
-          if (self.options.indexAddress) {
-            address = input.getHash('hex');
-            if (address)
-              batch.del('C/' + address + '/' + key);
-          }
-        }
-
-        next();
-      });
-    }, function(err) {
-      if (err)
-        return callback(err);
-
-      utils.forEachSerial(tx.outputs, function(output, next, i) {
-        var key = hash + '/' + i;
-        var address;
-
-        if (output.script.isUnspendable())
           return next();
 
-        batch.del('c/' + key);
+        self.removeUnchecked(entry, limit, next, true);
-
-        if (self.options.indexAddress) {
-          address = output.getHash('hex');
-          if (address)
-            batch.del('C/' + address + '/' + key);
-        }
-
-        if (!limit)
-          return next();
-
-        self.isSpent(hash, i, function(err, spender) {
-          if (err)
-            return next(err);
-
-          if (!spender)
-            return next();
-
-          self._removeUnchecked(spender, limit, next);
-        });
-      }, function(err) {
-        if (err)
-          return callback(err);
-
-        return batch.write(callback);
       });
     });
-  });
+  }, callback);
 };
 
 /**

lib/bcoin/txdb.js

@@ -250,7 +250,7 @@ TXDB.prototype._getOrphans = function _getOrphans(key, callback) {
     if (!orphans)
       return callback();
 
-    utils.forEach(orphans, function(orphan, next) {
+    utils.forEachSerial(orphans, function(orphan, next) {
       self.getTX(orphan.hash, function(err, tx) {
         if (err)
           return next(err);
@@ -1452,18 +1452,12 @@ TXDB.prototype.getCoins = function getCoins(id, callback) {
 TXDB.prototype.fillHistory = function fillHistory(tx, callback) {
   var self = this;
 
-  if (Array.isArray(tx)) {
+  if (tx.isCoinbase()) {
-    return utils.forEachSerial(tx, function(tx, next) {
+    callback = utils.asyncify(callback);
-      self.fillHistory(tx, next);
+    return callback(null, tx);
-    }, callback);
   }
 
-  callback = utils.asyncify(callback);
+  utils.forEachSerial(tx.inputs, function(input, next) {
-
-  if (tx.isCoinbase())
-    return callback(null, tx);
-
-  utils.forEach(tx.inputs, function(input, next) {
     if (input.coin)
       return next();
 
@@ -1492,18 +1486,12 @@ TXDB.prototype.fillHistory = function fillHistory(tx, callback) {
 TXDB.prototype.fillCoins = function fillCoins(tx, callback) {
   var self = this;
 
-  if (Array.isArray(tx)) {
+  if (tx.isCoinbase()) {
-    return utils.forEachSerial(tx, function(tx, next) {
+    callback = utils.asyncify(callback);
-      self.fillCoins(tx, next);
+    return callback(null, tx);
-    }, callback);
   }
 
-  callback = utils.asyncify(callback);
+  utils.forEachSerial(tx.inputs, function(input, next) {
-
-  if (tx.isCoinbase())
-    return callback(null, tx);
-
-  utils.forEach(tx.inputs, function(input, next) {
     if (input.coin)
       return next();
 

lib/bcoin/walletdb.js

@@ -692,7 +692,7 @@ WalletDB.prototype.saveAddress = function saveAddress(id, addresses, callback) {
       hashes.push([address.getProgramHash('hex'), address]);
   }
 
-  utils.forEach(hashes, function(hash, next) {
+  utils.forEachSerial(hashes, function(hash, next) {
     if (self.tx.filter)
       self.tx.filter.add(hash[0], 'hex');