[Mempool] Fix mempool limiting and replace-by-fee for PrioritiseTransaction

1) Fix mempool limiting for PrioritiseTransaction

Redo the feerate index to be based on mining score, rather than fee.

Update mempool_packages.py to test prioritisetransaction's effect on
package scores.

2) Update replace-by-fee logic to use fee deltas

3) Use fee deltas for determining mempool acceptance

4) Remove GetMinRelayFee

One test in AcceptToMemoryPool was to compare a transaction's fee
agains the value returned by GetMinRelayFee. This value was zero for
all small transactions.  For larger transactions (between
DEFAULT_BLOCK_PRIORITY_SIZE and MAX_STANDARD_TX_SIZE), this function
was preventing low fee transactions from ever being accepted.

With this function removed, we will now allow transactions in that range
with fees (including modifications via PrioritiseTransaction) below
the minRelayTxFee, provided that they have sufficient priority.

Github-Pull: #7062
Rebased-From: eb306664e786ae43d539fde66f0fbe2a3e89d910 9ef2a25603c9ec4e44c4f45c6a5d4e4386ec86d3 27fae3484cdb21b0d24face833b966fce5926be5 901b01d674031f9aca717deeb372bafa160a24af

3 files changed, 142 insertions, 2 deletions

diff --git a/qa/rpc-tests/mempool_packages.py b/qa/rpc-tests/mempool_packages.py
index 34b316a6a3..063308d394 100755
--- a/qa/rpc-tests/mempool_packages.py
+++ b/qa/rpc-tests/mempool_packages.py
@@ -64,17 +64,41 @@ class MempoolPackagesTest(BitcoinTestFramework):
         for x in reversed(chain):
             assert_equal(mempool[x]['descendantcount'], descendant_count)
             descendant_fees += mempool[x]['fee']
+            assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee'])
             assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees)
             descendant_size += mempool[x]['size']
             assert_equal(mempool[x]['descendantsize'], descendant_size)
             descendant_count += 1
 
+        # Check that descendant modified fees includes fee deltas from
+        # prioritisetransaction
+        self.nodes[0].prioritisetransaction(chain[-1], 0, 1000)
+        mempool = self.nodes[0].getrawmempool(True)
+
+        descendant_fees = 0
+        for x in reversed(chain):
+            descendant_fees += mempool[x]['fee']
+            assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees+1000)
+
         # Adding one more transaction on to the chain should fail.
         try:
             self.chain_transaction(self.nodes[0], txid, vout, value, fee, 1)
         except JSONRPCException as e:
             print "too-long-ancestor-chain successfully rejected"
 
+        # Check that prioritising a tx before it's added to the mempool works
+        self.nodes[0].generate(1)
+        self.nodes[0].prioritisetransaction(chain[-1], 0, 2000)
+        self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash())
+        mempool = self.nodes[0].getrawmempool(True)
+
+        descendant_fees = 0
+        for x in reversed(chain):
+            descendant_fees += mempool[x]['fee']
+            if (x == chain[-1]):
+                assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee']+satoshi_round(0.00002))
+            assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees+2000)
+
         # TODO: check that node1's mempool is as expected
 
         # TODO: test ancestor size limits
diff --git a/qa/rpc-tests/prioritise_transaction.py b/qa/rpc-tests/prioritise_transaction.py
index f376ceee5e..d9492f27a4 100755
--- a/qa/rpc-tests/prioritise_transaction.py
+++ b/qa/rpc-tests/prioritise_transaction.py
@@ -143,5 +143,45 @@ class PrioritiseTransactionTest(BitcoinTestFramework):
             if (x != high_fee_tx):
                 assert(x not in mempool)
 
+        # Create a free, low priority transaction.  Should be rejected.
+        utxo_list = self.nodes[0].listunspent()
+        assert(len(utxo_list) > 0)
+        utxo = utxo_list[0]
+
+        inputs = []
+        outputs = {}
+        inputs.append({"txid" : utxo["txid"], "vout" : utxo["vout"]})
+        outputs[self.nodes[0].getnewaddress()] = utxo["amount"] - self.relayfee
+        raw_tx = self.nodes[0].createrawtransaction(inputs, outputs)
+        tx_hex = self.nodes[0].signrawtransaction(raw_tx)["hex"]
+        txid = self.nodes[0].sendrawtransaction(tx_hex)
+
+        # A tx that spends an in-mempool tx has 0 priority, so we can use it to
+        # test the effect of using prioritise transaction for mempool acceptance
+        inputs = []
+        inputs.append({"txid": txid, "vout": 0})
+        outputs = {}
+        outputs[self.nodes[0].getnewaddress()] = utxo["amount"] - self.relayfee
+        raw_tx2 = self.nodes[0].createrawtransaction(inputs, outputs)
+        tx2_hex = self.nodes[0].signrawtransaction(raw_tx2)["hex"]
+        tx2_id = self.nodes[0].decoderawtransaction(tx2_hex)["txid"]
+
+        try:
+            self.nodes[0].sendrawtransaction(tx2_hex)
+        except JSONRPCException as exp:
+            assert_equal(exp.error['code'], -26) # insufficient fee
+            assert(tx2_id not in self.nodes[0].getrawmempool())
+        else:
+            assert(False)
+
+        # This is a less than 1000-byte transaction, so just set the fee
+        # to be the minimum for a 1000 byte transaction and check that it is
+        # accepted.
+        self.nodes[0].prioritisetransaction(tx2_id, 0, int(self.relayfee*COIN))
+
+        print "Assert that prioritised free transaction is accepted to mempool"
+        assert_equal(self.nodes[0].sendrawtransaction(tx2_hex), tx2_id)
+        assert(tx2_id in self.nodes[0].getrawmempool())
+
 if __name__ == '__main__':
     PrioritiseTransactionTest().main()
