refactor: Use name constants in chainparams initialization

jtimon commented at 7:32 pm on October 29, 2019: contributor

I thought this wouldn’t work for some reason, but it seems it does. Just a little bit more consistency. I’m still not able to use them in qt/networkstyle.cpp though, not sure why.

Chainparams: Use name constants in chainparams initialization 37b8475dcf

fanquake added the label Refactoring on Oct 29, 2019

MarcoFalke renamed this:
~~Use name constants in chainparams initialization~~
refactor: Use name constants in chainparams initialization
on Oct 29, 2019

MarcoFalke commented at 7:36 pm on October 29, 2019: member

ACK 37b8475dcf61383dffffebae374eab07c14aee80

in src/chainparams.cpp:257 in 37b8475dcf

253@@ -254,7 +254,7 @@ class CTestNetParams : public CChainParams {
254 class CRegTestParams : public CChainParams {
255 public:
256     explicit CRegTestParams(const ArgsManager& args) {
257-        strNetworkID = "regtest";
258+        strNetworkID =  CBaseChainParams::REGTEST;

fjahr commented at 7:46 pm on October 29, 2019:

There seems to be an unnecessary white space here.

MarcoFalke commented at 8:07 pm on October 29, 2019:

You may install clang-format and run the https://github.com/bitcoin/bitcoin/tree/master/contrib/devtools#clang-format-diffpy script to preempt whitespace nitpicking.

hebasto approved

hebasto commented at 7:48 pm on October 29, 2019: member

ACK 37b8475dcf61383dffffebae374eab07c14aee80, I have reviewed the code and it looks OK, I agree it can be merged.

fjahr approved

fjahr commented at 7:48 pm on October 29, 2019: member

ACK 37b8475

DrahtBot commented at 8:05 pm on October 29, 2019: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#17032 (Tests: Chainparams: Make regtest almost fully customizable by jtimon)
#16411 (Signet support by kallewoof)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

practicalswift commented at 11:34 pm on October 29, 2019: contributor

Concept ACK

FWIW:

 0$ git grep -E '("main"|"test"|"regtest")' -- "*.cpp" "*.h" 
 1src/chainparams.cpp:        strNetworkID = "main";
 2src/chainparams.cpp:        strNetworkID = "test";
 3src/chainparams.cpp:        strNetworkID = "regtest";
 4src/chainparamsbase.cpp:const std::string CBaseChainParams::MAIN = "main";
 5src/chainparamsbase.cpp:const std::string CBaseChainParams::TESTNET = "test";
 6src/chainparamsbase.cpp:const std::string CBaseChainParams::REGTEST = "regtest";
 7src/chainparamsbase.cpp:        return MakeUnique<CBaseChainParams>("regtest", 18443);
 8src/qt/bitcoin.cpp:    util::ThreadSetInternalName("main");
 9src/qt/networkstyle.cpp:    {"main", QAPP_APP_NAME_DEFAULT, 0, 0},
10src/qt/networkstyle.cpp:    {"test", QAPP_APP_NAME_TESTNET, 70, 30},
11src/qt/networkstyle.cpp:    {"regtest", QAPP_APP_NAME_REGTEST, 160, 30}
12src/qt/test/apptests.cpp:    QCOMPARE(value["chain"].get_str(), std::string("regtest"));
13src/qt/test/rpcnestedtests.cpp:    { "test", "rpcNestedTest", &rpcNestedTest_rpc, {} },
14src/qt/test/rpcnestedtests.cpp:    QVERIFY(result=="main");
15src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "main");
16src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
17src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "regtest");
18src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
19src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
20src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
21src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
22src/test/util_tests.cpp:    test_args.SelectConfigNetwork("test");
23src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
24src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
25src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
26src/test/util_tests.cpp:// - Testing "main" and "test" network values to make sure settings from network
27src/test/util_tests.cpp:        parser.AddArg("-regtest", "regtest", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
28src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(FormatParagraph("test", 79, 0), "test");
29src/wallet/db.cpp:                            "main",             // Logical db name
30src/wallet/db.cpp:                            fMockDb ? strFilename.c_str() : "main",   // Logical db name
31src/wallet/db.cpp:                                            "main",             // Logical db name
32src/wallet/rpcwallet.cpp:    // Release the "main" shared pointer and prevent further notifications.
33src/wallet/test/wallet_crypto_tests.cpp:    TestCrypter::TestPassphrase(ParseHex("0000deadbeef0000"), "test", 25000, \

MarcoFalke added the label Waiting for author on Oct 30, 2019

laanwj commented at 11:22 am on October 30, 2019: member

ACK 37b8475dcf61383dffffebae374eab07c14aee80

laanwj referenced this in commit f129170b85 on Oct 30, 2019

laanwj merged this on Oct 30, 2019

laanwj closed this on Oct 30, 2019

jtimon commented at 12:37 pm on October 30, 2019: contributor

This is one of the few times when I feel you guys merged this too fast. I would have happily removed the extra white space. I would also had happily added more cases like @practicalswift seems to be suggesting.

laanwj commented at 12:43 pm on October 30, 2019: member

I didn’t see that the whitespace nit wasn’t solved yet.

I would also had happily added more cases like @practicalswift seems to be suggesting.

I think it’s a bad idea to extend the scope of a PR after two people have ACKed it. It’s a trivially correct change and people agree with it, so it should be merged quickly.

laanwj removed the label Waiting for author on Oct 30, 2019

MarcoFalke commented at 12:46 pm on October 30, 2019: member

It is not a disaster to have this merged, mistakes can always happen. Let’s just fix up the nits in a follow-up.

jtimon deleted the branch on Oct 30, 2019

MarcoFalke locked this on Dec 16, 2021

refactor: Use name constants in chainparams initialization #17306

Conflicts