Merge pull request #4499 from vijaydasmp/backport_v18_vijay_batch4_3

merge bitcoin#13258, #14951

src/addrman.cpp

@@ -12,8 +12,8 @@
 
 int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asmap) const
 {
-    uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetKey()).GetHash().GetCheapHash();
-    uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetHash().GetCheapHash();
+    uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetKey()).GetCheapHash();
+    uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetCheapHash();
     int tried_bucket = hash2 % ADDRMAN_TRIED_BUCKET_COUNT;
     uint32_t mapped_as = GetMappedAS(asmap);
     LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to tried bucket %i\n", ToStringIP(), mapped_as, tried_bucket);
@@ -23,8 +23,8 @@ int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asma
 int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector<bool> &asmap) const
 {
     std::vector<unsigned char> vchSourceGroupKey = src.GetGroup(asmap);
-    uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetHash().GetCheapHash();
-    uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetHash().GetCheapHash();
+    uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash();
+    uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetCheapHash();
     int new_bucket = hash2 % ADDRMAN_NEW_BUCKET_COUNT;
     uint32_t mapped_as = GetMappedAS(asmap);
     LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to new bucket %i\n", ToStringIP(), mapped_as, new_bucket);
@@ -33,7 +33,7 @@ int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std:
 
 int CAddrInfo::GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const
 {
-    uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << (fNew ? 'N' : 'K') << nBucket << GetKey()).GetHash().GetCheapHash();
+    uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << (fNew ? 'N' : 'K') << nBucket << GetKey()).GetCheapHash();
     return hash1 % ADDRMAN_BUCKET_SIZE;
 }
 

src/evo/deterministicmns.h

@@ -6,6 +6,7 @@
 #define BITCOIN_EVO_DETERMINISTICMNS_H
 
 #include <arith_uint256.h>
+#include <crypto/common.h>
 #include <evo/evodb.h>
 #include <evo/providertx.h>
 #include <saltedhasher.h>
