BIP125: Enable replace-by-fee in mempool

lib/mempool/mempool.js

@@ -817,15 +817,19 @@ class Mempool extends EventEmitter {
         0);
     }
 
-    // Quick and dirty test to verify we're
-    // not double-spending an output in the
-    // mempool.
-    if (this.isDoubleSpend(tx)) {
-      this.emit('conflict', tx);
-      throw new VerifyError(tx,
-        'duplicate',
-        'bad-txns-inputs-spent',
-        0);
+    if (!this.options.replaceByFee) {
+      // Quick and dirty test to verify we're
+      // not double-spending an output in the
+      // mempool.
+      // If we are verifying RBF, we will do a
+      // slow and clean test later...
+      if (this.isDoubleSpend(tx)) {
+        this.emit('conflict', tx);
+        throw new VerifyError(tx,
+          'duplicate',
+          'bad-txns-inputs-spent',
+          0);
+      }
     }
 
     // Get coin viewpoint as it
@@ -844,7 +848,20 @@ class Mempool extends EventEmitter {
     const entry = MempoolEntry.fromTX(tx, view, height);
 
     // Contextual verification.
-    await this.verify(entry, view);
+    const conflicts = await this.verify(entry, view);
+
+    // RBF only: Remove conflicting TXs and their descendants
+    if (!this.options.replaceByFee) {
+      assert(conflicts.length === 0);
+    } else {
+      for (const conflict of conflicts) {
+        this.logger.debug(
+          'Replacing tx %h with %h',
+          conflict.tx.hash(),
+          tx.hash());
+        this.evictEntry(conflict);
+      }
+    }
 
     // Add and index the entry.
     await this.addEntry(entry, view);
@@ -862,10 +879,11 @@ class Mempool extends EventEmitter {
 
   /**
    * Verify a transaction with mempool standards.
+   * Returns an array of conflicting TXs
    * @method
    * @param {MempoolEntry} entry
    * @param {CoinView} view
-   * @returns {Promise}
+   * @returns {Promise} - Returns {@link MempoolEntry[]}
    */
 
   async verify(entry, view) {
@@ -953,6 +971,16 @@ class Mempool extends EventEmitter {
         0);
     }
 
+    // If we reject RBF transactions altogether we can skip these checks,
+    // because incoming conflicts are already rejected as double spends.
+    let conflicts = [];
+    if (this.options.replaceByFee)
+      conflicts = this.getConflicts(tx);
+
+    if (conflicts.length) {
+      this.verifyRBF(entry, view, conflicts);
+    }
+
     // Contextual sanity checks.
     const [fee, reason, score] = tx.checkInputs(view, height);
 
@@ -995,6 +1023,111 @@ class Mempool extends EventEmitter {
       assert(await this.verifyResult(tx, view, flags),
         'BUG: Verify failed for mandatory but not standard.');
     }
+
+    return conflicts;
+  }
+
+  /**
+   * Verify BIP 125 replaceability
+   * https://github.com/bitcoin/bips/blob/master/bip-0125.mediawiki
+   * Also see documented discrepancy between specification and implementation
+   * regarding Rule #1:
+   * https://www.cve.org/CVERecord?id=CVE-2021-31876
+   * Bitcoin Core policy may also change regularly, see:
+   * https://github.com/bitcoin/bitcoin/blob/master/doc/policy/
+   *   mempool-replacements.md
+   * @method
+   * @param {MempoolEntry} entry
+   * @param {CoinView} view
+   * @param {MempoolEntry[]} conflicts
+   * @returns {Boolean}
+   */
+
+  verifyRBF(entry, view, conflicts) {
+    const tx = entry.tx;
+
+    let conflictingFees = 0;
+    let totalEvictions = 0;
+
+    for (const conflict of conflicts) {
+      // Rule #1 - as implemented in Bitcoin Core (not BIP 125)
+      // We do not replace TXs that do not signal opt-in RBF
+      if (!conflict.tx.isRBF()) {
+        throw new VerifyError(tx,
+          'duplicate',
+          'bad-txns-inputs-spent',
+          0);
+      }
+
+      // Rule #2
+      // Replacement TXs can not consume any unconfirmed outputs that were not
+      // already included in the original transactions. They can only add
+      // confirmed UTXO, saving the trouble of checking new mempool ancestors.
+      // This also prevents the case of replacing our own inputs.
+      PREVOUTS: for (const {prevout} of tx.inputs) {
+        // Confirmed inputs are not relevant to Rule #2
+        if (view.getHeight(prevout) > 0) {
+          continue;
+        }
+
+        // Any unconfirmed inputs spent by the replacement
+        // must also be spent by the conflict.
+        for (const {prevout: conflictPrevout} of conflict.tx.inputs) {
+          if (conflictPrevout.hash.equals(prevout.hash)
+              && conflictPrevout.index === prevout.index) {
+            // Once we find a match we don't need to check any more
+            // of the conflict's inputs. Continue by testing the
+            // next input in the potential replacement tx.
+            continue PREVOUTS;
+          }
+        }
+
+        // Rule #2 violation
+        throw new VerifyError(tx,
+          'nonstandard',
+          'replacement-adds-unconfirmed',
+          0);
+      }
+
+      conflictingFees += conflict.descFee;
+      totalEvictions += this.countDescendants(conflict) + 1;
+
+      // Rule #5
+      // Replacement TXs must not evict/replace more than 100 descendants
+      if (totalEvictions > policy.MEMPOOL_MAX_REPLACEMENTS) {
+        throw new VerifyError(tx,
+        'nonstandard',
+        'too many potential replacements',
+        0);
+      }
+
+      // Rule #6 - currently implemented by Bitcoin Core but not in BIP 125
+      // The replacement transaction must have a higher feerate than its
+      // direct conflicts.
+      // We compare using deltaFee to account for prioritiseTransaction()
+      if (entry.getDeltaRate() <= conflict.getDeltaRate()) {
+        throw new VerifyError(tx,
+        'insufficientfee',
+        'insufficient fee: must not reduce total mempool fee rate',
+        0);
+      }
+    }
+
+    // Rule #3 and #4
+    // Replacement TX must pay for the total fees of all descendant
+    // transactions that will be evicted if an ancestor is replaced.
+    // Thus the replacement "pays for the bandwidth" of all the conflicts.
+    // Once the conflicts are all paid for, the replacement TX fee
+    // still needs to cover it's own bandwidth.
+    // We use our policy min fee, Bitcoin Core has a separate incremental fee
+    const minFee = policy.getMinFee(entry.size, this.options.minRelay);
+    const feeRemaining = entry.deltaFee - conflictingFees;
+    if (feeRemaining < minFee) {
+      throw new VerifyError(tx,
+        'insufficientfee',
+        'insufficient fee: must pay for fees including conflicts',
+        0);
+    }
   }
 
   /**
@@ -1425,9 +1558,22 @@ class Mempool extends EventEmitter {
     const missing = [];
 
     for (const {prevout} of tx.inputs) {
+      // We assume the view came from mempool.getCoinView()
+      // which will only exclude spent coins if the spender
+      // is not replaceable
       if (view.hasEntry(prevout))
         continue;
 
+      // If this prevout is missing from the view because
+      // it has a spender in the mempool it's not an orphan,
+      // it's a double-spend.
+      if (this.isSpent(prevout.hash, prevout.index)) {
+        throw new VerifyError(tx,
+          'duplicate',
+          'bad-txns-inputs-spent',
+          0);
+      }
+
       if (this.hasReject(prevout.hash)) {
         this.logger.debug(
           'Not storing orphan %h (rejected parents).',
@@ -1659,13 +1805,35 @@ class Mempool extends EventEmitter {
   isDoubleSpend(tx) {
     for (const {prevout} of tx.inputs) {
       const {hash, index} = prevout;
-      if (this.isSpent(hash, index))
+      const conflict = this.getSpent(hash, index);
+
+      if (conflict)
         return true;
     }
 
     return false;
   }
 
+  /**
+   * Get an array of all transactions currently in the mempool that
+   * spend one or more of the same outputs as an incoming transaction.
+   * @param {TX} tx
+   * @returns {Promise} - Returns (@link MempoolEntry[]}.
+   */
+
+  getConflicts(tx) {
+    const conflicts = new Set();
+
+    for (const { prevout: { hash, index } } of tx.inputs) {
+      const conflict = this.getSpent(hash, index);
+
+      if (conflict) {
+        conflicts.add(conflict);
+      }
+    }
+    return Array.from(conflicts);
+  }
+
   /**
    * Get coin viewpoint (lock).
    * Note: this does not return
@@ -1713,7 +1881,7 @@ class Mempool extends EventEmitter {
   }
 
   /**
-   * Get coin viewpoint (no lock).
+   * Get coin viewpoint as it pertains to mempool (no lock).
    * @method
    * @param {TX} tx
    * @returns {Promise} - Returns {@link CoinView}.
@@ -1724,14 +1892,41 @@ class Mempool extends EventEmitter {
 
     for (const {prevout} of tx.inputs) {
       const {hash, index} = prevout;
-      const tx = this.getTX(hash);
 
-      if (tx) {
-        if (this.hasCoin(hash, index))
-          view.addIndex(tx, index, -1);
-        continue;
+      // First check mempool for the TX
+      // that created the coin we need for the view
+      const parentTX = this.getTX(hash);
+
+      if (parentTX) {
+        // Does parent TX even have the output index?
+        if (index >= parentTX.outputs.length)
+          continue;
+
+        // Check to see if this output is already spent
+        // by another unconfirmed TX in the mempool
+        const spender = this.getSpent(hash, index);
+
+        if (!spender) {
+          // Parent TX output is unspent, add it to the view
+          view.addIndex(parentTX, index, -1);
+          continue;
+        }
+
+        // If the spender TX signals opt-in RBF, then we
+        // can still consider the parent TX output as spendable.
+        // Note that at this point we are not fully checking all the
+        // replaceability rules and the tx initally passed to this
+        // function may not be actually allowed to replace the spender.
+        if (spender.tx.isRBF()) {
+          // If any TX in the mempool signals opt-in RBF
+          // we already know this option is set.
+          assert(this.options.replaceByFee);
+          view.addIndex(parentTX, index, -1);
+        }
       }
 
+      // Parent TX is not in mempool.
+      // Check the chain (UTXO set) for the coin.
       const coin = await this.chain.readCoin(prevout);
 
       if (coin)

lib/protocol/policy.js

@@ -172,6 +172,13 @@ exports.MEMPOOL_EXPIRY_TIME = 72 * 60 * 60;
 
 exports.MEMPOOL_MAX_ORPHANS = 100;
 
+/**
+ * BIP125 Rule #5
+ * Maximum number of transactions that can be replaced.
+ */
+
+exports.MEMPOOL_MAX_REPLACEMENTS = 100;
+
 /**
  * Minimum block size to create. Block will be
  * filled with free transactions until block