Merge 18b2c263aa into c5e44a0435

32d55e28af test: Use the correct node for doubled keypath test (Ava Chow)

Pull request description:

  #29124 had a silent merge conflict with #32350 which resulted in it using the wrong node. Fix the test to use the correct v22 node.

ACKs for top commit:
  maflcko:
    lgtm ACK 32d55e28af
  rkrux:
    ACK 32d55e28af
  BrandonOdiwuor:
    Code Review ACK 32d55e28af

Tree-SHA512: 1e0231985beb382b16e1d608c874750423d0502388db0c8ad450b22d17f9d96f5e16a6b44948ebda5efc750f62b60d0de8dd20131f449427426a36caf374af92

cmake/module/AddBoostIfNeeded.cmake

@@ -26,7 +26,7 @@ function(add_boost_if_needed)
     cmake_policy(SET CMP0167 OLD)
   endif()
   set(Boost_NO_BOOST_CMAKE ON)
-  find_package(Boost 1.73.0 REQUIRED)
+  find_package(Boost 1.82.0 REQUIRED)
   mark_as_advanced(Boost_INCLUDE_DIR)
   set_target_properties(Boost::headers PROPERTIES IMPORTED_GLOBAL TRUE)
   target_compile_definitions(Boost::headers INTERFACE

doc/dependencies.md

@@ -18,7 +18,7 @@ Bitcoin Core requires one of the following compilers.
 | Dependency | Releases | Version used | Minimum required | Runtime |
 | --- | --- | --- | --- | --- |
 | CMake | [link](https://cmake.org/) | N/A | [3.22](https://github.com/bitcoin/bitcoin/pull/30454) | No |
-| [Boost](../depends/packages/boost.mk) | [link](https://www.boost.org/users/download/) | [1.81.0](https://github.com/bitcoin/bitcoin/pull/26557) | [1.73.0](https://github.com/bitcoin/bitcoin/pull/29066) | No |
+| [Boost](../depends/packages/boost.mk) | [link](https://www.boost.org/users/download/) | [1.82.0](https://github.com/bitcoin/bitcoin/pull/26557) | [1.82.0](https://github.com/bitcoin/bitcoin/pull/29066) | No |
 | [libevent](../depends/packages/libevent.mk) | [link](https://github.com/libevent/libevent/releases) | [2.1.12-stable](https://github.com/bitcoin/bitcoin/pull/21991) | [2.1.8](https://github.com/bitcoin/bitcoin/pull/24681) | No |
 | glibc | [link](https://www.gnu.org/software/libc/) | N/A | [2.31](https://github.com/bitcoin/bitcoin/pull/29987) | Yes |
 | Linux Kernel (if building that platform) | [link](https://www.kernel.org/) | N/A | [3.17.0](https://github.com/bitcoin/bitcoin/pull/27699) | Yes |

src/coins.cpp

@@ -186,30 +186,34 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha
         if (!it->second.IsDirty()) {
             continue;
         }
-        CCoinsMap::iterator itUs = cacheCoins.find(it->first);
-        if (itUs == cacheCoins.end()) {
+        CCoinsMap::iterator itUs;
+        bool isInserted = false;
+        if (!(it->second.IsFresh() && it->second.coin.IsSpent())) {
+            std::tie(itUs, isInserted) = cacheCoins.try_emplace(it->first);
+        } else {
+            itUs = cacheCoins.find(it->first);
+        }
+
+        if (isInserted) {
             // The parent cache does not have an entry, while the child cache does.
             // We can ignore it if it's both spent and FRESH in the child
-            if (!(it->second.IsFresh() && it->second.coin.IsSpent())) {
-                // Create the coin in the parent cache, move the data up
-                // and mark it as dirty.
-                itUs = cacheCoins.try_emplace(it->first).first;
-                CCoinsCacheEntry& entry{itUs->second};
-                if (cursor.WillErase(*it)) {
-                    // Since this entry will be erased,
-                    // we can move the coin into us instead of copying it
-                    entry.coin = std::move(it->second.coin);
-                } else {
-                    entry.coin = it->second.coin;
-                }
-                cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
-                CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
-                // We can mark it FRESH in the parent if it was FRESH in the child
-                // Otherwise it might have just been flushed from the parent's cache
-                // and already exist in the grandparent
-                if (it->second.IsFresh()) CCoinsCacheEntry::SetFresh(*itUs, m_sentinel);
+            // Create the coin in the parent cache, move the data up
+            // and mark it as dirty.
+            CCoinsCacheEntry& entry{itUs->second};
+            if (cursor.WillErase(*it)) {
+                // Since this entry will be erased,
+                // we can move the coin into us instead of copying it
+                entry.coin = std::move(it->second.coin);
+            } else {
+                entry.coin = it->second.coin;
             }
-        } else {
+            cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
+            CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
+            // We can mark it FRESH in the parent if it was FRESH in the child
+            // Otherwise it might have just been flushed from the parent's cache
+            // and already exist in the grandparent
+            if (it->second.IsFresh()) CCoinsCacheEntry::SetFresh(*itUs, m_sentinel);
+        } else if (itUs != cacheCoins.end()) {
             // Found the entry in the parent cache
             if (it->second.IsFresh() && !itUs->second.coin.IsSpent()) {
                 // The coin was marked FRESH in the child cache, but the coin

src/coins.h

@@ -15,6 +15,7 @@
 #include <uint256.h>
 #include <util/check.h>
 #include <util/hasher.h>
+#include <boost/unordered/unordered_node_map.hpp>
 
 #include <assert.h>
 #include <stdint.h>
@@ -221,12 +222,12 @@ public:
  * Using an additional sizeof(void*)*4 for MAX_BLOCK_SIZE_BYTES should thus be sufficient so that
  * all implementations can allocate the nodes from the PoolAllocator.
  */
-using CCoinsMap = std::unordered_map<COutPoint,
-                                     CCoinsCacheEntry,
-                                     SaltedOutpointHasher,
-                                     std::equal_to<COutPoint>,
-                                     PoolAllocator<CoinsCachePair,
-                                                   sizeof(CoinsCachePair) + sizeof(void*) * 4>>;
+using CCoinsMap = boost::unordered_node_map<COutPoint,
+                                            CCoinsCacheEntry,
+                                            SaltedOutpointHasher,
+                                            std::equal_to<COutPoint>,
+                                            PoolAllocator<CoinsCachePair,
+                                                          sizeof(CoinsCachePair) + sizeof(void*) * 4>>;
 
 using CCoinsMapMemoryResource = CCoinsMap::allocator_type::ResourceType;
 

src/crypto/siphash.cpp

@@ -172,3 +172,53 @@ uint64_t SipHashUint256Extra(uint64_t k0, uint64_t k1, const uint256& val, uint3
     SIPROUND;
     return v0 ^ v1 ^ v2 ^ v3;
 }
+
+
+uint64_t ModifiedRapidHash(uint64_t k0, uint64_t k1, const uint256& val, uint32_t extra)
+{
+    auto const rapid_mum = [](uint64_t* a, uint64_t* b) {
+#if defined(__SIZEOF_INT128__)
+        __uint128_t r = *a;
+        r *= *b;
+        *a = (uint64_t)r;
+        *b = (uint64_t)(r >> 64);
+#elif defined(_MSC_VER) && (defined(_WIN64) || defined(_M_HYBRID_CHPE_ARM64))
+#if defined(_M_X64)
+        *a = _umul128(*a, *b, b);
+#else
+        uint64_t c = __umulh(*a, *b);
+        *a = *a * *b;
+        *b = c;
+#endif
+#else
+        uint64_t ha = *a >> 32, hb = *b >> 32, la = (uint32_t)*a, lb = (uint32_t)*b, hi, lo;
+        uint64_t rh = ha * hb, rm0 = ha * lb, rm1 = hb * la, rl = la * lb, t = rl + (rm0 << 32), c = t < rl;
+        lo = t + (rm1 << 32);
+        c += lo < t;
+        hi = rh + (rm0 >> 32) + (rm1 >> 32) + c;
+        *a = lo;
+        *b = hi;
+#endif
+    };
+
+    auto const rapid_mix = [&rapid_mum](uint64_t a, uint64_t b) -> uint64_t {
+        rapid_mum(&a, &b);
+        return a ^ b;
+    };
+
+    // This effectifely behaves like rapidhash with that input:
+    // seed: k0, data: [val, k1, extra]
+    // So it hashes 32+8+4 = 44 bytes, plus uses the 8 byte as seed.
+
+    // Default secret parameters.
+    static constexpr uint64_t secret[3] = {0x2d358dccaa6c78a5ull, 0x8bb84b93962eacc9ull, 0x4b33a62ed433d4a3ull};
+
+    // no need to mix the seed itself, as it is purely random.
+    uint64_t seed = k0;
+    seed = rapid_mix(val.GetUint64(0) ^ secret[2], val.GetUint64(1) ^ seed ^ secret[1]);
+    seed = rapid_mix(val.GetUint64(2) ^ secret[2], val.GetUint64(3) ^ seed);
+    uint64_t a = k1 ^ secret[1];
+    uint64_t b = extra ^ seed;
+    rapid_mum(&a, &b);
+    return rapid_mix(a ^ secret[0], b ^ secret[1]);
+}

src/crypto/siphash.h

@@ -44,5 +44,6 @@ public:
  */
 uint64_t SipHashUint256(uint64_t k0, uint64_t k1, const uint256& val);
 uint64_t SipHashUint256Extra(uint64_t k0, uint64_t k1, const uint256& val, uint32_t extra);
+uint64_t ModifiedRapidHash(uint64_t k0, uint64_t k1, const uint256& val, uint32_t extra);
 
 #endif // BITCOIN_CRYPTO_SIPHASH_H

src/memusage.h

@@ -19,6 +19,7 @@
 #include <vector>
 #include <unordered_map>
 #include <unordered_set>
+#include <boost/unordered/unordered_node_map.hpp>
 
 
 namespace memusage
@@ -215,6 +216,25 @@ static inline size_t DynamicUsage(const std::unordered_map<Key,
     return usage_resource + usage_chunks + MallocUsage(sizeof(void*) * m.bucket_count());
 }
 
+template <class Key, class T, class Hash, class Pred, std::size_t MAX_BLOCK_SIZE_BYTES, std::size_t ALIGN_BYTES>
+static inline size_t DynamicUsage(const boost::unordered_node_map<Key,
+                                                           T,
+                                                           Hash,
+                                                           Pred,
+                                                           PoolAllocator<std::pair<const Key, T>,
+                                                                         MAX_BLOCK_SIZE_BYTES,
+                                                                         ALIGN_BYTES>>& m)
+{
+    auto* pool_resource = m.get_allocator().resource();
+
+    // The allocated chunks are stored in a std::list. Size per node should
+    // therefore be 3 pointers: next, previous, and a pointer to the chunk.
+    size_t estimated_list_node_size = MallocUsage(sizeof(void*) * 3);
+    size_t usage_resource = estimated_list_node_size * pool_resource->NumAllocatedChunks();
+    size_t usage_chunks = MallocUsage(pool_resource->ChunkSizeBytes()) * pool_resource->NumAllocatedChunks();
+    return usage_resource + usage_chunks + MallocUsage(sizeof(void*) * m.bucket_count());
+}
+
 } // namespace memusage
 
 #endif // BITCOIN_MEMUSAGE_H

src/test/pool_tests.cpp

@@ -158,7 +158,7 @@ BOOST_AUTO_TEST_CASE(memusage_test)
 {
     auto std_map = std::unordered_map<int64_t, int64_t>{};
 
-    using Map = std::unordered_map<int64_t,
+    using Map = boost::unordered_node_map<int64_t,
                                    int64_t,
                                    std::hash<int64_t>,
                                    std::equal_to<int64_t>,

src/util/hasher.h

@@ -35,6 +35,10 @@ private:
     const uint64_t k0, k1;
 
 public:
+    // instructs Boost.Unordered to not use post-mixing. We can do this because the hash is of high quality.
+    // This should have a slight performance benefit.
+    using is_avalanching = std::true_type;
+
     SaltedOutpointHasher(bool deterministic = false);
 
     /**
@@ -47,7 +51,7 @@ public:
      * @see https://gcc.gnu.org/onlinedocs/gcc-13.2.0/libstdc++/manual/manual/unordered_associative.html
      */
     size_t operator()(const COutPoint& id) const noexcept {
-        return SipHashUint256Extra(k0, k1, id.hash, id.n);
+        return ModifiedRapidHash(k0, k1, id.hash, id.n);
     }
 };
 

test/functional/wallet_backwards_compatibility.py

@@ -87,7 +87,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         # 0.21.x and 22.x would both produce bad derivation paths when topping up an inactive hd chain
         # Make sure that this is being automatically cleaned up by migration
         node_master = self.nodes[1]
-        node_v22 = self.nodes[self.num_nodes - 5]
+        node_v22 = self.nodes[self.num_nodes - 3]
         wallet_name = "bad_deriv_path"
         node_v22.createwallet(wallet_name=wallet_name, descriptors=False)
         bad_deriv_wallet = node_v22.get_wallet_rpc(wallet_name)