From fa7dde1c418e2e700853bd30cc9e012c4e4c5ef2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 17 Dec 2020 19:18:34 +0100 Subject: [PATCH 1/3] wallet: Pass ArgsManager into ExecuteWalletToolFunc instead of using global --- src/bitcoin-wallet.cpp | 3 ++- src/wallet/wallettool.cpp | 8 ++++---- src/wallet/wallettool.h | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index 0f8d312c5e6..ef26ed3b95d 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -120,8 +120,9 @@ int main(int argc, char* argv[]) ECCVerifyHandle globalVerifyHandle; ECC_Start(); - if (!WalletTool::ExecuteWalletToolFunc(method, name)) + if (!WalletTool::ExecuteWalletToolFunc(gArgs, method, name)) { return EXIT_FAILURE; + } ECC_Stop(); return EXIT_SUCCESS; } diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index fe3fcb32c26..e16d7f63385 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -103,17 +103,17 @@ static void WalletShowInfo(CWallet* wallet_instance) tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->m_address_book.size()); } -bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) +bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, const std::string& name) { fs::path path = fs::absolute(name, GetWalletDir()); // -format is only allowed with createfromdump. Disallow it for all other commands. - if (gArgs.IsArgSet("-format") && command != "createfromdump") { + if (args.IsArgSet("-format") && command != "createfromdump") { tfm::format(std::cerr, "The -format option can only be used with the \"createfromdump\" command.\n"); return false; } // -dumpfile is only allowed with dump and createfromdump. Disallow it for all other commands. - if (gArgs.IsArgSet("-dumpfile") && command != "dump" && command != "createfromdump") { + if (args.IsArgSet("-dumpfile") && command != "dump" && command != "createfromdump") { tfm::format(std::cerr, "The -dumpfile option can only be used with the \"dump\" and \"createfromdump\" commands.\n"); return false; } @@ -121,7 +121,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) if (command == "create") { DatabaseOptions options; options.require_create = true; - if (gArgs.GetBoolArg("-descriptors", false)) { + if (args.GetBoolArg("-descriptors", false)) { options.create_flags |= WALLET_FLAG_DESCRIPTORS; options.require_format = DatabaseFormat::SQLITE; } diff --git a/src/wallet/wallettool.h b/src/wallet/wallettool.h index d0b8d6812a7..6a9a8102316 100644 --- a/src/wallet/wallettool.h +++ b/src/wallet/wallettool.h @@ -10,7 +10,7 @@ namespace WalletTool { void WalletShowInfo(CWallet* wallet_instance); -bool ExecuteWalletToolFunc(const std::string& command, const std::string& file); +bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, const std::string& file); } // namespace WalletTool From faf8f61368696b9cbbea55ead30d6a48203235ff Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 17 Dec 2020 19:52:58 +0100 Subject: [PATCH 2/3] test: Add missing check for is_sqlite_compiled --- test/functional/tool_wallet.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 10939829295..288128e00e7 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -348,7 +348,8 @@ class ToolWalletTest(BitcoinTestFramework): self.log.info('Checking createfromdump') self.do_tool_createfromdump("load", "wallet.dump") self.do_tool_createfromdump("load-bdb", "wallet.dump", "bdb") - self.do_tool_createfromdump("load-sqlite", "wallet.dump", "sqlite") + if self.is_sqlite_compiled(): + self.do_tool_createfromdump("load-sqlite", "wallet.dump", "sqlite") self.log.info('Checking createfromdump handling of magic and versions') bad_ver_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_ver1.dump") From fae32f295cc5b57c1cb95090bb60cddb42f9778a Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 17 Dec 2020 19:52:36 +0100 Subject: [PATCH 3/3] wallet: Add missing check for -descriptors wallet tool option --- src/bitcoin-wallet.cpp | 2 +- src/wallet/wallettool.cpp | 6 ++++-- test/functional/tool_wallet.py | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index ef26ed3b95d..3e8e5fc7bc5 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -29,7 +29,7 @@ static void SetupWalletToolArgs(ArgsManager& argsman) argsman.AddArg("-wallet=", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS); argsman.AddArg("-dumpfile=", "When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet.", ArgsManager::ALLOW_STRING, OptionsCategory::OPTIONS); argsman.AddArg("-debug=", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-descriptors", "Create descriptors wallet. Only for create", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); + argsman.AddArg("-descriptors", "Create descriptors wallet. Only for 'create'", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); argsman.AddArg("-format=", "The format of the wallet file to create. Either \"bdb\" or \"sqlite\". Only used with 'createfromdump'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index e16d7f63385..a1bb7343f42 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -107,16 +107,18 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, { fs::path path = fs::absolute(name, GetWalletDir()); - // -format is only allowed with createfromdump. Disallow it for all other commands. if (args.IsArgSet("-format") && command != "createfromdump") { tfm::format(std::cerr, "The -format option can only be used with the \"createfromdump\" command.\n"); return false; } - // -dumpfile is only allowed with dump and createfromdump. Disallow it for all other commands. if (args.IsArgSet("-dumpfile") && command != "dump" && command != "createfromdump") { tfm::format(std::cerr, "The -dumpfile option can only be used with the \"dump\" and \"createfromdump\" commands.\n"); return false; } + if (args.IsArgSet("-descriptors") && command != "create") { + tfm::format(std::cerr, "The -descriptors option can only be used with the 'create' command.\n"); + return false; + } if (command == "create") { DatabaseOptions options; diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 288128e00e7..8a1af24dcfe 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -31,7 +31,7 @@ class ToolWalletTest(BitcoinTestFramework): def bitcoin_wallet_process(self, *args): binary = self.config["environment"]["BUILDDIR"] + '/src/bitcoin-wallet' + self.config["environment"]["EXEEXT"] default_args = ['-datadir={}'.format(self.nodes[0].datadir), '-chain=%s' % self.chain] - if self.options.descriptors: + if self.options.descriptors and 'create' in args: default_args.append('-descriptors') return subprocess.Popen([binary] + default_args + list(args), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) @@ -344,6 +344,7 @@ class ToolWalletTest(BitcoinTestFramework): self.assert_raises_tool_error('Dump file {} does not exist.'.format(non_exist_dump), '-wallet=todump', '-dumpfile={}'.format(non_exist_dump), 'createfromdump') wallet_path = os.path.join(self.nodes[0].datadir, 'regtest/wallets/todump2') self.assert_raises_tool_error('Failed to create database path \'{}\'. Database already exists.'.format(wallet_path), '-wallet=todump2', '-dumpfile={}'.format(wallet_dump), 'createfromdump') + self.assert_raises_tool_error("The -descriptors option can only be used with the 'create' command.", '-descriptors', '-wallet=todump2', '-dumpfile={}'.format(wallet_dump), 'createfromdump') self.log.info('Checking createfromdump') self.do_tool_createfromdump("load", "wallet.dump")