Merge bitcoin/bitcoin#28085: refactor: use Span for SipHash::Write

7d92b1430a refactor: use Span for SipHash::Write (Sebastian Falbesoner)

Pull request description:

  This simple refactoring PR changes the interface for the `SipHash` arbitrary-data `Write` method to take a `Span<unsigned char>` instead of having to pass data and length. (`Span<std::byte>` seems to be more modern, but vectors of `unsigned char` are still used prety much everywhere where SipHash is called, and I didn't find it very appealing having to clutter the code with `Make(Writable)ByteSpan` helpers).

ACKs for top commit:
  sipa:
    utACK 7d92b1430a
  MarcoFalke:
    lgtm ACK 7d92b1430a
  achow101:
    ACK 7d92b1430a

Tree-SHA512: f17a27013c942aead4b09f5a64e0c3ff8dbc7e83fe63eb9a2e3ace8be9921c9cbba3ec67e3e83fbe3332ca941c42370efd059e702c060f9b508307e9657c66f2

src/blockfilter.cpp

@@ -27,7 +27,7 @@ static const std::map<BlockFilterType, std::string> g_filter_types = {
 uint64_t GCSFilter::HashToRange(const Element& element) const
 {
     uint64_t hash = CSipHasher(m_params.m_siphash_k0, m_params.m_siphash_k1)
-        .Write(element.data(), element.size())
+        .Write(element)
         .Finalize();
     return FastRange64(hash, m_F);
 }

src/crypto/siphash.cpp

@@ -45,14 +45,14 @@ CSipHasher& CSipHasher::Write(uint64_t data)
     return *this;
 }
 
-CSipHasher& CSipHasher::Write(const unsigned char* data, size_t size)
+CSipHasher& CSipHasher::Write(Span<const unsigned char> data)
 {
     uint64_t v0 = v[0], v1 = v[1], v2 = v[2], v3 = v[3];
     uint64_t t = tmp;
     uint8_t c = count;
 
-    while (size--) {
-        t |= ((uint64_t)(*(data++))) << (8 * (c % 8));
+    while (data.size() > 0) {
+        t |= uint64_t{data.front()} << (8 * (c % 8));
         c++;
         if ((c & 7) == 0) {
             v3 ^= t;
@@ -61,6 +61,7 @@ CSipHasher& CSipHasher::Write(const unsigned char* data, size_t size)
             v0 ^= t;
             t = 0;
         }
+        data = data.subspan(1);
     }
 
     v[0] = v0;

src/crypto/siphash.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <span.h>
 #include <uint256.h>
 
 /** SipHash-2-4 */
@@ -26,7 +27,7 @@ public:
      */
     CSipHasher& Write(uint64_t data);
     /** Hash arbitrary bytes. */
-    CSipHasher& Write(const unsigned char* data, size_t size);
+    CSipHasher& Write(Span<const unsigned char> data);
     /** Compute the 64-bit SipHash-2-4 of the data written so far. The object remains untouched. */
     uint64_t Finalize() const;
 };

src/net.cpp

@@ -2529,7 +2529,7 @@ std::vector<CAddress> CConnman::GetAddresses(CNode& requestor, size_t max_addres
     auto local_socket_bytes = requestor.addrBind.GetAddrBytes();
     uint64_t cache_id = GetDeterministicRandomizer(RANDOMIZER_ID_ADDRCACHE)
         .Write(requestor.ConnectedThroughNetwork())
-        .Write(local_socket_bytes.data(), local_socket_bytes.size())
+        .Write(local_socket_bytes)
         // For outbound connections, the port of the bound address is randomly
         // assigned by the OS and would therefore not be useful for seeding.
         .Write(requestor.IsInboundConn() ? requestor.addrBind.GetPort() : 0)
@@ -2912,7 +2912,7 @@ uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& address) const
 {
     std::vector<unsigned char> vchNetGroup(m_netgroupman.GetGroup(address));
 
-    return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup.data(), vchNetGroup.size()).Finalize();
+    return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup).Finalize();
 }
 
 void CaptureMessageToFile(const CAddress& addr,

src/netaddress.h

@@ -564,7 +564,7 @@ public:
         CSipHasher hasher(m_salt_k0, m_salt_k1);
         hasher.Write(a.m_net);
         hasher.Write(a.port);
-        hasher.Write(a.m_addr.data(), a.m_addr.size());
+        hasher.Write(a.m_addr);
         return static_cast<size_t>(hasher.Finalize());
     }
 

src/test/fuzz/addrman.cpp

@@ -171,8 +171,8 @@ public:
             hasher.Write(a.source.GetNetwork());
             hasher.Write(addr_key.size());
             hasher.Write(source_key.size());
-            hasher.Write(addr_key.data(), addr_key.size());
-            hasher.Write(source_key.data(), source_key.size());
+            hasher.Write(addr_key);
+            hasher.Write(source_key);
             return (size_t)hasher.Finalize();
         };
 

src/test/fuzz/crypto.cpp

@@ -57,7 +57,7 @@ FUZZ_TARGET(crypto)
                 (void)sha256.Write(data.data(), data.size());
                 (void)sha3.Write(data);
                 (void)sha512.Write(data.data(), data.size());
