random: convert XoRoShiRo128PlusPlus into full RNG

Convert XoRoShiRo128PlusPlus into a full RandomMixin-based RNG class,
providing all utility functionality that FastRandomContext has. In doing so,
it is renamed to InsecureRandomContext, highlighting its non-cryptographic
nature.

To do this, a fillrand fallback is added to RandomMixin (where it is used by
InsecureRandomContext), but FastRandomContext still uses its own fillrand.

src/random.h

@@ -147,8 +147,6 @@ template<typename T>
 concept RandomNumberGenerator = requires(T& rng, Span<std::byte> s) {
     // A random number generator must provide rand64().
     { rng.rand64() } noexcept -> std::same_as<uint64_t>;
-    // A random number generator must provide randfill(Span<std::byte>).
-    { rng.fillrand(s) } noexcept;
     // A random number generator must derive from RandomMixin, which adds other rand* functions.
     requires std::derived_from<std::remove_reference_t<T>, RandomMixin<std::remove_reference_t<T>>>;
 };
@@ -261,6 +259,25 @@ public:
         }
     }
 
+    /** Fill a Span with random bytes. */
+    void fillrand(Span<std::byte> span) noexcept
+    {
+        while (span.size() >= 8) {
+            uint64_t gen = Impl().rand64();
+            WriteLE64(UCharCast(span.data()), gen);
+            span = span.subspan(8);
+        }
+        if (span.size() >= 4) {
+            uint32_t gen = Impl().rand32();
+            WriteLE32(UCharCast(span.data()), gen);
+            span = span.subspan(4);
+        }
+        while (span.size()) {
+            span[0] = std::byte(Impl().template randbits<8>());
+            span = span.subspan(1);
+        }
+    }
+
     /** Generate random bytes. */
     template <BasicByte B = unsigned char>
     std::vector<B> randbytes(size_t len) noexcept
@@ -345,19 +362,19 @@ public:
         return ReadLE64(UCharCast(buf.data()));
     }
 
-    /** Fill a byte Span with random bytes. */
+    /** Fill a byte Span with random bytes. This overrides the RandomMixin version. */
     void fillrand(Span<std::byte> output) noexcept;
 };
 
 /** xoroshiro128++ PRNG. Extremely fast, not appropriate for cryptographic purposes.
  *
- * Memory footprint is 128bit, period is 2^128 - 1.
+ * Memory footprint is very small, period is 2^128 - 1.
  * This class is not thread-safe.
  *
  * Reference implementation available at https://prng.di.unimi.it/xoroshiro128plusplus.c
  * See https://prng.di.unimi.it/
  */
-class XoRoShiRo128PlusPlus
+class InsecureRandomContext : public RandomMixin<InsecureRandomContext>
 {
     uint64_t m_s0;
     uint64_t m_s1;
@@ -371,21 +388,19 @@ class XoRoShiRo128PlusPlus
     }
 
 public:
-    using result_type = uint64_t;
+    constexpr explicit InsecureRandomContext(uint64_t seedval) noexcept
-
-    constexpr explicit XoRoShiRo128PlusPlus(uint64_t seedval) noexcept
         : m_s0(SplitMix64(seedval)), m_s1(SplitMix64(seedval)) {}
 
     // no copy - that is dangerous, we don't want accidentally copy the RNG and then have two streams
     // with exactly the same results.
-    XoRoShiRo128PlusPlus(const XoRoShiRo128PlusPlus&) = delete;
+    InsecureRandomContext(const InsecureRandomContext&) = delete;
-    XoRoShiRo128PlusPlus& operator=(const XoRoShiRo128PlusPlus&) = delete;
+    InsecureRandomContext& operator=(const InsecureRandomContext&) = delete;
 
     // allow moves
-    XoRoShiRo128PlusPlus(XoRoShiRo128PlusPlus&&) = default;
+    InsecureRandomContext(InsecureRandomContext&&) = default;
-    XoRoShiRo128PlusPlus& operator=(XoRoShiRo128PlusPlus&&) = default;
+    InsecureRandomContext& operator=(InsecureRandomContext&&) = default;
 
