This PR improves the test wallet_listtransactions.py in three ways:
-whitelist=noban@127.0.0.1)self.log.info(...) calls
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, 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.
ACK c0abfa2d11a3ef847c68ef1a88bb3aa63d6fb7b2 modulo updating the positions of wallet_listtransactions.py --legacy-wallet and wallet_listtransactions.py --descriptors in the test runner from 2m to under 30s (per my testing, the run time drops from ~1m to ~10-15s).
<details><summary>Some quick suggestions for your consideration in the last commit</summary><p>
--- a/test/functional/wallet_listtransactions.py
+++ b/test/functional/wallet_listtransactions.py
@@ -105,19 +105,20 @@ class ListTransactionsTest(BitcoinTestFramework):
self.run_rbf_opt_in_test()
- # Check that the opt-in-rbf flag works properly, for sent and received
- # transactions.
+
def run_rbf_opt_in_test(self):
- # Check whether a transaction signals opt-in RBF itself
+ """Test the opt-in-rbf flag for sent and received transactions."""
+
def is_opt_in(node, txid):
+ """Check whether a transaction signals opt-in RBF itself."""
rawtx = node.getrawtransaction(txid, 1)
for x in rawtx["vin"]:
if x["sequence"] < 0xfffffffe:
return True
return False
- # Find an unconfirmed output matching a certain txid
def get_unconfirmed_utxo_entry(node, txid_to_match):
+ """Find an unconfirmed output matching a certain txid."""
utxo = node.listunspent(0, 0)
for i in utxo:
if i["txid"] == txid_to_match:
@@ -125,7 +126,7 @@ class ListTransactionsTest(BitcoinTestFramework):
return None
self.log.info("Test txs w/o opt-in RBF (bip125-replaceable=no)")
- # Chain a few transactions that don't opt-in.
+ # Chain a few transactions that don't opt in.
txid_1 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1)
assert not is_opt_in(self.nodes[0], txid_1)
@@ -205,5 +205,6 @@ class ListTransactionsTest(BitcoinTestFramework):
assert_equal(self.nodes[0].gettransaction(txid_3b)["bip125-replaceable"], "no")
assert_equal(self.nodes[0].gettransaction(txid_4)["bip125-replaceable"], "unknown")
+
if __name__ == '__main__':
ListTransactionsTest().main()
</p></details>
By whitelisting the peers via -whitelist, the inventory is transmissioned
immediately rather than on average every 5 seconds, speeding up the test by at
least a factor of two:
before:
$ time ./wallet_listtransactions.py
...
0m40.25s real 0m01.74s user 0m01.70s system
with this PR:
$ time ./wallet_listtransactions.py
...
0m14.93s real 0m01.68s user 0m01.87s system
This commit also moves the wallet_listtransactions tests into the < 30s group.
-> remove unneeded get-out-of IBD generate()
(The test framework already sets up the nodes to be out of IBD
in setup_nodes(), if setup_clean_chain is not set to True)
-> remove duplicate code line assigning an utxo
Co-authored-by: Jon Atack <jon@atack.com>
@jonatack: Thanks for your review!
[...] modulo updating the positions of
wallet_listtransactions.py --legacy-walletandwallet_listtransactions.py --descriptorsin the test runner from 2m to under 30s (per my testing, the run time drops from ~1m to ~10-15s).
Good idea, done. Put that into the first commit, due to a conflict in test_runner.py I also had to rebase the PR branch.
Some quick suggestions for your consideration in the last commit
Nice, agree that this PR is also a good oportunity to change to docstrings. Took your suggestions and added you as co-author in the last commit.
Thanks for updating!
ACK a006d7d73019b8cf4d68626c019c3d69729dda69
ACK a006d7d73019b8cf4d68626c019c3d69729dda69
Concept ACK