Merge #14624: Some simple improvements to the RNG code

e414486d56b9f06af7aeb07ce13e3c3780c2b69b Do not permit copying FastRandomContexts (Pieter Wuille)
022cf47dd7ef8f46e32a184e84f94d1e9f3a495c Simplify testing RNG code (Pieter Wuille)
fd3e7973ffaaa15ed32e5aeadcb02956849b8fc7 Make unit tests use the insecure_rand_ctx exclusively (Pieter Wuille)
8d98d426116f0178612f14d1874d331042c4c4b7 Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille)
273d02580aa736b7ccea8fce51d90541665fdbd1 Use a FastRandomContext in LimitOrphanTxSize (Pieter Wuille)
3db746beb407f7cdd9cd6a605a195bef1254b4c0 Introduce a Shuffle for FastRandomContext and use it in wallet and coinselection (Pieter Wuille)
8098379be5465f598220e1d6174fc57c56f9da42 Use a local FastRandomContext in a few more places in net (Pieter Wuille)
9695f31d7544778853aa373f0aeed629fa68d85e Make addrman use its local RNG exclusively (Pieter Wuille)

Pull request description:

  This improves a few minor issues with the RNG code:
  * Avoid calling `GetRand*()` functions (which currently invoke OpenSSL, later may switch to using our own RNG pool) inside loops in addrman, networking code, `KnapsackSolver`, and `LimitOrphanSize`
  * Fix a currently unreachable bug in `FastRandomContext::randbytes`.
  * Make a number of simplifications to the unit tests' randomness code (some tests unnecessarily used their own RNG or the OpenSSL one, instead of using the unit test specific `insecure_rand_ctx`).
  * As a precaution, make it illegal to copy a `FastRandomContext`.

Tree-SHA512: 084c70b533ea68ca7adc0186c39f0b3e0a5c0ae43a12c37286e5d42086e056a8cd026dde61b12c0a296dc80f87fdc87fe303b9e8e6161b460ac2086cf7615f9d

16 files changed, 134 insertions, 82 deletions

diff --git a/src/addrman.cpp b/src/addrman.cpp
index 44328c3056..06c342ba73 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -217,7 +217,7 @@ void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime
         return;
 
     // find a bucket it is in now
