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

948238a test: Remove FastRandomContext global (Ryan Ofsky)
fa0fe08 scripted-diff: [test] Use g_rng/m_rng directly (MarcoFalke)
fa54cab test: refactor: Accept any RandomNumberGenerator in RandMoney (MarcoFalke)
68f77dd test: refactor: Pass rng parameters to test functions (Ryan Ofsky)
fa19af5 test: refactor: Move g_insecure_rand_ctx.Reseed out of the helper that calls MakeRandDeterministicDANGEROUS (MarcoFalke)
3dc527f test: refactor: Give unit test functions access to test state (Ryan Ofsky)
fab023e test: refactor: Make unsigned promotion explicit (MarcoFalke)
fa2cb65 test: Add m_rng alias for the global random context (MarcoFalke)
fae7e37 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 948238a
  ryanofsky:
    Code review ACK 948238a. Only changes since last review were changing a comments a little bit.
  marcofleon:
    Code review ACK 948238a. Only changes since my last review are the improvements in `prevector_tests`.

Tree-SHA512: 69c6b46a42cb743138ee8c87ff26a588dbe083e3efb3dca49b8a133ba5d3b09e8bf01c590ec7e121a7d77cb1fd7dcacd927a9ca139ac65e1f7c6d1ec46f93b57

src/bench/sign_transaction.cpp

