Merge pull request #1630 from evoskuil/master

Optimize point hash to avoid loss of 1 bit of sha256 entropy.

Author: evoskuil

include/bitcoin/system/chain/point.hpp

@@ -134,8 +134,9 @@ struct hash<bc::system::chain::point>
 {
     size_t operator()(const bc::system::chain::point& value) const NOEXCEPT
     {
-        return bc::system::hash_combine(value.index(),
-            bc::system::unique_hash(value.hash()));
+        // Right should always be the lower entropy value (i.e. point index).
+        return bc::system::hash_combine(bc::system::unique_hash(value.hash()),
+            value.index());
     }
 };
 

include/bitcoin/system/impl/hash/functions.ipp

@@ -237,30 +237,10 @@ INLINE constexpr size_t djb2_hash(const data_slice& data) NOEXCEPT
 }
 
 // Combine hash values, such as djb2_hash or unique_hash outputs.
+// Right should always be the lower entropy value (e.g. point index).
 INLINE constexpr size_t hash_combine(size_t left, size_t right) NOEXCEPT
 {
-    //// This leads to mixing null points (850k identical) with other buckets.
-    ////constexpr auto prime1 = possible_narrow_cast<size_t>(0x9e3779b97f4a7c15_u64);
-    ////constexpr auto prime2 = possible_narrow_cast<size_t>(0x517cc1b727220a95_u64);
-    ////
-    ////auto first = left;
-    ////first ^= shift_right(first, 23);
-    ////first *= prime1;
-    ////first ^= shift_right(first, 19);
-    ////
-    ////auto second = right;
-    ////second ^= shift_right(second, 13);
-    ////second *= prime2;
-    ////second ^= shift_right(second, 31);
-    ////
-    ////// seed parameter, defaults to zero.
-    ////first ^= second;
-    ////first += seed;
-    ////first ^= shift_right(first, 17);
-    ////first *= prime1;
-    ////first ^= shift_right(first, 29);
-
-    ////return first;
+    // Shift ensures left == right does not always return zero.
     return bit_xor(left, shift_left(right));
 }
 

src/wallet/keys/hd_private.cpp

@@ -128,7 +128,7 @@ hd_private hd_private::from_entropy(const data_slice& entropy,
     static const auto magic = to_chunk("Bitcoin seed");
     const auto intermediate = split(hmac<sha512>::code(entropy, magic));
 
-    return hd_private(intermediate.first, intermediate.second, prefixes);
+    return { intermediate.first, intermediate.second, prefixes };
 }
 
 hd_private hd_private::from_key(const hd_key& key,
@@ -162,7 +162,7 @@ hd_private hd_private::from_key(const hd_key& key, uint64_t prefixes) NOEXCEPT
         child
     };
 
-    return hd_private(secret, chain, lineage);
+    return { secret, chain, lineage };
 }
 
 hd_private hd_private::from_string(const std::string& encoded,
@@ -231,8 +231,8 @@ hd_key hd_private::to_hd_key() const NOEXCEPT
 
 hd_public hd_private::to_public() const NOEXCEPT
 {
-    return hd_public(((hd_public)*this).to_hd_key(),
-        hd_public::to_prefix(lineage_.prefixes));
+    const auto key = static_cast<hd_public>(*this).to_hd_key();
+    return { key, hd_public::to_prefix(lineage_.prefixes) };
 }
 
 hd_private hd_private::derive_private(uint32_t index) const NOEXCEPT
@@ -261,7 +261,7 @@ hd_private hd_private::derive_private(uint32_t index) const NOEXCEPT
         index
     };
 
-    return hd_private(child, intermediate.second, lineage);
+    return { child, intermediate.second, lineage };
 }
 
 hd_public hd_private::derive_public(uint32_t index) const NOEXCEPT

src/wallet/keys/hd_public.cpp

@@ -132,7 +132,7 @@ hd_public hd_public::from_key(const hd_key& key, uint32_t prefix) NOEXCEPT
         child
     };
 
-    return hd_public(compressed, chain, lineage);
+    return { compressed, chain, lineage };
 }
 
 hd_public hd_public::from_string(const std::string& encoded,
@@ -142,7 +142,7 @@ hd_public hd_public::from_string(const std::string& encoded,
     if (!decode_base58(key, encoded))
         return {};
 
-    return hd_public(from_key(key, prefix));
+    return { from_key(key, prefix) };
 }
 
 // Cast operators.
@@ -227,7 +227,7 @@ hd_public hd_public::derive_public(uint32_t index) const NOEXCEPT
         index
     };
 
-    return hd_public(child, intermediate.second, lineage);
+    return { child, intermediate.second, lineage };
 }
 
 // Helpers.