[Tests] Add prioritisetransaction RPC test #7063

1. 
   sdaftuar commented at 6:36 pm on November 19, 2015: member

   @morcos mentioned in #6898 (comment) that there’s no rpc test for prioritisetransaction. This adds a simple test to exercise this logic.

   In writing this test I discovered something unusual – if you try to set a fee delta using prioritisetransaction that is negative and greater in magnitude than the transaction’s fee, you get incorrect results. The CFeeRate constructor that takes a fee and size doesn’t work if you pass in a negative fee. I don’t really understand the details of how conversion and type promotion work in this kind of situation, but as far as I can tell, on my machine, I think the compiler promotes the int64_t nFeePaid to a uint64_t (nSize is a size_t):

   0nSatoshisPerK = nFeePaid*1000/nSize;
   

   Link to the code: https://github.com/bitcoin/bitcoin/blob/c983d6fcb47bafb4b82529f512310ccaef076ca2/src/amount.cpp#L15

   I think this arithmetic may work on a 32-bit platform, because it looked to me like doing arithmetic with an unsigned int rather than a size_t appeared to work.

   Anyway I expect that #6898 will fix prioritisetransaction to work properly with fee deltas that bring the modified fee below 0, so I just avoided that issue for now in this test; I can update it in the future to include that case after #6898.

2. in qa/rpc-tests/prioritise_transaction.py: in 4c6b6f3560 outdated

   108+        self.nodes[0].generate(1)
   109+
   110+        mempool = self.nodes[0].getrawmempool()
   111+        assert(txids[0][0] not in mempool)
   112+        assert(txids[0][1] in mempool)
   113+        print "Prioritised transaction mined: success"
   

   ---

   ---

   

   MarcoFalke commented at 7:40 pm on November 19, 2015:

   Nit: Shouldn’t the print appear in front of assert? So when it fails, we know where it did (Not just by line number)?

   ---

   

   sdaftuar commented at 8:30 pm on November 19, 2015:

   Well then it would be printing something incorrect if the assertion failed. I guess I could add a different print statement above the asserts, but not sure it’s worth it? If the assertion fails I think the stack trace is informative enough, and I don’t think the style here is inconsistent with the rest of the tests.

   ---

   

   MarcoFalke commented at 10:10 pm on November 19, 2015:

   Where would you put a comment, when you had put one? Then just replace the # with print and you get in L109:

   print "Assert that prioritised transaction was mined"

   ---

   

   MarcoFalke commented at 10:10 pm on November 19, 2015:

   rest of the tests

   I see no reason to do this. You could as well just remove the print completely.

3. in qa/rpc-tests/smartfees.py: in 4c6b6f3560 outdated

   18@@ -19,9 +19,6 @@
   19 # 4 bytes of OP_TRUE and push 2-byte redeem script of "OP_1 OP_DROP" or "OP_2 OP_DROP"
   20 SCRIPT_SIG = ["0451025175", "0451025275"]
   21 
   22-def satoshi_round(amount):
   23-    return  Decimal(amount).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN)
   

   ---

   ---

   

   MarcoFalke commented at 7:41 pm on November 19, 2015:

   Long overdue to have this moved to the test framework. Thanks!
4. jonasschnelli added the label Tests on Nov 20, 2015
5. 
   sipa commented at 6:57 pm on November 28, 2015: member

   utACK
6. Add rounding helper function to util.py 826079ef34
7. Add rpc test for prioritisetransaction 596e8399d4
8. sdaftuar force-pushed on Nov 30, 2015
9. 
   sdaftuar commented at 10:03 pm on November 30, 2015: member

   Rebased, moved prints
10. laanwj referenced this in commit 6abf6eb7bb on Dec 1, 2015
11. 
    laanwj commented at 10:05 am on December 1, 2015: member

    Merged via 6abf6eb7bb777a5c4f22e9db6d4544281277378f (needed trivial rebase change in rpc-tests.py)
12. 
    MarcoFalke commented at 12:51 pm on December 1, 2015: member

    utACK 6abf6eb
13. 
    MarcoFalke commented at 12:51 pm on December 1, 2015: member

    PR can be closed!?
14. laanwj closed this on Dec 1, 2015

    ---

15. MarcoFalke locked this on Sep 8, 2021

Contributors
sdaftuar MarcoFalke sipa laanwj

Labels
Tests