From fb6c6a7938cb7c4808ad88d23bfc2b7408407b12 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 18 Jun 2021 18:09:27 +0200 Subject: [PATCH 1/3] test: speedup wallet_listtransactions by whitelisting peers (immediate tx relay) 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. --- test/functional/test_runner.py | 4 ++-- test/functional/wallet_listtransactions.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 8afd8b3bc10..cbd773b7048 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -109,8 +109,6 @@ BASE_SCRIPTS = [ 'p2p_tx_download.py', 'mempool_updatefromblock.py', 'wallet_dump.py --legacy-wallet', - 'wallet_listtransactions.py --legacy-wallet', - 'wallet_listtransactions.py --descriptors', 'feature_taproot.py --previous_release', 'feature_taproot.py', 'rpc_signer.py', @@ -159,6 +157,8 @@ BASE_SCRIPTS = [ 'wallet_createwallet.py --legacy-wallet', 'wallet_createwallet.py --usecli', 'wallet_createwallet.py --descriptors', + 'wallet_listtransactions.py --legacy-wallet', + 'wallet_listtransactions.py --descriptors', 'wallet_watchonly.py --legacy-wallet', 'wallet_watchonly.py --usecli --legacy-wallet', 'wallet_reorgsrestore.py', diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index 8b503f5971b..df1cbd5eded 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -18,6 +18,9 @@ from test_framework.util import ( class ListTransactionsTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 + # This test isn't testing txn relay/timing, so set whitelist on the + # peers for instant txn relay. This speeds up the test run time 2-3x. + self.extra_args = [["-whitelist=noban@127.0.0.1"]] * self.num_nodes def skip_test_if_missing_module(self): self.skip_if_no_wallet() From 47915b118720c6e2b2ec9f599f25848041b42b99 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 19 Jun 2021 22:42:08 +0200 Subject: [PATCH 2/3] test: remove unneeded/redundant code in wallet_listtransactions -> 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 --- test/functional/wallet_listtransactions.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index df1cbd5eded..7557dbe8c8d 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -26,8 +26,6 @@ class ListTransactionsTest(BitcoinTestFramework): self.skip_if_no_wallet() def run_test(self): - self.nodes[0].generate(1) # Get out of IBD - self.sync_all() # Simple send, 0 to 1: txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 0.1) self.sync_all() @@ -136,7 +134,6 @@ class ListTransactionsTest(BitcoinTestFramework): utxo_to_use = get_unconfirmed_utxo_entry(self.nodes[0], txid_1) assert_equal(utxo_to_use["safe"], True) utxo_to_use = get_unconfirmed_utxo_entry(self.nodes[1], txid_1) - utxo_to_use = get_unconfirmed_utxo_entry(self.nodes[1], txid_1) assert_equal(utxo_to_use["safe"], False) # Create tx2 using createrawtransaction From a006d7d73019b8cf4d68626c019c3d69729dda69 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sun, 20 Jun 2021 13:28:22 +0200 Subject: [PATCH 3/3] test: add logging to wallet_listtransactions Co-authored-by: Jon Atack --- test/functional/wallet_listtransactions.py | 29 +++++++++++++--------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index 7557dbe8c8d..c0386f5d702 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -26,7 +26,7 @@ class ListTransactionsTest(BitcoinTestFramework): self.skip_if_no_wallet() def run_test(self): - # Simple send, 0 to 1: + self.log.info("Test simple send from node0 to node1") txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 0.1) self.sync_all() assert_array_result(self.nodes[0].listtransactions(), @@ -35,7 +35,7 @@ class ListTransactionsTest(BitcoinTestFramework): assert_array_result(self.nodes[1].listtransactions(), {"txid": txid}, {"category": "receive", "amount": Decimal("0.1"), "confirmations": 0}) - # mine a block, confirmations should change: + self.log.info("Test confirmations change after mining a block") blockhash = self.nodes[0].generate(1)[0] blockheight = self.nodes[0].getblockheader(blockhash)['height'] self.sync_all() @@ -46,7 +46,7 @@ class ListTransactionsTest(BitcoinTestFramework): {"txid": txid}, {"category": "receive", "amount": Decimal("0.1"), "confirmations": 1, "blockhash": blockhash, "blockheight": blockheight}) - # send-to-self: + self.log.info("Test send-to-self on node0") txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 0.2) assert_array_result(self.nodes[0].listtransactions(), {"txid": txid, "category": "send"}, @@ -55,7 +55,7 @@ class ListTransactionsTest(BitcoinTestFramework): {"txid": txid, "category": "receive"}, {"amount": Decimal("0.2")}) - # sendmany from node1: twice to self, twice to node2: + self.log.info("Test sendmany from node1: twice to self, twice to node0") send_to = {self.nodes[0].getnewaddress(): 0.11, self.nodes[1].getnewaddress(): 0.22, self.nodes[0].getnewaddress(): 0.33, @@ -89,6 +89,7 @@ class ListTransactionsTest(BitcoinTestFramework): if not self.options.descriptors: # include_watchonly is a legacy wallet feature, so don't test it for descriptor wallets + self.log.info("Test 'include_watchonly' feature (legacy wallet)") pubkey = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress())['pubkey'] multisig = self.nodes[1].createmultisig(1, [pubkey]) self.nodes[0].importaddress(multisig["redeemScript"], "watchonly", False, True) @@ -104,33 +105,35 @@ 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: return i return None - # 1. Chain a few transactions that don't opt-in. + self.log.info("Test txs w/o opt-in RBF (bip125-replaceable=no)") + # 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) assert_array_result(self.nodes[0].listtransactions(), {"txid": txid_1}, {"bip125-replaceable": "no"}) self.sync_mempools() assert_array_result(self.nodes[1].listtransactions(), {"txid": txid_1}, {"bip125-replaceable": "no"}) - # Tx2 will build off txid_1, still not opting in to RBF. + # Tx2 will build off tx1, still not opting in to RBF. utxo_to_use = get_unconfirmed_utxo_entry(self.nodes[0], txid_1) assert_equal(utxo_to_use["safe"], True) utxo_to_use = get_unconfirmed_utxo_entry(self.nodes[1], txid_1) @@ -149,6 +152,7 @@ class ListTransactionsTest(BitcoinTestFramework): self.sync_mempools() assert_array_result(self.nodes[0].listtransactions(), {"txid": txid_2}, {"bip125-replaceable": "no"}) + self.log.info("Test txs with opt-in RBF (bip125-replaceable=yes)") # Tx3 will opt-in to RBF utxo_to_use = get_unconfirmed_utxo_entry(self.nodes[0], txid_2) inputs = [{"txid": txid_2, "vout": utxo_to_use["vout"]}] @@ -179,6 +183,7 @@ class ListTransactionsTest(BitcoinTestFramework): self.sync_mempools() assert_array_result(self.nodes[0].listtransactions(), {"txid": txid_4}, {"bip125-replaceable": "yes"}) + self.log.info("Test tx with unknown RBF state (bip125-replaceable=unknown)") # Replace tx3, and check that tx4 becomes unknown tx3_b = tx3_modified tx3_b.vout[0].nValue -= int(Decimal("0.004") * COIN) # bump the fee @@ -191,7 +196,7 @@ class ListTransactionsTest(BitcoinTestFramework): self.sync_mempools() assert_array_result(self.nodes[1].listtransactions(), {"txid": txid_4}, {"bip125-replaceable": "unknown"}) - # Check gettransaction as well: + self.log.info("Test bip125-replaceable status with gettransaction RPC") for n in self.nodes[0:2]: assert_equal(n.gettransaction(txid_1)["bip125-replaceable"], "no") assert_equal(n.gettransaction(txid_2)["bip125-replaceable"], "no") @@ -199,7 +204,7 @@ class ListTransactionsTest(BitcoinTestFramework): assert_equal(n.gettransaction(txid_3b)["bip125-replaceable"], "yes") assert_equal(n.gettransaction(txid_4)["bip125-replaceable"], "unknown") - # After mining a transaction, it's no longer BIP125-replaceable + self.log.info("Test mined transactions are no longer bip125-replaceable") self.nodes[0].generate(1) assert txid_3b not in self.nodes[0].getrawmempool() assert_equal(self.nodes[0].gettransaction(txid_3b)["bip125-replaceable"], "no")