WIP Refactor large transaction generation for tests and make generic #12930

In playing with some of the large transaction / full block functional tests, I found some of the construction logic for large transactions to be a bit confusing. In particular, you could make a 'large transaction', but the actual size was not exact in any way, making more advanced testing harder to reason about. This change allows customization in number of outputs / size of output to add to transactions (via gen_return_txouts) and allows creating a tx with a particular target size simply with

def create_tx_with_size(node, target_vsize)

This allows much more flexible testing of certain scenarios, for example, easily filling the mempool to an exact size, with various size txs and fees (tx radius is size).

And from there, evaluating something like getblocktemplate selection:

There is a tests of this functionality in this commit but I was thinking to not include this in the PR because it's just testing what was added to test_framework. This tests was also used to generate the mempools above, and graphed with a draw mempool tool I have been working on.

I wanted to get feedback before possibly updating / adding more functional tests to use this kind of construction. @jnewbery @MarcoFalke ?

Concept ACK. A few high-level comments:

• create_tx_with_size() is defined, but doesn't seem to be used anywhere.
• you're doing a lot of manual transaction construction and byte wrangling. Is it possible to use the CTransaction class in messages.py to achieve this more easily
• I much prefer docstrings (https://www.python.org/dev/peps/pep-0257/) for commenting functions than a block comment above (it makes the function stand out more, and the comment is available in interactive sessions).
• I think the new code can be moved around a bit. Pure util functions should live in util.py in the Utility Functions section. Transaction and block building functions should live in blocktools.py

Agree with what @jnewbery already said.

create_tx_with_size() is defined, but doesn't seem to be used anywhere.

If the code is not simpler (or faster) or better any other way than the existing code, I see not much reason to review/merge this. Please adjust the call sites to use the new code, so reviewers can get an idea what this is supposed to be replacing.

Thanks for looking, will close until I have more time to update this with the usage I had in mind.

@conscott I am looking forward to this. (We might be able to speed up the pruning functional test with a version that is optimized for speed, haven't looked if that is actually the bottleneck, though)

I tried to refactor the pruning test here: #10591 but got bored of rebasing. The original comment in that PR did suggest that constructing large blocks was a bottleneck.

Thanks for the suggestion; I will try to use this with the pruning test and see what happens. I won't have time to work for the next few days, which is another reason I chose to close temporarily. Once I get some solid time to test this with large block tests I will definitely re-open :)