Conversation
str4d
left a comment
There was a problem hiding this comment.
Looks good overall! See comments.
There was a problem hiding this comment.
Should also call getblocktemplate() before the prioritisetransaction() call, and assert that priority_tx_0 is not in the block template.
There was a problem hiding this comment.
found = False
for tx in block_template['transactions']:
if tx['hash'] == priority_tx_0:
print "\nPrioritized tx found in getblocktemplate\n"
found = True
break
assert(found)There was a problem hiding this comment.
After this, you could mine a block on node 1 and sync, after which you could then check on node 0 again to see that priority_tx_1 was now in a block.
There was a problem hiding this comment.
# Copyright (c) 2017 The Zcash developers
|
@arcalinea If you update your PR before Thursday we can get it into the release. :) |
9152d74 to
cc64e2c
Compare
|
Incorporated str4d's changes and squashed the commits |
str4d
left a comment
There was a problem hiding this comment.
Looking good, but see comments.
|
|
||
| # Check that priority_tx_0 is not in block_template() prior to prioritisation | ||
| block_template = self.nodes[0].getblocktemplate() | ||
| in_mempool = True |
There was a problem hiding this comment.
Non-blocking: in_block_template would make more sense here (and assert(!in_block_template) below).
| break | ||
| # NOTE: getblocktemplate() should return prioritized transaction, but is not | ||
| # Noted by user in issue #1884 | ||
| assert_equal(prioritized, False) |
There was a problem hiding this comment.
Confirmed that the test fails if this is set to True. Excellent!
| # Check to see if priority_tx_1 is now mined | ||
| mempool_1 = self.nodes[1].getrawmempool() | ||
| assert_equal(priority_tx_1 in mempool_1, False) | ||
| assert_equal(priority_tx_1 in block_1['tx'], True) |
There was a problem hiding this comment.
This, however, is not. If prioritisation is failing, then why does this prioritisation work when the earlier one doesn't? I assume because there aren't enough other transactions in the mempool at this point?
There was a problem hiding this comment.
Prioritisation works when blocks are actually mined--what's not working is "getblocktemplate()" returning prioritised transactions
|
utACK. |
|
📌 Commit 8d2dac6 has been approved by |
Test prioritisetransaction After talking with @str4d about #1884 , I wrote a test for prioritisetransaction. It uses small blocks (11kb), and checks whether a transaction makes it into the next block after being prioritized by that node. Should this be improved with a larger number of txs in the mempool, or by testing over multiple runs? As for getblocktemplate(), it seems to return the prioritized transaction within the block size set by the node (about 50 txs fit in an 11kb block), but the block "sizelimit" it displays is set at 2 MB in `rpcmining.cpp` line 690: ``` result.push_back(Pair("sizelimit", (int64_t)MAX_BLOCK_SIZE)); ``` This was quite confusing, I didn't think the `-blockmaxsize` parameter I was setting was working for awhile.
|
☀️ Test successful - zcash |
After talking with @str4d about #1884 , I wrote a test for prioritisetransaction. It uses small blocks (11kb), and checks whether a transaction makes it into the next block after being prioritized by that node.
Should this be improved with a larger number of txs in the mempool, or by testing over multiple runs?
As for getblocktemplate(), it seems to return the prioritized transaction within the block size set by the node (about 50 txs fit in an 11kb block), but the block "sizelimit" it displays is set at 2 MB in
rpcmining.cppline 690:This was quite confusing, I didn't think the
-blockmaxsizeparameter I was setting was working for awhile.