highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-04-21 14:05:06 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge #20715: util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet

Browse Source 
fa61b9d1a68820758f9540653920deaeae6abe79 util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet (MarcoFalke)
7777105a24a36b62df35d12ecf6c6370671568c8 refactor: Move all command dependend checks to ExecuteWalletToolFunc (MarcoFalke)
fa06bce4ac17f93decd4ee38c956e7aa55983f0d test: Add tests (MarcoFalke)
fac05ccdade8b34c969b9cd9b37b355bc0aabf9c wallet: [refactor] Pass ArgsManager to WalletAppInit (MarcoFalke)

Pull request description:

  This not only moves the parsing responsibility out from the wallet tool, but it also makes it easier to implement bitcoin-util #19937

  Fixes: #20902

ACKs for top commit:
  ajtowns:
    ACK fa61b9d1a68820758f9540653920deaeae6abe79
  fjahr:
    Code review ACK fa61b9d1a68820758f9540653920deaeae6abe79

Tree-SHA512: 79622b806e8bf9dcd0dc24a8a6687345710df57720992e83a41cd8d6762a6dc112044ebc58fcf6e8fbf45de29a79b04873c5b8c2494a1eaaf902a2884703e47b
This commit is contained in:
MarcoFalke 2021-02-04 09:11:54 +01:00
commit 4e946ebcf1
No known key found for this signature in database
GPG Key ID: D2EA4850E7528B25
7 changed files with 115 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
src/bitcoin-wallet.cpp
View File