-    constexpr result_type operator()() noexcept
+    constexpr uint64_t rand64() noexcept
     {
         uint64_t s0 = m_s0, s1 = m_s1;
         const uint64_t result = std::rotl(s0 + s1, 17) + s0;
@@ -394,10 +409,6 @@ public:
         m_s1 = std::rotl(s1, 28);
         return result;
     }
-
-    static constexpr result_type min() noexcept { return std::numeric_limits<result_type>::min(); }
-    static constexpr result_type max() noexcept { return std::numeric_limits<result_type>::max(); }
-    static constexpr double entropy() noexcept { return 0.0; }
 };
 
 /** More efficient than using std::shuffle on a FastRandomContext.

src/test/fuzz/bip324.cpp

@@ -56,7 +56,7 @@ FUZZ_TARGET(bip324_cipher_roundtrip, .init=Initialize)
     // (potentially buggy) edge cases triggered by specific values of contents/AAD, so we can avoid
     // reading the actual data for those from the fuzzer input (which would need large amounts of
     // data).
-    XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral<uint64_t>());
+    InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>());
 
     // Compare session IDs and garbage terminators.
     assert(initiator.GetSessionID() == responder.GetSessionID());
@@ -79,10 +79,8 @@ FUZZ_TARGET(bip324_cipher_roundtrip, .init=Initialize)
         unsigned length_bits = 2 * ((mode >> 5) & 7);
         unsigned length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << length_bits) - 1);
         // Generate aad and content.
-        std::vector<std::byte> aad(aad_length);
+        auto aad = rng.randbytes<std::byte>(aad_length);
-        for (auto& val : aad) val = std::byte{(uint8_t)rng()};
+        auto contents = rng.randbytes<std::byte>(length);
-        std::vector<std::byte> contents(length);
-        for (auto& val : contents) val = std::byte{(uint8_t)rng()};
 
         // Pick sides.
         auto& sender{from_init ? initiator : responder};

src/test/fuzz/bitset.cpp

@@ -29,7 +29,7 @@ void TestType(FuzzBufferType buffer)
      *  bitsets and their simulations do not matter for the purpose of detecting edge cases, thus
      *  these are taken from a deterministically-seeded RNG instead. To provide some level of
      *  variation however, pick the seed based on the buffer size and size of the chosen bitset. */
-    XoRoShiRo128PlusPlus rng(buffer.size() + 0x10000 * S::Size());
+    InsecureRandomContext rng(buffer.size() + 0x10000 * S::Size());
 
     using Sim = std::bitset<S::Size()>;
     // Up to 4 real BitSets (initially 2).