-    int nRnd = RandomInt(ADDRMAN_NEW_BUCKET_COUNT);
+    int nRnd = insecure_rand.randrange(ADDRMAN_NEW_BUCKET_COUNT);
     int nUBucket = -1;
     for (unsigned int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) {
         int nB = (n + nRnd) % ADDRMAN_NEW_BUCKET_COUNT;
@@ -291,7 +291,7 @@ bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimeP
         int nFactor = 1;
         for (int n = 0; n < pinfo->nRefCount; n++)
             nFactor *= 2;
-        if (nFactor > 1 && (RandomInt(nFactor) != 0))
+        if (nFactor > 1 && (insecure_rand.randrange(nFactor) != 0))
             return false;
     } else {
         pinfo = Create(addr, source, &nId);
@@ -356,12 +356,12 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
 
     // Use a 50% chance for choosing between tried and new table entries.
     if (!newOnly &&
-       (nTried > 0 && (nNew == 0 || RandomInt(2) == 0))) {
+       (nTried > 0 && (nNew == 0 || insecure_rand.randbool() == 0))) {
         // use a tried node
         double fChanceFactor = 1.0;
         while (1) {
-            int nKBucket = RandomInt(ADDRMAN_TRIED_BUCKET_COUNT);
-            int nKBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
+            int nKBucket = insecure_rand.randrange(ADDRMAN_TRIED_BUCKET_COUNT);
+            int nKBucketPos = insecure_rand.randrange(ADDRMAN_BUCKET_SIZE);
             while (vvTried[nKBucket][nKBucketPos] == -1) {
                 nKBucket = (nKBucket + insecure_rand.randbits(ADDRMAN_TRIED_BUCKET_COUNT_LOG2)) % ADDRMAN_TRIED_BUCKET_COUNT;
                 nKBucketPos = (nKBucketPos + insecure_rand.randbits(ADDRMAN_BUCKET_SIZE_LOG2)) % ADDRMAN_BUCKET_SIZE;
@@ -369,7 +369,7 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
             int nId = vvTried[nKBucket][nKBucketPos];
             assert(mapInfo.count(nId) == 1);
             CAddrInfo& info = mapInfo[nId];
-            if (RandomInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
+            if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30))
                 return info;
             fChanceFactor *= 1.2;
         }
@@ -377,8 +377,8 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
         // use a new node
         double fChanceFactor = 1.0;
         while (1) {
-            int nUBucket = RandomInt(ADDRMAN_NEW_BUCKET_COUNT);
-            int nUBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
+            int nUBucket = insecure_rand.randrange(ADDRMAN_NEW_BUCKET_COUNT);
+            int nUBucketPos = insecure_rand.randrange(ADDRMAN_BUCKET_SIZE);
             while (vvNew[nUBucket][nUBucketPos] == -1) {
                 nUBucket = (nUBucket + insecure_rand.randbits(ADDRMAN_NEW_BUCKET_COUNT_LOG2)) % ADDRMAN_NEW_BUCKET_COUNT;
                 nUBucketPos = (nUBucketPos + insecure_rand.randbits(ADDRMAN_BUCKET_SIZE_LOG2)) % ADDRMAN_BUCKET_SIZE;
@@ -386,7 +386,7 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
             int nId = vvNew[nUBucket][nUBucketPos];
             assert(mapInfo.count(nId) == 1);
             CAddrInfo& info = mapInfo[nId];
-            if (RandomInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
+            if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30))
                 return info;
             fChanceFactor *= 1.2;
         }
@@ -482,7 +482,7 @@ void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr)
         if (vAddr.size() >= nNodes)
             break;
 
-        int nRndPos = RandomInt(vRandom.size() - n) + n;
+        int nRndPos = insecure_rand.randrange(vRandom.size() - n) + n;
         SwapRandom(n, nRndPos);
         assert(mapInfo.count(vRandom[n]) == 1);
 
@@ -530,10 +530,6 @@ void CAddrMan::SetServices_(const CService& addr, ServiceFlags nServices)
     info.nServices = nServices;
 }
 
