test: Move helper function create_large_block #25546

pull aureleoules wants to merge 1 commits into bitcoin:master from aureleoules:2022-07-test-refactor-large-block changing 2 files +10 −23

aureleoules commented at 3:07 PM on July 5, 2022: member

Following #24865 (review).

This PR adds a helper function to create large blocks for pruning related tests.
test: Move helper function create_large_block 58c49c865b
fanquake added the label Tests on Jul 5, 2022
in test/functional/test_framework/blocktools.py:92 in 58c49c865b
```
  86 | @@ -87,6 +87,14 @@ def create_block(hashprev=None, coinbase=None, ntime=None, *, version=None, tmpl
  87 |      block.calc_sha256()
  88 |      return block
  89 |  
  90 | +def create_large_block(hashprev=None, ntime=None, height=None, scriptPubKey=CScript([OP_RETURN] + [OP_TRUE] * 950000)):
  91 | +    coinbase_tx = create_coinbase(height)
  92 | +    coinbase_tx.vin[0].nSequence = 2 ** 32 - 1
```
MarcoFalke commented at 3:24 PM on July 5, 2022:

I wonder if there is reason to change the sequence?

aureleoules commented at 3:28 PM on July 5, 2022:

I can make it a parameter if needed.

MarcoFalke commented at 3:31 PM on July 5, 2022:

I don't think the sequence is used anywhere at all, is it?

aureleoules commented at 4:31 PM on July 5, 2022:

I removed it in #24865, thanks.
kouloumos commented at 3:38 PM on July 5, 2022: contributor

Why not extracting this function as part of the linked PR? Wouldn't this make more sense as that's where you would better show the reasoning for this change?

Also, would it make sense to use this mine_large_block method instead? It's not a drop-in replacement, but my thought is if doing minor changes in order to reuse logic would be more beneficial. https://github.com/bitcoin/bitcoin/blob/9fb2a2bc6768ab03fcada9155d52a16ce6f6a0cc/test/functional/test_framework/util.py#L532-L538
aureleoules commented at 4:35 PM on July 5, 2022: member

Why not extracting this function as part of the linked PR? Wouldn't this make more sense as that's where you would better show the reasoning for this change?

Sure, I'll close this PR and continue in #24865.

Also, would it make sense to use this mine_large_block method instead? It's not a drop-in replacement, but my thought is if doing minor changes in order to reuse logic would be more beneficial.

I'll take a look at it but it might not easily fit my use-case as I need to control the block nTime...
aureleoules closed this on Jul 5, 2022
aureleoules deleted the branch on Aug 25, 2022
bitcoin locked this on Aug 25, 2023

Contributors

Labels

Linked (view graph)

#24865 rpc: Enable wallet import on pruned nodes and add test