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

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>
 #endif
@@ -582,16 +580,6 @@ static void ProcRand(unsigned char* out, int num, RNGLevel level) noexcept
     }
 }
 
-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())};
-}
-
 void GetRandBytes(unsigned char* buf, int num) noexcept { ProcRand(buf, num, RNGLevel::FAST); }
 void GetStrongRandBytes(unsigned char* buf, int num) noexcept { ProcRand(buf, num, RNGLevel::SLOW); }
 void RandAddPeriodic() noexcept { ProcRand(nullptr, 0, RNGLevel::PERIODIC); }

src/random.h

@@ -70,9 +70,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;
 

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);
     }
     {
         constexpr SteadySeconds time_point{1s};
@@ -67,6 +69,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});
     }
 
     {
@@ -108,7 +112,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());
@@ -118,7 +122,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. */