-int CAddrMan::RandomInt(int nMax){
-    return GetRandInt(nMax);
-}
-
 void CAddrMan::ResolveCollisions_()
 {
     for (std::set<int>::iterator it = m_tried_collisions.begin(); it != m_tried_collisions.end();) {
@@ -593,7 +589,7 @@ CAddrInfo CAddrMan::SelectTriedCollision_()
     std::set<int>::iterator it = m_tried_collisions.begin();
 
     // Selects a random element from m_tried_collisions
-    std::advance(it, GetRandInt(m_tried_collisions.size()));
+    std::advance(it, insecure_rand.randrange(m_tried_collisions.size()));
     int id_new = *it;
 
     // If id_new not found in mapInfo remove it from m_tried_collisions
diff --git a/src/addrman.h b/src/addrman.h
index b97feb6f08..af5a1d3b23 100644
--- a/src/addrman.h
+++ b/src/addrman.h
@@ -266,9 +266,6 @@ protected:
     //! Return a random to-be-evicted tried table address.
     CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs);
 
-    //! Wraps GetRandInt to allow tests to override RandomInt and make it determinismistic.
-    virtual int RandomInt(int nMax);
-
 #ifdef DEBUG_ADDRMAN
     //! Perform consistency check. Returns an error code or zero.
     int Check_() EXCLUSIVE_LOCKS_REQUIRED(cs);
@@ -473,7 +470,7 @@ public:
     {
         LOCK(cs);
         std::vector<int>().swap(vRandom);
-        nKey = GetRandHash();
+        nKey = insecure_rand.rand256();
         for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) {
             for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) {
                 vvNew[bucket][entry] = -1;
diff --git a/src/net.cpp b/src/net.cpp
index fde85b0f2a..e595fb0b0b 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -134,11 +134,12 @@ static std::vector<CAddress> convertSeed6(const std::vector<SeedSpec6> &vSeedsIn
     const int64_t nOneWeek = 7*24*60*60;
     std::vector<CAddress> vSeedsOut;
     vSeedsOut.reserve(vSeedsIn.size());
+    FastRandomContext rng;
     for (const auto& seed_in : vSeedsIn) {
         struct in6_addr ip;
         memcpy(&ip, seed_in.addr, sizeof(ip));
         CAddress addr(CService(ip, seed_in.port), GetDesirableServiceFlags(NODE_NONE));
-        addr.nTime = GetTime() - GetRand(nOneWeek) - nOneWeek;
+        addr.nTime = GetTime() - rng.randrange(nOneWeek) - nOneWeek;
         vSeedsOut.push_back(addr);
     }
     return vSeedsOut;
@@ -189,16 +190,16 @@ void AdvertiseLocal(CNode *pnode)
         // If discovery is enabled, sometimes give our peer the address it
         // tells us that it sees us as in case it has a better idea of our
         // address than we do.
+        FastRandomContext rng;
         if (IsPeerAddrLocalGood(pnode) && (!addrLocal.IsRoutable() ||
-             GetRand((GetnScore(addrLocal) > LOCAL_MANUAL) ? 8:2) == 0))
+             rng.randbits((GetnScore(addrLocal) > LOCAL_MANUAL) ? 3 : 1) == 0))
         {
             addrLocal.SetIP(pnode->GetAddrLocal());
         }
         if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false))
         {
             LogPrint(BCLog::NET, "AdvertiseLocal: advertising address %s\n", addrLocal.ToString());
-            FastRandomContext insecure_rand;
-            pnode->PushAddress(addrLocal, insecure_rand);
+            pnode->PushAddress(addrLocal, rng);
         }
     }
 }
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 40c5112578..0e222bdfa4 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -779,10 +779,11 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
         nNextSweep = nMinExpTime + ORPHAN_TX_EXPIRE_INTERVAL;
         if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx due to expiration\n", nErased);
     }
