diff options
author | Ava Chow <github@achow101.com> | 2024-09-09 12:44:29 -0400 |
---|---|---|
committer | Ava Chow <github@achow101.com> | 2024-09-09 12:44:29 -0400 |
commit | fb52023ee69c346dd101770716b6d8c0525c38aa (patch) | |
tree | 46e5cdf49c51bbffe0fe6b7fc32109765918ae33 /test | |
parent | 746f88000e8b219b1318839ca1cd7fd9f2057793 (diff) | |
parent | ee47ca29d6ef55650a0af63bca817c5d494f31ef (diff) |
Merge bitcoin/bitcoin#30684: init: fix init fatal error on invalid negated option value
ee47ca29d6ef55650a0af63bca817c5d494f31ef init: fix fatal error on '-wallet' negated option value (furszy)
Pull request description:
Currently, if users provide a double negated value such as '-nowallet=0' or a non-boolean
convertible value to a negated option such as '-nowallet=not_a_boolean', the initialization
process results in a fatal error, causing an unclean shutdown and displaying a poorly
descriptive error message:
"JSON value of type bool is not of expected type string." (On bitcoind. The GUI
does not display any error msg - upcoming PR -).
This PR fixes the issue by ensuring that only string values are returned in the
the "wallet" settings list, failing otherwise. It also improves the clarity of the
returned error message.
Note:
This bug was introduced in https://github.com/bitcoin/bitcoin/pull/22217. Where the `GetArgs("-wallet")` call was
replaced by `GetSettingsList("-wallet")`.
ACKs for top commit:
achow101:
ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef
ryanofsky:
Code review ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef, just adding the suggested test since last review
TheCharlatan:
ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef
ismaelsadeeq:
Tested ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef
Tree-SHA512: 5f01076f74a048019bb70791160f0accc2db7a457d969cb23687bed81ccbbdec1dda68311e7c6e2dd56250e23e8d926d4066e5014b2a99a2fc202e24ed264fbd
Diffstat (limited to 'test')
-rwxr-xr-x | test/functional/feature_config_args.py | 7 | ||||
-rwxr-xr-x | test/functional/feature_settings.py | 23 |
2 files changed, 30 insertions, 0 deletions
diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index bb20e2baa8..44c7edf962 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -153,6 +153,13 @@ class ConfArgsTest(BitcoinTestFramework): expected_msg='Error: Error parsing command line arguments: Can not set -proxy with no value. Please specify value with -proxy=value.', extra_args=['-proxy'], ) + # Provide a value different from 1 to the -wallet negated option + if self.is_wallet_compiled(): + for value in [0, 'not_a_boolean']: + self.nodes[0].assert_start_raises_init_error( + expected_msg="Error: Invalid value detected for '-wallet' or '-nowallet'. '-wallet' requires a string value, while '-nowallet' accepts only '1' to disable all wallets", + extra_args=[f'-nowallet={value}'], + ) def test_log_buffer(self): self.stop_node(0) diff --git a/test/functional/feature_settings.py b/test/functional/feature_settings.py index 2189eac7dd..a7294944bf 100755 --- a/test/functional/feature_settings.py +++ b/test/functional/feature_settings.py @@ -13,11 +13,32 @@ from test_framework.util import assert_equal class SettingsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 self.wallet_names = [] + def test_wallet_settings(self, settings_path): + if not self.is_wallet_compiled(): + return + + self.log.info("Testing wallet settings..") + node = self.nodes[0] + # Create wallet to use it during tests + self.start_node(0) + node.createwallet(wallet_name='w1') + self.stop_node(0) + + # Verify wallet settings can only be strings. Either names or paths. Not booleans, nums nor anything else. + for wallets_data in [[10], [True], [[]], [{}], ["w1", 10], ["w1", False]]: + with settings_path.open("w") as fp: + json.dump({"wallet": wallets_data}, fp) + node.assert_start_raises_init_error(expected_msg="Error: Invalid value detected for '-wallet' or '-nowallet'. '-wallet' requires a string value, while '-nowallet' accepts only '1' to disable all wallets", + extra_args=[f'-settings={settings_path}']) + def run_test(self): node, = self.nodes settings = node.chain_path / "settings.json" @@ -86,6 +107,8 @@ class SettingsTest(BitcoinTestFramework): self.start_node(0, extra_args=[f"-settings={altsettings}"]) self.stop_node(0) + self.test_wallet_settings(settings) + if __name__ == '__main__': SettingsTest(__file__).main() |