Merge bitcoin#31519: refactor: Use std::span over Span

ffff4a2 bench: Update span-serialize comment (MarcoFalke)
fa4d6ec refactor: Avoid false-positive gcc warning (MarcoFalke)
fa94233 scripted-diff: Bump copyright headers after std::span changes (MarcoFalke)
fa0c6b7 refactor: Remove unused Span alias (MarcoFalke)
fade0b5 scripted-diff: Use std::span over Span (MarcoFalke)
fadccc2 refactor: Make Span an alias of std::span (MarcoFalke)
fa27e36 test: Fix broken span_tests (MarcoFalke)
fadf02e refactor: Return std::span from MakeUCharSpan (MarcoFalke)
fa720b9 refactor: Return std::span from MakeByteSpan (MarcoFalke)

Pull request description:

  `Span` has some issues:

  * It does not support fixed-size spans, which are available through `std::span`.
  * It is confusing to have it available and in use at the same time with `std::span`.
  * It does not obey the standard library iterator build hardening flags. See bitcoin#31272 for a discussion. For example, this allows to catch issues like the one fixed in commit fabeca3.

  Both types are type-safe and can even implicitly convert into each other in most contexts.

  However, exclusively using `std::span` seems less confusing, so do it here with a scripted-diff.

ACKs for top commit:
  l0rinc:
    reACK ffff4a2
  theuni:
    ACK ffff4a2.

Tree-SHA512: 9cc2f1f43551e2c07cc09f38b1f27d11e57e9e9bc0c6138c8fddd0cef54b91acd8b14711205ff949be874294a121910d0aceffe0e8914c4cff07f1e0e87ad5b8

Author: fanquake

doc/developer-notes.md

@@ -856,14 +856,14 @@ class A
   - *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those
   that are not language lawyers.
 
