test: Speed up mempool_spend_coinbase.py #21762
Conversation
Can be reviewed with --ignore-all-space --color-moved=dimmed-zebra
The documentation did not match the implementation, no coins were mined to the OP_TRUE address.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, 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. |
|
I don't know either why the assertion is there, but the value changes because the block size changes due to the scriptPubKey (size) change.
Yes, |
| wallet.scan_blocks(start=chain_height - 100 + 1, num=1) | ||
| utxo_mature = wallet.get_utxo() | ||
| wallet.scan_blocks(start=chain_height - 100 + 2, num=1) | ||
| utxo_immature = wallet.get_utxo() |
There was a problem hiding this comment.
I don't really like this pattern. The method name sync_blocks seems confusing because I wouldn't expect a state change (self._utxos) by this method. It seems this would have to be used very carefully, with knowledge of MiniWallet's internals to avoid bugs. For example, if scan_blocks is called multiple times with any overlapping blocks, then we'll start to get duplicate utxos in self._utxos (maybe this should be a set instead of a list)?
In general I also don't really like how self._utxos is sorted (in send_self_transfer by value), while here it isn't sorted but will have an implicit ordering based on when/how scan_blocks is called, and then based on order of blocks/vouts inside scan_blocks. There's just a lot of different ways it's ordered and we are always relying on that ordering
I'm gonna think this through, don't think this should hold up this PR. But at least I can start the discussion and see what you think / if this can be improved.
There was a problem hiding this comment.
I agree. Do you have specific suggestions to improve that pass with all existing tests? Feel free to ping me for review.
|
crACK fa40eb5 |
fa40eb5 test: Speed up mempool_spend_coinbase.py (MarcoFalke) fa29382 test: Fix test cache issue (MarcoFalke) fa085b4 test: Create MiniWallet.create_self_transfer (MarcoFalke) fa1bedb test: Add MiniWallet.sendrawtransaction (MarcoFalke) Pull request description: Locally the test will run 4 seconds faster with `--valgrind` (18s vs 14s) ACKs for top commit: mjdietzx: crACK bitcoin@fa40eb5 Tree-SHA512: ecfb60dda5ca5d7e6367bb9c6210390d95ebf6396ce657728901d118b75bb90c98f9351df3b01004d00682234448d6c6a13338d12097f7dced2cf7f1bd84d924
Locally the test will run 4 seconds faster with
--valgrind(18s vs 14s)