@@ -124,7 +124,7 @@ void TestType(FuzzBufferType buffer)
                 sim[dest].reset();
                 real[dest] = S{};
                 for (unsigned i = 0; i < S::Size(); ++i) {
-                    if (rng() & 1) {
+                    if (rng.randbool()) {
                         sim[dest][i] = true;
                         real[dest].Set(i);
                     }
@@ -132,9 +132,9 @@ void TestType(FuzzBufferType buffer)
                 break;
             } else if (dest < sim.size() && command-- == 0) {
                 /* Assign initializer list. */
-                unsigned r1 = rng() % S::Size();
+                unsigned r1 = rng.randrange(S::Size());
-                unsigned r2 = rng() % S::Size();
+                unsigned r2 = rng.randrange(S::Size());
-                unsigned r3 = rng() % S::Size();
+                unsigned r3 = rng.randrange(S::Size());
                 compare_fn(dest);
                 sim[dest].reset();
                 real[dest] = {r1, r2, r3};
@@ -166,8 +166,8 @@ void TestType(FuzzBufferType buffer)
                 break;
             } else if (sim.size() < 4 && command-- == 0) {
                 /* Construct with initializer list. */
-                unsigned r1 = rng() % S::Size();
+                unsigned r1 = rng.randrange(S::Size());
-                unsigned r2 = rng() % S::Size();
+                unsigned r2 = rng.randrange(S::Size());
                 sim.emplace_back();
                 sim.back().set(r1);
                 sim.back().set(r2);

src/test/fuzz/crypto_chacha20.cpp

@@ -53,7 +53,7 @@ namespace
     once for a large block at once, and then the same data in chunks, comparing
     the outcome.
 
-    If UseCrypt, seeded Xoroshiro128++ output is used as input to Crypt().
+    If UseCrypt, seeded InsecureRandomContext output is used as input to Crypt().
     If not, Keystream() is used directly, or sequences of 0x00 are encrypted.
 */
 template<bool UseCrypt>
@@ -78,25 +78,11 @@ void ChaCha20SplitFuzz(FuzzedDataProvider& provider)
     data1.resize(total_bytes);
     data2.resize(total_bytes);
 
-    // If using Crypt(), initialize data1 and data2 with the same Xoroshiro128++ based
+    // If using Crypt(), initialize data1 and data2 with the same InsecureRandomContext based
     // stream.
     if constexpr (UseCrypt) {
-        uint64_t seed = provider.ConsumeIntegral<uint64_t>();
+        InsecureRandomContext(provider.ConsumeIntegral<uint64_t>()).fillrand(data1);
-        XoRoShiRo128PlusPlus rng(seed);
+        std::copy(data1.begin(), data1.end(), data2.begin());
-        uint64_t bytes = 0;
-        while (bytes < (total_bytes & ~uint64_t{7})) {
-            uint64_t val = rng();
-            WriteLE64(UCharCast(data1.data() + bytes), val);
-            WriteLE64(UCharCast(data2.data() + bytes), val);
-            bytes += 8;
-        }
-        if (bytes < total_bytes) {
-            std::byte valbytes[8];
-            uint64_t val = rng();
-            WriteLE64(UCharCast(valbytes), val);
-            std::copy(valbytes, valbytes + (total_bytes - bytes), data1.data() + bytes);
-            std::copy(valbytes, valbytes + (total_bytes - bytes), data2.data() + bytes);
-        }
     }
 
     // Whether UseCrypt is used or not, the two byte arrays must match.

src/test/fuzz/p2p_transport_serialization.cpp

@@ -103,7 +103,7 @@ FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serial
 
 namespace {
 
-template<typename R>
+template<RandomNumberGenerator R>
 void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDataProvider& provider)
 {
     // Simulation test with two Transport objects, which send messages to each other, with
@@ -164,8 +164,7 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa
         // Determine size of message to send (limited to 75 kB for performance reasons).
         size_t size = provider.ConsumeIntegralInRange<uint32_t>(0, 75000);
         // Get payload of message from RNG.
-        msg.data.resize(size);
+        msg.data = rng.randbytes(size);
-        for (auto& v : msg.data) v = uint8_t(rng());
         // Return.
         return msg;
     };
@@ -336,7 +335,7 @@ std::unique_ptr<Transport> MakeV1Transport(NodeId nodeid) noexcept
     return std::make_unique<V1Transport>(nodeid);
 }
 
-template<typename RNG>
+template<RandomNumberGenerator RNG>
 std::unique_ptr<Transport> MakeV2Transport(NodeId nodeid, bool initiator, RNG& rng, FuzzedDataProvider& provider)
 {
     // Retrieve key
@@ -352,8 +351,7 @@ std::unique_ptr<Transport> MakeV2Transport(NodeId nodeid, bool initiator, RNG& r
     } else {
         // If it's longer, generate it from the RNG. This avoids having large amounts of
         // (hopefully) irrelevant data needing to be stored in the fuzzer data.
-        garb.resize(garb_len);
+        garb = rng.randbytes(garb_len);
-        for (auto& v : garb) v = uint8_t(rng());
     }
     // Retrieve entropy
     auto ent = provider.ConsumeBytes<std::byte>(32);
@@ -377,7 +375,7 @@ FUZZ_TARGET(p2p_transport_bidirectional, .init = initialize_p2p_transport_serial
 {
     // Test with two V1 transports talking to each other.
     FuzzedDataProvider provider{buffer.data(), buffer.size()};
-    XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral<uint64_t>());
+    InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>());
     auto t1 = MakeV1Transport(NodeId{0});
     auto t2 = MakeV1Transport(NodeId{1});
     if (!t1 || !t2) return;
@@ -388,7 +386,7 @@ FUZZ_TARGET(p2p_transport_bidirectional_v2, .init = initialize_p2p_transport_ser
 {
     // Test with two V2 transports talking to each other.
     FuzzedDataProvider provider{buffer.data(), buffer.size()};
-    XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral<uint64_t>());
+    InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>());
     auto t1 = MakeV2Transport(NodeId{0}, true, rng, provider);
     auto t2 = MakeV2Transport(NodeId{1}, false, rng, provider);
     if (!t1 || !t2) return;
@@ -399,7 +397,7 @@ FUZZ_TARGET(p2p_transport_bidirectional_v1v2, .init = initialize_p2p_transport_s
 {
     // Test with a V1 initiator talking to a V2 responder.
     FuzzedDataProvider provider{buffer.data(), buffer.size()};
-    XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral<uint64_t>());
+    InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>());
     auto t1 = MakeV1Transport(NodeId{0});
     auto t2 = MakeV2Transport(NodeId{1}, false, rng, provider);
     if (!t1 || !t2) return;

src/test/fuzz/poolresource.cpp

@@ -71,41 +71,14 @@ public:
 
     void RandomContentFill(Entry& entry)
     {
-        XoRoShiRo128PlusPlus rng(entry.seed);
+        InsecureRandomContext(entry.seed).fillrand(entry.span);
-        auto ptr = entry.span.data();
-        auto size = entry.span.size();
-
-        while (size >= 8) {
-            auto r = rng();
-            std::memcpy(ptr, &r, 8);
-            size -= 8;
-            ptr += 8;
-        }
-        if (size > 0) {
-            auto r = rng();
-            std::memcpy(ptr, &r, size);
-        }
     }
 
     void RandomContentCheck(const Entry& entry)
     {
-        XoRoShiRo128PlusPlus rng(entry.seed);
+        std::vector<std::byte> expect(entry.span.size());
-        auto ptr = entry.span.data();
+        InsecureRandomContext(entry.seed).fillrand(expect);
-        auto size = entry.span.size();
+        assert(entry.span == expect);
-
-        std::byte buf[8];
-        while (size >= 8) {
-            auto r = rng();
-            std::memcpy(buf, &r, 8);
-            assert(std::memcmp(buf, ptr, 8) == 0);
-            size -= 8;
-            ptr += 8;
-        }
-        if (size > 0) {
-            auto r = rng();
-            std::memcpy(buf, &r, size);
-            assert(std::memcmp(buf, ptr, size) == 0);
-        }
     }
 
     void Deallocate(const Entry& entry)

src/test/fuzz/vecdeque.cpp

@@ -28,7 +28,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
 {
     FuzzedDataProvider provider(buffer.data(), buffer.size());
     // Local RNG, only used for the seeds to initialize T objects with.
-    XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral<uint64_t>() ^ rng_tweak);
+    InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>() ^ rng_tweak);
 
     // Real circular buffers.
     std::vector<VecDeque<T>> real;
@@ -175,7 +175,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
             }
             if (existing_buffer_non_full && command-- == 0) {
                 /* push_back() (copying) */
-                tmp = T(rng());
+                tmp = T(rng.rand64());
                 size_t old_size = real[idx].size();
                 size_t old_cap = real[idx].capacity();
                 real[idx].push_back(*tmp);
@@ -191,7 +191,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
             }
             if (existing_buffer_non_full && command-- == 0) {
                 /* push_back() (moving) */
-                tmp = T(rng());
+                tmp = T(rng.rand64());
                 size_t old_size = real[idx].size();
                 size_t old_cap = real[idx].capacity();
                 sim[idx].push_back(*tmp);
@@ -207,7 +207,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
             }
             if (existing_buffer_non_full && command-- == 0) {
                 /* emplace_back() */
-                uint64_t seed{rng()};
+                uint64_t seed{rng.rand64()};
                 size_t old_size = real[idx].size();
                 size_t old_cap = real[idx].capacity();
                 sim[idx].emplace_back(seed);
@@ -223,7 +223,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
             }
             if (existing_buffer_non_full && command-- == 0) {
                 /* push_front() (copying) */
-                tmp = T(rng());
+                tmp = T(rng.rand64());
                 size_t old_size = real[idx].size();
                 size_t old_cap = real[idx].capacity();
                 real[idx].push_front(*tmp);
@@ -239,7 +239,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
             }
             if (existing_buffer_non_full && command-- == 0) {
                 /* push_front() (moving) */
-                tmp = T(rng());
+                tmp = T(rng.rand64());
                 size_t old_size = real[idx].size();
                 size_t old_cap = real[idx].capacity();
                 sim[idx].push_front(*tmp);
@@ -255,7 +255,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
             }
             if (existing_buffer_non_full && command-- == 0) {
                 /* emplace_front() */
-                uint64_t seed{rng()};
+                uint64_t seed{rng.rand64()};
                 size_t old_size = real[idx].size();
                 size_t old_cap = real[idx].capacity();
                 sim[idx].emplace_front(seed);
@@ -271,7 +271,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
             }
             if (existing_buffer_non_empty && command-- == 0) {
                 /* front() [modifying] */
-                tmp = T(rng());
+                tmp = T(rng.rand64());
                 size_t old_size = real[idx].size();
                 assert(sim[idx].front() == real[idx].front());
                 sim[idx].front() = *tmp;
@@ -281,7 +281,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
             }
             if (existing_buffer_non_empty && command-- == 0) {
                 /* back() [modifying] */
-                tmp = T(rng());
+                tmp = T(rng.rand64());
                 size_t old_size = real[idx].size();
                 assert(sim[idx].back() == real[idx].back());
                 sim[idx].back() = *tmp;
@@ -291,7 +291,7 @@ void TestType(Span<const uint8_t> buffer, uint64_t rng_tweak)
             }
             if (existing_buffer_non_empty && command-- == 0) {
                 /* operator[] [modifying] */
-                tmp = T(rng());
+                tmp = T(rng.rand64());
                 size_t pos = provider.ConsumeIntegralInRange<size_t>(0, sim[idx].size() - 1);
                 size_t old_size = real[idx].size();
                 assert(sim[idx][pos] == real[idx][pos]);

src/test/random_tests.cpp

@@ -222,14 +222,14 @@ BOOST_AUTO_TEST_CASE(shuffle_stat_test)
 BOOST_AUTO_TEST_CASE(xoroshiro128plusplus_reference_values)
 {
     // numbers generated from reference implementation
-    XoRoShiRo128PlusPlus rng(0);
+    InsecureRandomContext rng(0);
     BOOST_TEST(0x6f68e1e7e2646ee1 == rng());
     BOOST_TEST(0xbf971b7f454094ad == rng());
     BOOST_TEST(0x48f2de556f30de38 == rng());
     BOOST_TEST(0x6ea7c59f89bbfc75 == rng());
 
     // seed with a random number
-    rng = XoRoShiRo128PlusPlus(0x1a26f3fa8546b47a);
+    rng = InsecureRandomContext(0x1a26f3fa8546b47a);
     BOOST_TEST(0xc8dc5e08d844ac7d == rng());
     BOOST_TEST(0x5b5f1f6d499dad1b == rng());
     BOOST_TEST(0xbeb0031f93313d6f == rng());

test/sanitizer_suppressions/ubsan

@@ -57,8 +57,8 @@ unsigned-integer-overflow:CBlockPolicyEstimator::processBlockTx
 unsigned-integer-overflow:TxConfirmStats::EstimateMedianVal
 unsigned-integer-overflow:prevector.h
 unsigned-integer-overflow:EvalScript
-unsigned-integer-overflow:XoRoShiRo128PlusPlus::operator()
+unsigned-integer-overflow:InsecureRandomContext::rand64
-unsigned-integer-overflow:XoRoShiRo128PlusPlus::SplitMix64
+unsigned-integer-overflow:InsecureRandomContext::SplitMix64
 unsigned-integer-overflow:bitset_detail::PopCount
 implicit-integer-sign-change:CBlockPolicyEstimator::processBlockTx
 implicit-integer-sign-change:SetStdinEcho
@@ -76,6 +76,6 @@ shift-base:arith_uint256.cpp
 shift-base:crypto/
 shift-base:streams.h
 shift-base:FormatHDKeypath
-shift-base:XoRoShiRo128PlusPlus::operator()
+shift-base:InsecureRandomContext::rand64
 shift-base:RandomMixin<*>::randbits
 shift-base:RandomMixin<*>::randbits<*>