Merge #20687: wallet: Add missing check for -descriptors wallet tool option

fae32f295cc5b57c1cb95090bb60cddb42f9778a wallet: Add missing check for -descriptors wallet tool option (MarcoFalke)
faf8f61368696b9cbbea55ead30d6a48203235ff test: Add missing check for is_sqlite_compiled (MarcoFalke)
fa7dde1c418e2e700853bd30cc9e012c4e4c5ef2 wallet: Pass ArgsManager into ExecuteWalletToolFunc instead of using global (MarcoFalke)

Pull request description:

  Also, fix a test failure when compiled without sqlite

ACKs for top commit:
  ryanofsky:
    Code review ACK fae32f295cc5b57c1cb95090bb60cddb42f9778a. Thanks for implementing the -descriptors check and dealing with the test failure!
  jonatack:
    Code review utACK fae32f295cc5b57c1cb95090bb60cddb42f9778a

Tree-SHA512: 3d7710694085822739a8316e4abc6db270799ca6ff6b0f9e5563ae240da65ae6a9cab7ba2647feae6ba540dac40b55b38ed41c8f6ed0bf02a3d1536284448927
This commit is contained in:
MarcoFalke 2020-12-18 07:55:51 +01:00
commit b27104dc52
No known key found for this signature in database
GPG Key ID: D2EA4850E7528B25
4 changed files with 16 additions and 11 deletions

View File

@ -29,7 +29,7 @@ static void SetupWalletToolArgs(ArgsManager& argsman)
argsman.AddArg("-wallet=<wallet-name>", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS);
argsman.AddArg("-dumpfile=<file name>", "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=<category>", "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=<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);
@ -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;
}

View File

@ -103,25 +103,27 @@ 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;
}
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;
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;
}

View File

@ -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

View File

@ -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,11 +344,13 @@ 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")
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")