Merge bitcoin/bitcoin#30571: test: [refactor] Use m_rng directly

948238a683b6c99f4e91114aa75680c6c2d73714 test: Remove FastRandomContext global (Ryan Ofsky)
fa0fe08eca48064b2a42789571fea017e455d820 scripted-diff: [test] Use g_rng/m_rng directly (MarcoFalke)
fa54cab4734f02422f28fdffc0f11e6d3d51b8f0 test: refactor: Accept any RandomNumberGenerator in RandMoney (MarcoFalke)
68f77dd21e4aaf4f09d36d6e5ddd7d260824b94b test: refactor: Pass rng parameters to test functions (Ryan Ofsky)
fa19af555dff6d6c722caf36319b158699d2aa95 test: refactor: Move g_insecure_rand_ctx.Reseed out of the helper that calls MakeRandDeterministicDANGEROUS (MarcoFalke)
3dc527f4602297ffcec3a578eadc480a620d01ec test: refactor: Give unit test functions access to test state (Ryan Ofsky)
fab023e177d7eaef73902869ae1c95693f1e268b test: refactor: Make unsigned promotion explicit (MarcoFalke)
fa2cb654eca8dd6ed89101cd6d199ba1de0b81e0 test: Add m_rng alias for the global random context (MarcoFalke)
fae7e3791c9ed8053166773fcfb583ad19d006dd test: Correct the random seed log on a prevector test failure (MarcoFalke)

Pull request description:

  This is mostly a style-cleanup for the tests' random generation:

  1) `g_insecure_rand_ctx` in the tests is problematic, because the name is a leftover when the generator was indeed insecure. However, now the generator is *deterministic*, because the seed is either passed in or printed (c.f. RANDOM_CTX_SEED). Stating that deterministic randomness is insecure in the tests seems redundant at best. Fix it by just using `m_rng` for the name.

  2) The global random context has many one-line aliases, such as `InsecureRand32`. This is problematic, because the same line of code may use the context directly and through a wrapper at the same time. For example in net_tests (see below). This inconsistency is harmless, but confusing. Fix it by just removing the one-line aliases.

  ```
  src/test/net_tests.cpp:        auto msg_data_1 = g_insecure_rand_ctx.randbytes<uint8_t>(InsecureRandRange(100000));
  ````

  3) The wrapper for randmoney has the same problem that the same unit test uses the context directly and through a wrapper at the same time. Also, it has a single type of Rng hardcoded. Fix it by accepting any type.

ACKs for top commit:
  hodlinator:
    ACK 948238a683b6c99f4e91114aa75680c6c2d73714
  ryanofsky:
    Code review ACK 948238a683b6c99f4e91114aa75680c6c2d73714. Only changes since last review were changing a comments a little bit.
  marcofleon:
    Code review ACK 948238a683b6c99f4e91114aa75680c6c2d73714. Only changes since my last review are the improvements in `prevector_tests`.

Tree-SHA512: 69c6b46a42cb743138ee8c87ff26a588dbe083e3efb3dca49b8a133ba5d3b09e8bf01c590ec7e121a7d77cb1fd7dcacd927a9ca139ac65e1f7c6d1ec46f93b57

1 files changed, 4 insertions, 4 deletions

diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 62ff61b227..dcf72cf627 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -75,8 +75,8 @@ using node::VerifyLoadedChainstate;
 
 const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
 
-/** 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;
+/** Random context to get unique temp data dirs. Separate from m_rng, which can be seeded from a const env var */
+static FastRandomContext g_rng_temp_path;
 
 std::ostream& operator<<(std::ostream& os, const arith_uint256& num)
 {
@@ -158,7 +158,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)
 
     if (!m_node.args->IsArgSet("-testdatadir")) {
         // By default, the data directory has a random name
-        const auto rand_str{g_insecure_rand_ctx_temp_path.rand256().ToString()};
+        const auto rand_str{g_rng_temp_path.rand256().ToString()};
         m_path_root = fs::temp_directory_path() / "test_common_" PACKAGE_NAME / rand_str;
         TryCreateDirectories(m_path_root);
     } else {
@@ -580,7 +580,7 @@ void TestChain100Setup::MockMempoolMinFee(const CFeeRate& target_feerate)
     // Manually create an invalid transaction. Manually set the fee in the CTxMemPoolEntry to
     // achieve the exact target feerate.
     CMutableTransaction mtx = CMutableTransaction();
-    mtx.vin.emplace_back(COutPoint{Txid::FromUint256(g_insecure_rand_ctx.rand256()), 0});
+    mtx.vin.emplace_back(COutPoint{Txid::FromUint256(m_rng.rand256()), 0});
     mtx.vout.emplace_back(1 * COIN, GetScriptForDestination(WitnessV0ScriptHash(CScript() << OP_TRUE)));
     const auto tx{MakeTransactionRef(mtx)};
     LockPoints lp;