refactor: Use name constants in chainparams initialization #17306

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.

ACK 37b8475dcf61383dffffebae374eab07c14aee80

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

ACK 37b8475

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

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.

Concept ACK

FWIW:

$ git grep -E '("main"|"test"|"regtest")' -- "*.cpp" "*.h" 
src/chainparams.cpp:        strNetworkID = "main";
src/chainparams.cpp:        strNetworkID = "test";
src/chainparams.cpp:        strNetworkID = "regtest";
src/chainparamsbase.cpp:const std::string CBaseChainParams::MAIN = "main";
src/chainparamsbase.cpp:const std::string CBaseChainParams::TESTNET = "test";
src/chainparamsbase.cpp:const std::string CBaseChainParams::REGTEST = "regtest";
src/chainparamsbase.cpp:        return MakeUnique<CBaseChainParams>("regtest", 18443);
src/qt/bitcoin.cpp:    util::ThreadSetInternalName("main");
src/qt/networkstyle.cpp:    {"main", QAPP_APP_NAME_DEFAULT, 0, 0},
src/qt/networkstyle.cpp:    {"test", QAPP_APP_NAME_TESTNET, 70, 30},
src/qt/networkstyle.cpp:    {"regtest", QAPP_APP_NAME_REGTEST, 160, 30}
src/qt/test/apptests.cpp:    QCOMPARE(value["chain"].get_str(), std::string("regtest"));
src/qt/test/rpcnestedtests.cpp:    { "test", "rpcNestedTest", &rpcNestedTest_rpc, {} },
src/qt/test/rpcnestedtests.cpp:    QVERIFY(result=="main");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "main");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "regtest");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
src/test/util_tests.cpp:    test_args.SelectConfigNetwork("test");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
src/test/util_tests.cpp:// - Testing "main" and "test" network values to make sure settings from network
src/test/util_tests.cpp:        parser.AddArg("-regtest", "regtest", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
src/test/util_tests.cpp:    BOOST_CHECK_EQUAL(FormatParagraph("test", 79, 0), "test");
src/wallet/db.cpp:                            "main",             // Logical db name
src/wallet/db.cpp:                            fMockDb ? strFilename.c_str() : "main",   // Logical db name
src/wallet/db.cpp:                                            "main",             // Logical db name
src/wallet/rpcwallet.cpp:    // Release the "main" shared pointer and prevent further notifications.
src/wallet/test/wallet_crypto_tests.cpp:    TestCrypter::TestPassphrase(ParseHex("0000deadbeef0000"), "test", 25000, \

ACK 37b8475dcf61383dffffebae374eab07c14aee80

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.

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.

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