From 95e61c1cf2a91d041c8025306ba36f0ea2806894 Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Tue, 17 Sep 2019 17:02:56 -0400
Subject: [PATCH 1/3] Move Hashers to util/hasher.{cpp/h}

Move the hashers that we use for hash tables to a common place.

Moved hashers:
- SaltedTxidHasher
- SaltedOutpointHasher
- FilterHeaderHasher
- SignatureCacheHasher
- BlockHasher
---
 src/Makefile.am              |  2 +
 src/coins.cpp                |  2 -
 src/coins.h                  | 29 +-----------
 src/index/blockfilterindex.h |  6 +--
 src/script/sigcache.h        | 22 +--------
 src/txmempool.cpp            |  2 -
 src/txmempool.h              | 16 +------
 src/util/hasher.cpp          | 12 +++++
 src/util/hasher.h            | 87 ++++++++++++++++++++++++++++++++++++
 src/validation.h             |  9 +---
 10 files changed, 106 insertions(+), 81 deletions(-)
 create mode 100644 src/util/hasher.cpp
 create mode 100644 src/util/hasher.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 67fd402603..a0ba756d99 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -224,6 +224,7 @@ BITCOIN_CORE_H = \
   util/error.h \
   util/fees.h \
   util/golombrice.h \
+  util/hasher.h \
   util/macros.h \
   util/memory.h \
   util/message.h \
@@ -539,6 +540,7 @@ libbitcoin_util_a_SOURCES = \
   util/bytevectorhash.cpp \
   util/error.cpp \
   util/fees.cpp \
+  util/hasher.cpp \
   util/system.cpp \
   util/message.cpp \
   util/moneystr.cpp \
diff --git a/src/coins.cpp b/src/coins.cpp
index 5de2ed7810..abba61d3d2 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -31,8 +31,6 @@ bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock)
 CCoinsViewCursor *CCoinsViewBacked::Cursor() const { return base->Cursor(); }
 size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); }
 
-SaltedOutpointHasher::SaltedOutpointHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
-
 CCoinsViewCache::CCoinsViewCache(CCoinsView *baseIn) : CCoinsViewBacked(baseIn), cachedCoinsUsage(0) {}
 
 size_t CCoinsViewCache::DynamicMemoryUsage() const {
diff --git a/src/coins.h b/src/coins.h
index a3e241ac90..d2eb42d8cf 100644
--- a/src/coins.h
+++ b/src/coins.h
@@ -8,11 +8,11 @@
 
 #include <compressor.h>
 #include <core_memusage.h>
-#include <crypto/siphash.h>
 #include <memusage.h>
 #include <primitives/transaction.h>
 #include <serialize.h>
 #include <uint256.h>
+#include <util/hasher.h>
 
 #include <assert.h>
 #include <stdint.h>
@@ -82,33 +82,6 @@ public:
     }
 };
 
-class SaltedOutpointHasher
-{
-private:
-    /** Salt */
-    const uint64_t k0, k1;
-
-public:
-    SaltedOutpointHasher();
-
-    /**
-     * This *must* return size_t. With Boost 1.46 on 32-bit systems the
-     * unordered_map will behave unpredictably if the custom hasher returns a
-     * uint64_t, resulting in failures when syncing the chain (#4634).
-     *
-     * Having the hash noexcept allows libstdc++'s unordered_map to recalculate
-     * the hash during rehash, so it does not have to cache the value. This
-     * reduces node's memory by sizeof(size_t). The required recalculation has
-     * a slight performance penalty (around 1.6%), but this is compensated by
-     * memory savings of about 9% which allow for a larger dbcache setting.
-     *
-     * @see https://gcc.gnu.org/onlinedocs/gcc-9.2.0/libstdc++/manual/manual/unordered_associative.html
-     */
-    size_t operator()(const COutPoint& id) const noexcept {
-        return SipHashUint256Extra(k0, k1, id.hash, id.n);
-    }
-};
-
 /**
  * A Coin in one level of the coins database caching hierarchy.
  *
diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h
index 317f8c0e40..bb75eee960 100644
--- a/src/index/blockfilterindex.h
+++ b/src/index/blockfilterindex.h
@@ -9,15 +9,11 @@
 #include <chain.h>
 #include <flatfile.h>
 #include <index/base.h>
+#include <util/hasher.h>
 
 /** Interval between compact filter checkpoints. See BIP 157. */
 static constexpr int CFCHECKPT_INTERVAL = 1000;
 