+    FastRandomContext rng;
     while (mapOrphanTransactions.size() > nMaxOrphans)
     {
         // Evict a random orphan:
-        uint256 randomhash = GetRandHash();
+        uint256 randomhash = rng.rand256();
         std::map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.lower_bound(randomhash);
         if (it == mapOrphanTransactions.end())
             it = mapOrphanTransactions.begin();
diff --git a/src/random.cpp b/src/random.cpp
index a34c70e1d5..f8ffda136d 100644
--- a/src/random.cpp
+++ b/src/random.cpp
@@ -398,6 +398,7 @@ uint256 FastRandomContext::rand256()
 
 std::vector<unsigned char> FastRandomContext::randbytes(size_t len)
 {
+    if (requires_seed) RandomSeed();
     std::vector<unsigned char> ret(len);
     if (len > 0) {
         rng.Output(&ret[0], len);
@@ -463,6 +464,20 @@ FastRandomContext::FastRandomContext(bool fDeterministic) : requires_seed(!fDete
     rng.SetKey(seed.begin(), 32);
 }
 
+FastRandomContext& FastRandomContext::operator=(FastRandomContext&& from) noexcept
+{
+    requires_seed = from.requires_seed;
+    rng = from.rng;
+    std::copy(std::begin(from.bytebuf), std::end(from.bytebuf), std::begin(bytebuf));
+    bytebuf_size = from.bytebuf_size;
+    bitbuf = from.bitbuf;
+    bitbuf_size = from.bitbuf_size;
+    from.requires_seed = true;
+    from.bytebuf_size = 0;
+    from.bitbuf_size = 0;
+    return *this;
+}
+
 void RandomInit()
 {
     RDRandInit();
diff --git a/src/random.h b/src/random.h
index 3d5421eb3e..00e90abbc5 100644
--- a/src/random.h
+++ b/src/random.h
@@ -76,6 +76,14 @@ public:
     /** Initialize with explicit seed (only for testing) */
     explicit FastRandomContext(const uint256& seed);
 
+    // Do not permit copying a FastRandomContext (move it, or create a new one to get reseeded).
+    FastRandomContext(const FastRandomContext&) = delete;
+    FastRandomContext(FastRandomContext&&) = delete;
+    FastRandomContext& operator=(const FastRandomContext&) = delete;
+
+    /** Move a FastRandomContext. If the original one is used again, it will be reseeded. */
+    FastRandomContext& operator=(FastRandomContext&& from) noexcept;
+
     /** Generate a random 64-bit integer. */
     uint64_t rand64()
     {
@@ -130,6 +138,29 @@ public:
     inline uint64_t operator()() { return rand64(); }
 };
 
+/** More efficient than using std::shuffle on a FastRandomContext.
+ *
+ * This is more efficient as std::shuffle will consume entropy in groups of
+ * 64 bits at the time and throw away most.
+ *
+ * This also works around a bug in libstdc++ std::shuffle that may cause
+ * type::operator=(type&&) to be invoked on itself, which the library's
+ * debug mode detects and panics on. This is a known issue, see
+ * https://stackoverflow.com/questions/22915325/avoiding-self-assignment-in-stdshuffle
+ */
+template<typename I, typename R>
+void Shuffle(I first, I last, R&& rng)
+{
+    while (first != last) {
+        size_t j = rng.randrange(last - first);
+        if (j) {
+            using std::swap;
+            swap(*first, *(first + j));
+        }
+        ++first;
+    }
+}
+
 /* Number of random bytes returned by GetOSRand.
  * When changing this constant make sure to change all call sites, and make
  * sure that the underlying OS APIs for all platforms support the number.
diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index 55fe19cebe..12e6ace213 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -32,12 +32,6 @@ public:
         insecure_rand = FastRandomContext(true);
     }
 
-    int RandomInt(int nMax) override
-    {
-        state = (CHashWriter(SER_GETHASH, 0) << state).GetCheapHash();
-        return (unsigned int)(state % nMax);
-    }
-
     CAddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr)
     {
         LOCK(cs);
diff --git a/src/test/cuckoocache_tests.cpp b/src/test/cuckoocache_tests.cpp
index dbceb9d2e0..fea1e1b841 100644
--- a/src/test/cuckoocache_tests.cpp
+++ b/src/test/cuckoocache_tests.cpp
@@ -21,40 +21,23 @@
  *  using BOOST_CHECK_CLOSE to fail.
  *
  */
-FastRandomContext local_rand_ctx(true);
-
 BOOST_AUTO_TEST_SUITE(cuckoocache_tests);
 
-
-/** insecure_GetRandHash fills in a uint256 from local_rand_ctx
- */
-static void insecure_GetRandHash(uint256& t)
-{
-    uint32_t* ptr = (uint32_t*)t.begin();
-    for (uint8_t j = 0; j < 8; ++j)
-        *(ptr++) = local_rand_ctx.rand32();
-}
-
-
-
 /* Test that no values not inserted into the cache are read out of it.
  *
  * There are no repeats in the first 200000 insecure_GetRandHash calls
  */
 BOOST_AUTO_TEST_CASE(test_cuckoocache_no_fakes)
 {
-    local_rand_ctx = FastRandomContext(true);
+    SeedInsecureRand(true);
     CuckooCache::cache<uint256, SignatureCacheHasher> cc{};
     size_t megabytes = 4;
     cc.setup_bytes(megabytes << 20);
-    uint256 v;
     for (int x = 0; x < 100000; ++x) {
-        insecure_GetRandHash(v);
-        cc.insert(v);
+        cc.insert(InsecureRand256());
     }
     for (int x = 0; x < 100000; ++x) {
-        insecure_GetRandHash(v);
-        BOOST_CHECK(!cc.contains(v, false));
+        BOOST_CHECK(!cc.contains(InsecureRand256(), false));
     }
 };
 
@@ -64,7 +47,7 @@ BOOST_AUTO_TEST_CASE(test_cuckoocache_no_fakes)
 template <typename Cache>
 static double test_cache(size_t megabytes, double load)
 {
-    local_rand_ctx = FastRandomContext(true);
+    SeedInsecureRand(true);
     std::vector<uint256> hashes;
     Cache set{};
     size_t bytes = megabytes * (1 << 20);
@@ -74,7 +57,7 @@ static double test_cache(size_t megabytes, double load)
     for (uint32_t i = 0; i < n_insert; ++i) {
         uint32_t* ptr = (uint32_t*)hashes[i].begin();
         for (uint8_t j = 0; j < 8; ++j)
-            *(ptr++) = local_rand_ctx.rand32();
+            *(ptr++) = InsecureRand32();
     }
     /** We make a copy of the hashes because future optimizations of the
      * cuckoocache may overwrite the inserted element, so the test is
@@ -135,7 +118,7 @@ template <typename Cache>
 static void test_cache_erase(size_t megabytes)
 {
     double load = 1;
-    local_rand_ctx = FastRandomContext(true);
+    SeedInsecureRand(true);
     std::vector<uint256> hashes;
     Cache set{};
     size_t bytes = megabytes * (1 << 20);
@@ -145,7 +128,7 @@ static void test_cache_erase(size_t megabytes)
     for (uint32_t i = 0; i < n_insert; ++i) {
         uint32_t* ptr = (uint32_t*)hashes[i].begin();
         for (uint8_t j = 0; j < 8; ++j)
-            *(ptr++) = local_rand_ctx.rand32();
+            *(ptr++) = InsecureRand32();
     }
     /** We make a copy of the hashes because future optimizations of the
      * cuckoocache may overwrite the inserted element, so the test is
@@ -198,7 +181,7 @@ template <typename Cache>
 static void test_cache_erase_parallel(size_t megabytes)
 {
     double load = 1;
-    local_rand_ctx = FastRandomContext(true);
+    SeedInsecureRand(true);
     std::vector<uint256> hashes;
     Cache set{};
     size_t bytes = megabytes * (1 << 20);
@@ -208,7 +191,7 @@ static void test_cache_erase_parallel(size_t megabytes)
     for (uint32_t i = 0; i < n_insert; ++i) {
         uint32_t* ptr = (uint32_t*)hashes[i].begin();
         for (uint8_t j = 0; j < 8; ++j)
-            *(ptr++) = local_rand_ctx.rand32();
+            *(ptr++) = InsecureRand32();
     }
     /** We make a copy of the hashes because future optimizations of the
      * cuckoocache may overwrite the inserted element, so the test is
@@ -300,7 +283,7 @@ static void test_cache_generations()
     // iterations with non-deterministic values, so it isn't "overfit" to the
     // specific entropy in FastRandomContext(true) and implementation of the
     // cache.
-    local_rand_ctx = FastRandomContext(true);
+    SeedInsecureRand(true);
 
     // block_activity models a chunk of network activity. n_insert elements are
     // added to the cache. The first and last n/4 are stored for removal later
@@ -317,7 +300,7 @@ static void test_cache_generations()
             for (uint32_t i = 0; i < n_insert; ++i) {
                 uint32_t* ptr = (uint32_t*)inserts[i].begin();
                 for (uint8_t j = 0; j < 8; ++j)
-                    *(ptr++) = local_rand_ctx.rand32();
+                    *(ptr++) = InsecureRand32();
             }
             for (uint32_t i = 0; i < n_insert / 4; ++i)
                 reads.push_back(inserts[i]);
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 97cf5ed345..5291dfdd4b 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -111,7 +111,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
 
 static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerLogicValidation &peerLogic)
 {
-    CAddress addr(ip(GetRandInt(0xffffffff)), NODE_NONE);
+    CAddress addr(ip(insecure_rand_ctx.randbits(32)), NODE_NONE);
     vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", /*fInboundIn=*/ false));
     CNode &node = *vNodes.back();
     node.SetSendVersion(PROTOCOL_VERSION);
diff --git a/src/test/prevector_tests.cpp b/src/test/prevector_tests.cpp
index c488d3edcf..7341389208 100644
--- a/src/test/prevector_tests.cpp
+++ b/src/test/prevector_tests.cpp
@@ -189,8 +189,8 @@ public:
 
     prevector_tester() {
         SeedInsecureRand();
-        rand_seed = insecure_rand_seed;
-        rand_cache = insecure_rand_ctx;
+        rand_seed = InsecureRand256();
+        rand_cache = FastRandomContext(rand_seed);
     }
 };
 
diff --git a/src/test/random_tests.cpp b/src/test/random_tests.cpp
index 679e857ce6..1057d09471 100644
--- a/src/test/random_tests.cpp
+++ b/src/test/random_tests.cpp
@@ -38,11 +38,18 @@ BOOST_AUTO_TEST_CASE(fastrandom_tests)
     BOOST_CHECK(ctx1.randbytes(50) == ctx2.randbytes(50));
 
     // Check that a nondeterministic ones are not
-    FastRandomContext ctx3;
-    FastRandomContext ctx4;
-    BOOST_CHECK(ctx3.rand64() != ctx4.rand64()); // extremely unlikely to be equal
-    BOOST_CHECK(ctx3.rand256() != ctx4.rand256());
-    BOOST_CHECK(ctx3.randbytes(7) != ctx4.randbytes(7));
+    {
+        FastRandomContext ctx3, ctx4;
+        BOOST_CHECK(ctx3.rand64() != ctx4.rand64()); // extremely unlikely to be equal
+    }
+    {
+        FastRandomContext ctx3, ctx4;
+        BOOST_CHECK(ctx3.rand256() != ctx4.rand256());
+    }
+    {
+        FastRandomContext ctx3, ctx4;
+        BOOST_CHECK(ctx3.randbytes(7) != ctx4.randbytes(7));
+    }
 }
 
 BOOST_AUTO_TEST_CASE(fastrandom_randbits)
@@ -75,8 +82,42 @@ BOOST_AUTO_TEST_CASE(stdrandom_test)
         for (int j = 1; j <= 10; ++j) {
             BOOST_CHECK(std::find(test.begin(), test.end(), j) != test.end());
         }
+        Shuffle(test.begin(), test.end(), ctx);
+        for (int j = 1; j <= 10; ++j) {
+            BOOST_CHECK(std::find(test.begin(), test.end(), j) != test.end());
+        }
     }
 
 }
 