diff --git a/qa/rpc-tests/replace-by-fee.py b/qa/rpc-tests/replace-by-fee.py
index 6e9e0b304c..734db33b51 100755
--- a/qa/rpc-tests/replace-by-fee.py
+++ b/qa/rpc-tests/replace-by-fee.py
@@ -63,8 +63,14 @@ def make_utxo(node, amount, confirmed=True, scriptPubKey=CScript([1])):
 
     # If requested, ensure txouts are confirmed.
     if confirmed:
-        while len(node.getrawmempool()):
+        mempool_size = len(node.getrawmempool())
+        while mempool_size > 0:
             node.generate(1)
+            new_size = len(node.getrawmempool())
+            # Error out if we have something stuck in the mempool, as this
+            # would likely be a bug.
+            assert(new_size < mempool_size)
+            mempool_size = new_size
 
     return COutPoint(int(txid, 16), 0)
 
@@ -72,7 +78,7 @@ class ReplaceByFeeTest(BitcoinTestFramework):
 
     def setup_network(self):
         self.nodes = []
-        self.nodes.append(start_node(0, self.options.tmpdir, ["-maxorphantx=1000",
+        self.nodes.append(start_node(0, self.options.tmpdir, ["-maxorphantx=1000", "-debug",
                                                               "-relaypriority=0", "-whitelist=127.0.0.1",
                                                               "-limitancestorcount=50",
                                                               "-limitancestorsize=101",
@@ -108,6 +114,9 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         print "Running test opt-in..."
         self.test_opt_in()
 
+        print "Running test prioritised transactions..."
+        self.test_prioritised_transactions()
+
         print "Passed\n"
 
     def test_simple_doublespend(self):
@@ -513,5 +522,72 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         # but make sure it is accepted anyway
         self.nodes[0].sendrawtransaction(tx3c_hex, True)
 
+    def test_prioritised_transactions(self):
+        # Ensure that fee deltas used via prioritisetransaction are
+        # correctly used by replacement logic
+
+        # 1. Check that feeperkb uses modified fees
+        tx0_outpoint = make_utxo(self.nodes[0], 1.1*COIN)
+
+        tx1a = CTransaction()
+        tx1a.vin = [CTxIn(tx0_outpoint, nSequence=0)]
+        tx1a.vout = [CTxOut(1*COIN, CScript([b'a']))]
+        tx1a_hex = txToHex(tx1a)
+        tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex, True)
+
+        # Higher fee, but the actual fee per KB is much lower.
+        tx1b = CTransaction()
+        tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)]
+        tx1b.vout = [CTxOut(0.001*COIN, CScript([b'a'*740000]))]
+        tx1b_hex = txToHex(tx1b)
+
+        # Verify tx1b cannot replace tx1a.
+        try:
+            tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, True)
+        except JSONRPCException as exp:
+            assert_equal(exp.error['code'], -26)
+        else:
+            assert(False)
+
+        # Use prioritisetransaction to set tx1a's fee to 0.
+        self.nodes[0].prioritisetransaction(tx1a_txid, 0, int(-0.1*COIN))
+
+        # Now tx1b should be able to replace tx1a
+        tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, True)
+
+        assert(tx1b_txid in self.nodes[0].getrawmempool())
+
+        # 2. Check that absolute fee checks use modified fee.
+        tx1_outpoint = make_utxo(self.nodes[0], 1.1*COIN)
+
+        tx2a = CTransaction()
+        tx2a.vin = [CTxIn(tx1_outpoint, nSequence=0)]
+        tx2a.vout = [CTxOut(1*COIN, CScript([b'a']))]
+        tx2a_hex = txToHex(tx2a)
+        tx2a_txid = self.nodes[0].sendrawtransaction(tx2a_hex, True)
+
+        # Lower fee, but we'll prioritise it
+        tx2b = CTransaction()
+        tx2b.vin = [CTxIn(tx1_outpoint, nSequence=0)]
+        tx2b.vout = [CTxOut(1.01*COIN, CScript([b'a']))]
+        tx2b.rehash()
+        tx2b_hex = txToHex(tx2b)
+
+        # Verify tx2b cannot replace tx2a.
+        try:
+            tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, True)
+        except JSONRPCException as exp:
+            assert_equal(exp.error['code'], -26)
+        else:
+            assert(False)
+
+        # Now prioritise tx2b to have a higher modified fee
+        self.nodes[0].prioritisetransaction(tx2b.hash, 0, int(0.1*COIN))
+
+        # tx2b should now be accepted
+        tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, True)
+
+        assert(tx2b_txid in self.nodes[0].getrawmempool())
+
 if __name__ == '__main__':
     ReplaceByFeeTest().main()