diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -151,7 +151,7 @@ // Check for -testnet or -regtest parameter (BaseParams() calls are only // valid after this clause) try { - SelectBaseParams(ChainNameFromCommandLine()); + SelectBaseParams(gArgs.GetChainName()); } catch (const std::exception &e) { fprintf(stderr, "Error: %s\n", e.what()); return EXIT_FAILURE; diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -44,7 +44,7 @@ // Check for -testnet or -regtest parameter (Params() calls are only valid // after this clause) try { - SelectParams(ChainNameFromCommandLine()); + SelectParams(gArgs.GetChainName()); } catch (const std::exception &e) { fprintf(stderr, "Error: %s\n", e.what()); return EXIT_FAILURE; diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -117,7 +117,7 @@ // Check for -testnet or -regtest parameter (Params() calls are only // valid after this clause) try { - SelectParams(ChainNameFromCommandLine()); + SelectParams(gArgs.GetChainName()); } catch (const std::exception &e) { fprintf(stderr, "Error: %s\n", e.what()); return false; diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h --- a/src/chainparamsbase.h +++ b/src/chainparamsbase.h @@ -54,11 +54,4 @@ /** Sets the params returned by Params() to those for the given network. */ void SelectBaseParams(const std::string &chain); -/** - * Looks for -regtest, -testnet and returns the appropriate BIP70 chain name. - * @return CBaseChainParams::MAX_NETWORK_TYPES if an invalid combination is - * given. CBaseChainParams::MAIN by default. - */ -std::string ChainNameFromCommandLine(); - #endif // BITCOIN_CHAINPARAMSBASE_H diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -79,15 +79,3 @@ void SelectBaseParams(const std::string &chain) { globalChainBaseParams = CreateBaseChainParams(chain); } - -std::string ChainNameFromCommandLine() { - bool fRegTest = gArgs.GetBoolArg("-regtest", false); - bool fTestNet = gArgs.GetBoolArg("-testnet", false); - - if (fTestNet && fRegTest) - throw std::runtime_error( - "Invalid combination of -regtest and -testnet."); - if (fRegTest) return CBaseChainParams::REGTEST; - if (fTestNet) return CBaseChainParams::TESTNET; - return CBaseChainParams::MAIN; -} diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -700,7 +700,7 @@ // Check for -testnet or -regtest parameter (Params() calls are only valid // after this clause) try { - SelectParams(ChainNameFromCommandLine()); + SelectParams(gArgs.GetChainName()); } catch (std::exception &e) { QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), QObject::tr("Error: %1").arg(e.what())); diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -605,7 +605,7 @@ #ifdef WIN32 static fs::path StartupShortcutPath() { - std::string chain = ChainNameFromCommandLine(); + std::string chain = gArgs.GetChainName(); if (chain == CBaseChainParams::MAIN) return GetSpecialFolderPath(CSIDL_STARTUP) / "Bitcoin.lnk"; // Remove this special case when CBaseChainParams::TESTNET = "testnet4" @@ -703,7 +703,7 @@ } static fs::path GetAutostartFilePath() { - std::string chain = ChainNameFromCommandLine(); + std::string chain = gArgs.GetChainName(); if (chain == CBaseChainParams::MAIN) return GetAutostartDir() / "bitcoin.desktop"; return GetAutostartDir() / strprintf("bitcoin-%s.lnk", chain); @@ -740,7 +740,7 @@ fs::ofstream optionFile(GetAutostartFilePath(), std::ios_base::out | std::ios_base::trunc); if (!optionFile.good()) return false; - std::string chain = ChainNameFromCommandLine(); + std::string chain = gArgs.GetChainName(); // Write a bitcoin.desktop file to the autostart directory: optionFile << "[Desktop Entry]\n"; optionFile << "Type=Application\n"; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -108,6 +108,10 @@ const std::unordered_set &GetNegatedArgs() { return m_negated_args; } + void ReadConfigString(const std::string str_config) { + std::istringstream stream(str_config); + ReadConfigStream(stream); + } }; BOOST_AUTO_TEST_CASE(util_ParseParameters) { @@ -178,16 +182,149 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) { // Test some awful edge cases that hopefully no user will ever exercise. TestArgsManager testArgs; + + // Params test const char *argv_test[] = {"ignored", "-nofoo", "-foo", "-nobar=0"}; testArgs.ParseParameters(4, (char **)argv_test); // This was passed twice, second one overrides the negative setting. BOOST_CHECK(!testArgs.IsArgNegated("-foo")); - BOOST_CHECK(testArgs.GetBoolArg("-foo", false) == true); + BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == ""); + + // A double negative is a positive. + BOOST_CHECK(testArgs.IsArgNegated("-bar")); + BOOST_CHECK(testArgs.GetArg("-bar", "xxx") == "1"); + + // Config test + const char *conf_test = "nofoo=1\nfoo=1\nnobar=0\n"; + testArgs.ParseParameters(1, (char **)argv_test); + testArgs.ReadConfigString(conf_test); + + // This was passed twice, second one overrides the negative setting, + // but not the value. + BOOST_CHECK(!testArgs.IsArgNegated("-foo")); + BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == "0"); // A double negative is a positive. BOOST_CHECK(testArgs.IsArgNegated("-bar")); - BOOST_CHECK(testArgs.GetBoolArg("-bar", false) == true); + BOOST_CHECK(testArgs.GetArg("-bar", "xxx") == "1"); + + // Combined test + const char *combo_test_args[] = {"ignored", "-nofoo", "-bar"}; + const char *combo_test_conf = "foo=1\nnobar=1\n"; + testArgs.ParseParameters(3, (char **)combo_test_args); + testArgs.ReadConfigString(combo_test_conf); + + // Command line overrides, but doesn't erase old setting + BOOST_CHECK(!testArgs.IsArgNegated("-foo")); + BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == "0"); + BOOST_CHECK(testArgs.GetArgs("-foo").size() == 2 && + testArgs.GetArgs("-foo").front() == "0" && + testArgs.GetArgs("-foo").back() == "1"); + + // Command line overrides, but doesn't erase old setting + BOOST_CHECK(testArgs.IsArgNegated("-bar")); + BOOST_CHECK(testArgs.GetArg("-bar", "xxx") == ""); + BOOST_CHECK(testArgs.GetArgs("-bar").size() == 2 && + testArgs.GetArgs("-bar").front() == "" && + testArgs.GetArgs("-bar").back() == "0"); +} + +BOOST_AUTO_TEST_CASE(util_ReadConfigStream) { + const char *str_config = "a=\n" + "b=1\n" + "ccc=argument\n" + "ccc=multiple\n" + "d=e\n" + "nofff=1\n" + "noggg=0\n" + "h=1\n" + "noh=1\n" + "noi=1\n" + "i=1\n"; + + TestArgsManager test_args; + + test_args.ReadConfigString(str_config); + // expectation: a, b, ccc, d, fff, ggg, h, i end up in map + + BOOST_CHECK(test_args.GetMapArgs().size() == 8); + BOOST_CHECK(test_args.GetMapMultiArgs().size() == 8); + + BOOST_CHECK(test_args.GetMapArgs().count("-a") && + test_args.GetMapArgs().count("-b") && + test_args.GetMapArgs().count("-ccc") && + test_args.GetMapArgs().count("-d") && + test_args.GetMapArgs().count("-fff") && + test_args.GetMapArgs().count("-ggg") && + test_args.GetMapArgs().count("-h") && + test_args.GetMapArgs().count("-i")); + + BOOST_CHECK(test_args.IsArgSet("-a") && test_args.IsArgSet("-b") && + test_args.IsArgSet("-ccc") && test_args.IsArgSet("-d") && + test_args.IsArgSet("-fff") && test_args.IsArgSet("-ggg") && + test_args.IsArgSet("-h") && test_args.IsArgSet("-i") && + !test_args.IsArgSet("-zzz")); + + BOOST_CHECK(test_args.GetArg("-a", "xxx") == "" && + test_args.GetArg("-b", "xxx") == "1" && + test_args.GetArg("-ccc", "xxx") == "argument" && + test_args.GetArg("-d", "xxx") == "e" && + test_args.GetArg("-fff", "xxx") == "0" && + test_args.GetArg("-ggg", "xxx") == "1" && + // 1st value takes precedence + test_args.GetArg("-h", "xxx") == "1" && + // 1st value takes precedence + test_args.GetArg("-i", "xxx") == "0" && + test_args.GetArg("-zzz", "xxx") == "xxx"); + + for (bool def : {false, true}) { + BOOST_CHECK(test_args.GetBoolArg("-a", def) && + test_args.GetBoolArg("-b", def) && + !test_args.GetBoolArg("-ccc", def) && + !test_args.GetBoolArg("-d", def) && + !test_args.GetBoolArg("-fff", def) && + test_args.GetBoolArg("-ggg", def) && + test_args.GetBoolArg("-h", def) && + !test_args.GetBoolArg("-i", def) && + test_args.GetBoolArg("-zzz", def) == def); + } + + BOOST_CHECK(test_args.GetArgs("-a").size() == 1 && + test_args.GetArgs("-a").front() == ""); + BOOST_CHECK(test_args.GetArgs("-b").size() == 1 && + test_args.GetArgs("-b").front() == "1"); + BOOST_CHECK(test_args.GetArgs("-ccc").size() == 2 && + test_args.GetArgs("-ccc").front() == "argument" && + test_args.GetArgs("-ccc").back() == "multiple"); + BOOST_CHECK(test_args.GetArgs("-fff").size() == 1 && + test_args.GetArgs("-fff").front() == "0"); + BOOST_CHECK(test_args.GetArgs("-nofff").size() == 0); + BOOST_CHECK(test_args.GetArgs("-ggg").size() == 1 && + test_args.GetArgs("-ggg").front() == "1"); + BOOST_CHECK(test_args.GetArgs("-noggg").size() == 0); + BOOST_CHECK(test_args.GetArgs("-h").size() == 2 && + test_args.GetArgs("-h").front() == "1" && + test_args.GetArgs("-h").back() == "0"); + BOOST_CHECK(test_args.GetArgs("-noh").size() == 0); + BOOST_CHECK(test_args.GetArgs("-i").size() == 2 && + test_args.GetArgs("-i").front() == "0" && + test_args.GetArgs("-i").back() == "1"); + BOOST_CHECK(test_args.GetArgs("-noi").size() == 0); + BOOST_CHECK(test_args.GetArgs("-zzz").size() == 0); + + BOOST_CHECK(!test_args.IsArgNegated("-a")); + BOOST_CHECK(!test_args.IsArgNegated("-b")); + BOOST_CHECK(!test_args.IsArgNegated("-ccc")); + BOOST_CHECK(!test_args.IsArgNegated("-d")); + BOOST_CHECK(test_args.IsArgNegated("-fff")); + // IsArgNegated==true when noggg=0 + BOOST_CHECK(test_args.IsArgNegated("-ggg")); + // last setting takes precedence + BOOST_CHECK(test_args.IsArgNegated("-h")); + // last setting takes precedence + BOOST_CHECK(!test_args.IsArgNegated("-i")); + BOOST_CHECK(!test_args.IsArgNegated("-zzz")); } BOOST_AUTO_TEST_CASE(util_GetArg) { @@ -214,6 +351,53 @@ BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest4", false), true); } +BOOST_AUTO_TEST_CASE(util_GetChainName) { + TestArgsManager test_args; + + const char *argv_testnet[] = {"cmd", "-testnet"}; + const char *argv_regtest[] = {"cmd", "-regtest"}; + const char *argv_test_no_reg[] = {"cmd", "-testnet", "-noregtest"}; + const char *argv_both[] = {"cmd", "-testnet", "-regtest"}; + + // equivalent to "-testnet" + const char *testnetconf = "testnet=1\nregtest=0\n"; + + test_args.ParseParameters(0, (char **)argv_testnet); + BOOST_CHECK_EQUAL(test_args.GetChainName(), "main"); + + test_args.ParseParameters(2, (char **)argv_testnet); + BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); + + test_args.ParseParameters(2, (char **)argv_regtest); + BOOST_CHECK_EQUAL(test_args.GetChainName(), "regtest"); + + test_args.ParseParameters(3, (char **)argv_test_no_reg); + BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); + + test_args.ParseParameters(3, (char **)argv_both); + BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); + + test_args.ParseParameters(0, (char **)argv_testnet); + test_args.ReadConfigString(testnetconf); + BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); + + test_args.ParseParameters(2, (char **)argv_testnet); + test_args.ReadConfigString(testnetconf); + BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); + + test_args.ParseParameters(2, (char **)argv_regtest); + test_args.ReadConfigString(testnetconf); + BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); + + test_args.ParseParameters(3, (char **)argv_test_no_reg); + test_args.ReadConfigString(testnetconf); + BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); + + test_args.ParseParameters(3, (char **)argv_both); + test_args.ReadConfigString(testnetconf); + BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); +} + BOOST_AUTO_TEST_CASE(util_FormatMoney) { BOOST_CHECK_EQUAL(FormatMoney(Amount::zero()), "0.00"); BOOST_CHECK_EQUAL(FormatMoney(123456789 * (COIN / 10000)), "12345.6789"); @@ -627,10 +811,10 @@ comments.push_back(std::string("comment1")); std::vector comments2; comments2.push_back(std::string("comment1")); + // Semicolon is discouraged but not forbidden by BIP-0014 comments2.push_back(SanitizeString( std::string("Comment2; .,_?@-; !\"#$%&'()*+/<=>[]\\^`{|}~"), - SAFE_CHARS_UA_COMMENT)); // Semicolon is discouraged but not forbidden - // by BIP-0014 + SAFE_CHARS_UA_COMMENT)); BOOST_CHECK_EQUAL( FormatSubVersion("Test", 99900, std::vector()), std::string("/Test:0.9.99/")); diff --git a/src/util.h b/src/util.h --- a/src/util.h +++ b/src/util.h @@ -99,6 +99,8 @@ std::map> mapMultiArgs; std::unordered_set m_negated_args; + void ReadConfigStream(std::istream &stream); + public: void ParseParameters(int argc, const char *const argv[]); void ReadConfigFile(const std::string &confPath); @@ -181,6 +183,14 @@ void ForceSetMultiArg(const std::string &strArg, const std::string &strValue); + /** + * Looks for -regtest, -testnet and returns the appropriate BIP70 chain + * name. + * @return CBaseChainParams::MAIN by default; raises runtime error if an + * invalid combination is given. + */ + std::string GetChainName() const; + // Remove an arg setting, used only in testing void ClearArg(const std::string &strArg); diff --git a/src/util.cpp b/src/util.cpp --- a/src/util.cpp +++ b/src/util.cpp @@ -441,38 +441,57 @@ return pathConfigFile; } -void ArgsManager::ReadConfigFile(const std::string &confPath) { - fs::ifstream streamConfig(GetConfigFile(confPath)); +void ArgsManager::ReadConfigStream(std::istream &stream) { + LOCK(cs_args); - // No bitcoin.conf file is OK - if (!streamConfig.good()) { - return; + std::set setOptions; + setOptions.insert("*"); + + for (boost::program_options::detail::config_file_iterator + it(stream, setOptions), + end; + it != end; ++it) { + // Don't overwrite existing settings so command line settings override + // bitcoin.conf + std::string strKey = std::string("-") + it->string_key; + std::string strValue = it->value[0]; + InterpretNegatedOption(strKey, strValue); + if (mapArgs.count(strKey) == 0) { + mapArgs[strKey] = strValue; + } + mapMultiArgs[strKey].push_back(strValue); } +} - { - LOCK(cs_args); - std::set setOptions; - setOptions.insert("*"); - - for (boost::program_options::detail::config_file_iterator - it(streamConfig, setOptions), - end; - it != end; ++it) { - // Don't overwrite existing settings so command line settings - // override bitcoin.conf - std::string strKey = std::string("-") + it->string_key; - std::string strValue = it->value[0]; - InterpretNegatedOption(strKey, strValue); - if (mapArgs.count(strKey) == 0) { - mapArgs[strKey] = strValue; - } - mapMultiArgs[strKey].push_back(strValue); - } +void ArgsManager::ReadConfigFile(const std::string &confPath) { + fs::ifstream stream(GetConfigFile(confPath)); + + // ok to not have a config file + if (stream.good()) { + ReadConfigStream(stream); } + // If datadir is changed in .conf file: ClearDatadirCache(); } +std::string ArgsManager::GetChainName() const { + bool fRegTest = GetBoolArg("-regtest", false); + bool fTestNet = GetBoolArg("-testnet", false); + + if (fTestNet && fRegTest) { + throw std::runtime_error( + "Invalid combination of -regtest and -testnet."); + } + if (fRegTest) { + return CBaseChainParams::REGTEST; + } + if (fTestNet) { + return CBaseChainParams::TESTNET; + } + return CBaseChainParams::MAIN; +} + #ifndef WIN32 fs::path GetPidFile() { fs::path pathPidFile(gArgs.GetArg("-pid", BITCOIN_PID_FILENAME));