@@ -300,10 +301,16 @@ inline void SerReadWrite(Stream& s, immer::map<K, T, Hash, Equal>& obj, CSerActi
 
 class CDeterministicMNList
 {
+private:
+    struct ImmerHasher
+    {
+        size_t operator()(const uint256& hash) const { return ReadLE64(hash.begin()); }
+    };
+
 public:
-    using MnMap = immer::map<uint256, CDeterministicMNCPtr>;
+    using MnMap = immer::map<uint256, CDeterministicMNCPtr, ImmerHasher>;
     using MnInternalIdMap = immer::map<uint64_t, uint256>;
-    using MnUniquePropertyMap = immer::map<uint256, std::pair<uint256, uint32_t> >;
+    using MnUniquePropertyMap = immer::map<uint256, std::pair<uint256, uint32_t>, ImmerHasher>;
 
 private:
     uint256 blockHash;

src/hash.h

@@ -7,6 +7,7 @@
 #ifndef BITCOIN_HASH_H
 #define BITCOIN_HASH_H
 
+#include <crypto/common.h>
 #include <crypto/ripemd160.h>
 #include <crypto/sha256.h>
 #include <prevector.h>
@@ -208,6 +209,15 @@ public:
         return result;
     }
 
+    /**
+     * Returns the first 64 bits from the resulting hash.
+     */
+    inline uint64_t GetCheapHash() {
+        unsigned char result[CHash256::OUTPUT_SIZE];
+        ctx.Finalize(result);
+        return ReadLE64(result);
+    }
+
     template<typename T>
     CHashWriter& operator<<(const T& obj) {
         // Serialize to this stream

src/llmq/chainlocks.h

@@ -9,6 +9,7 @@
 #include <llmq/signing.h>
 
 #include <chainparams.h>
+#include <crypto/common.h>
 #include <net.h>
 #include <saltedhasher.h>
 #include <streams.h>
@@ -70,9 +71,13 @@ private:
     uint256 lastSignedMsgHash GUARDED_BY(cs);
 
     // We keep track of txids from recently received blocks so that we can check if all TXs got islocked
-    using BlockTxs = std::unordered_map<uint256, std::shared_ptr<std::unordered_set<uint256, StaticSaltedHasher>>>;
+    struct BlockHasher
+    {
+        size_t operator()(const uint256& hash) const { return ReadLE64(hash.begin()); }
+    };
+    using BlockTxs = std::unordered_map<uint256, std::shared_ptr<std::unordered_set<uint256, StaticSaltedHasher>>, BlockHasher>;
     BlockTxs blockTxs GUARDED_BY(cs);
-    std::unordered_map<uint256, int64_t> txFirstSeenTime GUARDED_BY(cs);
+    std::unordered_map<uint256, int64_t, StaticSaltedHasher> txFirstSeenTime GUARDED_BY(cs);
 
     std::map<uint256, int64_t> seenChainLocks GUARDED_BY(cs);
 

src/llmq/instantsend.cpp

@@ -169,7 +169,7 @@ void CInstantSendDb::WriteInstantSendLockArchived(CDBBatch& batch, const uint256
     batch.Write(std::make_tuple(DB_ARCHIVED_BY_HASH, hash), true);
 }
 
-std::unordered_map<uint256, CInstantSendLockPtr> CInstantSendDb::RemoveConfirmedInstantSendLocks(int nUntilHeight)
+std::unordered_map<uint256, CInstantSendLockPtr, StaticSaltedHasher> CInstantSendDb::RemoveConfirmedInstantSendLocks(int nUntilHeight)
 {
     LOCK(cs_db);
     if (nUntilHeight <= best_confirmed_height) {
@@ -186,7 +186,7 @@ std::unordered_map<uint256, CInstantSendLockPtr> CInstantSendDb::RemoveConfirmed
     it->Seek(firstKey);
 
     CDBBatch batch(*db);
-    std::unordered_map<uint256, CInstantSendLockPtr> ret;
+    std::unordered_map<uint256, CInstantSendLockPtr, StaticSaltedHasher> ret;
     while (it->Valid()) {
         decltype(firstKey) curKey;
         if (!it->GetKey(curKey) || std::get<0>(curKey) != DB_MINED_BY_HEIGHT_AND_HASH) {
@@ -900,12 +900,12 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks()
     return fMoreWork;
 }
 
-std::unordered_set<uint256> CInstantSendManager::ProcessPendingInstantSendLocks(int signOffset, const std::unordered_map<uint256, std::pair<NodeId, CInstantSendLockPtr>, StaticSaltedHasher>& pend, bool ban)
+std::unordered_set<uint256, StaticSaltedHasher> CInstantSendManager::ProcessPendingInstantSendLocks(int signOffset, const std::unordered_map<uint256, std::pair<NodeId, CInstantSendLockPtr>, StaticSaltedHasher>& pend, bool ban)
 {
     auto llmqType = Params().GetConsensus().llmqTypeInstantSend;
 
     CBLSBatchVerifier<NodeId, uint256> batchVerifier(false, true, 8);
-    std::unordered_map<uint256, CRecoveredSig> recSigs;
+    std::unordered_map<uint256, CRecoveredSig, StaticSaltedHasher> recSigs;
 
     size_t verifyCount = 0;
     size_t alreadyVerified = 0;
@@ -977,7 +977,7 @@ std::unordered_set<uint256> CInstantSendManager::ProcessPendingInstantSendLocks(
     LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- verified locks. count=%d, alreadyVerified=%d, vt=%d, nodes=%d\n", __func__,
             verifyCount, alreadyVerified, verifyTimer.count(), batchVerifier.GetUniqueSourceCount());
 
-    std::unordered_set<uint256> badISLocks;
+    std::unordered_set<uint256, StaticSaltedHasher> badISLocks;
 
     if (ban && !batchVerifier.badSources.empty()) {
         LOCK(cs_main);
@@ -1335,7 +1335,7 @@ void CInstantSendManager::HandleFullyConfirmedBlock(const CBlockIndex* pindex)
 
 void CInstantSendManager::RemoveMempoolConflictsForLock(const uint256& hash, const CInstantSendLock& islock)
 {
-    std::unordered_map<uint256, CTransactionRef> toDelete;
+    std::unordered_map<uint256, CTransactionRef, StaticSaltedHasher> toDelete;
 
     {
         LOCK(mempool.cs);

src/llmq/instantsend.h

@@ -117,7 +117,7 @@ public:
      * @param nUntilHeight Removes all IS Locks confirmed up until nUntilHeight
      * @return returns an unordered_map of the hash of the IS Locks and a pointer object to the IS Locks for all IS Locks which were removed
      */
-    std::unordered_map<uint256, CInstantSendLockPtr> RemoveConfirmedInstantSendLocks(int nUntilHeight);
+    std::unordered_map<uint256, CInstantSendLockPtr, StaticSaltedHasher> RemoveConfirmedInstantSendLocks(int nUntilHeight);
     /**
      * Removes IS Locks from the archive if the tx was confirmed 100 blocks before nUntilHeight
      * @param nUntilHeight the height from which to base the remove of archive IS Locks
@@ -230,7 +230,7 @@ private:
     void ProcessMessageInstantSendLock(const CNode* pfrom, const CInstantSendLockPtr& islock) LOCKS_EXCLUDED(cs);
     static bool PreVerifyInstantSendLock(const CInstantSendLock& islock);
     bool ProcessPendingInstantSendLocks();
-    std::unordered_set<uint256> ProcessPendingInstantSendLocks(int signOffset, const std::unordered_map<uint256, std::pair<NodeId, CInstantSendLockPtr>, StaticSaltedHasher>& pend, bool ban);
+    std::unordered_set<uint256, StaticSaltedHasher> ProcessPendingInstantSendLocks(int signOffset, const std::unordered_map<uint256, std::pair<NodeId, CInstantSendLockPtr>, StaticSaltedHasher>& pend, bool ban);
     void ProcessInstantSendLock(NodeId from, const uint256& hash, const CInstantSendLockPtr& islock);
 
     void AddNonLockedTx(const CTransactionRef& tx, const CBlockIndex* pindexMined);

src/llmq/signing_shares.cpp

@@ -991,7 +991,7 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_map<No
 {
     AssertLockHeld(cs);
 
-    std::unordered_map<uint256, CNode*> proTxToNode;
+    std::unordered_map<uint256, CNode*, StaticSaltedHasher> proTxToNode;
     for (const auto& pnode : vNodes) {
         auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash();
         if (verifiedProRegTxHash.IsNull()) {

src/spork.h

@@ -7,6 +7,7 @@
 
 #include <hash.h>
 #include <net.h>
+#include <saltedhasher.h>
 #include <util/strencodings.h>
 #include <key.h>
 
@@ -162,7 +163,7 @@ private:
     mutable std::unordered_map<const SporkId, bool> mapSporksCachedActive GUARDED_BY(cs);
 
     mutable std::unordered_map<SporkId, int64_t> mapSporksCachedValues GUARDED_BY(cs);
-    std::unordered_map<uint256, CSporkMessage> mapSporksByHash GUARDED_BY(cs);
+    std::unordered_map<uint256, CSporkMessage, StaticSaltedHasher> mapSporksByHash GUARDED_BY(cs);
     std::unordered_map<SporkId, std::map<CKeyID, CSporkMessage> > mapSporksActive GUARDED_BY(cs);
 
     std::set<CKeyID> setSporkPubKeyIDs GUARDED_BY(cs);

src/txdb.cpp

@@ -139,7 +139,7 @@ size_t CCoinsViewDB::EstimateSize() const
     return db.EstimateSize(DB_COIN, (char)(DB_COIN+1));
 }
 
-CBlockTreeDB::CBlockTreeDB(size_t nCacheSize, bool fMemory, bool fWipe) : CDBWrapper(gArgs.IsArgSet("-blocksdir") ? GetDataDir() / "blocks" / "index" : GetBlocksDir() / "index", nCacheSize, fMemory, fWipe), mapHasTxIndexCache(10000, 20000) {
+CBlockTreeDB::CBlockTreeDB(size_t nCacheSize, bool fMemory, bool fWipe) : CDBWrapper(gArgs.IsArgSet("-blocksdir") ? GetDataDir() / "blocks" / "index" : GetBlocksDir() / "index", nCacheSize, fMemory, fWipe) {
 }
 
 bool CBlockTreeDB::ReadBlockFileInfo(int nFile, CBlockFileInfo &info) {

src/txdb.h

@@ -9,10 +9,8 @@
 #include <coins.h>
 #include <dbwrapper.h>
 #include <chain.h>
-#include <limitedmap.h>
 #include <primitives/block.h>
 #include <spentindex.h>
-#include <sync.h>
 
 #include <map>
 #include <memory>
@@ -91,10 +89,6 @@ private:
 /** Access to the block database (blocks/index/) */
 class CBlockTreeDB : public CDBWrapper
 {
-private:
-    CCriticalSection cs;
-    unordered_limitedmap<uint256, bool> mapHasTxIndexCache;
-
 public:
     explicit CBlockTreeDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false);
 

src/uint256.h

@@ -13,7 +13,6 @@
 #include <stdint.h>
 #include <string>
 #include <vector>
-#include <crypto/common.h>
 
 /** Template base class for fixed-sized opaque blobs. */
 template<unsigned int BITS>
@@ -127,16 +126,6 @@ class uint256 : public base_blob<256> {
 public:
     uint256() {}
     explicit uint256(const std::vector<unsigned char>& vch) : base_blob<256>(vch) {}
-
-    /** A cheap hash function that just returns 64 bits from the result, it can be
-     * used when the contents are considered uniformly random. It is not appropriate
-     * when the value can easily be influenced from outside as e.g. a network adversary could
-     * provide values to trigger worst-case behavior.
-     */
-    uint64_t GetCheapHash() const
-    {
-        return ReadLE64(m_data);
-    }
 };
 
 /* uint256 from const char *.
@@ -175,15 +164,5 @@ public:
     }
 };
 
-namespace std {
-    template <>
-    struct hash<uint256>
-    {
-        std::size_t operator()(const uint256& k) const
-        {
-            return (std::size_t)k.GetCheapHash();
-        }
-    };
-}
 
 #endif // BITCOIN_UINT256_H

src/validation.h

@@ -13,6 +13,7 @@
 
 #include <amount.h>
 #include <coins.h>
+#include <crypto/common.h> // for ReadLE64
 #include <fs.h>
 #include <protocol.h> // For CMessageHeader::MessageStartChars
 #include <policy/feerate.h>
@@ -114,7 +115,10 @@ static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 945 * 1024 * 1024;
 
 struct BlockHasher
 {
-    size_t operator()(const uint256& hash) const { return hash.GetCheapHash(); }
+    // this used to call `GetCheapHash()` in uint256, which was later moved; the
+    // cheap hash function simply calls ReadLE64() however, so the end result is
+    // identical
+    size_t operator()(const uint256& hash) const { return ReadLE64(hash.begin()); }
 };
 
 extern CCriticalSection cs_main;

src/wallet/wallet.cpp

@@ -10,6 +10,7 @@
 #include <wallet/coinselection.h>
 #include <consensus/consensus.h>
 #include <consensus/validation.h>
+#include <crypto/common.h>
 #include <fs.h>
 #include <interfaces/chain.h>
 #include <key.h>
@@ -1650,7 +1651,7 @@ bool CWallet::IsFullyMixed(const COutPoint& outpoint) const
         ss << outpoint << nCoinJoinSalt;
         uint256 nHash;
         CSHA256().Write((const unsigned char*)ss.data(), ss.size()).Finalize(nHash.begin());
-        if (nHash.GetCheapHash() % 2 == 0) {
+        if (ReadLE64(nHash.begin()) % 2 == 0) {
             return false;
         }
     }

test/functional/test_framework/test_framework.py

@@ -357,7 +357,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
     # Public helper methods. These can be accessed by the subclass test scripts.
 
     def add_nodes(self, num_nodes, extra_args=None, *, rpchost=None, binary=None):
-        """Instantiate TestNode objects"""
+        """Instantiate TestNode objects.
+
+        Should only be called once after the nodes have been specified in
+        set_test_params()."""
         if self.bind_to_localhost_only:
             extra_confs = [["bind=127.0.0.1"]] * num_nodes
         else:
@@ -369,9 +372,24 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         assert_equal(len(extra_confs), num_nodes)
         assert_equal(len(extra_args), num_nodes)
         assert_equal(len(binary), num_nodes)
+        old_num_nodes = len(self.nodes)
         for i in range(num_nodes):
-            numnode = len(self.nodes)
-            self.nodes.append(TestNode(numnode, get_datadir_path(self.options.tmpdir, numnode), self.extra_args_from_options, chain=self.chain, rpchost=rpchost, timewait=self.rpc_timeout, bitcoind=binary[i], bitcoin_cli=self.options.bitcoincli, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, extra_conf=extra_confs[i], extra_args=extra_args[i], use_cli=self.options.usecli, start_perf=self.options.perf))
+            self.nodes.append(TestNode(
+                old_num_nodes + i,
+                get_datadir_path(self.options.tmpdir, old_num_nodes + i),
+                self.extra_args_from_options,
+                chain=self.chain,
+                rpchost=rpchost,
+                timewait=self.rpc_timeout,
+                bitcoind=binary[i],
+                bitcoin_cli=self.options.bitcoincli,
+                mocktime=self.mocktime,
+                coverage_dir=self.options.coveragedir,
+                extra_conf=extra_confs[i],
+                extra_args=extra_args[i],
+                use_cli=self.options.usecli,
+                start_perf=self.options.perf,
+            ))
 
     def start_node(self, i, *args, **kwargs):
         """Start a dashd"""