refactor: make GetRand a template, remove GetRandInt

bitcoin · Jan 31, 2022 · 79deab2 · 79deab2
1 parent 51ad2b6
commit 79deab2
Show file tree

Hide file tree

Showing 13 changed files with 36 additions and 33 deletions.
diff --git a/src/addrdb.cpp b/src/addrdb.cpp
@@ -47,8 +47,7 @@ template <typename Data>
 bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data& data, int version)
 {
     // Generate random temporary filename
-    uint16_t randv = 0;
-    GetRandBytes({(unsigned char*)&randv, sizeof(randv)});
+    uint16_t randv = GetRand<uint16_t>();
     std::string tmpfn = strprintf("%s.%04x", prefix, randv);
 
     // open temp output file, and associate with CAutoFile

diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp
@@ -17,7 +17,7 @@
 #include <unordered_map>
 
 CBlockHeaderAndShortTxIDs::CBlockHeaderAndShortTxIDs(const CBlock& block, bool fUseWTXID) :
-        nonce(GetRand(std::numeric_limits<uint64_t>::max())),
+        nonce(GetRand<uint64_t>()),
         shorttxids(block.vtx.size() - 1), prefilledtxn(1), header(block) {
     FillShortTxIDSelector();
     //TODO: Use our mempool prior to block acceptance to predictively fill more than just the coinbase

diff --git a/src/common/bloom.cpp b/src/common/bloom.cpp
@@ -239,7 +239,7 @@ bool CRollingBloomFilter::contains(Span<const unsigned char> vKey) const
 
 void CRollingBloomFilter::reset()
 {
-    nTweak = GetRand(std::numeric_limits<unsigned int>::max());
+    nTweak = GetRand<unsigned int>();
     nEntriesThisGeneration = 0;
     nGeneration = 1;
     std::fill(data.begin(), data.end(), 0);

diff --git a/src/init.cpp b/src/init.cpp
@@ -1274,7 +1274,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     assert(!node.banman);
     node.banman = std::make_unique<BanMan>(gArgs.GetDataDirNet() / "banlist", &uiInterface, args.GetIntArg("-bantime", DEFAULT_MISBEHAVING_BANTIME));
     assert(!node.connman);
-    node.connman = std::make_unique<CConnman>(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max()), *node.addrman, args.GetBoolArg("-networkactive", true));
+    node.connman = std::make_unique<CConnman>(GetRand<uint64_t>(), GetRand<uint64_t>(), *node.addrman, args.GetBoolArg("-networkactive", true));
 
     assert(!node.fee_estimator);
     // Don't initialize fee estimation with old data if we don't relay transactions,

diff --git a/src/net.cpp b/src/net.cpp
@@ -2115,7 +2115,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
 
             if (fFeeler) {
                 // Add small amount of random noise before connection to avoid synchronization.
-                int randsleep = GetRandInt(FEELER_SLEEP_WINDOW * 1000);
+                int randsleep = GetRand<int>(FEELER_SLEEP_WINDOW * 1000);
                 if (!interruptNet.sleep_for(std::chrono::milliseconds(randsleep)))
                     return;
                 LogPrint(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToString());

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -4414,10 +4414,10 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic
     }
 
     if (pingSend) {
-        uint64_t nonce = 0;
-        while (nonce == 0) {
-            GetRandBytes({(unsigned char*)&nonce, sizeof(nonce)});
-        }
+        uint64_t nonce;
+        do {
+            nonce = GetRand<uint64_t>();
+        } while (nonce == 0);
         peer.m_ping_queued = false;
         peer.m_ping_start = now;
         if (node_to.GetCommonVersion() > BIP0031_VERSION) {

diff --git a/src/netaddress.h b/src/netaddress.h
@@ -572,8 +572,8 @@ class CServiceHash
     }
 
 private:
-    const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max());
-    const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max());
+    const uint64_t m_salt_k0 = GetRand<uint64_t>();
+    const uint64_t m_salt_k1 = GetRand<uint64_t>();
 };
 
 #endif // BITCOIN_NETADDRESS_H
