Merge bitcoin/bitcoin#27425: test: move remaining rand code from util/setup_common to util/random

1cd45d4 test: move random.h include header from setup_common.h to cpp (Jon Atack)
1b246fd test: move remaining random test util code from setup_common to random (jonatack)

Pull request description:

  and drop the `util/random` dependency on `util/setup_common`.  This improves code separation and allows `util/setup_common` to call `util/random` functions without creating a circular dependency, thereby addressing bitcoin/bitcoin#26940 (comment) by glozow (thanks!)

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 1cd45d4 🌂

Tree-SHA512: 6ce63d9103ba9b04eebbd8ad02fe9aa79e356296533404034a1ae88e9b7ca0bc9a5c51fd754b71cf4e7b55b18bcd4d5474b2d588edee3851e3b3ce0e4d309a93

Author: fanquake

src/Makefile.test_util.include

@@ -40,6 +40,7 @@ libtest_util_a_SOURCES = \
   test/util/logging.cpp \
   test/util/mining.cpp \
   test/util/net.cpp \
+  test/util/random.cpp \
   test/util/script.cpp \
   test/util/setup_common.cpp \
   test/util/str.cpp \

src/test/denialofservice_tests.cpp

@@ -15,6 +15,7 @@
 #include <script/standard.h>
 #include <serialize.h>
 #include <test/util/net.h>
+#include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <timedata.h>
 #include <util/string.h>

src/test/miniscript_tests.cpp

@@ -6,6 +6,7 @@
 #include <string>
 #include <vector>
 
+#include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <boost/test/unit_test.hpp>
 

src/test/random_tests.cpp

@@ -4,6 +4,7 @@
 
 #include <random.h>
 
+#include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <util/time.h>
 

src/test/util/random.cpp

@@ -0,0 +1,33 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <test/util/random.h>
+
+#include <logging.h>
+#include <random.h>
+#include <uint256.h>
+
+#include <cstdlib>
+#include <string>
+
+FastRandomContext g_insecure_rand_ctx;
+
+/** Return the unsigned from the environment var if available, otherwise 0 */
+static uint256 GetUintFromEnv(const std::string& env_name)
+{
+    const char* num = std::getenv(env_name.c_str());
+    if (!num) return {};
+    return uint256S(num);
+}
+
+void Seed(FastRandomContext& ctx)
+{
+    // Should be enough to get the seed once for the process
+    static uint256 seed{};
+    static const std::string RANDOM_CTX_SEED{"RANDOM_CTX_SEED"};
+    if (seed.IsNull()) seed = GetUintFromEnv(RANDOM_CTX_SEED);
+    if (seed.IsNull()) seed = GetRandHash();
+    LogPrintf("%s: Setting random seed for current tests to %s=%s\n", __func__, RANDOM_CTX_SEED, seed.GetHex());
+    ctx = FastRandomContext(seed);
+}

src/test/util/random.h

@@ -7,11 +7,41 @@
 
 #include <consensus/amount.h>
 #include <random.h>
-#include <test/util/setup_common.h>
 #include <uint256.h>
 
 #include <cstdint>
 
