test: simplify and speedup mempool_updatefromblock.py by using MiniWallet #27035
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
brunoerg
reviewed
Feb 3, 2023
There was a problem hiding this comment.
nice speed-up! my result:
master:
➜ bitcoin-core-dev git:(master) ✗ time ./test/functional/mempool_updatefromblock.py
2023-02-03T19:32:04.575000Z TestFramework (INFO): Initializing test directory /var/folders/7j/m0yjzmhj4ys9jgl353v2mqph0000gn/T/bitcoin_func_test_h40yt0t5
2023-02-03T19:32:04.870000Z TestFramework (INFO): Creating 100 transactions...
2023-02-03T19:32:10.305000Z TestFramework (INFO): The batch of 25 transactions has been accepted into the mempool.
2023-02-03T19:32:10.312000Z TestFramework (INFO): All of the transactions from the current batch have been mined into a block.
2023-02-03T19:32:15.681000Z TestFramework (INFO): The batch of 25 transactions has been accepted into the mempool.
2023-02-03T19:32:15.690000Z TestFramework (INFO): All of the transactions from the current batch have been mined into a block.
2023-02-03T19:32:20.717000Z TestFramework (INFO): The batch of 25 transactions has been accepted into the mempool.
2023-02-03T19:32:20.726000Z TestFramework (INFO): All of the transactions from the current batch have been mined into a block.
2023-02-03T19:32:25.189000Z TestFramework (INFO): The last batch of 25 transactions has been accepted into the mempool.
2023-02-03T19:32:25.229000Z TestFramework (INFO): All of the recently mined transactions have been re-added into the mempool in 0.0395050048828125 seconds.
2023-02-03T19:32:25.229000Z TestFramework (INFO): Checking descendants/ancestors properties of all of the in-mempool transactions...
2023-02-03T19:32:25.320000Z TestFramework (INFO): Stopping nodes
2023-02-03T19:32:25.431000Z TestFramework (INFO): Cleaning up /var/folders/7j/m0yjzmhj4ys9jgl353v2mqph0000gn/T/bitcoin_func_test_h40yt0t5 on exit
2023-02-03T19:32:25.431000Z TestFramework (INFO): Tests successful
./test/functional/mempool_updatefromblock.py 19.97s user 0.41s system 94% cpu 21.519 totalpr branch:
➜ bitcoin-core-dev git:(thestack) ✗ time ./test/functional/mempool_updatefromblock.py
2023-02-03T19:30:49.236000Z TestFramework (INFO): Initializing test directory /var/folders/7j/m0yjzmhj4ys9jgl353v2mqph0000gn/T/bitcoin_func_test_qvob42yw
2023-02-03T19:30:50.067000Z TestFramework (INFO): Creating 100 transactions...
2023-02-03T19:30:50.299000Z TestFramework (INFO): The batch of 25 transactions has been accepted into the mempool.
2023-02-03T19:30:50.306000Z TestFramework (INFO): All of the transactions from the current batch have been mined into a block.
2023-02-03T19:30:50.997000Z TestFramework (INFO): The batch of 25 transactions has been accepted into the mempool.
2023-02-03T19:30:51.004000Z TestFramework (INFO): All of the transactions from the current batch have been mined into a block.
2023-02-03T19:30:52.080000Z TestFramework (INFO): The batch of 25 transactions has been accepted into the mempool.
2023-02-03T19:30:52.088000Z TestFramework (INFO): All of the transactions from the current batch have been mined into a block.
2023-02-03T19:30:52.916000Z TestFramework (INFO): The last batch of 25 transactions has been accepted into the mempool.
2023-02-03T19:30:53.018000Z TestFramework (INFO): All of the recently mined transactions have been re-added into the mempool in 0.10210895538330078 seconds.
2023-02-03T19:30:53.018000Z TestFramework (INFO): Checking descendants/ancestors properties of all of the in-mempool transactions...
2023-02-03T19:30:53.107000Z TestFramework (INFO): Stopping nodes
2023-02-03T19:30:53.218000Z TestFramework (INFO): Cleaning up /var/folders/7j/m0yjzmhj4ys9jgl353v2mqph0000gn/T/bitcoin_func_test_qvob42yw on exit
2023-02-03T19:30:53.218000Z TestFramework (INFO): Tests successful
./test/functional/mempool_updatefromblock.py 3.14s user 0.12s system 68% cpu 4.758 total
brunoerg
reviewed
Feb 3, 2023
| return address | ||
|
|
||
| def transaction_graph_test(self, size, n_tx_to_mine=None, start_input_txid='', end_address='', fee=Decimal(0.00100000)): | ||
| def transaction_graph_test(self, size, n_tx_to_mine=None, fee=100_000): |
There was a problem hiding this comment.
feel free to ignore:
perhaps we could initialize n_tx_to_mine with an empty array since we're expecting it.
Suggested change
| def transaction_graph_test(self, size, n_tx_to_mine=None, fee=100_000): | |
| def transaction_graph_test(self, size, n_tx_to_mine=[], fee=100_000): |
Also, the name of that variable and its explanation seems a little weird.
n_tx_to_mine -- the number of transaction that should be mined into a block
maybe the numbers of the transactions?
brunoerg
approved these changes
Feb 6, 2023
maflcko
approved these changes
Feb 15, 2023
There was a problem hiding this comment.
lgtm ACK dee8549 🚏
Show signature
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
lgtm ACK dee8549be39f841a24c3c8a3af3c5f787b9ad880 🚏
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgRvAv+Ocqz1YXq+DqusVBZ2r4gPtEhoLtBvJoNlBshKtRL0BW8/eiLvfFQ6uuC
ecuJmi2m4bVO8HvieSSUpkJ0A8qPy5j+5USRYa9OM7MAHJq13x35GoaQkFIt5Dw0
WR+CvnojJZ6+pqdSQbBG9JpuU4p8D2vhjP4TSCpXhlaQl5/TbaYK1TN1fLPx+4qX
5/7JdEjbNi6fTc2NYWOTVpyy0G+I0tVRwzuIbfb2v33A26JyPs92QSJGTVfryyt4
TpFOZ7slxLliag6JHUsCRd+XCNSd+JSwzSOKvSQ5I000DcpVDyyP8cmdmsHkxc2t
MG186XP25WfZnU+oUu4YA6/4RFbhP20c8HrKHzyh0/aFfLBU3aA2Xk5lpURGIsKf
ykW00gboBsan2Y6NZY9QBaC0L/g2G7DxUHdQhb2NbjaadPzjmao7xJo50mhskhKL
nciyIdvbb+KtXOSHNkskQQQ2HANDUk/zOVMRycm4x23nT15o7F6qMcgzbolhDiXM
pBAC0tg9
=CxF6
-----END PGP SIGNATURE-----
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Feb 15, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR simplifies the functional test mempool_updatefromblock.py by using MiniWallet in order to avoid manual low-level tx creation (signing, outputs selection, fee calculation). Most of the tedious work is done by the method
MiniWallet.send_self_transfer_multi(callingcreate_self_transfer_multiinternally) which supports spending a given set of UTXOs and creating a certain number of outputs.As a nice side-effect, the test's performance increases significantly (~3.5x on my system):
The arguments
start_input_txidandend_addresshave been removed from thetransaction_graph_testmethod, as they are currently unused and I don't see them being needed for future tests.