tests: Speedup feature_pruning test and refactor big transaction logic #14691

1. 
   conscott commented at 4:08 PM on November 8, 2018: contributor

   Description

   Extended tests like feature_pruning.py create large blocks by making many large transactions with a lot of OP_RETURNS in a single tx. Under this model roughly 14 of these transactions make a full-ish block.

   These transactions are already non-standard, so I have just modified the logic to instead just create 1 large output, making a single large transaction, mining a block of roughly the same size (947k). The reduction in tx creation / number of outputs seems to significantly improve performance.

   Another optimization is to splice in the OP_RETURN output after transaction signing, since it reduces the copying of the large OP_RETURN portion during signing. All signatures are SIGHHASH_NONE, so splicing after is okay.

   feature_pruning.py has about a 1.5x speedup on my machine (~12 minutes down to ~8 minutes), and mempool_limit, and feature_maxuploadtarget seem to benefit as well.

   Refactoring

   • The original OP_RETURN generating function is now just a string. If there is a better place to put this, I will move it. At the very least I just wanted to document the format because the previous function had little explanation for all the hex values.
   • create_lots_of_big_transactions no longer requires providing the OP_RETURN splice, and uses the default from before, with an optional op_return_txout= keyword args for using the giant OP_RETURN mentioned above.
2. conscott renamed this:
   Speedup feature_pruning test and refactor big transaction logic
   [Tests] - Speedup feature_pruning test and refactor big transaction logic
   on Nov 8, 2018
3. 
   DrahtBot commented at 5:08 PM on November 8, 2018: member

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #15282 (test: Replace hard-coded hex tx with class in test framework by stevenroose)
   • #13541 (wallet/rpc: sendrawtransaction maxfeerate by kallewoof)

   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.<!--2502f1a698b3751726fa55edcda76cd3-->

   Coverage

   Coverage	Change (pull 14691)	Reference (master)
   Lines	+0.0395 %	87.0730 %
   Functions	+0.0153 %	84.3717 %
   Branches	+0.0151 %	51.5638 %

   ^{Updated at: 2018-11-08T22:02:58.107718.}

4. meshcollider added the label Tests on Nov 8, 2018
5. fanquake renamed this:
   [Tests] - Speedup feature_pruning test and refactor big transaction logic
   tests: Speedup feature_pruning test and refactor big transaction logic
   on Nov 9, 2018
6. fanquake requested review from MarcoFalke on Nov 9, 2018
7. 
   fanquake commented at 2:10 AM on November 10, 2018: member

   Concept ACK

   Haven't looked at the changes, but I'm seeing a ~1.4x speedup on my machine between feature_pruning.py in master (e70a19e7132dac91b7948fcbfac086f86fec3d88) and c8ef522:

   feature_pruning.py | ✓ Passed  | 819 s
   ALL                | ✓ Passed  | 819 s (accumulated) 
   Runtime: 819 s
   

   feature_pruning.py | ✓ Passed  | 577 s
   ALL                | ✓ Passed  | 577 s (accumulated) 
   Runtime: 577 s
   

8. conscott force-pushed on Nov 10, 2018
9. [Tests] - Add giant OP_RETURN for mining large blocks and refactor large tx code 9c930190ec
10. [Tests] - Make mempool_limit test use larger tx to hit mempool limit ecf8389c1f
11. conscott force-pushed on Nov 12, 2018
12. DrahtBot added the label Needs rebase on Mar 18, 2019
13. 
    DrahtBot commented at 6:08 PM on March 18, 2019: member

    <!--cf906140f33d8803c4a75a2196329ecb-->Needs rebase

14. 
    fanquake commented at 1:11 AM on June 19, 2019: member

    @conscott I realise this hasn't seen much traction / concept ACKs, however are you interested in rebasing?

15. 
    conscott commented at 1:55 AM on June 19, 2019: contributor

    I almost forgot this was still alive :)

    I will rebase when I get the chance, hopefully this week!

16. 
    sipa commented at 7:09 AM on June 19, 2019: member

    Concept ACK

17. in test/functional/test_framework/util.py:577 in 9c930190ec outdated

     609 | +
     610 | +
     611 | +# Create a spend of each passed-in utxo, splicing in "txout" to each raw
     612 | +# transaction to make it large. By default, it will splice in
     613 | +# 128 OP_RETURN outputs of 512 data bytes (see MULTI_OP_RETURN)
     614 | +def create_lots_of_big_transactions(node, utxos, num, fee, op_return_txout=MULTI_OP_RETURN):
    

    ---

    ---

    

    promag commented at 9:11 AM on June 19, 2019:

    Could simplify by ditching num parameter? This way below could be

    return [create_big_transaction(node, utxo, fee, op_return_txout) for utxo in utxos]
    

18. 
    promag commented at 9:12 AM on June 19, 2019: member

    Concept ACK.

19. fanquake added the label Waiting for author on Jun 26, 2019
20. 
    jamesob commented at 3:08 PM on June 26, 2019: member

    Concept ACK. Will review on rebase.

21. 
    jnewbery commented at 3:52 PM on July 8, 2019: member

    Is this still required after #15686?

22. 
    conscott commented at 11:11 AM on July 9, 2019: contributor

    @jnewbery - Looks like you got it covered more elegantly with OP_NOPs :) It was a fun learning experience anyhow.

    I will go ahead and close this.

23. conscott closed this on Jul 9, 2019

    ---

24. laanwj removed the label Needs rebase on Oct 24, 2019
25. laanwj removed the label Waiting for author on Oct 24, 2019
26. MarcoFalke locked this on Dec 16, 2021

Contributors

conscottDrahtBotfanquakesipapromagjamesobjnewbery

Review Requested
MarcoFalke