+/** Test that Shuffle reaches every permutation with equal probability. */
+BOOST_AUTO_TEST_CASE(shuffle_stat_test)
+{
+    FastRandomContext ctx(true);
+    uint32_t counts[5 * 5 * 5 * 5 * 5] = {0};
+    for (int i = 0; i < 12000; ++i) {
+        int data[5] = {0, 1, 2, 3, 4};
+        Shuffle(std::begin(data), std::end(data), ctx);
+        int pos = data[0] + data[1] * 5 + data[2] * 25 + data[3] * 125 + data[4] * 625;
+        ++counts[pos];
+    }
+    unsigned int sum = 0;
+    double chi_score = 0.0;
+    for (int i = 0; i < 5 * 5 * 5 * 5 * 5; ++i) {
+        int i1 = i % 5, i2 = (i / 5) % 5, i3 = (i / 25) % 5, i4 = (i / 125) % 5, i5 = i / 625;
+        uint32_t count = counts[i];
+        if (i1 == i2 || i1 == i3 || i1 == i4 || i1 == i5 || i2 == i3 || i2 == i4 || i2 == i5 || i3 == i4 || i3 == i5 || i4 == i5) {
+            BOOST_CHECK(count == 0);
+        } else {
+            chi_score += ((count - 100.0) * (count - 100.0)) / 100.0;
+            BOOST_CHECK(count > 50);
+            BOOST_CHECK(count < 150);
+            sum += count;
+        }
+    }
+    BOOST_CHECK(chi_score > 58.1411); // 99.9999% confidence interval
+    BOOST_CHECK(chi_score < 210.275);
+    BOOST_CHECK_EQUAL(sum, 12000);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp
index f7874e6882..9b4076d956 100644
--- a/src/test/test_bitcoin.cpp
+++ b/src/test/test_bitcoin.cpp
@@ -36,8 +36,7 @@ void CConnmanTest::ClearNodes()
     g_connman->vNodes.clear();
 }
 