@ -33,51 +33,52 @@ static void SetupWalletToolArgs(ArgsManager& argsman)
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);
argsman.AddArg("info", "Get wallet info", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
argsman.AddArg("create", "Create new wallet file", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
argsman.AddArg("salvage", "Attempt to recover private keys from a corrupt wallet. Warning: 'salvage' is experimental.", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
argsman.AddArg("dump", "Print out all of the wallet key-value records", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
argsman.AddArg("createfromdump", "Create new wallet file from dumped records", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
argsman.AddCommand("info", "Get wallet info", OptionsCategory::COMMANDS);
argsman.AddCommand("create", "Create new wallet file", OptionsCategory::COMMANDS);
argsman.AddCommand("salvage", "Attempt to recover private keys from a corrupt wallet. Warning: 'salvage' is experimental.", OptionsCategory::COMMANDS);
argsman.AddCommand("dump", "Print out all of the wallet key-value records", OptionsCategory::COMMANDS);
argsman.AddCommand("createfromdump", "Create new wallet file from dumped records", OptionsCategory::COMMANDS);
}
static bool WalletAppInit(int argc, char* argv[])
static bool WalletAppInit(ArgsManager& args, int argc, char* argv[])
{
SetupWalletToolArgs(gArgs);
SetupWalletToolArgs(args);
std::string error_message;
if (!gArgs.ParseParameters(argc, argv, error_message)) {
if (!args.ParseParameters(argc, argv, error_message)) {
tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error_message);
return false;
}
if (argc < 2 || HelpRequested(gArgs) || gArgs.IsArgSet("-version")) {
if (argc < 2 || HelpRequested(args) || args.IsArgSet("-version")) {
std::string strUsage = strprintf("%s bitcoin-wallet version", PACKAGE_NAME) + " " + FormatFullVersion() + "\n";
if (!gArgs.IsArgSet("-version")) {
strUsage += "\n"
"bitcoin-wallet is an offline tool for creating and interacting with " PACKAGE_NAME " wallet files.\n"
"By default bitcoin-wallet will act on wallets in the default mainnet wallet directory in the datadir.\n"
"To change the target wallet, use the -datadir, -wallet and -testnet/-regtest arguments.\n\n"
"Usage:\n"
" bitcoin-wallet [options] <command>\n";
strUsage += "\n" + gArgs.GetHelpMessage();
}
if (!args.IsArgSet("-version")) {
strUsage += "\n"
"bitcoin-wallet is an offline tool for creating and interacting with " PACKAGE_NAME " wallet files.\n"
"By default bitcoin-wallet will act on wallets in the default mainnet wallet directory in the datadir.\n"
"To change the target wallet, use the -datadir, -wallet and -testnet/-regtest arguments.\n\n"
"Usage:\n"
" bitcoin-wallet [options] <command>\n";
strUsage += "\n" + args.GetHelpMessage();
}
tfm::format(std::cout, "%s", strUsage);
return false;
}
// check for printtoconsole, allow -debug
LogInstance().m_print_to_console = gArgs.GetBoolArg("-printtoconsole", gArgs.GetBoolArg("-debug", false));
LogInstance().m_print_to_console = args.GetBoolArg("-printtoconsole", args.GetBoolArg("-debug", false));
if (!CheckDataDirOption()) {
tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", ""));
tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", args.GetArg("-datadir", ""));
return false;
}
// Check for chain settings (Params() calls are only valid after this clause)
SelectParams(gArgs.GetChainName());
SelectParams(args.GetChainName());
return true;
}
int main(int argc, char* argv[])
{
ArgsManager& args = gArgs;
#ifdef WIN32
util::WinCmdLineArgs winArgs;
std::tie(argc, argv) = winArgs.get();
@ -85,7 +86,7 @@ int main(int argc, char* argv[])
SetupEnvironment();
RandomInit();
try {
if (!WalletAppInit(argc, argv)) return EXIT_FAILURE;
if (!WalletAppInit(args, argc, argv)) return EXIT_FAILURE;
} catch (const std::exception& e) {
PrintExceptionContinue(&e, "WalletAppInit()");
return EXIT_FAILURE;
@ -94,33 +95,19 @@ int main(int argc, char* argv[])
return EXIT_FAILURE;
}
std::string method {};
for(int i = 1; i < argc; ++i) {
if (!IsSwitchChar(argv[i][0])) {
if (!method.empty()) {
tfm::format(std::cerr, "Error: two methods provided (%s and %s). Only one method should be provided.\n", method, argv[i]);
return EXIT_FAILURE;
}
method = argv[i];
}
}
if (method.empty()) {
const auto command = args.GetCommand();
if (!command) {
tfm::format(std::cerr, "No method provided. Run `bitcoin-wallet -help` for valid methods.\n");
return EXIT_FAILURE;
}
// A name must be provided when creating a file
if (method == "create" && !gArgs.IsArgSet("-wallet")) {
tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n");
if (command->args.size() != 0) {
tfm::format(std::cerr, "Error: Additional arguments provided (%s). Methods do not take arguments. Please refer to `-help`.\n", Join(command->args, ", "));
return EXIT_FAILURE;
}
std::string name = gArgs.GetArg("-wallet", "");
ECCVerifyHandle globalVerifyHandle;
ECC_Start();
if (!WalletTool::ExecuteWalletToolFunc(gArgs, method, name)) {
if (!WalletTool::ExecuteWalletToolFunc(args, command->command)) {
return EXIT_FAILURE;
}
ECC_Stop();

2
src/test/fuzz/system.cpp
View File

@ -54,7 +54,7 @@ FUZZ_TARGET(system)
if (args_manager.GetArgFlags(argument_name) != nullopt) {
return;
}
args_manager.AddArg(argument_name, fuzzed_data_provider.ConsumeRandomLengthString(16), fuzzed_data_provider.ConsumeIntegral<unsigned int>(), options_category);
args_manager.AddArg(argument_name, fuzzed_data_provider.ConsumeRandomLengthString(16), fuzzed_data_provider.ConsumeIntegral<unsigned int>() & ~ArgsManager::COMMAND, options_category);
},
[&] {
// Avoid hitting:

53
src/util/system.cpp
View File

@ -3,7 +3,6 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <sync.h>
#include <util/system.h>
#ifdef HAVE_BOOST_PROCESS
@ -11,6 +10,8 @@
#endif // HAVE_BOOST_PROCESS
#include <chainparamsbase.h>
#include <sync.h>
#include <util/check.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <util/translation.h>
@ -310,8 +311,22 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
key[0] = '-';
#endif
if (key[0] != '-')
if (key[0] != '-') {
if (!m_accept_any_command && m_command.empty()) {
// The first non-dash arg is a registered command
Optional<unsigned int> flags = GetArgFlags(key);
if (!flags || !(*flags & ArgsManager::COMMAND)) {
error = strprintf("Invalid command '%s'", argv[i]);
return false;
}
}
m_command.push_back(key);
while (++i < argc) {
// The remaining args are command args
m_command.push_back(argv[i]);
}
break;
}
// Transform --foo to -foo
if (key.length() > 1 && key[1] == '-')
@ -359,6 +374,26 @@ Optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) const
return nullopt;
}
std::optional<const ArgsManager::Command> ArgsManager::GetCommand() const
{
Command ret;
LOCK(cs_args);
auto it = m_command.begin();
if (it == m_command.end()) {
// No command was passed
return std::nullopt;
}
if (!m_accept_any_command) {
// The registered command
ret.command = *(it++);
}
while (it != m_command.end()) {
// The unregistered command and args (if any)
ret.args.push_back(*(it++));
}
return ret;
}
std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
{
std::vector<std::string> result;
@ -504,8 +539,22 @@ void ArgsManager::ForceSetArg(const std::string& strArg, const std::string& strV
m_settings.forced_settings[SettingName(strArg)] = strValue;
}
void ArgsManager::AddCommand(const std::string& cmd, const std::string& help, const OptionsCategory& cat)
{
Assert(cmd.find('=') == std::string::npos);
Assert(cmd.at(0) != '-');
LOCK(cs_args);
m_accept_any_command = false; // latch to false
std::map<std::string, Arg>& arg_map = m_available_args[cat];
auto ret = arg_map.emplace(cmd, Arg{"", help, ArgsManager::COMMAND});
Assert(ret.second); // Fail on duplicate commands
}
void ArgsManager::AddArg(const std::string& name, const std::string& help, unsigned int flags, const OptionsCategory& cat)
{
Assert((flags & ArgsManager::COMMAND) == 0); // use AddCommand
// Split arg name from its help param
size_t eq_index = name.find('=');
if (eq_index == std::string::npos) {

22
src/util/system.h
View File

@ -181,6 +181,7 @@ public:
NETWORK_ONLY = 0x200,
// This argument's value is sensitive (such as a password).
SENSITIVE = 0x400,
COMMAND = 0x800,
};
protected:
@ -193,9 +194,11 @@ protected:
mutable RecursiveMutex cs_args;
util::Settings m_settings GUARDED_BY(cs_args);
std::vector<std::string> m_command GUARDED_BY(cs_args);
std::string m_network GUARDED_BY(cs_args);
std::set<std::string> m_network_only_args GUARDED_BY(cs_args);
std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
bool m_accept_any_command GUARDED_BY(cs_args){true};
std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
[[nodiscard]] bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false);
@ -246,6 +249,20 @@ public:
*/
const std::list<SectionInfo> GetUnrecognizedSections() const;
struct Command {
/** The command (if one has been registered with AddCommand), or empty */
std::string command;
/**
* If command is non-empty: Any args that followed it
* If command is empty: The unregistered command and any args that followed it
*/
std::vector<std::string> args;
};
/**
* Get the command and command args (returns std::nullopt if no command provided)
*/
std::optional<const Command> GetCommand() const;
/**
* Return a vector of strings of the given argument
*
@ -331,6 +348,11 @@ public:
*/
void AddArg(const std::string& name, const std::string& help, unsigned int flags, const OptionsCategory& cat);
/**
* Add subcommand
*/
void AddCommand(const std::string& cmd, const std::string& help, const OptionsCategory& cat);
/**
* Add many hidden arguments
*/

10
src/wallet/wallettool.cpp
View File

@ -103,10 +103,8 @@ static void WalletShowInfo(CWallet* wallet_instance)
tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->m_address_book.size());
}
bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, const std::string& name)
bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
{
const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), name);
if (args.IsArgSet("-format") && command != "createfromdump") {
tfm::format(std::cerr, "The -format option can only be used with the \"createfromdump\" command.\n");
return false;
@ -119,6 +117,12 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command,
tfm::format(std::cerr, "The -descriptors option can only be used with the 'create' command.\n");
return false;
}
if (command == "create" && !args.IsArgSet("-wallet")) {
tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n");
return false;
}
const std::string name = args.GetArg("-wallet", "");
const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), name);
if (command == "create") {
DatabaseOptions options;

2
src/wallet/wallettool.h
View File

@ -10,7 +10,7 @@
namespace WalletTool {
void WalletShowInfo(CWallet* wallet_instance);
bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, const std::string& file);
bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command);
} // namespace WalletTool

8
test/functional/tool_wallet.py
View File

@ -183,11 +183,13 @@ class ToolWalletTest(BitcoinTestFramework):
def test_invalid_tool_commands_and_args(self):
self.log.info('Testing that various invalid commands raise with specific error messages')
self.assert_raises_tool_error('Invalid command: foo', 'foo')
self.assert_raises_tool_error("Error parsing command line arguments: Invalid command 'foo'", 'foo')
# `bitcoin-wallet help` raises an error. Use `bitcoin-wallet -help`.
self.assert_raises_tool_error('Invalid command: help', 'help')
self.assert_raises_tool_error('Error: two methods provided (info and create). Only one method should be provided.', 'info', 'create')
self.assert_raises_tool_error("Error parsing command line arguments: Invalid command 'help'", 'help')
self.assert_raises_tool_error('Error: Additional arguments provided (create). Methods do not take arguments. Please refer to `-help`.', 'info', 'create')
self.assert_raises_tool_error('Error parsing command line arguments: Invalid parameter -foo', '-foo')
self.assert_raises_tool_error('No method provided. Run `bitcoin-wallet -help` for valid methods.')
self.assert_raises_tool_error('Wallet name must be provided when creating a new wallet.', 'create')
locked_dir = os.path.join(self.options.tmpdir, "node0", "regtest", "wallets")
error = 'Error initializing wallet database environment "{}"!'.format(locked_dir)
if self.options.descriptors: