From e2d1f84858485650ff743753ffa5c679f210a992 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 10 Mar 2024 23:38:31 -0400 Subject: [PATCH] random: make GetRand() support entire range (incl. max) The existing code uses GetRand(nMax), with a default value for nMax, where nMax is the range of values (not the maximum!) that the output is allowed to take. This will always miss the last possible value (e.g. GetRand() will never return 0xffffffff). Fix this, by moving the functionality largely in RandomMixin, and also adding a separate RandomMixin::rand function, which returns a value in the entire (non-negative) range of an integer. --- src/random.cpp | 5 --- src/random.h | 78 +++++++++++++++++++++++++------------------ src/test/util/net.cpp | 2 +- 3 files changed, 46 insertions(+), 39 deletions(-) diff --git a/src/random.cpp b/src/random.cpp index 95e4806aa43..5067bf259c3 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -679,11 +679,6 @@ void RandAddPeriodic() noexcept void RandAddEvent(const uint32_t event_info) noexcept { GetRNGState().AddEvent(event_info); } -uint64_t GetRandInternal(uint64_t nMax) noexcept -{ - return FastRandomContext().randrange(nMax); -} - uint256 GetRandHash() noexcept { uint256 hash; diff --git a/src/random.h b/src/random.h index fb83eebbf2f..afbae0cec3b 100644 --- a/src/random.h +++ b/src/random.h @@ -80,31 +80,6 @@ * Thread-safe. */ void GetRandBytes(Span bytes) noexcept; -/** Generate a uniform random integer in the range [0..range). Precondition: range > 0 */ -uint64_t GetRandInternal(uint64_t nMax) noexcept; -/** Generate a uniform random integer of type T in the range [0..nMax) - * nMax defaults to std::numeric_limits::max() - * Precondition: nMax > 0, T is an integral type, no larger than uint64_t - */ -template -T GetRand(T nMax=std::numeric_limits::max()) noexcept { - static_assert(std::is_integral(), "T must be integral"); - static_assert(std::numeric_limits::max() <= std::numeric_limits::max(), "GetRand only supports up to uint64_t"); - return T(GetRandInternal(nMax)); -} -/** Generate a uniform random duration in the range [0..max). Precondition: max.count() > 0 */ -template -D GetRandomDuration(typename std::common_type::type max) noexcept -// Having the compiler infer the template argument from the function argument -// is dangerous, because the desired return value generally has a different -// type than the function argument. So std::common_type is used to force the -// call site to specify the type of the return value. -{ - assert(max.count() > 0); - return D{GetRand(max.count())}; -}; -constexpr auto GetRandMicros = GetRandomDuration; -constexpr auto GetRandMillis = GetRandomDuration; /** * Return a timestamp in the future sampled from an exponential distribution @@ -251,17 +226,17 @@ public: } } - /** Generate a random integer in the range [0..range). - * Precondition: range > 0. - */ - uint64_t randrange(uint64_t range) noexcept + /** Generate a random integer in the range [0..range), with range > 0. */ + template + I randrange(I range) noexcept { - assert(range); - --range; - int bits = std::bit_width(range); + static_assert(std::numeric_limits::max() <= std::numeric_limits::max()); + Assume(range > 0); + uint64_t maxval = range - 1U; + int bits = std::bit_width(maxval); while (true) { uint64_t ret = Impl().randbits(bits); - if (ret <= range) return ret; + if (ret <= maxval) return ret; } } @@ -284,6 +259,16 @@ public: } } + /** Generate a random integer in its entire (non-negative) range. */ + template + I rand() noexcept + { + static_assert(std::numeric_limits::max() <= std::numeric_limits::max()); + static constexpr auto BITS = std::bit_width(uint64_t(std::numeric_limits::max())); + static_assert(std::numeric_limits::max() == std::numeric_limits::max() >> (64 - BITS)); + return I(Impl().template randbits()); + } + /** Generate random bytes. */ template std::vector randbytes(size_t len) noexcept @@ -441,6 +426,33 @@ void Shuffle(I first, I last, R&& rng) } } +/** Generate a uniform random integer of type T in the range [0..nMax) + * Precondition: nMax > 0, T is an integral type, no larger than uint64_t + */ +template +T GetRand(T nMax) noexcept { + return T(FastRandomContext().randrange(nMax)); +} + +/** Generate a uniform random integer of type T in its entire non-negative range. */ +template +T GetRand() noexcept { + return T(FastRandomContext().rand()); +} + +/** Generate a uniform random duration in the range [0..max). Precondition: max.count() > 0 */ +template +D GetRandomDuration(typename std::common_type::type max) noexcept +// Having the compiler infer the template argument from the function argument +// is dangerous, because the desired return value generally has a different +// type than the function argument. So std::common_type is used to force the +// call site to specify the type of the return value. +{ + return D{GetRand(max.count())}; +}; +constexpr auto GetRandMicros = GetRandomDuration; +constexpr auto GetRandMillis = GetRandomDuration; + /* 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/util/net.cpp b/src/test/util/net.cpp index 9257a4964ac..9b38d85d582 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -126,7 +126,7 @@ std::vector GetRandomNodeEvictionCandidates(int n_candida /*fRelevantServices=*/random_context.randbool(), /*m_relay_txs=*/random_context.randbool(), /*fBloomFilter=*/random_context.randbool(), - /*nKeyedNetGroup=*/random_context.randrange(100), + /*nKeyedNetGroup=*/random_context.randrange(100u), /*prefer_evict=*/random_context.randbool(), /*m_is_local=*/random_context.randbool(), /*m_network=*/ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())],