diff options
author | Wladimir J. van der Laan <laanwj@gmail.com> | 2018-04-16 20:26:42 +0200 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@gmail.com> | 2018-04-16 20:52:38 +0200 |
commit | 4366f61cc9d417b4c24b7a7605c869fa60d8ddb2 (patch) | |
tree | 0db40c35e8606c56816ae7fd27557ab126afcc76 | |
parent | 6a278e06400934df0fc6ed81b0d7c645efc50bdf (diff) | |
parent | c25321ff96737bdba80d626d2425ef02c7a4c181 (diff) |
Merge #11862: Network specific conf sections
c25321f Add config changes to release notes (Anthony Towns)
5e3cbe0 [tests] Unit tests for -testnet/-regtest in [test]/[regtest] sections (Anthony Towns)
005ad26 ArgsManager: special handling for -regtest and -testnet (Anthony Towns)
608415d [tests] Unit tests for network-specific config entries (Anthony Towns)
68797e2 ArgsManager: Warn when ignoring network-specific config setting (Anthony Towns)
d1fc4d9 ArgsManager: limit some options to only apply on mainnet when in default section (Anthony Towns)
8a9817d [tests] Use regtest section in functional tests configs (Anthony Towns)
30f9407 [tests] Unit tests for config file sections (Anthony Towns)
95eb66d ArgsManager: support config file sections (Anthony Towns)
4d34fcc ArgsManager: drop m_negated_args (Anthony Towns)
3673ca3 ArgsManager: keep command line and config file arguments separate (Anthony Towns)
Pull request description:
The weekly meeting on [2017-12-07](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-12-07-19.00.log.html) discussed allowing options to bitcoin to have some sensitivity to what network is in use. @theuni suggested having sections in the config file:
<cfields> an alternative to that would be sections in a config file. and on the
cmdline they'd look like namespaces. so, [testnet] port=5. or -testnet::port=5.
This approach is (more or less) supported by `boost::program_options::detail::config_file_iterator` -- when it sees a `[testnet]` section with `port=5`, it will treat that the same as "testnet.port=5". So `[testnet] port=5` (or `testnet.port=5` without the section header) in bitcoin.conf and `-testnet.port=5` on the command line.
The other aspect to this question is possibly limiting some options so that there is no possibility of accidental cross-contamination across networks. For example, if you're using a particular wallet.dat on mainnet, you may not want to accidentally use the same wallet on testnet and risk reusing keys.
I've set this up so that the `-addnode` and `-wallet` options are `NETWORK_ONLY`, so that if you have a bitcoin.conf:
wallet=/secret/wallet.dat
upnp=1
and you run `bitcoind -testnet` or `bitcoind -regtest`, then the `wallet=` setting will be ignored, and should behave as if your bitcoin.conf had specified:
upnp=1
[main]
wallet=/secret/wallet.dat
For any `NETWORK_ONLY` options, if you're using `-testnet` or `-regtest`, you'll have to add the prefix to any command line options. This was necessary for `multiwallet.py` for instance.
I've left the "default" options as taking precedence over network specific ones, which might be backwards. So if you have:
maxmempool=200
[regtest]
maxmempool=100
your maxmempool will still be 200 on regtest. The advantage of doing it this way is that if you have `[regtest] maxmempool=100` in bitcoin.conf, and then say `bitcoind -regtest -maxmempool=200`, the same result is probably in line with what you expect...
The other thing to note is that I'm using the chain names from `chainparamsbase.cpp` / `ChainNameFromCommandLine`, so the sections are `[main]`, `[test]` and `[regtest]`; not `[mainnet]` or `[testnet]` as might be expected.
Thoughts? Ping @MeshCollider @laanwj @jonasschnelli @morcos
Tree-SHA512: f00b5eb75f006189987e5c15e154a42b66ee251777768c1e185d764279070fcb7c41947d8794092b912a03d985843c82e5189871416995436a6260520fb7a4db
-rw-r--r-- | doc/release-notes-pr12823.md | 20 | ||||
-rw-r--r-- | src/chainparamsbase.cpp | 1 | ||||
-rw-r--r-- | src/init.cpp | 5 | ||||
-rw-r--r-- | src/test/util_tests.cpp | 252 | ||||
-rw-r--r-- | src/util.cpp | 279 | ||||
-rw-r--r-- | src/util.h | 30 | ||||
-rwxr-xr-x | test/functional/feature_config_args.py | 7 | ||||
-rw-r--r-- | test/functional/test_framework/util.py | 1 |
8 files changed, 480 insertions, 115 deletions
diff --git a/doc/release-notes-pr12823.md b/doc/release-notes-pr12823.md new file mode 100644 index 0000000000..b493908716 --- /dev/null +++ b/doc/release-notes-pr12823.md @@ -0,0 +1,20 @@ +Configuration sections for testnet and regtest +---------------------------------------------- + +It is now possible for a single configuration file to set different +options for different networks. This is done by using sections or by +prefixing the option with the network, such as: + + main.uacomment=bitcoin + test.uacomment=bitcoin-testnet + regtest.uacomment=regtest + [main] + mempoolsize=300 + [test] + mempoolsize=100 + [regtest] + mempoolsize=20 + +The `addnode=`, `connect=`, `port=`, `bind=`, `rpcport=`, `rpcbind=` +and `wallet=` options will only apply to mainnet when specified in the +configuration file, unless a network is specified. diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index e840a2ed30..3ef9c2cfe5 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -47,4 +47,5 @@ std::unique_ptr<CBaseChainParams> CreateBaseChainParams(const std::string& chain void SelectBaseParams(const std::string& chain) { globalChainBaseParams = CreateBaseChainParams(chain); + gArgs.SelectConfigNetwork(chain); } diff --git a/src/init.cpp b/src/init.cpp index 486c84f5a3..abff5ec79d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -803,6 +803,11 @@ void InitParameterInteraction() if (gArgs.SoftSetBoolArg("-whitelistrelay", true)) LogPrintf("%s: parameter interaction: -whitelistforcerelay=1 -> setting -whitelistrelay=1\n", __func__); } + + // Warn if network-specific options (-addnode, -connect, etc) are + // specified in default section of config file, but not overridden + // on the command line or in this network's section of the config file. + gArgs.WarnForSectionOnlyArgs(); } static std::string ResolveErrMsg(const char * const optname, const std::string& strBind) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index e8c7b80c3a..344113b60c 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -176,13 +176,22 @@ BOOST_AUTO_TEST_CASE(util_FormatISO8601Time) 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; } + TestArgsManager() { m_network_only_args.clear(); } + std::map<std::string, std::vector<std::string> >& GetOverrideArgs() { return m_override_args; } + std::map<std::string, std::vector<std::string> >& GetConfigArgs() { return m_config_args; } void ReadConfigString(const std::string str_config) { - std::istringstream stream(str_config); - ReadConfigStream(stream); + std::istringstream streamConfig(str_config); + { + LOCK(cs_args); + m_config_args.clear(); + } + ReadConfigStream(streamConfig); + } + void SetNetworkOnlyArg(const std::string arg) + { + LOCK(cs_args); + m_network_only_args.insert(arg); } }; @@ -192,22 +201,26 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters) const char *argv_test[] = {"-ignored", "-a", "-b", "-ccc=argument", "-ccc=multiple", "f", "-d=e"}; testArgs.ParseParameters(0, (char**)argv_test); - BOOST_CHECK(testArgs.GetMapArgs().empty() && testArgs.GetMapMultiArgs().empty()); + BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty()); testArgs.ParseParameters(1, (char**)argv_test); - BOOST_CHECK(testArgs.GetMapArgs().empty() && testArgs.GetMapMultiArgs().empty()); + BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty()); testArgs.ParseParameters(7, (char**)argv_test); // expectation: -ignored is ignored (program name argument), // -a, -b and -ccc end up in map, -d ignored because it is after // a non-option argument (non-GNU option parsing) - BOOST_CHECK(testArgs.GetMapArgs().size() == 3 && testArgs.GetMapMultiArgs().size() == 3); + BOOST_CHECK(testArgs.GetOverrideArgs().size() == 3 && testArgs.GetConfigArgs().empty()); BOOST_CHECK(testArgs.IsArgSet("-a") && testArgs.IsArgSet("-b") && testArgs.IsArgSet("-ccc") && !testArgs.IsArgSet("f") && !testArgs.IsArgSet("-d")); - BOOST_CHECK(testArgs.GetMapMultiArgs().count("-a") && testArgs.GetMapMultiArgs().count("-b") && testArgs.GetMapMultiArgs().count("-ccc") - && !testArgs.GetMapMultiArgs().count("f") && !testArgs.GetMapMultiArgs().count("-d")); - - BOOST_CHECK(testArgs.GetMapArgs()["-a"] == "" && testArgs.GetMapArgs()["-ccc"] == "multiple"); + BOOST_CHECK(testArgs.GetOverrideArgs().count("-a") && testArgs.GetOverrideArgs().count("-b") && testArgs.GetOverrideArgs().count("-ccc") + && !testArgs.GetOverrideArgs().count("f") && !testArgs.GetOverrideArgs().count("-d")); + + BOOST_CHECK(testArgs.GetOverrideArgs()["-a"].size() == 1); + BOOST_CHECK(testArgs.GetOverrideArgs()["-a"].front() == ""); + BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].size() == 2); + BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].front() == "argument"); + BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].back() == "multiple"); BOOST_CHECK(testArgs.GetArgs("-ccc").size() == 2); } @@ -223,15 +236,14 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg) BOOST_CHECK(testArgs.IsArgSet({'-', opt}) || !opt); // Nothing else should be in the map - BOOST_CHECK(testArgs.GetMapArgs().size() == 6 && - testArgs.GetMapMultiArgs().size() == 6); + BOOST_CHECK(testArgs.GetOverrideArgs().size() == 6 && + testArgs.GetConfigArgs().empty()); // The -no prefix should get stripped on the way in. BOOST_CHECK(!testArgs.IsArgSet("-nob")); // The -b option is flagged as negated, and nothing else is BOOST_CHECK(testArgs.IsArgNegated("-b")); - BOOST_CHECK(testArgs.GetNegatedArgs().size() == 1); BOOST_CHECK(!testArgs.IsArgNegated("-a")); // Check expected values. @@ -256,8 +268,8 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) BOOST_CHECK(!testArgs.IsArgNegated("-foo")); BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == ""); - // A double negative is a positive. - BOOST_CHECK(testArgs.IsArgNegated("-bar")); + // A double negative is a positive, and not marked as negated. + BOOST_CHECK(!testArgs.IsArgNegated("-bar")); BOOST_CHECK(testArgs.GetArg("-bar", "xxx") == "1"); // Config test @@ -266,12 +278,12 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) testArgs.ReadConfigString(conf_test); // This was passed twice, second one overrides the negative setting, - // but not the value. + // and the value. BOOST_CHECK(!testArgs.IsArgNegated("-foo")); - BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == "0"); + BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == "1"); - // A double negative is a positive. - BOOST_CHECK(testArgs.IsArgNegated("-bar")); + // A double negative is a positive, and does not count as negated. + BOOST_CHECK(!testArgs.IsArgNegated("-bar")); BOOST_CHECK(testArgs.GetArg("-bar", "xxx") == "1"); // Combined test @@ -281,18 +293,15 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) testArgs.ReadConfigString(combo_test_conf); // Command line overrides, but doesn't erase old setting - BOOST_CHECK(!testArgs.IsArgNegated("-foo")); + 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"); + BOOST_CHECK(testArgs.GetArgs("-foo").size() == 0); // Command line overrides, but doesn't erase old setting - BOOST_CHECK(testArgs.IsArgNegated("-bar")); + 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_CHECK(testArgs.GetArgs("-bar").size() == 1 + && testArgs.GetArgs("-bar").front() == ""); } BOOST_AUTO_TEST_CASE(util_ReadConfigStream) @@ -308,24 +317,39 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) "h=1\n" "noh=1\n" "noi=1\n" - "i=1\n"; + "i=1\n" + "sec1.ccc=extend1\n" + "\n" + "[sec1]\n" + "ccc=extend2\n" + "d=eee\n" + "h=1\n" + "[sec2]\n" + "ccc=extend3\n" + "iii=2\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") + // so do sec1.ccc, sec1.d, sec1.h, sec2.ccc, sec2.iii + + BOOST_CHECK(test_args.GetOverrideArgs().empty()); + BOOST_CHECK(test_args.GetConfigArgs().size() == 13); + + BOOST_CHECK(test_args.GetConfigArgs().count("-a") + && test_args.GetConfigArgs().count("-b") + && test_args.GetConfigArgs().count("-ccc") + && test_args.GetConfigArgs().count("-d") + && test_args.GetConfigArgs().count("-fff") + && test_args.GetConfigArgs().count("-ggg") + && test_args.GetConfigArgs().count("-h") + && test_args.GetConfigArgs().count("-i") + ); + BOOST_CHECK(test_args.GetConfigArgs().count("-sec1.ccc") + && test_args.GetConfigArgs().count("-sec1.h") + && test_args.GetConfigArgs().count("-sec2.ccc") + && test_args.GetConfigArgs().count("-sec2.iii") ); BOOST_CHECK(test_args.IsArgSet("-a") @@ -337,6 +361,7 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) && test_args.IsArgSet("-h") && test_args.IsArgSet("-i") && !test_args.IsArgSet("-zzz") + && !test_args.IsArgSet("-iii") ); BOOST_CHECK(test_args.GetArg("-a", "xxx") == "" @@ -345,9 +370,10 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) && 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("-h", "xxx") == "0" + && test_args.GetArg("-i", "xxx") == "1" && test_args.GetArg("-zzz", "xxx") == "xxx" + && test_args.GetArg("-iii", "xxx") == "xxx" ); for (bool def : {false, true}) { @@ -357,9 +383,10 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) && !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("-h", def) + && test_args.GetBoolArg("-i", def) && test_args.GetBoolArg("-zzz", def) == def + && test_args.GetBoolArg("-iii", def) == def ); } @@ -370,19 +397,15 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) 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("-fff").size() == 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("-h").size() == 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("-i").size() == 1 + && test_args.GetArgs("-i").front() == "1"); BOOST_CHECK(test_args.GetArgs("-noi").size() == 0); BOOST_CHECK(test_args.GetArgs("-zzz").size() == 0); @@ -391,25 +414,98 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) 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("-ggg")); 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")); + + // Test sections work + test_args.SelectConfigNetwork("sec1"); + + // same as original + BOOST_CHECK(test_args.GetArg("-a", "xxx") == "" + && test_args.GetArg("-b", "xxx") == "1" + && test_args.GetArg("-fff", "xxx") == "0" + && test_args.GetArg("-ggg", "xxx") == "1" + && test_args.GetArg("-zzz", "xxx") == "xxx" + && test_args.GetArg("-iii", "xxx") == "xxx" + ); + // d is overridden + BOOST_CHECK(test_args.GetArg("-d", "xxx") == "eee"); + // section-specific setting + BOOST_CHECK(test_args.GetArg("-h", "xxx") == "1"); + // section takes priority for multiple values + BOOST_CHECK(test_args.GetArg("-ccc", "xxx") == "extend1"); + // check multiple values works + const std::vector<std::string> sec1_ccc_expected = {"extend1","extend2","argument","multiple"}; + const auto& sec1_ccc_res = test_args.GetArgs("-ccc"); + BOOST_CHECK_EQUAL_COLLECTIONS(sec1_ccc_res.begin(), sec1_ccc_res.end(), sec1_ccc_expected.begin(), sec1_ccc_expected.end()); + + test_args.SelectConfigNetwork("sec2"); + + // same as original + BOOST_CHECK(test_args.GetArg("-a", "xxx") == "" + && test_args.GetArg("-b", "xxx") == "1" + && test_args.GetArg("-d", "xxx") == "e" + && test_args.GetArg("-fff", "xxx") == "0" + && test_args.GetArg("-ggg", "xxx") == "1" + && test_args.GetArg("-zzz", "xxx") == "xxx" + && test_args.GetArg("-h", "xxx") == "0" + ); + // section-specific setting + BOOST_CHECK(test_args.GetArg("-iii", "xxx") == "2"); + // section takes priority for multiple values + BOOST_CHECK(test_args.GetArg("-ccc", "xxx") == "extend3"); + // check multiple values works + const std::vector<std::string> sec2_ccc_expected = {"extend3","argument","multiple"}; + const auto& sec2_ccc_res = test_args.GetArgs("-ccc"); + BOOST_CHECK_EQUAL_COLLECTIONS(sec2_ccc_res.begin(), sec2_ccc_res.end(), sec2_ccc_expected.begin(), sec2_ccc_expected.end()); + + // Test section only options + + test_args.SetNetworkOnlyArg("-d"); + test_args.SetNetworkOnlyArg("-ccc"); + test_args.SetNetworkOnlyArg("-h"); + + test_args.SelectConfigNetwork(CBaseChainParams::MAIN); + BOOST_CHECK(test_args.GetArg("-d", "xxx") == "e"); + BOOST_CHECK(test_args.GetArgs("-ccc").size() == 2); + BOOST_CHECK(test_args.GetArg("-h", "xxx") == "0"); + + test_args.SelectConfigNetwork("sec1"); + BOOST_CHECK(test_args.GetArg("-d", "xxx") == "eee"); + BOOST_CHECK(test_args.GetArgs("-d").size() == 1); + BOOST_CHECK(test_args.GetArgs("-ccc").size() == 2); + BOOST_CHECK(test_args.GetArg("-h", "xxx") == "1"); + + test_args.SelectConfigNetwork("sec2"); + BOOST_CHECK(test_args.GetArg("-d", "xxx") == "xxx"); + BOOST_CHECK(test_args.GetArgs("-d").size() == 0); + BOOST_CHECK(test_args.GetArgs("-ccc").size() == 1); + BOOST_CHECK(test_args.GetArg("-h", "xxx") == "0"); } BOOST_AUTO_TEST_CASE(util_GetArg) { TestArgsManager testArgs; - testArgs.GetMapArgs().clear(); - testArgs.GetMapArgs()["strtest1"] = "string..."; + testArgs.GetOverrideArgs().clear(); + testArgs.GetOverrideArgs()["strtest1"] = {"string..."}; // strtest2 undefined on purpose - testArgs.GetMapArgs()["inttest1"] = "12345"; - testArgs.GetMapArgs()["inttest2"] = "81985529216486895"; + testArgs.GetOverrideArgs()["inttest1"] = {"12345"}; + testArgs.GetOverrideArgs()["inttest2"] = {"81985529216486895"}; // inttest3 undefined on purpose - testArgs.GetMapArgs()["booltest1"] = ""; + testArgs.GetOverrideArgs()["booltest1"] = {""}; // booltest2 undefined on purpose - testArgs.GetMapArgs()["booltest3"] = "0"; - testArgs.GetMapArgs()["booltest4"] = "1"; + testArgs.GetOverrideArgs()["booltest3"] = {"0"}; + testArgs.GetOverrideArgs()["booltest4"] = {"1"}; + + // priorities + testArgs.GetOverrideArgs()["pritest1"] = {"a", "b"}; + testArgs.GetConfigArgs()["pritest2"] = {"a", "b"}; + testArgs.GetOverrideArgs()["pritest3"] = {"a"}; + testArgs.GetConfigArgs()["pritest3"] = {"b"}; + testArgs.GetOverrideArgs()["pritest4"] = {"a","b"}; + testArgs.GetConfigArgs()["pritest4"] = {"c","d"}; BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "string..."); BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default"); @@ -420,6 +516,11 @@ BOOST_AUTO_TEST_CASE(util_GetArg) BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest2", false), false); BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest3", false), false); BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest4", false), true); + + BOOST_CHECK_EQUAL(testArgs.GetArg("pritest1", "default"), "b"); + BOOST_CHECK_EQUAL(testArgs.GetArg("pritest2", "default"), "a"); + BOOST_CHECK_EQUAL(testArgs.GetArg("pritest3", "default"), "a"); + BOOST_CHECK_EQUAL(testArgs.GetArg("pritest4", "default"), "b"); } BOOST_AUTO_TEST_CASE(util_GetChainName) @@ -432,7 +533,8 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) const char* argv_both[] = {"cmd", "-testnet", "-regtest"}; // equivalent to "-testnet" - const char* testnetconf = "testnet=1\nregtest=0\n"; + // regtest in testnet section is ignored + const char* testnetconf = "testnet=1\nregtest=0\n[test]\nregtest=1"; test_args.ParseParameters(0, (char**)argv_testnet); BOOST_CHECK_EQUAL(test_args.GetChainName(), "main"); @@ -468,6 +570,30 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) test_args.ParseParameters(3, (char**)argv_both); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); + + // check setting the network to test (and thus making + // [test] regtest=1 potentially relevent) doesn't break things + test_args.SelectConfigNetwork("test"); + + 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(2, (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) diff --git a/src/util.cpp b/src/util.cpp index f55c9c8c34..1fb40ae7a1 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -455,39 +455,204 @@ static bool InterpretBool(const std::string& strValue) return (atoi(strValue) != 0); } +/** Internal helper functions for ArgsManager */ +class ArgsManagerHelper { +public: + typedef std::map<std::string, std::vector<std::string>> MapArgs; + + /** Determine whether to use config settings in the default section, + * See also comments around ArgsManager::ArgsManager() below. */ + static inline bool UseDefaultSection(const ArgsManager& am, const std::string& arg) + { + return (am.m_network == CBaseChainParams::MAIN || am.m_network_only_args.count(arg) == 0); + } + + /** Convert regular argument into the network-specific setting */ + static inline std::string NetworkArg(const ArgsManager& am, const std::string& arg) + { + assert(arg.length() > 1 && arg[0] == '-'); + return "-" + am.m_network + "." + arg.substr(1); + } + + /** Find arguments in a map and add them to a vector */ + static inline void AddArgs(std::vector<std::string>& res, const MapArgs& map_args, const std::string& arg) + { + auto it = map_args.find(arg); + if (it != map_args.end()) { + res.insert(res.end(), it->second.begin(), it->second.end()); + } + } + + /** Return true/false if an argument is set in a map, and also + * return the first (or last) of the possibly multiple values it has + */ + static inline std::pair<bool,std::string> GetArgHelper(const MapArgs& map_args, const std::string& arg, bool getLast = false) + { + auto it = map_args.find(arg); + + if (it == map_args.end() || it->second.empty()) { + return std::make_pair(false, std::string()); + } + + if (getLast) { + return std::make_pair(true, it->second.back()); + } else { + return std::make_pair(true, it->second.front()); + } + } + + /* Get the string value of an argument, returning a pair of a boolean + * indicating the argument was found, and the value for the argument + * if it was found (or the empty string if not found). + */ + static inline std::pair<bool,std::string> GetArg(const ArgsManager &am, const std::string& arg) + { + LOCK(am.cs_args); + std::pair<bool,std::string> found_result(false, std::string()); + + // We pass "true" to GetArgHelper in order to return the last + // argument value seen from the command line (so "bitcoind -foo=bar + // -foo=baz" gives GetArg(am,"foo")=={true,"baz"} + found_result = GetArgHelper(am.m_override_args, arg, true); + if (found_result.first) { + return found_result; + } + + // But in contrast we return the first argument seen in a config file, + // so "foo=bar \n foo=baz" in the config file gives + // GetArg(am,"foo")={true,"bar"} + if (!am.m_network.empty()) { + found_result = GetArgHelper(am.m_config_args, NetworkArg(am, arg)); + if (found_result.first) { + return found_result; + } + } + + if (UseDefaultSection(am, arg)) { + found_result = GetArgHelper(am.m_config_args, arg); + if (found_result.first) { + return found_result; + } + } + + return found_result; + } + + /* Special test for -testnet and -regtest args, because we + * don't want to be confused by craziness like "[regtest] testnet=1" + */ + static inline bool GetNetBoolArg(const ArgsManager &am, const std::string& net_arg) + { + std::pair<bool,std::string> found_result(false,std::string()); + found_result = GetArgHelper(am.m_override_args, net_arg, true); + if (!found_result.first) { + found_result = GetArgHelper(am.m_config_args, net_arg, true); + if (!found_result.first) { + return false; // not set + } + } + return InterpretBool(found_result.second); // is set, so evaluate + } +}; + /** * Interpret -nofoo as if the user supplied -foo=0. * - * This method also tracks when the -no form was supplied, and treats "-foo" as - * a negated option when this happens. This can be later checked using the + * This method also tracks when the -no form was supplied, and if so, + * checks whether there was a double-negative (-nofoo=0 -> -foo=1). + * + * If there was not a double negative, it removes the "no" from the key, + * and returns true, indicating the caller should clear the args vector + * to indicate a negated option. + * + * If there was a double negative, it removes "no" from the key, sets the + * value to "1" and returns false. + * + * If there was no "no", it leaves key and value untouched and returns + * false. + * + * Where an option was negated can be later checked using the * IsArgNegated() method. One use case for this is to have a way to disable * options that are not normally boolean (e.g. using -nodebuglogfile to request * that debug log output is not sent to any file at all). */ -void ArgsManager::InterpretNegatedOption(std::string& key, std::string& val) +static bool InterpretNegatedOption(std::string& key, std::string& val) { - if (key.substr(0, 3) == "-no") { + assert(key[0] == '-'); + + size_t option_index = key.find('.'); + if (option_index == std::string::npos) { + option_index = 1; + } else { + ++option_index; + } + if (key.substr(option_index, 2) == "no") { bool bool_val = InterpretBool(val); + key.erase(option_index, 2); if (!bool_val ) { // Double negatives like -nofoo=0 are supported (but discouraged) LogPrintf("Warning: parsed potentially confusing double-negative %s=%s\n", key, val); + val = "1"; + } else { + return true; } - key.erase(1, 2); - m_negated_args.insert(key); - val = bool_val ? "0" : "1"; - } else { - // In an invocation like "bitcoind -nofoo -foo" we want to unmark -foo - // as negated when we see the second option. - m_negated_args.erase(key); } + return false; +} + +ArgsManager::ArgsManager() : + /* These options would cause cross-contamination if values for + * mainnet were used while running on regtest/testnet (or vice-versa). + * Setting them as section_only_args ensures that sharing a config file + * between mainnet and regtest/testnet won't cause problems due to these + * parameters by accident. */ + m_network_only_args{ + "-addnode", "-connect", + "-port", "-bind", + "-rpcport", "-rpcbind", + "-wallet", + } +{ + // nothing to do +} + +void ArgsManager::WarnForSectionOnlyArgs() +{ + // if there's no section selected, don't worry + if (m_network.empty()) return; + + // if it's okay to use the default section for this network, don't worry + if (m_network == CBaseChainParams::MAIN) return; + + for (const auto& arg : m_network_only_args) { + std::pair<bool, std::string> found_result; + + // if this option is overridden it's fine + found_result = ArgsManagerHelper::GetArgHelper(m_override_args, arg); + if (found_result.first) continue; + + // if there's a network-specific value for this option, it's fine + found_result = ArgsManagerHelper::GetArgHelper(m_config_args, ArgsManagerHelper::NetworkArg(*this, arg)); + if (found_result.first) continue; + + // if there isn't a default value for this option, it's fine + found_result = ArgsManagerHelper::GetArgHelper(m_config_args, arg); + if (!found_result.first) continue; + + // otherwise, issue a warning + LogPrintf("Warning: Config setting for %s only applied on %s network when in [%s] section.\n", arg, m_network, m_network); + } +} + +void ArgsManager::SelectConfigNetwork(const std::string& network) +{ + m_network = network; } void ArgsManager::ParseParameters(int argc, const char* const argv[]) { LOCK(cs_args); - mapArgs.clear(); - mapMultiArgs.clear(); - m_negated_args.clear(); + m_override_args.clear(); for (int i = 1; i < argc; i++) { std::string key(argv[i]); @@ -510,55 +675,79 @@ void ArgsManager::ParseParameters(int argc, const char* const argv[]) if (key.length() > 1 && key[1] == '-') key.erase(0, 1); - // Transform -nofoo to -foo=0 - InterpretNegatedOption(key, val); - - mapArgs[key] = val; - mapMultiArgs[key].push_back(val); + // Check for -nofoo + if (InterpretNegatedOption(key, val)) { + m_override_args[key].clear(); + } else { + m_override_args[key].push_back(val); + } } } std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const { + std::vector<std::string> result = {}; + if (IsArgNegated(strArg)) return result; // special case + LOCK(cs_args); - auto it = mapMultiArgs.find(strArg); - if (it != mapMultiArgs.end()) return it->second; - return {}; + + ArgsManagerHelper::AddArgs(result, m_override_args, strArg); + if (!m_network.empty()) { + ArgsManagerHelper::AddArgs(result, m_config_args, ArgsManagerHelper::NetworkArg(*this, strArg)); + } + + if (ArgsManagerHelper::UseDefaultSection(*this, strArg)) { + ArgsManagerHelper::AddArgs(result, m_config_args, strArg); + } + + return result; } bool ArgsManager::IsArgSet(const std::string& strArg) const { - LOCK(cs_args); - return mapArgs.count(strArg); + if (IsArgNegated(strArg)) return true; // special case + return ArgsManagerHelper::GetArg(*this, strArg).first; } bool ArgsManager::IsArgNegated(const std::string& strArg) const { LOCK(cs_args); - return m_negated_args.find(strArg) != m_negated_args.end(); + + const auto& ov = m_override_args.find(strArg); + if (ov != m_override_args.end()) return ov->second.empty(); + + if (!m_network.empty()) { + const auto& cfs = m_config_args.find(ArgsManagerHelper::NetworkArg(*this, strArg)); + if (cfs != m_config_args.end()) return cfs->second.empty(); + } + + const auto& cf = m_config_args.find(strArg); + if (cf != m_config_args.end()) return cf->second.empty(); + + return false; } std::string ArgsManager::GetArg(const std::string& strArg, const std::string& strDefault) const { - LOCK(cs_args); - auto it = mapArgs.find(strArg); - if (it != mapArgs.end()) return it->second; + if (IsArgNegated(strArg)) return "0"; + std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg); + if (found_res.first) return found_res.second; return strDefault; } int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const { - LOCK(cs_args); - auto it = mapArgs.find(strArg); - if (it != mapArgs.end()) return atoi64(it->second); + if (IsArgNegated(strArg)) return 0; + std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg); + if (found_res.first) return atoi64(found_res.second); return nDefault; } bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const { - LOCK(cs_args); - auto it = mapArgs.find(strArg); - if (it != mapArgs.end()) return InterpretBool(it->second); + if (IsArgNegated(strArg)) return false; + std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg); + if (found_res.first) return InterpretBool(found_res.second); return fDefault; } @@ -581,8 +770,7 @@ bool ArgsManager::SoftSetBoolArg(const std::string& strArg, bool fValue) void ArgsManager::ForceSetArg(const std::string& strArg, const std::string& strValue) { LOCK(cs_args); - mapArgs[strArg] = strValue; - mapMultiArgs[strArg] = {strValue}; + m_override_args[strArg] = {strValue}; } bool HelpRequested(const ArgsManager& args) @@ -745,18 +933,23 @@ void ArgsManager::ReadConfigStream(std::istream& stream) 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); + if (InterpretNegatedOption(strKey, strValue)) { + m_config_args[strKey].clear(); + } else { + m_config_args[strKey].push_back(strValue); + } } } void ArgsManager::ReadConfigFile(const std::string& confPath) { + { + LOCK(cs_args); + m_config_args.clear(); + } + fs::ifstream stream(GetConfigFile(confPath)); // ok to not have a config file @@ -773,8 +966,8 @@ void ArgsManager::ReadConfigFile(const std::string& confPath) std::string ArgsManager::GetChainName() const { - bool fRegTest = GetBoolArg("-regtest", false); - bool fTestNet = GetBoolArg("-testnet", false); + bool fRegTest = ArgsManagerHelper::GetNetBoolArg(*this, "-regtest"); + bool fTestNet = ArgsManagerHelper::GetNetBoolArg(*this, "-testnet"); if (fTestNet && fRegTest) throw std::runtime_error("Invalid combination of -regtest and -testnet."); diff --git a/src/util.h b/src/util.h index 18d2076a05..ffdee99d27 100644 --- a/src/util.h +++ b/src/util.h @@ -24,6 +24,7 @@ #include <exception> #include <map> #include <memory> +#include <set> #include <stdint.h> #include <string> #include <unordered_set> @@ -225,18 +226,36 @@ inline bool IsSwitchChar(char c) class ArgsManager { protected: + friend class ArgsManagerHelper; + mutable CCriticalSection cs_args; - std::map<std::string, std::string> mapArgs; - std::map<std::string, std::vector<std::string>> mapMultiArgs; - std::unordered_set<std::string> m_negated_args; + std::map<std::string, std::vector<std::string>> m_override_args; + std::map<std::string, std::vector<std::string>> m_config_args; + std::string m_network; + std::set<std::string> m_network_only_args; void ReadConfigStream(std::istream& stream); public: + ArgsManager(); + + /** + * Select the network in use + */ + void SelectConfigNetwork(const std::string& network); + void ParseParameters(int argc, const char*const argv[]); void ReadConfigFile(const std::string& confPath); /** + * Log warnings for options in m_section_only_args when + * they are specified in the default section but not overridden + * on the command line or in a network-specific section in the + * config file. + */ + void WarnForSectionOnlyArgs(); + + /** * Return a vector of strings of the given argument * * @param strArg Argument to get (e.g. "-foo") @@ -315,11 +334,6 @@ public: * @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. - void InterpretNegatedOption(std::string &key, std::string &val); }; extern ArgsManager gArgs; diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index a1d22191af..e9924451d1 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -29,8 +29,13 @@ class ConfArgsTest(BitcoinTestFramework): # Check that using non-existent datadir in conf file fails conf_file = os.path.join(default_data_dir, "bitcoin.conf") - with open(conf_file, 'a', encoding='utf8') as f: + + # datadir needs to be set before [regtest] section + conf_file_contents = open(conf_file, encoding='utf8').read() + with open(conf_file, 'w', encoding='utf8') as f: f.write("datadir=" + new_data_dir + "\n") + f.write(conf_file_contents) + self.nodes[0].assert_start_raises_init_error(['-conf=' + conf_file], 'Error reading configuration file: specified data directory "' + new_data_dir + '" does not exist.') # Create the directory and ensure the config file now works diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index f22322fbbc..4ec3175cd6 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -294,6 +294,7 @@ def initialize_datadir(dirname, n): os.makedirs(datadir) with open(os.path.join(datadir, "bitcoin.conf"), 'w', encoding='utf8') as f: f.write("regtest=1\n") + f.write("[regtest]\n") f.write("port=" + str(p2p_port(n)) + "\n") f.write("rpcport=" + str(rpc_port(n)) + "\n") f.write("server=1\n") |