Merge bitcoin#29071: refactor: Remove Span operator==, Use std::range…

…s::equal

fad0cf6 refactor: Use std::ranges::equal in GetNetworkForMagic (MarcoFalke)
fadf0a7 refactor: Remove Span operator==, Use std::ranges::equal (MarcoFalke)

Pull request description:

  `std::span` removed the comparison operators, so it makes sense to remove them for the `Span` "backport" as well. Using `std::ranges::equal` also has the benefit that some `Span` temporary constructions can now be dropped.

  This is required to move from `Span` toward `std::span`.

ACKs for top commit:
  hodlinator:
    Untested Code Review re-ACK fad0cf6
  stickies-v:
    ACK fad0cf6
  TheCharlatan:
    ACK fad0cf6

Tree-SHA512: 5b9d1826ceac2aabae2295bc89893dd23ac3a1cc0d41988331cdbdc21be531aa91795d5273819f349f79648c6c4f30ed31af6e7a3816153e92080061b92ffe00

src/external_signer.cpp

@@ -80,10 +80,10 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str
     // Check if signer fingerprint matches any input master key fingerprint
     auto matches_signer_fingerprint = [&](const PSBTInput& input) {
         for (const auto& entry : input.hd_keypaths) {
-            if (parsed_m_fingerprint == MakeUCharSpan(entry.second.fingerprint)) return true;
+            if (std::ranges::equal(parsed_m_fingerprint, entry.second.fingerprint)) return true;
         }
         for (const auto& entry : input.m_tap_bip32_paths) {
-            if (parsed_m_fingerprint == MakeUCharSpan(entry.second.second.fingerprint)) return true;
+            if (std::ranges::equal(parsed_m_fingerprint, entry.second.second.fingerprint)) return true;
         }
         return false;
     };

src/kernel/chainparams.cpp

@@ -694,15 +694,15 @@ std::optional<ChainType> GetNetworkForMagic(const MessageStartChars& message)
     const auto regtest_msg = CChainParams::RegTest({})->MessageStart();
     const auto signet_msg = CChainParams::SigNet({})->MessageStart();
 
-    if (std::equal(message.begin(), message.end(), mainnet_msg.data())) {
+    if (std::ranges::equal(message, mainnet_msg)) {
         return ChainType::MAIN;
-    } else if (std::equal(message.begin(), message.end(), testnet_msg.data())) {
+    } else if (std::ranges::equal(message, testnet_msg)) {
         return ChainType::TESTNET;
-    } else if (std::equal(message.begin(), message.end(), testnet4_msg.data())) {
+    } else if (std::ranges::equal(message, testnet4_msg)) {
         return ChainType::TESTNET4;
-    } else if (std::equal(message.begin(), message.end(), regtest_msg.data())) {
+    } else if (std::ranges::equal(message, regtest_msg)) {
         return ChainType::REGTEST;
-    } else if (std::equal(message.begin(), message.end(), signet_msg.data())) {
+    } else if (std::ranges::equal(message, signet_msg)) {
         return ChainType::SIGNET;
     }
     return std::nullopt;

src/net.cpp

@@ -46,13 +46,12 @@
 
 #include <algorithm>
 #include <array>
+#include <cmath>
 #include <cstdint>
 #include <functional>
 #include <optional>
 #include <unordered_map>
 
-#include <math.h>
-
 /** Maximum number of block-relay-only anchor connections */
 static constexpr size_t MAX_BLOCK_RELAY_ONLY_ANCHORS = 2;
 static_assert (MAX_BLOCK_RELAY_ONLY_ANCHORS <= static_cast<size_t>(MAX_BLOCK_RELAY_ONLY_CONNECTIONS), "MAX_BLOCK_RELAY_ONLY_ANCHORS must not exceed MAX_BLOCK_RELAY_ONLY_CONNECTIONS.");
@@ -1150,7 +1149,7 @@ bool V2Transport::ProcessReceivedGarbageBytes() noexcept
     Assume(m_recv_state == RecvState::GARB_GARBTERM);
     Assume(m_recv_buffer.size() <= MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN);
     if (m_recv_buffer.size() >= BIP324Cipher::GARBAGE_TERMINATOR_LEN) {
-        if (MakeByteSpan(m_recv_buffer).last(BIP324Cipher::GARBAGE_TERMINATOR_LEN) == m_cipher.GetReceiveGarbageTerminator()) {
+        if (std::ranges::equal(MakeByteSpan(m_recv_buffer).last(BIP324Cipher::GARBAGE_TERMINATOR_LEN), m_cipher.GetReceiveGarbageTerminator())) {
             // Garbage terminator received. Store garbage to authenticate it as AAD later.
             m_recv_aad = std::move(m_recv_buffer);
             m_recv_aad.resize(m_recv_aad.size() - BIP324Cipher::GARBAGE_TERMINATOR_LEN);

src/netaddress.cpp

@@ -245,14 +245,14 @@ bool CNetAddr::SetTor(const std::string& addr)
         Span<const uint8_t> input_checksum{input->data() + ADDR_TORV3_SIZE, torv3::CHECKSUM_LEN};
         Span<const uint8_t> input_version{input->data() + ADDR_TORV3_SIZE + torv3::CHECKSUM_LEN, sizeof(torv3::VERSION)};
 
-        if (input_version != torv3::VERSION) {
+        if (!std::ranges::equal(input_version, torv3::VERSION)) {
             return false;
         }
 
         uint8_t calculated_checksum[torv3::CHECKSUM_LEN];
         torv3::Checksum(input_pubkey, calculated_checksum);
 
-        if (input_checksum != calculated_checksum) {
+        if (!std::ranges::equal(input_checksum, calculated_checksum)) {
             return false;
         }
 

src/script/descriptor.cpp

@@ -21,6 +21,7 @@
 #include <util/strencodings.h>
 #include <util/vector.h>
 
+#include <algorithm>
 #include <memory>
 #include <numeric>
 #include <optional>
@@ -1405,11 +1406,11 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(uint32_t key_exp_index, const S
     }
     KeyPath path;
     DeriveType type = DeriveType::NO;
-    if (split.back() == Span{"*"}.first(1)) {
+    if (std::ranges::equal(split.back(), Span{"*"}.first(1))) {
         split.pop_back();
         type = DeriveType::UNHARDENED;
-    } else if (split.back() == Span{"*'"}.first(2) || split.back() == Span{"*h"}.first(2)) {
-        apostrophe = split.back() == Span{"*'"}.first(2);
+    } else if (std::ranges::equal(split.back(), Span{"*'"}.first(2)) || std::ranges::equal(split.back(), Span{"*h"}.first(2))) {
+        apostrophe = std::ranges::equal(split.back(), Span{"*'"}.first(2));
         split.pop_back();
         type = DeriveType::HARDENED;
     }

src/signet.cpp

@@ -4,10 +4,6 @@
 
 #include <signet.h>
 
-#include <array>
-#include <cstdint>
-#include <vector>
-
 #include <common/system.h>
 #include <consensus/merkle.h>
 #include <consensus/params.h>
@@ -23,6 +19,11 @@
 #include <uint256.h>
 #include <util/strencodings.h>
 
+#include <algorithm>
+#include <array>
+#include <cstdint>
+#include <vector>
+
 static constexpr uint8_t SIGNET_HEADER[4] = {0xec, 0xc7, 0xda, 0xa2};
 
 static constexpr unsigned int BLOCK_SCRIPT_VERIFY_FLAGS = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_NULLDUMMY;
@@ -38,7 +39,7 @@ static bool FetchAndClearCommitmentSection(const Span<const uint8_t> header, CSc
     std::vector<uint8_t> pushdata;
     while (witness_commitment.GetOp(pc, opcode, pushdata)) {
         if (pushdata.size() > 0) {
-            if (!found_header && pushdata.size() > (size_t)header.size() && Span{pushdata}.first(header.size()) == header) {
+            if (!found_header && pushdata.size() > header.size() && std::ranges::equal(Span{pushdata}.first(header.size()), header)) {
                 // pushdata only counts if it has the header _and_ some data
                 result.insert(result.end(), pushdata.begin() + header.size(), pushdata.end());
                 pushdata.erase(pushdata.begin() + header.size(), pushdata.end());

src/span.h

@@ -5,11 +5,11 @@
 #ifndef BITCOIN_SPAN_H
 #define BITCOIN_SPAN_H
 
-#include <algorithm>
 #include <cassert>
 #include <cstddef>
 #include <span>
 #include <type_traits>
+#include <utility>
 
 #ifdef DEBUG
 #define CONSTEXPR_IF_NOT_DEBUG
@@ -213,13 +213,6 @@ class Span
          return Span<C>(m_data + m_size - count, count);
     }
 
-    friend constexpr bool operator==(const Span& a, const Span& b) noexcept { return a.size() == b.size() && std::equal(a.begin(), a.end(), b.begin()); }
-    friend constexpr bool operator!=(const Span& a, const Span& b) noexcept { return !(a == b); }
-    friend constexpr bool operator<(const Span& a, const Span& b) noexcept { return std::lexicographical_compare(a.begin(), a.end(), b.begin(), b.end()); }
-    friend constexpr bool operator<=(const Span& a, const Span& b) noexcept { return !(b < a); }
-    friend constexpr bool operator>(const Span& a, const Span& b) noexcept { return (b < a); }
-    friend constexpr bool operator>=(const Span& a, const Span& b) noexcept { return !(a < b); }
-
     template <typename O> friend class Span;
 };
 

src/test/base32_tests.cpp

@@ -5,6 +5,8 @@
 #include <util/strencodings.h>
 
 #include <boost/test/unit_test.hpp>
+
+#include <algorithm>
 #include <string>
 
 using namespace std::literals;
@@ -24,7 +26,7 @@ BOOST_AUTO_TEST_CASE(base32_testvectors)
         BOOST_CHECK_EQUAL(strEnc, vstrOutNoPadding[i]);
         auto dec = DecodeBase32(vstrOut[i]);
         BOOST_REQUIRE(dec);
-        BOOST_CHECK_MESSAGE(MakeByteSpan(*dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]);
+        BOOST_CHECK_MESSAGE(std::ranges::equal(*dec, vstrIn[i]), vstrOut[i]);
     }
 
     // Decoding strings with embedded NUL characters should fail

src/test/base64_tests.cpp

@@ -5,6 +5,8 @@
 #include <util/strencodings.h>
 
 #include <boost/test/unit_test.hpp>
+
+#include <algorithm>
 #include <string>
 
 using namespace std::literals;
@@ -21,7 +23,7 @@ BOOST_AUTO_TEST_CASE(base64_testvectors)
         BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);
         auto dec = DecodeBase64(strEnc);
         BOOST_REQUIRE(dec);
-        BOOST_CHECK_MESSAGE(MakeByteSpan(*dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]);
+        BOOST_CHECK_MESSAGE(std::ranges::equal(*dec, vstrIn[i]), vstrOut[i]);
     }
 
     {

src/test/bip324_tests.cpp

@@ -11,6 +11,7 @@
 #include <test/util/setup_common.h>
 #include <util/strencodings.h>
 
+#include <algorithm>
 #include <array>
 #include <cstddef>
 #include <cstdint>
@@ -62,9 +63,9 @@ void TestBIP324PacketVector(
     BOOST_CHECK(cipher);
 
     // Compare session variables.
-    BOOST_CHECK(Span{out_session_id} == cipher.GetSessionID());
-    BOOST_CHECK(Span{mid_send_garbage} == cipher.GetSendGarbageTerminator());
-    BOOST_CHECK(Span{mid_recv_garbage} == cipher.GetReceiveGarbageTerminator());
+    BOOST_CHECK(std::ranges::equal(out_session_id, cipher.GetSessionID()));
+    BOOST_CHECK(std::ranges::equal(mid_send_garbage, cipher.GetSendGarbageTerminator()));
+    BOOST_CHECK(std::ranges::equal(mid_recv_garbage, cipher.GetReceiveGarbageTerminator()));
 
     // Vector of encrypted empty messages, encrypted in order to seek to the right position.
     std::vector<std::vector<std::byte>> dummies(in_idx);
@@ -89,7 +90,7 @@ void TestBIP324PacketVector(
         BOOST_CHECK(out_ciphertext == ciphertext);
     } else {
         BOOST_CHECK(ciphertext.size() >= out_ciphertext_endswith.size());
-        BOOST_CHECK(Span{out_ciphertext_endswith} == Span{ciphertext}.last(out_ciphertext_endswith.size()));
+        BOOST_CHECK(std::ranges::equal(out_ciphertext_endswith, Span{ciphertext}.last(out_ciphertext_endswith.size())));
     }
 
     for (unsigned error = 0; error <= 12; ++error) {
@@ -109,9 +110,9 @@ void TestBIP324PacketVector(
         BOOST_CHECK(dec_cipher);
 
         // Compare session variables.
-        BOOST_CHECK((Span{out_session_id} == dec_cipher.GetSessionID()) == (error != 1));
-        BOOST_CHECK((Span{mid_send_garbage} == dec_cipher.GetSendGarbageTerminator()) == (error != 1));
-        BOOST_CHECK((Span{mid_recv_garbage} == dec_cipher.GetReceiveGarbageTerminator()) == (error != 1));
+        BOOST_CHECK(std::ranges::equal(out_session_id, dec_cipher.GetSessionID()) == (error != 1));
+        BOOST_CHECK(std::ranges::equal(mid_send_garbage, dec_cipher.GetSendGarbageTerminator()) == (error != 1));
+        BOOST_CHECK(std::ranges::equal(mid_recv_garbage, dec_cipher.GetReceiveGarbageTerminator()) == (error != 1));
 
         // Seek to the numbered packet.
         if (in_idx == 0 && error == 12) continue;

src/test/crypto_tests.cpp

@@ -21,6 +21,7 @@
 #include <test/util/setup_common.h>
 #include <util/strencodings.h>
 
+#include <algorithm>
 #include <vector>
 
 #include <boost/test/unit_test.hpp>
@@ -833,9 +834,9 @@ BOOST_AUTO_TEST_CASE(chacha20_midblock)
     c20.Keystream(b2);
     c20.Keystream(b3);
 
-    BOOST_CHECK(Span{block}.first(5) == Span{b1});
-    BOOST_CHECK(Span{block}.subspan(5, 7) == Span{b2});
-    BOOST_CHECK(Span{block}.last(52) == Span{b3});
+    BOOST_CHECK(std::ranges::equal(Span{block}.first(5), b1));
+    BOOST_CHECK(std::ranges::equal(Span{block}.subspan(5, 7), b2));
+    BOOST_CHECK(std::ranges::equal(Span{block}.last(52), b3));
 }
 
 BOOST_AUTO_TEST_CASE(poly1305_testvector)

src/test/fuzz/bip324.cpp

@@ -10,6 +10,7 @@
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
 
+#include <algorithm>
 #include <cstdint>
 #include <vector>
 
@@ -59,9 +60,9 @@ FUZZ_TARGET(bip324_cipher_roundtrip, .init=Initialize)
     InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>());
 
     // Compare session IDs and garbage terminators.
-    assert(initiator.GetSessionID() == responder.GetSessionID());
-    assert(initiator.GetSendGarbageTerminator() == responder.GetReceiveGarbageTerminator());
-    assert(initiator.GetReceiveGarbageTerminator() == responder.GetSendGarbageTerminator());
+    assert(std::ranges::equal(initiator.GetSessionID(), responder.GetSessionID()));
+    assert(std::ranges::equal(initiator.GetSendGarbageTerminator(), responder.GetReceiveGarbageTerminator()));
+    assert(std::ranges::equal(initiator.GetReceiveGarbageTerminator(), responder.GetSendGarbageTerminator()));
 
     LIMITED_WHILE(provider.remaining_bytes(), 1000) {
         // Mode:

src/test/fuzz/hex.cpp

@@ -12,6 +12,7 @@
 #include <util/strencodings.h>
 #include <util/transaction_identifier.h>
 
+#include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <string>
@@ -22,7 +23,7 @@ FUZZ_TARGET(hex)
     const std::string random_hex_string(buffer.begin(), buffer.end());
     const std::vector<unsigned char> data = ParseHex(random_hex_string);
     const std::vector<std::byte> bytes{ParseHex<std::byte>(random_hex_string)};
-    assert(AsBytes(Span{data}) == Span{bytes});
+    assert(std::ranges::equal(AsBytes(Span{data}), bytes));
     const std::string hex_data = HexStr(data);
     if (IsHex(random_hex_string)) {
         assert(ToLower(random_hex_string) == hex_data);

src/test/fuzz/miniscript.cpp

@@ -13,6 +13,8 @@
 #include <test/fuzz/util.h>
 #include <util/strencodings.h>
 
+#include <algorithm>
+
 namespace {
 
 using Fragment = miniscript::Fragment;
@@ -293,7 +295,7 @@ const struct CheckerContext: BaseSignatureChecker {
         XOnlyPubKey pk{pubkey};
         auto it = TEST_DATA.schnorr_sigs.find(pk);
         if (it == TEST_DATA.schnorr_sigs.end()) return false;
-        return it->second.first == sig;
+        return std::ranges::equal(it->second.first, sig);
     }
     bool CheckLockTime(const CScriptNum& nLockTime) const override { return nLockTime.GetInt64() & 1; }
     bool CheckSequence(const CScriptNum& nSequence) const override { return nSequence.GetInt64() & 1; }

src/test/fuzz/p2p_transport_serialization.cpp

@@ -12,6 +12,7 @@
 #include <test/fuzz/util.h>
 #include <util/chaintype.h>
 
+#include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <limits>
@@ -185,12 +186,12 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa
         // Compare with expected more.
         if (expect_more[side].has_value()) assert(!bytes.empty() == *expect_more[side]);
         // Verify consistency between the two results.
-        assert(bytes == bytes_next);
+        assert(std::ranges::equal(bytes, bytes_next));
         assert(msg_type == msg_type_next);
         if (more_nonext) assert(more_next);
         // Compare with previously reported output.
         assert(to_send[side].size() <= bytes.size());
-        assert(to_send[side] == Span{bytes}.first(to_send[side].size()));
+        assert(std::ranges::equal(to_send[side], Span{bytes}.first(to_send[side].size())));
         to_send[side].resize(bytes.size());
         std::copy(bytes.begin(), bytes.end(), to_send[side].begin());
         // Remember 'more' results.
@@ -278,7 +279,7 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa
                 // The m_type must match what is expected.
                 assert(received.m_type == expected[side].front().m_type);
                 // The data must match what is expected.
-                assert(MakeByteSpan(received.m_recv) == MakeByteSpan(expected[side].front().data));
+                assert(std::ranges::equal(received.m_recv, MakeByteSpan(expected[side].front().data)));
                 expected[side].pop_front();
                 progress = true;
             }

src/test/fuzz/poolresource.cpp

@@ -78,7 +78,7 @@ class PoolResourceFuzzer
     {
         std::vector<std::byte> expect(entry.span.size());
         InsecureRandomContext(entry.seed).fillrand(expect);
-        assert(entry.span == expect);
+        assert(std::ranges::equal(entry.span, expect));
     }
 
     void Deallocate(const Entry& entry)

src/test/fuzz/span.cpp

@@ -30,10 +30,4 @@ FUZZ_TARGET(span)
         (void)span.subspan(idx, span.size() - idx);
         (void)span[idx];
     }
-
-    std::string another_str = fuzzed_data_provider.ConsumeBytesAsString(32);
-    const Span<const char> another_span{another_str};
-    assert((span <= another_span) != (span > another_span));
-    assert((span == another_span) != (span != another_span));
-    assert((span >= another_span) != (span < another_span));
 }

src/test/key_io_tests.cpp

@@ -10,12 +10,13 @@
 #include <script/script.h>
 #include <test/util/json.h>
 #include <test/util/setup_common.h>
+#include <univalue.h>
 #include <util/chaintype.h>
 #include <util/strencodings.h>
 
 #include <boost/test/unit_test.hpp>
 
-#include <univalue.h>
+#include <algorithm>
 
 BOOST_FIXTURE_TEST_SUITE(key_io_tests, BasicTestingSetup)
 
@@ -46,7 +47,7 @@ BOOST_AUTO_TEST_CASE(key_io_valid_parse)
             privkey = DecodeSecret(exp_base58string);
             BOOST_CHECK_MESSAGE(privkey.IsValid(), "!IsValid:" + strTest);
             BOOST_CHECK_MESSAGE(privkey.IsCompressed() == isCompressed, "compressed mismatch:" + strTest);
-            BOOST_CHECK_MESSAGE(Span{privkey} == Span{exp_payload}, "key mismatch:" + strTest);
+            BOOST_CHECK_MESSAGE(std::ranges::equal(privkey, exp_payload), "key mismatch:" + strTest);
 
             // Private key must be invalid public key
             destination = DecodeDestination(exp_base58string);