[qa] Change default block priority size to 0 #7177

Unit test should not run a "different branch" by default.

This is a cleanup PR of #7022

@MarcoFalke were these the only changes necessary to make all the RPC tests work with fees only and no longer depend priority? If so, that's fantastic, huge concept ACK.

@morcos Yes, as pointed out in https://github.com/luke-jr/bitcoin/commit/e4d7271457140ef36b2b0eee7ebb5783e8ccadb4#commitcomment-14810804 there is currently no rpc test to exercise priority sufficiently. (If you look at the diff, the only check was "Create a free transaction and a child and see if the child pays a fee." )

@MarcoFalke: I'm not fully understanding this PR (and it seems other also have problem with it). Could you update the PR description?

Oops, forgot to mention this is a cleanup PR of #7022. (Ideally it should have been squashed in there)

I've been running the extended tests (excluded pruning.py) with assert(fee!=0) in mempool to verify there are no zero-fee transactions any more. Haven't yet checked for free transactions (per policy) though.

ACK Thanks for doing this, I agree I should have done it originally.

Concept ACK

utACK

• Travis passes
• Extended tests do not fail with assert(nFees!=0); and assert(nFees >= ::minRelayTxFee.GetFee(nSize)); in mempool.

If someone wants to verify this, maybe the following diff might be useful:

$ qa/pull-tester/rpc-tests.py -extended; git diff

diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py
index e7173fd..bb89d65 100755
--- a/qa/pull-tester/rpc-tests.py
+++ b/qa/pull-tester/rpc-tests.py
@@ -117,5 +117,5 @@ testScriptsExt = [
     'txn_doublespend.py',
     'txn_clone.py --mineblock',
-    'pruning.py',
+#    'pruning.py', # too long
     'forknotify.py',
     'invalidateblock.py',
diff --git a/qa/rpc-tests/prioritise_transaction.py b/qa/rpc-tests/prioritise_transaction.py
index 4a79d38..2ada1be 100755
--- a/qa/rpc-tests/prioritise_transaction.py
+++ b/qa/rpc-tests/prioritise_transaction.py
@@ -107,10 +107,12 @@ class PrioritiseTransactionTest(BitcoinTestFramework):

         try:
-            self.nodes[0].sendrawtransaction(tx2_hex)
+            # Don't send 0 fee transaction
+            # self.nodes[0].sendrawtransaction(tx2_hex)
+            pass
         except JSONRPCException as exp:
             assert_equal(exp.error['code'], -26) # insufficient fee
             assert(tx2_id not in self.nodes[0].getrawmempool())
         else:
-            assert(False)
+            assert(True)

         # This is a less than 1000-byte transaction, so just set the fee
@@ -120,6 +122,6 @@ class PrioritiseTransactionTest(BitcoinTestFramework):

         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())
+        # assert_equal(self.nodes[0].sendrawtransaction(tx2_hex), tx2_id)
+        # assert(tx2_id in self.nodes[0].getrawmempool())

 if __name__ == '__main__':
diff --git a/src/main.cpp b/src/main.cpp
index 9870bee..4fda8fc 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -970,4 +970,8 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
                 strprintf("%d", nSigOps));

+        assert(nFees!=0);
+        assert(nFees >= ::minRelayTxFee.GetFee(nSize));
+        assert(nModifiedFees >= ::minRelayTxFee.GetFee(nSize));
+
         CAmount mempoolRejectFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nSize);
         if (mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) {

Needs rebase

@morcos I think @laanwj is waiting for you to re-ACK this.

I can't re-ACK right now as I'm out of town. I extensively reviewed and tested the first version. Without knowing what changed I don't know how to re-ACK without redoing all that work.

The only actual content conflict is in the line where I remove -blockprioritysize=50000. If you tell me the commit you reviewed initially I can upload it for you.

Indirect content conflicts are two rpc test which rely on priority transaction getting accepted into the mempool:

• abandonconflict.py
• prioritise_transaction.py

But those can be addressed in a later pull. (I will only focus on priority transaction getting into blocks)

utACK

As this is only a test change, and the test still passes after this, I think it's OK for merging even without re-ACK from @morcos.