LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #12878: [refactor] Config handling refactoring in preparation for network-specific sections

Browse source 
77a733a99 [tests] Add additional unit tests for -nofoo edge cases (Anthony Towns)
af173c2be [tests] Check GetChainName works with config entries (Anthony Towns)
fa27f1c23 [tests] Add unit tests for ReadConfigStream (Anthony Towns)
087c5d204 ReadConfigStream: assume the stream is good (Anthony Towns)
6d5815aad Separate out ReadConfigStream from ReadConfigFile (Anthony Towns)
834d30341 [tests] Add unit tests for GetChainName (Anthony Towns)
11b6b5b86 Move ChainNameFromCommandLine into ArgsManager and rename to GetChainName (Anthony Towns)

Pull request description:

  This does a bit of refactoring of the configuration handling code in order to add additional tests to make adding support for [test]/[regtest] sections in the config file in #11862 easier. Should not cause any behaviour changes.

Tree-SHA512: 8d2ce1449fc180de03414e7e569d1a21ba1e9f6564e13d3faf3961f710adc725fa0d4ab49b89ebd2baa11ea36ac5018377f693a84037d386a8b8697c9d6db3e9
This commit is contained in:
Jonas Schnelli 2018-04-08 16:24:22 +02:00
commit 25c56cdbe7
No known key found for this signature in database
GPG key ID: 1EB776BB03C7922D
10 changed files with 246 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/bitcoin-cli.cpp
View file

@ -114,7 +114,7 @@ static int AppInitRPC(int argc, char* argv[])
}
// 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;

2
src/bitcoin-tx.cpp
View file

@ -44,7 +44,7 @@ static int AppInitRawTx(int argc, char* argv[])
// 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;

2
src/bitcoind.cpp
View file

@ -102,7 +102,7 @@ bool AppInit(int argc, char* argv[])
}
// 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;

14
src/chainparamsbase.cpp
View file

@ -49,17 +49,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;
}

6
src/chainparamsbase.h
View file

@ -54,10 +54,4 @@ const CBaseChainParams& BaseParams();
/** 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

2
src/qt/bitcoin.cpp
View file

@ -627,7 +627,7 @@ int main(int argc, char *argv[])
// Check for -testnet or -regtest parameter (Params() calls are only valid after this clause)
try {
node->selectParams(ChainNameFromCommandLine());
node->selectParams(gArgs.GetChainName());
} catch(std::exception &e) {
QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), QObject::tr("Error: %1").arg(e.what()));
return EXIT_FAILURE;

6
src/qt/guiutil.cpp
View file

@ -599,7 +599,7 @@ TableViewLastColumnResizingFixer::TableViewLastColumnResizingFixer(QTableView* t
#ifdef WIN32
fs::path static StartupShortcutPath()
{
std::string chain = ChainNameFromCommandLine();
std::string chain = gArgs.GetChainName();
if (chain == CBaseChainParams::MAIN)
return GetSpecialFolderPath(CSIDL_STARTUP) / "Bitcoin.lnk";
if (chain == CBaseChainParams::TESTNET) // Remove this special case when CBaseChainParams::TESTNET = "testnet4"
@ -697,7 +697,7 @@ fs::path static GetAutostartDir()
fs::path static 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);
@ -739,7 +739,7 @@ bool SetStartOnSystemStartup(bool fAutoStart)
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";

195
src/test/util_tests.cpp
View file

@ -190,6 +190,11 @@ struct TestArgsManager : public ArgsManager
std::map<std::string, std::string>& GetMapArgs() { return mapArgs; }
const std::map<std::string, std::vector<std::string> >& GetMapMultiArgs() { return mapMultiArgs; }
const std::unordered_set<std::string>& 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)
@ -253,16 +258,154 @@ 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.GetBoolArg("-bar", false) == true);
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.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"
&& test_args.GetArg("-h", "xxx") == "1" // 1st value takes precedence
&& test_args.GetArg("-i", "xxx") == "0" // 1st value takes precedence
&& 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"));
BOOST_CHECK(test_args.IsArgNegated("-ggg")); // IsArgNegated==true when noggg=0
BOOST_CHECK(test_args.IsArgNegated("-h")); // last setting takes precedence
BOOST_CHECK(!test_args.IsArgNegated("-i")); // last setting takes precedence
BOOST_CHECK(!test_args.IsArgNegated("-zzz"));
}
BOOST_AUTO_TEST_CASE(util_GetArg)
@ -290,6 +433,54 @@ BOOST_AUTO_TEST_CASE(util_GetArg)
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(0), "0.00");

56
src/util.cpp
View file

@ -736,28 +736,34 @@ fs::path GetConfigFile(const std::string& confPath)
return AbsPathForConfigVal(fs::path(confPath), false);
}
void ArgsManager::ReadConfigStream(std::istream& stream)
{
LOCK(cs_args);
std::set<std::string> 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);
}
}
void ArgsManager::ReadConfigFile(const std::string& confPath)
{
fs::ifstream streamConfig(GetConfigFile(confPath));
if (!streamConfig.good())
return; // No bitcoin.conf file is OK
fs::ifstream stream(GetConfigFile(confPath));
{
LOCK(cs_args);
std::set<std::string> 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);
}
// ok to not have a config file
if (stream.good()) {
ReadConfigStream(stream);
}
// If datadir is changed in .conf file:
ClearDatadirCache();
if (!fs::is_directory(GetDataDir(false))) {
@ -765,6 +771,20 @@ void ArgsManager::ReadConfigFile(const std::string& confPath)
}
}
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()
{

8
src/util.h
View file

@ -228,6 +228,8 @@ protected:
std::map<std::string, std::vector<std::string>> mapMultiArgs;
std::unordered_set<std::string> m_negated_args;
void ReadConfigStream(std::istream& stream);
public:
void ParseParameters(int argc, const char*const argv[]);
void ReadConfigFile(const std::string& confPath);
@ -306,6 +308,12 @@ public:
// been set. Also called directly in testing.
void ForceSetArg(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;
private:
// Munge -nofoo into -foo=0 and track the value as negated.