+/**
+ * This global and the helpers that use it are not thread-safe.
+ *
+ * If thread-safety is needed, the global could be made thread_local (given
+ * that thread_local is supported on all architectures we support) or a
+ * per-thread instance could be used in the multi-threaded test.
+ */
+extern FastRandomContext g_insecure_rand_ctx;
+
+/**
+ * Flag to make GetRand in random.h return the same number
+ */
+extern bool g_mock_deterministic_tests;
+
+enum class SeedRand {
+    ZEROS, //!< Seed with a compile time constant of zeros
+    SEED,  //!< Call the Seed() helper
+};
+
+/** Seed the given random ctx or use the seed passed in via an environment var */
+void Seed(FastRandomContext& ctx);
+
+static inline void SeedInsecureRand(SeedRand seed = SeedRand::SEED)
+{
+    if (seed == SeedRand::ZEROS) {
+        g_insecure_rand_ctx = FastRandomContext(/*fDeterministic=*/true);
+    } else {
+        Seed(g_insecure_rand_ctx);
+    }
+}
+
 static inline uint32_t InsecureRand32()
 {
     return g_insecure_rand_ctx.rand32();

src/test/util/setup_common.cpp

@@ -32,6 +32,7 @@
 #include <policy/fees.h>
 #include <policy/fees_args.h>
 #include <pow.h>
+#include <random.h>
 #include <rpc/blockchain.h>
 #include <rpc/register.h>
 #include <rpc/server.h>
@@ -40,6 +41,7 @@
 #include <shutdown.h>
 #include <streams.h>
 #include <test/util/net.h>
+#include <test/util/random.h>
 #include <test/util/txmempool.h>
 #include <timedata.h>
 #include <txdb.h>
@@ -73,29 +75,9 @@ using node::VerifyLoadedChainstate;
 const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
 UrlDecodeFn* const URL_DECODE = nullptr;
 
-FastRandomContext g_insecure_rand_ctx;
 /** Random context to get unique temp data dirs. Separate from g_insecure_rand_ctx, which can be seeded from a const env var */
 static FastRandomContext g_insecure_rand_ctx_temp_path;
 
-/** Return the unsigned from the environment var if available, otherwise 0 */
-static uint256 GetUintFromEnv(const std::string& env_name)
-{
-    const char* num = std::getenv(env_name.c_str());
-    if (!num) return {};
-    return uint256S(num);
-}
-
-void Seed(FastRandomContext& ctx)
-{
-    // Should be enough to get the seed once for the process
-    static uint256 seed{};
-    static const std::string RANDOM_CTX_SEED{"RANDOM_CTX_SEED"};
-    if (seed.IsNull()) seed = GetUintFromEnv(RANDOM_CTX_SEED);
-    if (seed.IsNull()) seed = GetRandHash();
-    LogPrintf("%s: Setting random seed for current tests to %s=%s\n", __func__, RANDOM_CTX_SEED, seed.GetHex());
-    ctx = FastRandomContext(seed);
-}
-
 std::ostream& operator<<(std::ostream& os, const uint256& num)
 {
     os << num.ToString();

src/test/util/setup_common.h

@@ -11,7 +11,6 @@
 #include <node/context.h> // IWYU pragma: export
 #include <primitives/transaction.h>
 #include <pubkey.h>
-#include <random.h>
 #include <stdexcept>
 #include <util/chaintype.h>
 #include <util/check.h>
@@ -25,6 +24,7 @@
 
 class CFeeRate;
 class Chainstate;
+class FastRandomContext;
 
 /** This is connected to the logger. Can be used to redirect logs to any other log */
 extern const std::function<void(const std::string&)> G_TEST_LOG_FUN;
@@ -41,37 +41,6 @@ std::ostream& operator<<(typename std::enable_if<std::is_enum<T>::value, std::os
 }
 } // namespace std
 
-/**
- * This global and the helpers that use it are not thread-safe.
- *
- * If thread-safety is needed, the global could be made thread_local (given
- * that thread_local is supported on all architectures we support) or a
- * per-thread instance could be used in the multi-threaded test.
- */
-extern FastRandomContext g_insecure_rand_ctx;
-
-/**
- * Flag to make GetRand in random.h return the same number
- */
-extern bool g_mock_deterministic_tests;
-
-enum class SeedRand {
-    ZEROS, //!< Seed with a compile time constant of zeros
-    SEED,  //!< Call the Seed() helper
-};
-
-/** Seed the given random ctx or use the seed passed in via an environment var */
-void Seed(FastRandomContext& ctx);
-
-static inline void SeedInsecureRand(SeedRand seed = SeedRand::SEED)
-{
-    if (seed == SeedRand::ZEROS) {
-        g_insecure_rand_ctx = FastRandomContext(/*fDeterministic=*/true);
-    } else {
-        Seed(g_insecure_rand_ctx);
-    }
-}
-
 static constexpr CAmount CENT{1000000};
 
 /** Basic testing setup.

src/wallet/test/wallet_tests.cpp

@@ -15,6 +15,7 @@
 #include <policy/policy.h>
 #include <rpc/server.h>
 #include <test/util/logging.h>
+#include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <util/translation.h>
 #include <validation.h>