← Watch feed
refactor(qa): Lift out functions to outer scopes
Public commit record
92/100 · Strong
What the developer wrote
Authored by Hodlinator
refactor(qa): Lift out functions to outer scopes
This prepares for later breaking apart of run_test().
Note that the "wallet" lambda was renamed to "get_wallet" since otherwise the Python interpreter emitted:
"UnboundLocalError: cannot access local variable 'wallet' where it is not associated with a value"
✓ Descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This is a harmless code cleanup in a test file. It moves some helper functions from inside a test method to the top of the file and renames one variable to avoid a Python language quirk. There is no change to Bitcoin Core's actual wallet behavior or security.
Recommended action
No action needed; this is a test-only refactor with no security relevance.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors test/functional/wallet_multiwallet.py by lifting nested lambdas (data_dir, wallet_dir, wallet) and the wallet_file() method to outer/module scope. The ‘wallet’ lambda is renamed to ‘get_wallet’ to avoid an UnboundLocalError when used at module scope. All call sites are updated to pass the node argument explicitly. No production code, RPC semantics, or wallet logic are modified.
Changed components
test/functional/wallet_multiwallet.pyInspect captured patch +58 / −53
diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py
index 19e26540..86d2737d 100755
--- a/test/functional/wallet_multiwallet.py
+++ b/test/functional/wallet_multiwallet.py
@@ -39,6 +39,15 @@ def test_load_unload(node, name):
got_loading_error = True
return
+def data_dir(node, *p):
+ return os.path.join(node.chain_path, *p)
+
+def wallet_dir(node, *p):
+ return data_dir(node, 'wallets', *p)
+
+def get_wallet(node, name):
+ return node.get_wallet_rpc(name)
+
class MultiWalletTest(BitcoinTestFramework):
def set_test_params(self):
@@ -50,25 +59,21 @@ class MultiWalletTest(BitcoinTestFramework):
def skip_test_if_missing_module(self):
self.skip_if_no_wallet()
+ def wallet_file(self, node, name):
+ if name == self.default_wallet_name:
+ return wallet_dir(node, self.default_wallet_name, self.wallet_data_filename)
+ if os.path.isdir(wallet_dir(node, name)):
+ return wallet_dir(node, name, "wallet.dat")
+ return wallet_dir(node, name)
+
def run_test(self):
node = self.nodes[0]
- data_dir = lambda *p: os.path.join(node.chain_path, *p)
- wallet_dir = lambda *p: data_dir('wallets', *p)
- wallet = lambda name: node.get_wallet_rpc(name)
-
- def wallet_file(name):
- if name == self.default_wallet_name:
- return wallet_dir(self.default_wallet_name, self.wallet_data_filename)
- if os.path.isdir(wallet_dir(name)):
- return wallet_dir(name, "wallet.dat")
- return wallet_dir(name)
-
assert_equal(node.listwalletdir(), {'wallets': [{'name': self.default_wallet_name, "warnings": []}]})
# check wallet.dat is created
self.stop_nodes()
- assert_equal(os.path.isfile(wallet_dir(self.default_wallet_name, self.wallet_data_filename)), True)
+ assert_equal(os.path.isfile(wallet_dir(node, self.default_wallet_name, self.wallet_data_filename)), True)
self.log.info("Verify warning is emitted when failing to scan the wallets directory")
if platform.system() == 'Windows':
@@ -80,23 +85,23 @@ class MultiWalletTest(BitcoinTestFramework):
with node.assert_debug_log(unexpected_msgs=['Error scanning directory entries under'], expected_msgs=[]):
result = node.listwalletdir()
assert_equal(result, {'wallets': [{'name': 'default_wallet', 'warnings': []}]})
- os.chmod(data_dir('wallets'), 0)
+ os.chmod(data_dir(node, 'wallets'), 0)
with node.assert_debug_log(expected_msgs=['Error scanning directory entries under']):
result = node.listwalletdir()
assert_equal(result, {'wallets': []})
self.stop_node(0)
# Restore permissions
- os.chmod(data_dir('wallets'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
+ os.chmod(data_dir(node, 'wallets'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
# create symlink to verify wallet directory path can be referenced
# through symlink
- os.mkdir(wallet_dir('w7'))
- os.symlink('w7', wallet_dir('w7_symlink'))
+ os.mkdir(wallet_dir(node, 'w7'))
+ os.symlink('w7', wallet_dir(node, 'w7_symlink'))
- os.symlink('..', wallet_dir('recursive_dir_symlink'))
+ os.symlink('..', wallet_dir(node, 'recursive_dir_symlink'))
- os.mkdir(wallet_dir('self_walletdat_symlink'))
- os.symlink('wallet.dat', wallet_dir('self_walletdat_symlink/wallet.dat'))
+ os.mkdir(wallet_dir(node, 'self_walletdat_symlink'))
+ os.symlink('wallet.dat', wallet_dir(node, 'self_walletdat_symlink/wallet.dat'))
# rename wallet.dat to make sure plain wallet file paths (as opposed to
# directory paths) can be loaded
@@ -107,13 +112,13 @@ class MultiWalletTest(BitcoinTestFramework):
node.createwallet("created")
self.stop_nodes()
empty_wallet = os.path.join(self.options.tmpdir, 'empty.dat')
- os.rename(wallet_file("empty"), empty_wallet)
- shutil.rmtree(wallet_dir("empty"))
+ os.rename(self.wallet_file(node, "empty"), empty_wallet)
+ shutil.rmtree(wallet_dir(node, "empty"))
empty_created_wallet = os.path.join(self.options.tmpdir, 'empty.created.dat')
- os.rename(wallet_dir("created", self.wallet_data_filename), empty_created_wallet)
- shutil.rmtree(wallet_dir("created"))
- os.rename(wallet_file("plain"), wallet_dir("w8"))
- shutil.rmtree(wallet_dir("plain"))
+ os.rename(wallet_dir(node, "created", self.wallet_data_filename), empty_created_wallet)
+ shutil.rmtree(wallet_dir(node, "created"))
+ os.rename(self.wallet_file(node, "plain"), wallet_dir(node, "w8"))
+ shutil.rmtree(wallet_dir(node, "plain"))
# restart node with a mix of wallet names:
# w1, w2, w3 - to verify new wallets created when non-existing paths specified
@@ -136,14 +141,14 @@ class MultiWalletTest(BitcoinTestFramework):
for wallet_name in to_load:
node.loadwallet(wallet_name)
- os.mkdir(wallet_dir('no_access'))
- os.chmod(wallet_dir('no_access'), 0)
+ os.mkdir(wallet_dir(node, 'no_access'))
+ os.chmod(wallet_dir(node, 'no_access'), 0)
try:
with node.assert_debug_log(expected_msgs=["Error while scanning wallet dir"]):
walletlist = node.listwalletdir()['wallets']
finally:
# Need to ensure access is restored for cleanup
- os.chmod(wallet_dir('no_access'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
+ os.chmod(wallet_dir(node, 'no_access'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir))
assert_equal(set(node.listwallets()), set(wallet_names))
@@ -155,43 +160,43 @@ class MultiWalletTest(BitcoinTestFramework):
# check that all requested wallets were created
self.stop_node(0)
for wallet_name in wallet_names:
- assert_equal(os.path.isfile(wallet_file(wallet_name)), True)
+ assert_equal(os.path.isfile(self.wallet_file(node, wallet_name)), True)
node.assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist')
- node.assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir())
- node.assert_start_raises_init_error(['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir())
+ node.assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir(node))
+ node.assert_start_raises_init_error(['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir(node))
self.start_node(0, ['-wallet=w1', '-wallet=w1'])
self.stop_node(0, 'Warning: Ignoring duplicate -wallet w1.')
# should not initialize if wallet file is a symlink
- os.symlink('w8', wallet_dir('w8_symlink'))
+ os.symlink('w8', wallet_dir(node, 'w8_symlink'))
node.assert_start_raises_init_error(['-wallet=w8_symlink'], r'Error: Invalid -wallet path \'w8_symlink\'\. .*', match=ErrorMatch.FULL_REGEX)
# should not initialize if the specified walletdir does not exist
node.assert_start_raises_init_error(['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist')
# should not initialize if the specified walletdir is not a directory
- not_a_dir = wallet_dir('notadir')
+ not_a_dir = wallet_dir(node, 'notadir')
open(not_a_dir, 'a').close()
node.assert_start_raises_init_error(['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory')
# if wallets/ doesn't exist, datadir should be the default wallet dir
- wallet_dir2 = data_dir('walletdir')
- os.rename(wallet_dir(), wallet_dir2)
+ wallet_dir2 = data_dir(node, 'walletdir')
+ os.rename(wallet_dir(node), wallet_dir2)
self.start_node(0)
node.createwallet("w4")
node.createwallet("w5")
assert_equal(set(node.listwallets()), {"w4", "w5"})
- w5 = wallet("w5")
+ w5 = get_wallet(node, "w5")
self.generatetoaddress(node, nblocks=1, address=w5.getnewaddress(), sync_fun=self.no_op)
# now if wallets/ exists again, but the rootdir is specified as the walletdir, w4 and w5 should still be loaded
- os.rename(wallet_dir2, wallet_dir())
- self.restart_node(0, ['-nowallet', '-walletdir=' + data_dir()])
+ os.rename(wallet_dir2, wallet_dir(node))
+ self.restart_node(0, ['-nowallet', '-walletdir=' + data_dir(node)])
node.loadwallet("w4")
node.loadwallet("w5")
assert_equal(set(node.listwallets()), {"w4", "w5"})
- w5 = wallet("w5")
+ w5 = get_wallet(node, "w5")
assert_equal(w5.getbalances()["mine"]["immature"], 50)
competing_wallet_dir = os.path.join(self.options.tmpdir, 'competing_walletdir')
@@ -207,8 +212,8 @@ class MultiWalletTest(BitcoinTestFramework):
assert_equal(sorted(map(lambda w: w['name'], node.listwalletdir()['wallets'])), sorted(in_wallet_dir))
- wallets = [wallet(w) for w in wallet_names]
- wallet_bad = wallet("bad")
+ wallets = [get_wallet(node, w) for w in wallet_names]
+ wallet_bad = get_wallet(node, "bad")
# check wallet names and balances
self.generatetoaddress(node, nblocks=1, address=wallets[0].getnewaddress(), sync_fun=self.no_op)
@@ -253,7 +258,7 @@ class MultiWalletTest(BitcoinTestFramework):
assert_equal(loadwallet_name['name'], wallet_names[0])
assert_equal(node.listwallets(), wallet_names[0:1])
node.getwalletinfo()
- w1 = node.get_wallet_rpc(wallet_names[0])
+ w1 = get_wallet(node, wallet_names[0])
w1.getwalletinfo()
self.log.info("Load second wallet")
@@ -261,7 +266,7 @@ class MultiWalletTest(BitcoinTestFramework):
assert_equal(loadwallet_name['name'], wallet_names[1])
assert_equal(node.listwallets(), wallet_names[0:2])
assert_raises_rpc_error(-19, "Multiple wallets are loaded. Please select which wallet", node.getwalletinfo)
- w2 = node.get_wallet_rpc(wallet_names[1])
+ w2 = get_wallet(node, wallet_names[1])
w2.getwalletinfo()
self.log.info("Concurrent wallet loading")
@@ -284,7 +289,7 @@ class MultiWalletTest(BitcoinTestFramework):
assert_equal(set(node.listwallets()), set(wallet_names))
# Fail to load if wallet doesn't exist
- path = wallet_dir("wallets")
+ path = wallet_dir(node, "wallets")
assert_raises_rpc_error(-18, "Wallet file verification failed. Failed to load database path '{}'. Path does not exist.".format(path), node.loadwallet, 'wallets')
# Fail to load duplicate wallets
@@ -293,21 +298,21 @@ class MultiWalletTest(BitcoinTestFramework):
assert_raises_rpc_error(-4, "Wallet file verification failed. Invalid -wallet path 'w8_symlink'", node.loadwallet, 'w8_symlink')
# Fail to load if a directory is specified that doesn't contain a wallet
- os.mkdir(wallet_dir('empty_wallet_dir'))
- path = wallet_dir("empty_wallet_dir")
+ os.mkdir(wallet_dir(node, 'empty_wallet_dir'))
+ path = wallet_dir(node, "empty_wallet_dir")
assert_raises_rpc_error(-18, "Wallet file verification failed. Failed to load database path '{}'. Data is not in recognized format.".format(path), node.loadwallet, 'empty_wallet_dir')
self.log.info("Test dynamic wallet creation.")
# Fail to create a wallet if it already exists.
- path = wallet_dir("w2")
+ path = wallet_dir(node, "w2")
assert_raises_rpc_error(-4, "Failed to create database path '{}'. Database already exists.".format(path), node.createwallet, 'w2')
# Successfully create a wallet with a new name
loadwallet_name = node.createwallet('w9')
in_wallet_dir.append('w9')
assert_equal(loadwallet_name['name'], 'w9')
- w9 = node.get_wallet_rpc('w9')
+ w9 = get_wallet(node, 'w9')
assert_equal(w9.getwalletinfo()['walletname'], 'w9')
assert 'w9' in node.listwallets()
@@ -317,7 +322,7 @@ class MultiWalletTest(BitcoinTestFramework):
new_wallet_name = os.path.join(new_wallet_dir, 'w10')
loadwallet_name = node.createwallet(new_wallet_name)
assert_equal(loadwallet_name['name'], new_wallet_name)
- w10 = node.get_wallet_rpc(new_wallet_name)
+ w10 = get_wallet(node, new_wallet_name)
assert_equal(w10.getwalletinfo()['walletname'], new_wallet_name)
assert new_wallet_name in node.listwallets()
@@ -327,7 +332,7 @@ class MultiWalletTest(BitcoinTestFramework):
# Test `unloadwallet` errors
assert_raises_rpc_error(-8, "Either the RPC endpoint wallet or the wallet name parameter must be provided", node.unloadwallet)
assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", node.unloadwallet, "dummy")
- assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", node.get_wallet_rpc("dummy").unloadwallet)
+ assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", get_wallet(node, "dummy").unloadwallet)
assert_raises_rpc_error(-8, "The RPC endpoint wallet and the wallet name parameter specify different wallets", w1.unloadwallet, "w2"),
# Successfully unload the specified wallet name
@@ -366,18 +371,18 @@ class MultiWalletTest(BitcoinTestFramework):
for wallet_name in wallet_names:
node.loadwallet(wallet_name)
for wallet_name in wallet_names:
- rpc = node.get_wallet_rpc(wallet_name)
+ rpc = get_wallet(node, wallet_name)
addr = rpc.getnewaddress()
backup = os.path.join(self.options.tmpdir, 'backup.dat')
if os.path.exists(backup):
os.unlink(backup)
rpc.backupwallet(backup)
node.unloadwallet(wallet_name)
- shutil.copyfile(empty_created_wallet if wallet_name == self.default_wallet_name else empty_wallet, wallet_file(wallet_name))
+ shutil.copyfile(empty_created_wallet if wallet_name == self.default_wallet_name else empty_wallet, self.wallet_file(node, wallet_name))
node.loadwallet(wallet_name)
assert_equal(rpc.getaddressinfo(addr)['ismine'], False)
node.unloadwallet(wallet_name)
- shutil.copyfile(backup, wallet_file(wallet_name))
+ shutil.copyfile(backup, self.wallet_file(node, wallet_name))
node.loadwallet(wallet_name)
assert_equal(rpc.getaddressinfo(addr)['ismine'], True)
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.