-uint256 insecure_rand_seed = GetRandHash();
-FastRandomContext insecure_rand_ctx(insecure_rand_seed);
+FastRandomContext insecure_rand_ctx;
 
 extern bool fPrintToConsole;
 extern void noui_connect();
diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h
index 182571b004..124f862de8 100644
--- a/src/test/test_bitcoin.h
+++ b/src/test/test_bitcoin.h
@@ -26,17 +26,11 @@ std::ostream& operator<<(typename std::enable_if<std::is_enum<T>::value, std::os
     return stream << static_cast<typename std::underlying_type<T>::type>(e);
 }
 
-extern uint256 insecure_rand_seed;
 extern FastRandomContext insecure_rand_ctx;
 
-static inline void SeedInsecureRand(bool fDeterministic = false)
+static inline void SeedInsecureRand(bool deterministic = false)
 {
-    if (fDeterministic) {
-        insecure_rand_seed = uint256();
-    } else {
-        insecure_rand_seed = GetRandHash();
-    }
-    insecure_rand_ctx = FastRandomContext(insecure_rand_seed);
+    insecure_rand_ctx = FastRandomContext(deterministic);
 }
 
 static inline uint32_t InsecureRand32() { return insecure_rand_ctx.rand32(); }
diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
index 4316f37999..fb0b8c0e16 100644
--- a/src/test/validation_block_tests.cpp
+++ b/src/test/validation_block_tests.cpp
@@ -104,8 +104,8 @@ void BuildChain(const uint256& root, int height, const unsigned int invalid_rate
 {
     if (height <= 0 || blocks.size() >= max_size) return;
 
-    bool gen_invalid = GetRand(100) < invalid_rate;
-    bool gen_fork = GetRand(100) < branch_rate;
+    bool gen_invalid = InsecureRandRange(100) < invalid_rate;
+    bool gen_fork = InsecureRandRange(100) < branch_rate;
 
     const std::shared_ptr<const CBlock> pblock = gen_invalid ? BadBlock(root) : GoodBlock(root);
     blocks.push_back(pblock);
@@ -157,7 +157,7 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
         threads.create_thread([&blocks]() {
             bool ignored;
             for (int i = 0; i < 1000; i++) {
-                auto block = blocks[GetRand(blocks.size() - 1)];
+                auto block = blocks[InsecureRandRange(blocks.size() - 1)];
                 ProcessNewBlock(Params(), block, true, &ignored);
             }
 
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 5e955b8495..8a37f374a1 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -223,7 +223,7 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group
     std::vector<OutputGroup> applicable_groups;
     CAmount nTotalLower = 0;
 
-    random_shuffle(groups.begin(), groups.end(), GetRandInt);
+    Shuffle(groups.begin(), groups.end(), FastRandomContext());
 
     for (const OutputGroup& group : groups) {
         if (group.m_value == nTargetValue) {
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 829a1a2478..cea6564b4e 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2462,7 +2462,7 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
         // Cases where we have 11+ outputs all pointing to the same destination may result in
         // privacy leaks as they will potentially be deterministically sorted. We solve that by
         // explicitly shuffling the outputs before processing
-        std::shuffle(vCoins.begin(), vCoins.end(), FastRandomContext());
+        Shuffle(vCoins.begin(), vCoins.end(), FastRandomContext());
     }
     std::vector<OutputGroup> groups = GroupOutputs(vCoins, !coin_control.m_avoid_partial_spends);
 
@@ -2922,7 +2922,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
         // Shuffle selected coins and fill in final vin
         txNew.vin.clear();
         std::vector<CInputCoin> selected_coins(setCoins.begin(), setCoins.end());
-        std::shuffle(selected_coins.begin(), selected_coins.end(), FastRandomContext());
+        Shuffle(selected_coins.begin(), selected_coins.end(), FastRandomContext());
 
         // Note how the sequence number is set to non-maxint so that
         // the nLockTime set above actually works.