refactor: Replace string chain name constants with ChainTypes

This commit effectively moves the definition of these constants out of the chainparamsbase to their own file. Using the ChainType enums provides better type safety compared to passing around strings. The commit is part of an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager and other functionality that should not be part of the kernel library.
tethercoin · May 9, 2023 · ba8fc7d · ba8fc7d
1 parent 401453d
commit ba8fc7d
Show file tree

Hide file tree

Showing 78 changed files with 288 additions and 229 deletions.
diff --git a/src/bench/checkblock.cpp b/src/bench/checkblock.cpp
@@ -9,6 +9,7 @@
 #include <common/args.h>
 #include <consensus/validation.h>
 #include <streams.h>
+#include <util/chaintype.h>
 #include <validation.h>
 
 // These are the two major time-sinks which happen after we have fully received
@@ -36,7 +37,7 @@ static void DeserializeAndCheckBlockTest(benchmark::Bench& bench)
     stream.write({&a, 1}); // Prevent compaction
 
     ArgsManager bench_args;
-    const auto chainParams = CreateChainParams(bench_args, CBaseChainParams::MAIN);
+    const auto chainParams = CreateChainParams(bench_args, ChainType::MAIN);
 
     bench.unit("block").run([&] {
         CBlock block; // Note that CBlock caches its checked state, so we need to recreate it here

diff --git a/src/bench/load_external.cpp b/src/bench/load_external.cpp
@@ -6,6 +6,7 @@
 #include <bench/data.h>
 #include <chainparams.h>
 #include <test/util/setup_common.h>
+#include <util/chaintype.h>
 #include <validation.h>
 
 /**
@@ -22,7 +23,7 @@
  */
 static void LoadExternalBlockFile(benchmark::Bench& bench)
 {
-    const auto testing_setup{MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN)};
+    const auto testing_setup{MakeNoLogFileContext<const TestingSetup>(ChainType::MAIN)};
 
     // Create a single block as in the blocks files (magic bytes, block size,
     // block data) as a stream object.

diff --git a/src/bench/logging.cpp b/src/bench/logging.cpp
@@ -5,6 +5,7 @@
 #include <bench/bench.h>
 #include <logging.h>
 #include <test/util/setup_common.h>
+#include <util/chaintype.h>
 
 // All but 2 of the benchmarks should have roughly similar performance:
 //
@@ -18,7 +19,7 @@ static void Logging(benchmark::Bench& bench, const std::vector<const char*>& ext
     LogInstance().DisableCategory(BCLog::LogFlags::ALL);
 
     TestingSetup test_setup{
-        CBaseChainParams::REGTEST,
+        ChainType::REGTEST,
         extra_args,
     };
 

diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp
@@ -7,6 +7,7 @@
 #include <policy/policy.h>
 #include <test/util/setup_common.h>
 #include <txmempool.h>
+#include <util/chaintype.h>
 #include <validation.h>
 
 #include <vector>
@@ -88,7 +89,7 @@ static void ComplexMemPool(benchmark::Bench& bench)
         childTxs = static_cast<int>(bench.complexityN());
     }
     std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /*min_ancestors=*/1);
-    const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN);
+    const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(ChainType::MAIN);
     CTxMemPool& pool = *testing_setup.get()->m_node.mempool;
     LOCK2(cs_main, pool.cs);
     bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