-- Use `Span` as function argument when it can operate on any range-like container.
+- Use `std::span` as function argument when it can operate on any range-like container.
 
   - *Rationale*: Compared to `Foo(const vector<int>&)` this avoids the need for a (potentially expensive)
     conversion to vector if the caller happens to have the input stored in another type of container.
     However, be aware of the pitfalls documented in [span.h](../src/span.h).
 
 ```cpp
-void Foo(Span<const int> data);
+void Foo(std::span<const int> data);
 
 std::vector<int> vec{1,2,3};
 Foo(vec);

src/base58.cpp

@@ -1,4 +1,4 @@
-// Copyright (c) 2014-2022 The Bitcoin Core developers
+// Copyright (c) 2014-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -86,7 +86,7 @@ static const int8_t mapBase58[256] = {
     return true;
 }
 
-std::string EncodeBase58(Span<const unsigned char> input)
+std::string EncodeBase58(std::span<const unsigned char> input)
 {
     // Skip & count leading zeroes.
     int zeroes = 0;
@@ -134,7 +134,7 @@ bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet, in
     return DecodeBase58(str.c_str(), vchRet, max_ret_len);
 }
 
-std::string EncodeBase58Check(Span<const unsigned char> input)
+std::string EncodeBase58Check(std::span<const unsigned char> input)
 {
     // add 4-byte hash check to the end
     std::vector<unsigned char> vch(input.begin(), input.end());
@@ -151,7 +151,7 @@ std::string EncodeBase58Check(Span<const unsigned char> input)
         return false;
     }
     // re-calculate the checksum, ensure it matches the included 4-byte checksum
-    uint256 hash = Hash(Span{vchRet}.first(vchRet.size() - 4));
+    uint256 hash = Hash(std::span{vchRet}.first(vchRet.size() - 4));
     if (memcmp(&hash, &vchRet[vchRet.size() - 4], 4) != 0) {
         vchRet.clear();
         return false;

src/base58.h

@@ -1,5 +1,5 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
-// Copyright (c) 2009-2022 The Bitcoin Core developers
+// Copyright (c) 2009-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -22,7 +22,7 @@
 /**
  * Encode a byte span as a base58-encoded string
  */
-std::string EncodeBase58(Span<const unsigned char> input);
+std::string EncodeBase58(std::span<const unsigned char> input);
 
 /**
  * Decode a base58-encoded string (str) into a byte vector (vchRet).
@@ -33,7 +33,7 @@ std::string EncodeBase58(Span<const unsigned char> input);
 /**
  * Encode a byte span into a base58-encoded string, including checksum
  */
-std::string EncodeBase58Check(Span<const unsigned char> input);
+std::string EncodeBase58Check(std::span<const unsigned char> input);
 
 /**
  * Decode a base58-encoded string (str) that includes a checksum into a byte

src/bench/load_external.cpp

@@ -1,4 +1,4 @@
-// Copyright (c) 2022 The Bitcoin Core developers
+// Copyright (c) 2022-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or https://www.opensource.org/licenses/mit-license.php.
 
@@ -44,9 +44,8 @@ static void LoadExternalBlockFile(benchmark::Bench& bench)
     auto params{testing_setup->m_node.chainman->GetParams()};
     ss << params.MessageStart();
     ss << static_cast<uint32_t>(benchmark::data::block413567.size());
-    // We can't use the streaming serialization (ss << benchmark::data::block413567)
-    // because that first writes a compact size.
-    ss << Span{benchmark::data::block413567};
+    // Use span-serialization to avoid writing the size first.
+    ss << std::span{benchmark::data::block413567};
 
     // Create the test file.
     {

src/bip324.cpp

@@ -1,4 +1,4 @@
-// Copyright (c) 2023 The Bitcoin Core developers
+// Copyright (c) 2023-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -22,8 +22,8 @@
 #include <iterator>
 #include <string>
 
-BIP324Cipher::BIP324Cipher(const CKey& key, Span<const std::byte> ent32) noexcept :
-    m_key(key)
+BIP324Cipher::BIP324Cipher(const CKey& key, std::span<const std::byte> ent32) noexcept
+    : m_key(key)
 {
     m_our_pubkey = m_key.EllSwiftCreate(ent32);
 }
@@ -70,7 +70,7 @@ void BIP324Cipher::Initialize(const EllSwiftPubKey& their_pubkey, bool initiator
     m_key = CKey();
 }
 
-void BIP324Cipher::Encrypt(Span<const std::byte> contents, Span<const std::byte> aad, bool ignore, Span<std::byte> output) noexcept
+void BIP324Cipher::Encrypt(std::span<const std::byte> contents, std::span<const std::byte> aad, bool ignore, std::span<std::byte> output) noexcept
 {
     assert(output.size() == contents.size() + EXPANSION);
 
@@ -86,7 +86,7 @@ void BIP324Cipher::Encrypt(Span<const std::byte> contents, Span<const std::byte>
     m_send_p_cipher->Encrypt(header, contents, aad, output.subspan(LENGTH_LEN));
 }
 
-uint32_t BIP324Cipher::DecryptLength(Span<const std::byte> input) noexcept
+uint32_t BIP324Cipher::DecryptLength(std::span<const std::byte> input) noexcept
 {
     assert(input.size() == LENGTH_LEN);
 
@@ -97,7 +97,7 @@ uint32_t BIP324Cipher::DecryptLength(Span<const std::byte> input) noexcept
     return uint32_t(buf[0]) + (uint32_t(buf[1]) << 8) + (uint32_t(buf[2]) << 16);
 }
 
-bool BIP324Cipher::Decrypt(Span<const std::byte> input, Span<const std::byte> aad, bool& ignore, Span<std::byte> contents) noexcept
+bool BIP324Cipher::Decrypt(std::span<const std::byte> input, std::span<const std::byte> aad, bool& ignore, std::span<std::byte> contents) noexcept
 {
     assert(input.size() + LENGTH_LEN == contents.size() + EXPANSION);
 

src/bip324.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2023 The Bitcoin Core developers
+// Copyright (c) 2023-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -45,7 +45,7 @@ class BIP324Cipher
     BIP324Cipher() = delete;
 
     /** Initialize a BIP324 cipher with specified key and encoding entropy (testing only). */
-    BIP324Cipher(const CKey& key, Span<const std::byte> ent32) noexcept;
+    BIP324Cipher(const CKey& key, std::span<const std::byte> ent32) noexcept;
 
     /** Initialize a BIP324 cipher with specified key (testing only). */
     BIP324Cipher(const CKey& key, const EllSwiftPubKey& pubkey) noexcept;
@@ -68,29 +68,29 @@ class BIP324Cipher
      *
      * It must hold that output.size() == contents.size() + EXPANSION.
      */
-    void Encrypt(Span<const std::byte> contents, Span<const std::byte> aad, bool ignore, Span<std::byte> output) noexcept;
+    void Encrypt(std::span<const std::byte> contents, std::span<const std::byte> aad, bool ignore, std::span<std::byte> output) noexcept;
 
     /** Decrypt the length of a packet. Only after Initialize().
      *
      * It must hold that input.size() == LENGTH_LEN.
      */
-    unsigned DecryptLength(Span<const std::byte> input) noexcept;
+    unsigned DecryptLength(std::span<const std::byte> input) noexcept;
 
     /** Decrypt a packet. Only after Initialize().
      *
      * It must hold that input.size() + LENGTH_LEN == contents.size() + EXPANSION.
      * Contents.size() must equal the length returned by DecryptLength.
      */
-    bool Decrypt(Span<const std::byte> input, Span<const std::byte> aad, bool& ignore, Span<std::byte> contents) noexcept;
+    bool Decrypt(std::span<const std::byte> input, std::span<const std::byte> aad, bool& ignore, std::span<std::byte> contents) noexcept;
 
     /** Get the Session ID. Only after Initialize(). */
-    Span<const std::byte> GetSessionID() const noexcept { return m_session_id; }
+    std::span<const std::byte> GetSessionID() const noexcept { return m_session_id; }
 
     /** Get the Garbage Terminator to send. Only after Initialize(). */
-    Span<const std::byte> GetSendGarbageTerminator() const noexcept { return m_send_garbage_terminator; }
+    std::span<const std::byte> GetSendGarbageTerminator() const noexcept { return m_send_garbage_terminator; }
 
     /** Get the expected Garbage Terminator to receive. Only after Initialize(). */
-    Span<const std::byte> GetReceiveGarbageTerminator() const noexcept { return m_recv_garbage_terminator; }
+    std::span<const std::byte> GetReceiveGarbageTerminator() const noexcept { return m_recv_garbage_terminator; }
 };
 
 #endif // BITCOIN_BIP324_H

src/cluster_linearize.h

@@ -75,7 +75,7 @@ class DepGraph
      *
      * @param depgraph   The original DepGraph that is being remapped.
      *
-     * @param mapping    A Span such that mapping[i] gives the position in the new DepGraph
+     * @param mapping    A span such that mapping[i] gives the position in the new DepGraph
      *                   for position i in the old depgraph. Its size must be equal to
      *                   depgraph.PositionRange(). The value of mapping[i] is ignored if
      *                   position i is a hole in depgraph (i.e., if !depgraph.Positions()[i]).
@@ -86,7 +86,7 @@ class DepGraph
      *
      * Complexity: O(N^2) where N=depgraph.TxCount().
      */
-    DepGraph(const DepGraph<SetType>& depgraph, Span<const ClusterIndex> mapping, ClusterIndex pos_range) noexcept : entries(pos_range)
+    DepGraph(const DepGraph<SetType>& depgraph, std::span<const ClusterIndex> mapping, ClusterIndex pos_range) noexcept : entries(pos_range)
     {
         Assume(mapping.size() == depgraph.PositionRange());
         Assume((pos_range == 0) == (depgraph.TxCount() == 0));
@@ -371,7 +371,7 @@ struct SetInfo
 
 /** Compute the feerates of the chunks of linearization. */
 template<typename SetType>
-std::vector<FeeFrac> ChunkLinearization(const DepGraph<SetType>& depgraph, Span<const ClusterIndex> linearization) noexcept
+std::vector<FeeFrac> ChunkLinearization(const DepGraph<SetType>& depgraph, std::span<const ClusterIndex> linearization) noexcept
 {
     std::vector<FeeFrac> ret;
     for (ClusterIndex i : linearization) {
@@ -396,7 +396,7 @@ class LinearizationChunking
     const DepGraph<SetType>& m_depgraph;
 
     /** The linearization we started from, possibly with removed prefix stripped. */
-    Span<const ClusterIndex> m_linearization;
+    std::span<const ClusterIndex> m_linearization;
 
     /** Chunk sets and their feerates, of what remains of the linearization. */
     std::vector<SetInfo<SetType>> m_chunks;
@@ -437,7 +437,7 @@ class LinearizationChunking
 
 public:
     /** Initialize a LinearizationSubset object for a given length of linearization. */
-    explicit LinearizationChunking(const DepGraph<SetType>& depgraph LIFETIMEBOUND, Span<const ClusterIndex> lin LIFETIMEBOUND) noexcept :
+    explicit LinearizationChunking(const DepGraph<SetType>& depgraph LIFETIMEBOUND, std::span<const ClusterIndex> lin LIFETIMEBOUND) noexcept :
         m_depgraph(depgraph), m_linearization(lin)
     {
         // Mark everything in lin as todo still.
@@ -1016,7 +1016,7 @@ class SearchCandidateFinder
  * Complexity: possibly O(N * min(max_iterations + N, sqrt(2^N))) where N=depgraph.TxCount().
  */
 template<typename SetType>
-std::pair<std::vector<ClusterIndex>, bool> Linearize(const DepGraph<SetType>& depgraph, uint64_t max_iterations, uint64_t rng_seed, Span<const ClusterIndex> old_linearization = {}) noexcept
+std::pair<std::vector<ClusterIndex>, bool> Linearize(const DepGraph<SetType>& depgraph, uint64_t max_iterations, uint64_t rng_seed, std::span<const ClusterIndex> old_linearization = {}) noexcept
 {
     Assume(old_linearization.empty() || old_linearization.size() == depgraph.TxCount());
     if (depgraph.TxCount() == 0) return {{}, true};
@@ -1110,7 +1110,7 @@ std::pair<std::vector<ClusterIndex>, bool> Linearize(const DepGraph<SetType>& de
  *   postlinearize" process.
  */
 template<typename SetType>
-void PostLinearize(const DepGraph<SetType>& depgraph, Span<ClusterIndex> linearization)
+void PostLinearize(const DepGraph<SetType>& depgraph, std::span<ClusterIndex> linearization)
 {
     // This algorithm performs a number of passes (currently 2); the even ones operate from back to
     // front, the odd ones from front to back. Each results in an equal-or-better linearization
@@ -1299,7 +1299,7 @@ void PostLinearize(const DepGraph<SetType>& depgraph, Span<ClusterIndex> lineari
  * Complexity: O(N^2) where N=depgraph.TxCount(); O(N) if both inputs are identical.
  */
 template<typename SetType>
-std::vector<ClusterIndex> MergeLinearizations(const DepGraph<SetType>& depgraph, Span<const ClusterIndex> lin1, Span<const ClusterIndex> lin2)
+std::vector<ClusterIndex> MergeLinearizations(const DepGraph<SetType>& depgraph, std::span<const ClusterIndex> lin1, std::span<const ClusterIndex> lin2)
 {
     Assume(lin1.size() == depgraph.TxCount());
     Assume(lin2.size() == depgraph.TxCount());

src/common/bloom.cpp

@@ -1,4 +1,4 @@
-// Copyright (c) 2012-2022 The Bitcoin Core developers
+// Copyright (c) 2012-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -40,13 +40,13 @@ CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, c
 {
 }
 
-inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, Span<const unsigned char> vDataToHash) const
+inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, std::span<const unsigned char> vDataToHash) const
 {
     // 0xFBA4C795 chosen as it guarantees a reasonable bit difference between nHashNum values.
     return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash) % (vData.size() * 8);
 }
 
-void CBloomFilter::insert(Span<const unsigned char> vKey)
+void CBloomFilter::insert(std::span<const unsigned char> vKey)
 {
     if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700)
         return;
@@ -65,7 +65,7 @@ void CBloomFilter::insert(const COutPoint& outpoint)
     insert(MakeUCharSpan(stream));
 }
 
-bool CBloomFilter::contains(Span<const unsigned char> vKey) const
+bool CBloomFilter::contains(std::span<const unsigned char> vKey) const
 {
     if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700)
         return true;
@@ -187,12 +187,12 @@ CRollingBloomFilter::CRollingBloomFilter(const unsigned int nElements, const dou
 }
 
 /* Similar to CBloomFilter::Hash */
-static inline uint32_t RollingBloomHash(unsigned int nHashNum, uint32_t nTweak, Span<const unsigned char> vDataToHash)
+static inline uint32_t RollingBloomHash(unsigned int nHashNum, uint32_t nTweak, std::span<const unsigned char> vDataToHash)
 {
     return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash);
 }
 
-void CRollingBloomFilter::insert(Span<const unsigned char> vKey)
+void CRollingBloomFilter::insert(std::span<const unsigned char> vKey)
 {
     if (nEntriesThisGeneration == nEntriesPerGeneration) {
         nEntriesThisGeneration = 0;
@@ -223,7 +223,7 @@ void CRollingBloomFilter::insert(Span<const unsigned char> vKey)
     }
 }
 
-bool CRollingBloomFilter::contains(Span<const unsigned char> vKey) const
+bool CRollingBloomFilter::contains(std::span<const unsigned char> vKey) const
 {
     for (int n = 0; n < nHashFuncs; n++) {
         uint32_t h = RollingBloomHash(n, nTweak, vKey);

src/common/bloom.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2012-2021 The Bitcoin Core developers
+// Copyright (c) 2012-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -49,7 +49,7 @@ class CBloomFilter
     unsigned int nTweak;
     unsigned char nFlags;
 
-    unsigned int Hash(unsigned int nHashNum, Span<const unsigned char> vDataToHash) const;
+    unsigned int Hash(unsigned int nHashNum, std::span<const unsigned char> vDataToHash) const;
 
 public:
     /**
@@ -66,10 +66,10 @@ class CBloomFilter
 
     SERIALIZE_METHODS(CBloomFilter, obj) { READWRITE(obj.vData, obj.nHashFuncs, obj.nTweak, obj.nFlags); }
 
-    void insert(Span<const unsigned char> vKey);
+    void insert(std::span<const unsigned char> vKey);
     void insert(const COutPoint& outpoint);
 
-    bool contains(Span<const unsigned char> vKey) const;
+    bool contains(std::span<const unsigned char> vKey) const;
     bool contains(const COutPoint& outpoint) const;
 
     //! True if the size is <= MAX_BLOOM_FILTER_SIZE and the number of hash functions is <= MAX_HASH_FUNCS
@@ -110,8 +110,8 @@ class CRollingBloomFilter
 public:
     CRollingBloomFilter(const unsigned int nElements, const double nFPRate);
 
-    void insert(Span<const unsigned char> vKey);
-    bool contains(Span<const unsigned char> vKey) const;
+    void insert(std::span<const unsigned char> vKey);
+    bool contains(std::span<const unsigned char> vKey) const;
 
     void reset();