diff --git a/src/random.cpp b/src/random.cpp
@@ -590,16 +590,11 @@ void RandAddEvent(const uint32_t event_info) noexcept { GetRNGState().AddEvent(e
 
 bool g_mock_deterministic_tests{false};
 
-uint64_t GetRand(uint64_t nMax) noexcept
+uint64_t GetRandInternal(uint64_t nMax) noexcept
 {
     return FastRandomContext(g_mock_deterministic_tests).randrange(nMax);
 }
 
-int GetRandInt(int nMax) noexcept
-{
-    return GetRand(nMax);
-}
-
 uint256 GetRandHash() noexcept
 {
     uint256 hash;

diff --git a/src/random.h b/src/random.h
@@ -68,7 +68,17 @@
  */
 void GetRandBytes(Span<unsigned char> bytes) noexcept;
 /** Generate a uniform random integer in the range [0..range). Precondition: range > 0 */
-uint64_t GetRand(uint64_t nMax) noexcept;
+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<T>::max()
+ *  Precondition: nMax > 0, T is an integral type, no larger than uint64_t
+ */
+template<typename T>
+T GetRand(T nMax=std::numeric_limits<T>::max()) noexcept {
+    static_assert(std::is_integral<T>(), "T must be integral");
+    static_assert(std::numeric_limits<T>::max() <= std::numeric_limits<uint64_t>::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 <typename D>
 D GetRandomDuration(typename std::common_type<D>::type max) noexcept
@@ -94,7 +104,6 @@ constexpr auto GetRandMillis = GetRandomDuration<std::chrono::milliseconds>;
  * */
 std::chrono::microseconds GetExponentialRand(std::chrono::microseconds now, std::chrono::seconds average_interval);
 
-int GetRandInt(int nMax) noexcept;
 uint256 GetRandHash() noexcept;
 
 /**

diff --git a/src/test/random_tests.cpp b/src/test/random_tests.cpp
@@ -26,8 +26,8 @@ BOOST_AUTO_TEST_CASE(fastrandom_tests)
     FastRandomContext ctx2(true);
 
     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(GetRand<uint64_t>(), uint64_t{10393729187455219830U});
+        BOOST_CHECK_EQUAL(GetRand<int>(), int{769702006});
         BOOST_CHECK_EQUAL(GetRandMicros(std::chrono::hours{1}).count(), 2917185654);
         BOOST_CHECK_EQUAL(GetRandMillis(std::chrono::hours{1}).count(), 2144374);
     }
@@ -47,8 +47,8 @@ BOOST_AUTO_TEST_CASE(fastrandom_tests)
     // Check that a nondeterministic ones are not
     g_mock_deterministic_tests = false;
     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(GetRand<uint64_t>() != uint64_t{10393729187455219830U});
+        BOOST_CHECK(GetRand<int>() != int{769702006});
         BOOST_CHECK(GetRandMicros(std::chrono::hours{1}) != std::chrono::microseconds{2917185654});
         BOOST_CHECK(GetRandMillis(std::chrono::hours{1}) != std::chrono::milliseconds{2144374});
     }

diff --git a/src/util/bytevectorhash.cpp b/src/util/bytevectorhash.cpp
@@ -6,10 +6,10 @@
 #include <random.h>
 #include <util/bytevectorhash.h>
 
-ByteVectorHash::ByteVectorHash()
+ByteVectorHash::ByteVectorHash() :
+    m_k0(GetRand<uint64_t>()),
+    m_k1(GetRand<uint64_t>())
 {
-    GetRandBytes({reinterpret_cast<unsigned char*>(&m_k0), sizeof(m_k0)});
-    GetRandBytes({reinterpret_cast<unsigned char*>(&m_k1), sizeof(m_k1)});
 }
 
 size_t ByteVectorHash::operator()(const std::vector<unsigned char>& input) const

diff --git a/src/util/hasher.cpp b/src/util/hasher.cpp
@@ -7,11 +7,11 @@
 
 #include <limits>
 
-SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
+SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand<uint64_t>()), k1(GetRand<uint64_t>()) {}
 
-SaltedOutpointHasher::SaltedOutpointHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
+SaltedOutpointHasher::SaltedOutpointHasher() : k0(GetRand<uint64_t>()), k1(GetRand<uint64_t>()) {}
 
-SaltedSipHasher::SaltedSipHasher() : m_k0(GetRand(std::numeric_limits<uint64_t>::max())), m_k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
+SaltedSipHasher::SaltedSipHasher() : m_k0(GetRand<uint64_t>()), m_k1(GetRand<uint64_t>()) {}
 
 size_t SaltedSipHasher::operator()(const Span<const unsigned char>& script) const
 {

diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
@@ -616,8 +616,8 @@ static uint32_t GetLocktimeForNewTransaction(interfaces::Chain& chain, const uin
         // that transactions that are delayed after signing for whatever reason,
         // e.g. high-latency mix networks and some CoinJoin implementations, have
         // better privacy.
-        if (GetRandInt(10) == 0)
-            locktime = std::max(0, (int)locktime - GetRandInt(100));
+        if (GetRand<int>(10) == 0)
+            locktime = std::max(0, (int)locktime - GetRand<int>(100));
     } else {
         // If our chain is lagging behind, we can't discourage fee sniping nor help
         // the privacy of high-latency transactions. To avoid leaking a potentially
@@ -774,7 +774,7 @@ static bool CreateTransactionInternal(
     if (nChangePosInOut == -1)
     {
         // Insert change txn at random position:
-        nChangePosInOut = GetRandInt(txNew.vout.size()+1);
+        nChangePosInOut = GetRand<int>(txNew.vout.size()+1);
     }
     else if ((unsigned int)nChangePosInOut > txNew.vout.size())
     {