-                (void)sip_hasher.Write(data.data(), data.size());
+                (void)sip_hasher.Write(data);
 
                 (void)Hash(data);
                 (void)Hash160(data);

src/test/fuzz/golomb_rice.cpp

@@ -23,7 +23,7 @@ namespace {
 uint64_t HashToRange(const std::vector<uint8_t>& element, const uint64_t f)
 {
     const uint64_t hash = CSipHasher(0x0706050403020100ULL, 0x0F0E0D0C0B0A0908ULL)
-                              .Write(element.data(), element.size())
+                              .Write(element)
                               .Finalize();
     return FastRange64(hash, f);
 }

src/test/hash_tests.cpp

@@ -83,21 +83,21 @@ BOOST_AUTO_TEST_CASE(siphash)
     CSipHasher hasher(0x0706050403020100ULL, 0x0F0E0D0C0B0A0908ULL);
     BOOST_CHECK_EQUAL(hasher.Finalize(),  0x726fdb47dd0e0e31ull);
     static const unsigned char t0[1] = {0};
-    hasher.Write(t0, 1);
+    hasher.Write(t0);
     BOOST_CHECK_EQUAL(hasher.Finalize(),  0x74f839c593dc67fdull);
     static const unsigned char t1[7] = {1,2,3,4,5,6,7};
-    hasher.Write(t1, 7);
+    hasher.Write(t1);
     BOOST_CHECK_EQUAL(hasher.Finalize(),  0x93f5f5799a932462ull);
     hasher.Write(0x0F0E0D0C0B0A0908ULL);
     BOOST_CHECK_EQUAL(hasher.Finalize(),  0x3f2acc7f57c29bdbull);
     static const unsigned char t2[2] = {16,17};
-    hasher.Write(t2, 2);
+    hasher.Write(t2);
     BOOST_CHECK_EQUAL(hasher.Finalize(),  0x4bc1b3f0968dd39cull);
     static const unsigned char t3[9] = {18,19,20,21,22,23,24,25,26};
-    hasher.Write(t3, 9);
+    hasher.Write(t3);
     BOOST_CHECK_EQUAL(hasher.Finalize(),  0x2f2e6163076bcfadull);
     static const unsigned char t4[5] = {27,28,29,30,31};
-    hasher.Write(t4, 5);
+    hasher.Write(t4);
     BOOST_CHECK_EQUAL(hasher.Finalize(),  0x7127512f72f27cceull);
     hasher.Write(0x2726252423222120ULL);
     BOOST_CHECK_EQUAL(hasher.Finalize(),  0x0e3ea96b5304a7d0ull);
@@ -111,7 +111,7 @@ BOOST_AUTO_TEST_CASE(siphash)
     for (uint8_t x=0; x<std::size(siphash_4_2_testvec); ++x)
     {
         BOOST_CHECK_EQUAL(hasher2.Finalize(), siphash_4_2_testvec[x]);
-        hasher2.Write(&x, 1);
+        hasher2.Write(Span{&x, 1});
     }
     // Check test vectors from spec, eight bytes at a time
     CSipHasher hasher3(0x0706050403020100ULL, 0x0F0E0D0C0B0A0908ULL);
@@ -140,9 +140,9 @@ BOOST_AUTO_TEST_CASE(siphash)
         uint8_t nb[4];
         WriteLE32(nb, n);
         CSipHasher sip256(k1, k2);
-        sip256.Write(x.begin(), 32);
+        sip256.Write(x);
         CSipHasher sip288 = sip256;
-        sip288.Write(nb, 4);
+        sip288.Write(nb);
         BOOST_CHECK_EQUAL(SipHashUint256(k1, k2, x), sip256.Finalize());
         BOOST_CHECK_EQUAL(SipHashUint256Extra(k1, k2, x, n), sip288.Finalize());
     }

src/txrequest.cpp

@@ -124,7 +124,7 @@ public:
 
     Priority operator()(const uint256& txhash, NodeId peer, bool preferred) const
     {
-        uint64_t low_bits = CSipHasher(m_k0, m_k1).Write(txhash.begin(), txhash.size()).Write(peer).Finalize() >> 1;
+        uint64_t low_bits = CSipHasher(m_k0, m_k1).Write(txhash).Write(peer).Finalize() >> 1;
         return low_bits | uint64_t{preferred} << 63;
     }
 

src/util/bytevectorhash.cpp

@@ -16,5 +16,5 @@ ByteVectorHash::ByteVectorHash() :
 
 size_t ByteVectorHash::operator()(const std::vector<unsigned char>& input) const
 {
-    return CSipHasher(m_k0, m_k1).Write(input.data(), input.size()).Finalize();
+    return CSipHasher(m_k0, m_k1).Write(input).Finalize();
 }

src/util/hasher.cpp

@@ -18,5 +18,5 @@ SaltedSipHasher::SaltedSipHasher() : m_k0(GetRand<uint64_t>()), m_k1(GetRand<uin
 
 size_t SaltedSipHasher::operator()(const Span<const unsigned char>& script) const
 {
-    return CSipHasher(m_k0, m_k1).Write(script.data(), script.size()).Finalize();
+    return CSipHasher(m_k0, m_k1).Write(script).Finalize();
 }