-struct FilterHeaderHasher
-{
-    size_t operator()(const uint256& hash) const { return ReadLE64(hash.begin()); }
-};
-
 /**
  * BlockFilterIndex is used to store and retrieve block filters, hashes, and headers for a range of
  * blocks by height. An index is constructed for each supported filter type with its own database
diff --git a/src/script/sigcache.h b/src/script/sigcache.h
index 00534f9758..a945df0cc0 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -8,6 +8,7 @@
 
 #include <script/interpreter.h>
 #include <span.h>
+#include <util/hasher.h>
 
 #include <vector>
 
@@ -20,27 +21,6 @@ static const int64_t MAX_MAX_SIG_CACHE_SIZE = 16384;
 
 class CPubKey;
 
-/**
- * We're hashing a nonce into the entries themselves, so we don't need extra
- * blinding in the set hash computation.
- *
- * This may exhibit platform endian dependent behavior but because these are
- * nonced hashes (random) and this state is only ever used locally it is safe.
- * All that matters is local consistency.
- */
-class SignatureCacheHasher
-{
-public:
-    template <uint8_t hash_select>
-    uint32_t operator()(const uint256& key) const
-    {
-        static_assert(hash_select <8, "SignatureCacheHasher only has 8 hashes available.");
-        uint32_t u;
-        std::memcpy(&u, key.begin()+4*hash_select, 4);
-        return u;
-    }
-};
-
 class CachingTransactionSignatureChecker : public TransactionSignatureChecker
 {
 private:
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 0c2b731967..8a519fa08c 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -1134,5 +1134,3 @@ CTxMemPool::EpochGuard::~EpochGuard()
     ++pool.m_epoch;
     pool.m_has_epoch_guard = false;
 }
-
-SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
diff --git a/src/txmempool.h b/src/txmempool.h
index f513f14af6..e6ea09f869 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -15,13 +15,13 @@
 
 #include <amount.h>
 #include <coins.h>
-#include <crypto/siphash.h>
 #include <indirectmap.h>
 #include <optional.h>
 #include <policy/feerate.h>
 #include <primitives/transaction.h>
 #include <sync.h>
 #include <random.h>
+#include <util/hasher.h>
 
 #include <boost/multi_index_container.hpp>
 #include <boost/multi_index/hashed_index.hpp>
@@ -398,20 +398,6 @@ enum class MemPoolRemovalReason {
     REPLACED,    //!< Removed for replacement
 };
 
-class SaltedTxidHasher
-{
-private:
-    /** Salt */
-    const uint64_t k0, k1;
-
-public:
-    SaltedTxidHasher();
-
-    size_t operator()(const uint256& txid) const {
-        return SipHashUint256(k0, k1, txid);
-    }
-};
-
 /**
  * CTxMemPool stores valid-according-to-the-current-best-chain transactions
  * that may be included in the next block.
diff --git a/src/util/hasher.cpp b/src/util/hasher.cpp
new file mode 100644
index 0000000000..236b8e926f
--- /dev/null
+++ b/src/util/hasher.cpp
@@ -0,0 +1,12 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <random.h>
+#include <util/hasher.h>
+
+#include <limits>
+
+SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
+
+SaltedOutpointHasher::SaltedOutpointHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
diff --git a/src/util/hasher.h b/src/util/hasher.h
new file mode 100644
index 0000000000..77301df63d
--- /dev/null
+++ b/src/util/hasher.h
@@ -0,0 +1,87 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_HASHER_H
+#define BITCOIN_UTIL_HASHER_H
+
+#include <crypto/siphash.h>
+#include <primitives/transaction.h>
+#include <uint256.h>
+
+class SaltedTxidHasher
+{
+private:
+    /** Salt */
+    const uint64_t k0, k1;
+
+public:
+    SaltedTxidHasher();
+
+    size_t operator()(const uint256& txid) const {
+        return SipHashUint256(k0, k1, txid);
+    }
+};
+
+class SaltedOutpointHasher
+{
+private:
+    /** Salt */
+    const uint64_t k0, k1;
+
+public:
+    SaltedOutpointHasher();
+
+    /**
+     * This *must* return size_t. With Boost 1.46 on 32-bit systems the
+     * unordered_map will behave unpredictably if the custom hasher returns a
+     * uint64_t, resulting in failures when syncing the chain (#4634).
+     *
+     * Having the hash noexcept allows libstdc++'s unordered_map to recalculate
+     * the hash during rehash, so it does not have to cache the value. This
+     * reduces node's memory by sizeof(size_t). The required recalculation has
+     * a slight performance penalty (around 1.6%), but this is compensated by
+     * memory savings of about 9% which allow for a larger dbcache setting.
+     *
+     * @see https://gcc.gnu.org/onlinedocs/gcc-9.2.0/libstdc++/manual/manual/unordered_associative.html
+     */
+    size_t operator()(const COutPoint& id) const noexcept {
+        return SipHashUint256Extra(k0, k1, id.hash, id.n);
+    }
+};
+
+struct FilterHeaderHasher
+{
+    size_t operator()(const uint256& hash) const { return ReadLE64(hash.begin()); }
+};
+
+/**
+ * We're hashing a nonce into the entries themselves, so we don't need extra
+ * blinding in the set hash computation.
+ *
+ * This may exhibit platform endian dependent behavior but because these are
+ * nonced hashes (random) and this state is only ever used locally it is safe.
+ * All that matters is local consistency.
+ */
+class SignatureCacheHasher
+{
+public:
+    template <uint8_t hash_select>
+    uint32_t operator()(const uint256& key) const
+    {
+        static_assert(hash_select <8, "SignatureCacheHasher only has 8 hashes available.");
+        uint32_t u;
+        std::memcpy(&u, key.begin()+4*hash_select, 4);
+        return u;
+    }
+};
+
+struct BlockHasher
+{
+    // 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()); }
+};
+
+#endif // BITCOIN_UTIL_HASHER_H
diff --git a/src/validation.h b/src/validation.h
index 3d9fa92c15..a3dc0b75af 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -23,6 +23,7 @@
 #include <txdb.h>
 #include <versionbits.h>
 #include <serialize.h>
