Merge bitcoin/bitcoin#27287: test: Replace threading with concurrent.futures

fa0696e test: Replace threading with concurrent.futures (MarcoFalke)

Pull request description:

  `threading` has no easy way to get the return value or exception once the target function stops. Not checking the return value or exception can make tests more fragile and failures harder to debug.

  Fix this by checking the return value (or exception) by wrapping the function execution into a future and calling `result()` on it.

  Can be reviewed with `--ignore-all-space`.

  (There are still some uses of `threading` around, because some tests do expect an exception to be thrown and caught in the target function)

ACKs for top commit:
  ishaanam:
    ACK fa0696e
  stickies-v:
    ACK fa0696e

Tree-SHA512: d9ddf6b3c530cd8c485a030a3c84d4e03d3e9f9ea8240b050afcd566a884f5cabe816ac56910cec9ea9fa299239e5abb99e672dda05a74974f61bb68dc3c1d65

Author: fanquake

test/functional/wallet_importdescriptors.py

@@ -15,7 +15,7 @@
 - `test_address()` is called to call getaddressinfo for an address on node1
   and test the values returned."""
 
-import threading
+import concurrent.futures
 
 from test_framework.authproxy import JSONRPCException
 from test_framework.blocktools import COINBASE_MATURITY
@@ -691,25 +691,24 @@ def run_test(self):
         descriptor["next_index"] = 0
 
         encrypted_wallet.walletpassphrase("passphrase", 99999)
-        t = threading.Thread(target=encrypted_wallet.importdescriptors, args=([descriptor],))
+        with concurrent.futures.ThreadPoolExecutor(max_workers=1) as thread:
+            with self.nodes[0].assert_debug_log(expected_msgs=["Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)"], timeout=5):
+                importing = thread.submit(encrypted_wallet.importdescriptors, requests=[descriptor])
 
-        with self.nodes[0].assert_debug_log(expected_msgs=[f'Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)'], timeout=5):
-            t.start()
+            # Set the passphrase timeout to 1 to test that the wallet remains unlocked during the rescan
+            self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletpassphrase("passphrase", 1)
 
-        # Set the passphrase timeout to 1 to test that the wallet remains unlocked during the rescan
-        self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletpassphrase("passphrase", 1)
+            try:
+                self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletlock()
+            except JSONRPCException as e:
+                assert e.error["code"] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet." in e.error["message"]
 
-        try:
-            self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletlock()
-        except JSONRPCException as e:
-            assert e.error['code'] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet." in e.error['message']
+            try:
+                self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletpassphrasechange("passphrase", "newpassphrase")
+            except JSONRPCException as e:
+                assert e.error["code"] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before changing the passphrase." in e.error["message"]
 
-        try:
-            self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletpassphrasechange("passphrase", "newpassphrase")
-        except JSONRPCException as e:
-            assert e.error['code'] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before changing the passphrase." in e.error['message']
-
-        t.join()
+            assert_equal(importing.result(), [{"success": True}])
 
         assert_equal(temp_wallet.getbalance(), encrypted_wallet.getbalance())
 

test/functional/wallet_transactiontime_rescan.py

@@ -5,7 +5,7 @@
 """Test transaction time during old block rescanning
 """
 
-import threading
+import concurrent.futures
 import time
 
 from test_framework.authproxy import JSONRPCException
@@ -201,25 +201,24 @@ def run_test(self):
             encrypted_wallet.walletpassphrase("passphrase", 99999)
             encrypted_wallet.sethdseed(seed=hd_seed)
 
-            t = threading.Thread(target=encrypted_wallet.rescanblockchain)
+            with concurrent.futures.ThreadPoolExecutor(max_workers=1) as thread:
+                with minernode.assert_debug_log(expected_msgs=["Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)"], timeout=5):
+                    rescanning = thread.submit(encrypted_wallet.rescanblockchain)
 
-            with minernode.assert_debug_log(expected_msgs=[f'Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)'], timeout=5):
-                t.start()
+                # set the passphrase timeout to 1 to test that the wallet remains unlocked during the rescan
+                minernode.cli("-rpcwallet=encrypted_wallet").walletpassphrase("passphrase", 1)
 
-            # set the passphrase timeout to 1 to test that the wallet remains unlocked during the rescan
-            minernode.cli("-rpcwallet=encrypted_wallet").walletpassphrase("passphrase", 1)
+                try:
+                    minernode.cli("-rpcwallet=encrypted_wallet").walletlock()
+                except JSONRPCException as e:
+                    assert e.error["code"] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet." in e.error["message"]
 
-            try:
-                minernode.cli("-rpcwallet=encrypted_wallet").walletlock()
-            except JSONRPCException as e:
-                assert e.error['code'] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet." in e.error['message']
+                try:
+                    minernode.cli("-rpcwallet=encrypted_wallet").walletpassphrasechange("passphrase", "newpassphrase")
+                except JSONRPCException as e:
+                    assert e.error["code"] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before changing the passphrase." in e.error["message"]
 
-            try:
-                minernode.cli("-rpcwallet=encrypted_wallet").walletpassphrasechange("passphrase", "newpassphrase")
-            except JSONRPCException as e:
-                assert e.error['code'] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before changing the passphrase." in e.error['message']
-
-            t.join()
+                assert_equal(rescanning.result(), {"start_height": 0, "stop_height": 803})
 
             assert_equal(encrypted_wallet.getbalance(), temp_wallet.getbalance())