@@ -103,7 +104,7 @@ static void ComplexMemPool(benchmark::Bench& bench)
 static void MempoolCheck(benchmark::Bench& bench)
 {
     FastRandomContext det_rand{true};
-    auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(CBaseChainParams::REGTEST, {"-checkmempool=1"});
+    auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(ChainType::REGTEST, {"-checkmempool=1"});
     CTxMemPool& pool = *testing_setup.get()->m_node.mempool;
     LOCK2(cs_main, pool.cs);
     testing_setup->PopulateMempool(det_rand, 400, true);

diff --git a/src/bench/rpc_blockchain.cpp b/src/bench/rpc_blockchain.cpp
@@ -8,14 +8,15 @@
 #include <rpc/blockchain.h>
 #include <streams.h>
 #include <test/util/setup_common.h>
+#include <util/chaintype.h>
 #include <validation.h>
 
 #include <univalue.h>
 
 namespace {
 
 struct TestBlockAndIndex {
-    const std::unique_ptr<const TestingSetup> testing_setup{MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN)};
+    const std::unique_ptr<const TestingSetup> testing_setup{MakeNoLogFileContext<const TestingSetup>(ChainType::MAIN)};
     CBlock block{};
     uint256 blockHash{};
     CBlockIndex blockindex{};

diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp
@@ -9,6 +9,7 @@
 #include <rpc/mempool.h>
 #include <test/util/setup_common.h>
 #include <txmempool.h>
+#include <util/chaintype.h>
 
 #include <univalue.h>
 
@@ -21,7 +22,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& poo
 
 static void RpcMempool(benchmark::Bench& bench)
 {
-    const auto testing_setup = MakeNoLogFileContext<const ChainTestingSetup>(CBaseChainParams::MAIN);
+    const auto testing_setup = MakeNoLogFileContext<const ChainTestingSetup>(ChainType::MAIN);
     CTxMemPool& pool = *Assert(testing_setup->m_node.mempool);
     LOCK2(cs_main, pool.cs);
 

diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
@@ -25,6 +25,7 @@
 #include <node/chainstate.h>
 #include <scheduler.h>
 #include <script/sigcache.h>
+#include <util/chaintype.h>
 #include <util/thread.h>
 #include <validation.h>
 #include <validationinterface.h>
@@ -52,7 +53,7 @@ int main(int argc, char* argv[])
 
 
     // SETUP: Misc Globals
-    SelectParams(CBaseChainParams::MAIN);
+    SelectParams(ChainType::MAIN);
     auto chainparams = CChainParams::Main();
 
     kernel::Context kernel_context{};

diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
@@ -20,6 +20,7 @@
 #include <rpc/request.h>
 #include <tinyformat.h>
 #include <univalue.h>
+#include <util/chaintype.h>
 #include <util/exception.h>
 #include <util/strencodings.h>
 #include <util/system.h>
@@ -73,10 +74,10 @@ static void SetupCliArgs(ArgsManager& argsman)
 {
     SetupHelpOptions(argsman);
 
-    const auto defaultBaseParams = CreateBaseChainParams(CBaseChainParams::MAIN);
-    const auto testnetBaseParams = CreateBaseChainParams(CBaseChainParams::TESTNET);
-    const auto signetBaseParams = CreateBaseChainParams(CBaseChainParams::SIGNET);
-    const auto regtestBaseParams = CreateBaseChainParams(CBaseChainParams::REGTEST);
+    const auto defaultBaseParams = CreateBaseChainParams(ChainType::MAIN);
+    const auto testnetBaseParams = CreateBaseChainParams(ChainType::TESTNET);
+    const auto signetBaseParams = CreateBaseChainParams(ChainType::SIGNET);
+    const auto regtestBaseParams = CreateBaseChainParams(ChainType::REGTEST);
 
     argsman.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -174,7 +175,7 @@ static int AppInitRPC(int argc, char* argv[])
     }
     // Check for chain settings (BaseParams() calls are only valid after this clause)
     try {
-        SelectBaseParams(gArgs.GetChainName());
+        SelectBaseParams(gArgs.GetChainType());
     } catch (const std::exception& e) {
         tfm::format(std::cerr, "Error: %s\n", e.what());
         return EXIT_FAILURE;
@@ -426,10 +427,16 @@ class NetinfoRequestHandler : public BaseRequestHandler
     std::vector<Peer> m_peers;
     std::string ChainToString() const
     {
-        if (gArgs.GetChainName() == CBaseChainParams::TESTNET) return " testnet";
-        if (gArgs.GetChainName() == CBaseChainParams::SIGNET) return " signet";
-        if (gArgs.GetChainName() == CBaseChainParams::REGTEST) return " regtest";
-        return "";
+        switch (gArgs.GetChainType()) {
+        case ChainType::TESTNET:
+            return " testnet";
+        case ChainType::SIGNET:
+            return " signet";
+        case ChainType::REGTEST:
+            return " regtest";
+        default:
+            return "";
+        }
     }
     std::string PingTimeToString(double seconds) const
     {

diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
@@ -91,7 +91,7 @@ static int AppInitRawTx(int argc, char* argv[])
 
     // Check for chain settings (Params() calls are only valid after this clause)
     try {
-        SelectParams(gArgs.GetChainName());
+        SelectParams(gArgs.GetChainType());
     } catch (const std::exception& e) {
         tfm::format(std::cerr, "Error: %s\n", e.what());
         return EXIT_FAILURE;

diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp
@@ -75,7 +75,7 @@ static int AppInitUtil(ArgsManager& args, int argc, char* argv[])
 
     // Check for chain settings (Params() calls are only valid after this clause)
     try {
-        SelectParams(args.GetChainName());
+        SelectParams(args.GetChainType());
     } catch (const std::exception& e) {
         tfm::format(std::cerr, "Error: %s\n", e.what());
         return EXIT_FAILURE;

diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp
@@ -91,7 +91,7 @@ static std::optional<int> WalletAppInit(ArgsManager& args, int argc, char* argv[
         return EXIT_FAILURE;
     }
     // Check for chain settings (Params() calls are only valid after this clause)
-    SelectParams(args.GetChainName());
+    SelectParams(args.GetChainType());
 
     return std::nullopt;
 }

diff --git a/src/chainparams.cpp b/src/chainparams.cpp
@@ -12,6 +12,7 @@
 #include <hash.h> // for signet block challenge hash
 #include <logging.h>
 #include <script/interpreter.h>
+#include <util/chaintype.h>
 #include <util/string.h>
 
 #include <assert.h>
@@ -97,26 +98,29 @@ const CChainParams &Params() {
     return *globalChainParams;
 }
 
-std::unique_ptr<const CChainParams> CreateChainParams(const ArgsManager& args, const std::string& chain)
+std::unique_ptr<const CChainParams> CreateChainParams(const ArgsManager& args, const ChainType chain)
 {
-    if (chain == CBaseChainParams::MAIN) {
+    switch (chain) {
+    case ChainType::MAIN:
         return CChainParams::Main();
-    } else if (chain == CBaseChainParams::TESTNET) {
+    case ChainType::TESTNET:
         return CChainParams::TestNet();
-    } else if (chain == CBaseChainParams::SIGNET) {
+    case ChainType::SIGNET: {
         auto opts = CChainParams::SigNetOptions{};
         ReadSigNetArgs(args, opts);
         return CChainParams::SigNet(opts);
-    } else if (chain == CBaseChainParams::REGTEST) {
+    }
+    case ChainType::REGTEST: {
         auto opts = CChainParams::RegTestOptions{};
         ReadRegTestArgs(args, opts);
         return CChainParams::RegTest(opts);
     }
-    throw std::runtime_error(strprintf("%s: Unknown chain %s.", __func__, chain));
+    }
+    throw std::invalid_argument(strprintf("%s: Invalid ChainType value", __func__));
 }
 
-void SelectParams(const std::string& network)
+void SelectParams(const ChainType chain)
 {
-    SelectBaseParams(network);
-    globalChainParams = CreateChainParams(gArgs, network);
+    SelectBaseParams(chain);
+    globalChainParams = CreateChainParams(gArgs, chain);
 }
diff --git a/src/chainparams.h b/src/chainparams.h
@@ -13,6 +13,7 @@
 #include <netaddress.h>
 #include <primitives/block.h>
 #include <protocol.h>
+#include <util/chaintype.h>
 #include <util/hash_type.h>
 
 #include <cstdint>
@@ -26,7 +27,7 @@
  * @returns a CChainParams* of the chosen chain.
  * @throws a std::runtime_error if the chain is not supported.
  */
-std::unique_ptr<const CChainParams> CreateChainParams(const ArgsManager& args, const std::string& chain);
+std::unique_ptr<const CChainParams> CreateChainParams(const ArgsManager& args, const ChainType chain);
 
 /**
  * Return the currently selected parameters. This won't change after app
@@ -38,6 +39,6 @@ const CChainParams &Params();
  * Sets the params returned by Params() to those for the given chain name.
  * @throws std::runtime_error when the chain is not supported.
  */
-void SelectParams(const std::string& chain);
+void SelectParams(const ChainType chain);
 
 #endif // BITCOIN_CHAINPARAMS_H
diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp
@@ -7,14 +7,10 @@
 
 #include <common/args.h>
 #include <tinyformat.h>
+#include <util/chaintype.h>
 
 #include <assert.h>
 
-const std::string CBaseChainParams::MAIN = "main";
-const std::string CBaseChainParams::TESTNET = "test";
-const std::string CBaseChainParams::SIGNET = "signet";
-const std::string CBaseChainParams::REGTEST = "regtest";
-
 void SetupChainParamsBaseOptions(ArgsManager& argsman)
 {
     argsman.AddArg("-chain=<chain>", "Use the chain <chain> (default: main). Allowed values: main, test, signet, regtest", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
@@ -40,22 +36,23 @@ const CBaseChainParams& BaseParams()
  * Port numbers for incoming Tor connections (8334, 18334, 38334, 18445) have
  * been chosen arbitrarily to keep ranges of used ports tight.
  */
-std::unique_ptr<CBaseChainParams> CreateBaseChainParams(const std::string& chain)
+std::unique_ptr<CBaseChainParams> CreateBaseChainParams(const ChainType chain)
 {
-    if (chain == CBaseChainParams::MAIN) {
+    switch (chain) {
+    case ChainType::MAIN:
         return std::make_unique<CBaseChainParams>("", 8332, 8334);
-    } else if (chain == CBaseChainParams::TESTNET) {
+    case ChainType::TESTNET:
         return std::make_unique<CBaseChainParams>("testnet3", 18332, 18334);
-    } else if (chain == CBaseChainParams::SIGNET) {
+    case ChainType::SIGNET:
         return std::make_unique<CBaseChainParams>("signet", 38332, 38334);
-    } else if (chain == CBaseChainParams::REGTEST) {
+    case ChainType::REGTEST:
         return std::make_unique<CBaseChainParams>("regtest", 18443, 18445);
     }
-    throw std::runtime_error(strprintf("%s: Unknown chain %s.", __func__, chain));
+    throw std::invalid_argument(strprintf("%s: Invalid ChainType value", __func__));
 }
 
-void SelectBaseParams(const std::string& chain)
+void SelectBaseParams(const ChainType chain)
 {
     globalChainBaseParams = CreateBaseChainParams(chain);
-    gArgs.SelectConfigNetwork(chain);
+    gArgs.SelectConfigNetwork(ChainTypeToString(chain));
 }
diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h
@@ -5,6 +5,8 @@
 #ifndef BITCOIN_CHAINPARAMSBASE_H
 #define BITCOIN_CHAINPARAMSBASE_H
 
+#include <util/chaintype.h>
+
 #include <memory>
 #include <string>
 
@@ -17,14 +19,6 @@ class ArgsManager;
 class CBaseChainParams
 {
 public:
-    ///@{
-    /** Chain name strings */
-    static const std::string MAIN;
-    static const std::string TESTNET;
-    static const std::string SIGNET;
-    static const std::string REGTEST;
-    ///@}
-
     const std::string& DataDir() const { return strDataDir; }
     uint16_t RPCPort() const { return m_rpc_port; }
     uint16_t OnionServiceTargetPort() const { return m_onion_service_target_port; }
@@ -44,7 +38,7 @@ class CBaseChainParams
  * @returns a CBaseChainParams* of the chosen chain.
  * @throws a std::runtime_error if the chain is not supported.
  */
-std::unique_ptr<CBaseChainParams> CreateBaseChainParams(const std::string& chain);
+std::unique_ptr<CBaseChainParams> CreateBaseChainParams(const ChainType chain);
 
 /**
  *Set the arguments for chainparams
@@ -57,7 +51,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman);
  */
 const CBaseChainParams& BaseParams();
 
-/** Sets the params returned by Params() to those for the given network. */
-void SelectBaseParams(const std::string& chain);
+/** Sets the params returned by Params() to those for the given chain. */
+void SelectBaseParams(const ChainType chain);
 
 #endif // BITCOIN_CHAINPARAMSBASE_H