+#include <util/hasher.h>
 
 #include <atomic>
 #include <map>
@@ -94,14 +95,6 @@ static const unsigned int DEFAULT_CHECKLEVEL = 3;
 // Setting the target to >= 550 MiB will make it likely we can respect the target.
 static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 550 * 1024 * 1024;
 
-struct BlockHasher
-{
-    // 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()); }
-};
-
 /** Current sync state passed to tip changed callbacks. */
 enum class SynchronizationState {
     INIT_REINDEX,

From 210b693db66e7c5b618014b5a287aee15af00045 Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Tue, 17 Sep 2019 19:29:59 -0400
Subject: [PATCH 2/3] Add generic SaltedSipHasher

SaltedSipHasher is a generic hasher that can be used with most things we
would hash in an unordered container.
---
 src/util/hasher.cpp |  7 +++++++
 src/util/hasher.h   | 12 ++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/src/util/hasher.cpp b/src/util/hasher.cpp
index 236b8e926f..5900daf050 100644
--- a/src/util/hasher.cpp
+++ b/src/util/hasher.cpp
@@ -10,3 +10,10 @@
 SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
 
 SaltedOutpointHasher::SaltedOutpointHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
+
+SaltedSipHasher::SaltedSipHasher() : m_k0(GetRand(std::numeric_limits<uint64_t>::max())), m_k1(GetRand(std::numeric_limits<uint64_t>::max())) {}
+
+size_t SaltedSipHasher::operator()(const Span<const unsigned char>& script) const
+{
+    return CSipHasher(m_k0, m_k1).Write(script.data(), script.size()).Finalize();
+}
diff --git a/src/util/hasher.h b/src/util/hasher.h
index 77301df63d..fa2fea30d8 100644
--- a/src/util/hasher.h
+++ b/src/util/hasher.h
@@ -84,4 +84,16 @@ struct BlockHasher
     size_t operator()(const uint256& hash) const { return ReadLE64(hash.begin()); }
 };
 
+class SaltedSipHasher
+{
+private:
+    /** Salt */
+    const uint64_t m_k0, m_k1;
+
+public:
+    SaltedSipHasher();
+
+    size_t operator()(const Span<const unsigned char>& script) const;
+};
+
 #endif // BITCOIN_UTIL_HASHER_H

From 281fd1a4a032cded7f9ea9857e3e99fc793c714b Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Mon, 28 Sep 2020 19:34:38 -0400
Subject: [PATCH 3/3] Replace KeyIDHasher with SaltedSipHasher

---
 src/wallet/scriptpubkeyman.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index 63c10b7a0d..85d98eb4a7 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -303,7 +303,7 @@ private:
 
     /* the HD chain data model (external chain counters) */
     CHDChain m_hd_chain;
-    std::unordered_map<CKeyID, CHDChain, KeyIDHasher> m_inactive_hd_chains;
+    std::unordered_map<CKeyID, CHDChain, SaltedSipHasher> m_inactive_hd_chains;
 
     /* HD derive new child key (on internal or external chain) */
     void DeriveNewChildKey(WalletBatch& batch, CKeyMetadata& metadata, CKey& secret, CHDChain& hd_chain, bool internal = false) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);