This commit makes the pruning.py much faster.
Key insights to do this:
Very nice! Do you have any numbers to share? :-)
23 | @@ -21,19 +24,60 @@ 24 | # compatible with pruning based on key creation time. 25 | TIMESTAMP_WINDOW = 2 * 60 * 60 26 | 27 | +def mine_large_blocks(node, n): 28 | + # Make a large scriptsig for the coinbase transaction. This is OP_RETURN 29 | + # followed by 950k of OP_NOP. This is a non-standard transaction but it
one hand clapping: can a coinbase tx ever be "non-standard"? :)
I think all coinbases are "non-standard", since we never relay them. :)
Heh. Would you prefer "A transaction containing this scriptPubKey would be non-standard in a non-coinbase transaction, but is consensus valid"?
I also notice that the comment incorrectly says 'scriptsig' instead of 'scriptPubKey'. I'll update.
just a joke :)
Needed to fix other comments, so I've fixed this too. Thanks.
57 | + block.nBits = int('207fffff', 16) 58 | + block.nNonce = 0 59 | + block.vtx = [coinbase_tx] 60 | + block.hashMerkleRoot = block.calc_merkle_root() 61 | + block.solve() 62 | + block_binary = (b2a_hex(block.serialize()).decode('ascii'))
block_hex? I also think a simple .hex() is kosher on master now(or at least I see many incantations of it!)
Also just as test belt and suspenders, make sure the blocks you are making are actually the size you think they are:
assert_greater_than(len(block_hex), len(big_script)*2)
or there abouts
Thanks. I've changed this to use ToHex() to match other tests.
tACK https://github.com/bitcoin/bitcoin/pull/15686/commits/3ce84d764f28c17faa790fc9ff038626a9291a3b with non-blocking nits.
2019-03-28T18:08:04.929000Z TestFramework (INFO): Warning! This test requires 4GB of disk space and takes over 30 mins (up to 2 hours) no longer true as well for me, took far less than 5 minutes on my debug build.
Force pushed some minor fixups. I also tested sending the blocks over P2P, and it's actually slower, so I've removed the comment saying we should do that.
Concept ACK
~30 minutes -> ~4 minutes on my machine.
That's impressive. Mind to move it to the non-extended tests?
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Mind to move it to the non-extended tests?
Sadly, I think this still won't run on travis. Here's my comment on the original PR:
Unfortunately, pruning.py still isn't able to run on Travis. It appears that disk access is very slow for in the Travis environment, and this test still times out/fails. Unfortunately, it's not possible to run this test without very heavy disk access - the whole point of pruning is to remove block files when disk usage goes over a certain threshold.
You've also asked that I split out the style changes into a separate commit, which is a fair request. I'll do that tomorrow.
Minor style fixups. No functional change.
This commit makes the pruning.py much faster.
Key insights to do this:
- pruning.py doesn't care what kind of transactions make up the big
blocks that are pruned in the test. Instead of making blocks with
several large, expensive to construct and validate transactions,
instead make the large blocks contain a single coinbase transaction with
a huge OP_RETURN txout.
- avoid stop-starting nodes where possible.
This test could probably be made even faster by using the P2P interface
for submitting blocks instead of the submitblock RPC.
I've split style-only changes into their own commit, to make review easier. Pushed branch should be identical to previous.
utACK 03d6d238104d228acfae9f3e122879bddef8d27d
7 | @@ -8,11 +8,13 @@
8 | This test uses 4GB of disk space.
9 | This test takes 30 mins or more (up to 2 hours)
Could update this comment
Time: Less than 3 minutes on ssd and about 4 minutes on hdd. :rocket:
50 | + # Build the block 51 | + block = CBlock() 52 | + block.nVersion = best_block["version"] 53 | + block.hashPrevBlock = previousblockhash 54 | + block.nTime = mine_large_blocks.nTime 55 | + block.nBits = int('207fffff', 16)
Could write 0x207fffff
ACK 03d6d238104d228acfae9f3e122879bddef8d27d. Test also took ~3 minutes for me (on ssd)
Thanks for the review @ryanofsky . This has been merged so I'm not intending to make further changes (happy to review your PR if you want to make the changes though).