@@ -75,12 +75,13 @@ static void SignTransactionSchnorr(benchmark::Bench& bench) { SignTransactionSin
 
 static void SignSchnorrTapTweakBenchmark(benchmark::Bench& bench, bool use_null_merkle_root)
 {
+    FastRandomContext rng;
     ECC_Context ecc_context{};
 
     auto key = GenerateRandomKey();
-    auto msg = InsecureRand256();
-    auto merkle_root = use_null_merkle_root ? uint256() : InsecureRand256();
-    auto aux = InsecureRand256();
+    auto msg = rng.rand256();
+    auto merkle_root = use_null_merkle_root ? uint256() : rng.rand256();
+    auto aux = rng.rand256();
     std::vector<unsigned char> sig(64);
 
     bench.minEpochIterations(100).run([&] {

src/test/base58_tests.cpp

@@ -84,14 +84,14 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58)
 BOOST_AUTO_TEST_CASE(base58_random_encode_decode)
 {
     for (int n = 0; n < 1000; ++n) {
-        unsigned int len = 1 + InsecureRandBits(8);
-        unsigned int zeroes = InsecureRandBool() ? InsecureRandRange(len + 1) : 0;
-        auto data = Cat(std::vector<unsigned char>(zeroes, '\000'), g_insecure_rand_ctx.randbytes(len - zeroes));
+        unsigned int len = 1 + m_rng.randbits(8);
+        unsigned int zeroes = m_rng.randbool() ? m_rng.randrange(len + 1) : 0;
+        auto data = Cat(std::vector<unsigned char>(zeroes, '\000'), m_rng.randbytes(len - zeroes));
         auto encoded = EncodeBase58Check(data);
         std::vector<unsigned char> decoded;
-        auto ok_too_small = DecodeBase58Check(encoded, decoded, InsecureRandRange(len));
+        auto ok_too_small = DecodeBase58Check(encoded, decoded, m_rng.randrange(len));
         BOOST_CHECK(!ok_too_small);
-        auto ok = DecodeBase58Check(encoded, decoded, len + InsecureRandRange(257 - len));
+        auto ok = DecodeBase58Check(encoded, decoded, len + m_rng.randrange(257 - len));
         BOOST_CHECK(ok);
         BOOST_CHECK(data == decoded);
     }

src/test/bip324_tests.cpp

@@ -21,6 +21,7 @@
 
 namespace {
 
+struct BIP324Test : BasicTestingSetup {
 void TestBIP324PacketVector(
     uint32_t in_idx,
     const std::string& in_priv_ours_hex,
@@ -116,7 +117,7 @@ void TestBIP324PacketVector(
 
         // Seek to the numbered packet.
         if (in_idx == 0 && error == 12) continue;
-        uint32_t dec_idx = in_idx ^ (error == 12 ? (1U << InsecureRandRange(16)) : 0);
+        uint32_t dec_idx = in_idx ^ (error == 12 ? (1U << m_rng.randrange(16)) : 0);
         for (uint32_t i = 0; i < dec_idx; ++i) {
             unsigned use_idx = i < in_idx ? i : 0;
             bool dec_ignore{false};
@@ -128,7 +129,7 @@ void TestBIP324PacketVector(
         // Decrypt length
         auto to_decrypt = ciphertext;
         if (error >= 2 && error <= 9) {
-            to_decrypt[InsecureRandRange(to_decrypt.size())] ^= std::byte(1U << (error - 2));
+            to_decrypt[m_rng.randrange(to_decrypt.size())] ^= std::byte(1U << (error - 2));
         }
 
         // Decrypt length and resize ciphertext to accommodate.
@@ -139,7 +140,7 @@ void TestBIP324PacketVector(
         auto dec_aad = in_aad;
         if (error == 10) {
             if (in_aad.size() == 0) continue;
-            dec_aad[InsecureRandRange(dec_aad.size())] ^= std::byte(1U << InsecureRandRange(8));
+            dec_aad[m_rng.randrange(dec_aad.size())] ^= std::byte(1U << m_rng.randrange(8));
         }
         if (error == 11) dec_aad.push_back({});
 
@@ -156,10 +157,11 @@ void TestBIP324PacketVector(
         }
     }
 }
+}; // struct BIP324Test
 
 }  // namespace
 
-BOOST_FIXTURE_TEST_SUITE(bip324_tests, BasicTestingSetup)
+BOOST_FIXTURE_TEST_SUITE(bip324_tests, BIP324Test)
 
 BOOST_AUTO_TEST_CASE(packet_test_vectors) {
     // BIP324 key derivation uses network magic in the HKDF process. We use mainnet params here

src/test/blockencodings_tests.cpp

@@ -356,7 +356,7 @@ BOOST_AUTO_TEST_CASE(ReceiveWithExtraTransactions) {
 
 BOOST_AUTO_TEST_CASE(TransactionsRequestSerializationTest) {
     BlockTransactionsRequest req1;
-    req1.blockhash = InsecureRand256();
+    req1.blockhash = m_rng.rand256();
     req1.indexes.resize(4);
     req1.indexes[0] = 0;
     req1.indexes[1] = 1;
@@ -380,7 +380,7 @@ BOOST_AUTO_TEST_CASE(TransactionsRequestSerializationTest) {
 BOOST_AUTO_TEST_CASE(TransactionsRequestDeserializationMaxTest) {
     // Check that the highest legal index is decoded correctly
     BlockTransactionsRequest req0;
-    req0.blockhash = InsecureRand256();
+    req0.blockhash = m_rng.rand256();
     req0.indexes.resize(1);
     req0.indexes[0] = 0xffff;
     DataStream stream{};
@@ -398,7 +398,7 @@ BOOST_AUTO_TEST_CASE(TransactionsRequestDeserializationOverflowTest) {
     // a request cannot be created by serializing a real BlockTransactionsRequest
     // due to the overflow, so here we'll serialize from raw deltas.
     BlockTransactionsRequest req0;
-    req0.blockhash = InsecureRand256();
+    req0.blockhash = m_rng.rand256();
     req0.indexes.resize(3);
     req0.indexes[0] = 0x7000;
     req0.indexes[1] = 0x10000 - 0x7000 - 2;

src/test/bloom_tests.cpp

@@ -22,7 +22,13 @@
 
 #include <boost/test/unit_test.hpp>
 
-BOOST_FIXTURE_TEST_SUITE(bloom_tests, BasicTestingSetup)
+namespace bloom_tests {
+struct BloomTest : public BasicTestingSetup {
+    std::vector<unsigned char> RandomData();
+};
+} // namespace bloom_tests
+
+BOOST_FIXTURE_TEST_SUITE(bloom_tests, BloomTest)
 
 BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize)
 {
@@ -455,9 +461,9 @@ BOOST_AUTO_TEST_CASE(merkle_block_4_test_update_none)
     BOOST_CHECK(!filter.contains(COutPoint(Txid::FromHex("02981fa052f0481dbc5868f4fc2166035a10f27a03cfd2de67326471df5bc041").value(), 0)));
 }
 
-static std::vector<unsigned char> RandomData()
+std::vector<unsigned char> BloomTest::RandomData()
 {
-    uint256 r = InsecureRand256();
+    uint256 r = m_rng.rand256();
     return std::vector<unsigned char>(r.begin(), r.end());
 }
 

src/test/checkqueue_tests.cpp

@@ -34,7 +34,9 @@ struct NoLockLoggingTestingSetup : public TestingSetup {
 #endif
 };
 
-BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, NoLockLoggingTestingSetup)
+struct CheckQueueTest : NoLockLoggingTestingSetup {
+    void Correct_Queue_range(std::vector<size_t> range);
+};
 
 static const unsigned int QUEUE_BATCH_SIZE = 128;
 static const int SCRIPT_CHECK_THREADS = 3;
@@ -156,7 +158,7 @@ typedef CCheckQueue<FrozenCleanupCheck> FrozenCleanup_Queue;
 /** This test case checks that the CCheckQueue works properly
  * with each specified size_t Checks pushed.
  */
-static void Correct_Queue_range(std::vector<size_t> range)
+void CheckQueueTest::Correct_Queue_range(std::vector<size_t> range)
 {
     auto small_queue = std::make_unique<Correct_Queue>(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS);
     // Make vChecks here to save on malloc (this test can be slow...)
@@ -168,7 +170,7 @@ static void Correct_Queue_range(std::vector<size_t> range)
         CCheckQueueControl<FakeCheckCheckCompletion> control(small_queue.get());
         while (total) {
             vChecks.clear();
-            vChecks.resize(std::min<size_t>(total, InsecureRandRange(10)));
+            vChecks.resize(std::min<size_t>(total, m_rng.randrange(10)));
             total -= vChecks.size();
             control.Add(std::move(vChecks));
         }
@@ -177,6 +179,8 @@ static void Correct_Queue_range(std::vector<size_t> range)
     }
 }
 
+BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, CheckQueueTest)
+
 /** Test that 0 checks is correct
  */
 BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Zero)
@@ -207,7 +211,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Random)
 {
     std::vector<size_t> range;
     range.reserve(100000/1000);
-    for (size_t i = 2; i < 100000; i += std::max((size_t)1, (size_t)InsecureRandRange(std::min((size_t)1000, ((size_t)100000) - i))))
+    for (size_t i = 2; i < 100000; i += std::max((size_t)1, (size_t)m_rng.randrange(std::min((size_t)1000, ((size_t)100000) - i))))
         range.push_back(i);
     Correct_Queue_range(range);
 }
@@ -221,7 +225,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure)
         CCheckQueueControl<FailingCheck> control(fail_queue.get());
         size_t remaining = i;
         while (remaining) {
-            size_t r = InsecureRandRange(10);
+            size_t r = m_rng.randrange(10);
 
             std::vector<FailingCheck> vChecks;
             vChecks.reserve(r);
@@ -268,7 +272,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck)
     {
         CCheckQueueControl<UniqueCheck> control(queue.get());
         while (total) {
-            size_t r = InsecureRandRange(10);
+            size_t r = m_rng.randrange(10);
             std::vector<UniqueCheck> vChecks;
             for (size_t k = 0; k < r && total; k++)
                 vChecks.emplace_back(--total);
@@ -300,7 +304,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory)
         {
             CCheckQueueControl<MemoryCheck> control(queue.get());
             while (total) {
-                size_t r = InsecureRandRange(10);
+                size_t r = m_rng.randrange(10);
                 std::vector<MemoryCheck> vChecks;
                 for (size_t k = 0; k < r && total; k++) {
                     total--;