Merge #18781: Add templated GetRandDuration<>

0000ea32656833efa3d2ffd9bab66c88c83334f0 test: Add test for GetRandMillis and GetRandMicros (MarcoFalke)
fa0e5b89cf742df56c6c8f49fe9b3c54d2970a66 Add templated GetRandomDuration<> (MarcoFalke)

Pull request description:

  A naive implementation of this template is dangerous, because the call site might accidentally omit the template parameter:

  ```cpp
  template <typename D>
  D GetRandDur(const D& duration_max)
  {
      return D{GetRand(duration_max.count())};
  }

  BOOST_AUTO_TEST_CASE(util_time_GetRandTime)
  {
      std::chrono::seconds rand_hour = GetRandDur(std::chrono::hours{1});
      // Want seconds to be in range [0..1hour), but always get zero :((((
      BOOST_CHECK_EQUAL(rand_hour.count(), 0);
  }
  ```

  Luckily `std::common_type` is already specialised in the standard lib for `std::chrono::duration` (https://en.cppreference.com/w/cpp/chrono/duration/common_type). And its effect seem to be that the call site must always specify the template argument explicitly.

  So instead of implementing the function for each duration type by hand, replace it with a templated version that is safe to use.

ACKs for top commit:
  laanwj:
    Code review ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0
  promag:
    Code review ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0.
  jonatack:
    ACK 0000ea3 thanks for the improved documentation. Code review, built, ran `src/test/test_bitcoin -t random_tests -l test_suite` for the new unit tests, `git diff fa05a4c 0000ea3` since previous review:
  hebasto:
    ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0 with non-blocking [nit](https://github.com/bitcoin/bitcoin/pull/18781#discussion_r424924671).

Tree-SHA512: e89d46e31452be6ea14269ecbbb2cdd9ae83b4412cd14dff7d1084283092722a2f847cb501e8054394e4a3eff852f9c87f6d694fd008b3f7e8458cb5a3068af7

3 files changed, 21 insertions, 18 deletions

diff --git a/src/random.cpp b/src/random.cpp
index 5b8782d1ce..9c9a35709a 100644
--- a/src/random.cpp
+++ b/src/random.cpp
@@ -14,16 +14,14 @@
 #include <wincrypt.h>
 #endif
 #include <logging.h>  // for LogPrintf()
+#include <randomenv.h>
+#include <support/allocators/secure.h>
 #include <sync.h>     // for Mutex
 #include <util/time.h> // for GetTimeMicros()
 
 #include <stdlib.h>
 #include <thread>
 
-#include <randomenv.h>
-
-#include <support/allocators/secure.h>
-
 #ifndef WIN32
 #include <fcntl.h>
 #include <sys/time.h>
@@ -590,16 +588,6 @@ uint64_t GetRand(uint64_t nMax) noexcept
     return FastRandomContext(g_mock_deterministic_tests).randrange(nMax);
 }
 
-std::chrono::microseconds GetRandMicros(std::chrono::microseconds duration_max) noexcept
-{
-    return std::chrono::microseconds{GetRand(duration_max.count())};
-}
-
-std::chrono::milliseconds GetRandMillis(std::chrono::milliseconds duration_max) noexcept
-{
-    return std::chrono::milliseconds{GetRand(duration_max.count())};
-}
-
 int GetRandInt(int nMax) noexcept
 {
     return GetRand(nMax);
diff --git a/src/random.h b/src/random.h
index 690125079b..0c6dc24983 100644
--- a/src/random.h
+++ b/src/random.h
@@ -67,9 +67,21 @@
  * Thread-safe.
  */
 void GetRandBytes(unsigned char* buf, int num) noexcept;
+/** Generate a uniform random integer in the range [0..range). Precondition: range > 0 */
 uint64_t GetRand(uint64_t nMax) noexcept;
-std::chrono::microseconds GetRandMicros(std::chrono::microseconds duration_max) noexcept;
-std::chrono::milliseconds GetRandMillis(std::chrono::milliseconds duration_max) noexcept;
+/** Generate a uniform random duration in the range [0..max). Precondition: max.count() > 0 */
+template <typename D>
+D GetRandomDuration(typename std::common_type<D>::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<std::chrono::microseconds>;
+constexpr auto GetRandMillis = GetRandomDuration<std::chrono::milliseconds>;
 int GetRandInt(int nMax) noexcept;
 uint256 GetRandHash() noexcept;
 
diff --git a/src/test/random_tests.cpp b/src/test/random_tests.cpp
index d1f60e8972..978a7bee4d 100644
--- a/src/test/random_tests.cpp
+++ b/src/test/random_tests.cpp
@@ -28,6 +28,8 @@ BOOST_AUTO_TEST_CASE(fastrandom_tests)
     for (int i = 10; i > 0; --i) {
         BOOST_CHECK_EQUAL(GetRand(std::numeric_limits<uint64_t>::max()), uint64_t{10393729187455219830U});
         BOOST_CHECK_EQUAL(GetRandInt(std::numeric_limits<int>::max()), int{769702006});
+        BOOST_CHECK_EQUAL(GetRandMicros(std::chrono::hours{1}).count(), 2917185654);
+        BOOST_CHECK_EQUAL(GetRandMillis(std::chrono::hours{1}).count(), 2144374);
     }
     BOOST_CHECK_EQUAL(ctx1.rand32(), ctx2.rand32());
     BOOST_CHECK_EQUAL(ctx1.rand32(), ctx2.rand32());
@@ -47,6 +49,8 @@ BOOST_AUTO_TEST_CASE(fastrandom_tests)
     for (int i = 10; i > 0; --i) {
         BOOST_CHECK(GetRand(std::numeric_limits<uint64_t>::max()) != uint64_t{10393729187455219830U});
         BOOST_CHECK(GetRandInt(std::numeric_limits<int>::max()) != int{769702006});
+        BOOST_CHECK(GetRandMicros(std::chrono::hours{1}) != std::chrono::microseconds{2917185654});
+        BOOST_CHECK(GetRandMillis(std::chrono::hours{1}) != std::chrono::milliseconds{2144374});
     }
     {
         FastRandomContext ctx3, ctx4;
@@ -87,7 +91,7 @@ BOOST_AUTO_TEST_CASE(stdrandom_test)
         BOOST_CHECK(x >= 3);
         BOOST_CHECK(x <= 9);
 
-        std::vector<int> test{1,2,3,4,5,6,7,8,9,10};
+        std::vector<int> test{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
         std::shuffle(test.begin(), test.end(), ctx);
         for (int j = 1; j <= 10; ++j) {
             BOOST_CHECK(std::find(test.begin(), test.end(), j) != test.end());
@@ -97,7 +101,6 @@ BOOST_AUTO_TEST_CASE(stdrandom_test)
             BOOST_CHECK(std::find(test.begin(), test.end(), j) != test.end());
         }
     }
-
 }
 
 /** Test that Shuffle reaches every permutation with equal probability. */