Merge bitcoin#23451: span: Add std::byte helpers

faa3ec2 span: Add std::byte helpers (MarcoFalke)
fa18038 refactor: Use ignore helper when unserializing an invalid pubkey (MarcoFalke)
fabe18d Use value_type in CDataStream where possible (MarcoFalke)

Pull request description:

  This adds (currently unused) span std::byte helpers, so that they can be used in new code.

  The refactors are also required for bitcoin#23438, but they are split up because the other pull doesn't compile with msvc right now.

  The third commit is not needed for the other pull, but still nice.

ACKs for top commit:
  klementtan:
    reACK  faa3ec2. Verified that all the new `std::byte` helper functions are tested.
  laanwj:
    Code review ACK faa3ec2

Tree-SHA512: b1f6af39f03ea4dfebf20d4a8538fa993a6104e7fc92ddf0c4606a7efc3ca9a8c1a4741d98a1418569c11bb9ce9258bf0c0c06d93d85ed7e208902a2db04e407

Author: MarcoFalke

src/pubkey.h

@@ -141,17 +141,15 @@ class CPubKey
     template <typename Stream>
     void Unserialize(Stream& s)
     {
-        unsigned int len = ::ReadCompactSize(s);
+        const unsigned int len(::ReadCompactSize(s));
         if (len <= SIZE) {
             s.read((char*)vch, len);
             if (len != size()) {
                 Invalidate();
             }
         } else {
             // invalid pubkey, skip available data
-            char dummy;
-            while (len--)
-                s.read(&dummy, 1);
+            s.ignore(len);
             Invalidate();
         }
     }

src/span.h

@@ -184,6 +184,7 @@ class Span
         return m_data[m_size - 1];
     }
     constexpr std::size_t size() const noexcept { return m_size; }
+    constexpr std::size_t size_bytes() const noexcept { return sizeof(C) * m_size; }
     constexpr bool empty() const noexcept { return size() == 0; }
     CONSTEXPR_IF_NOT_DEBUG C& operator[](std::size_t pos) const noexcept
     {
@@ -240,11 +241,35 @@ T& SpanPopBack(Span<T>& span)
     return back;
 }
 
+// From C++20 as_bytes and as_writeable_bytes
+template <typename T>
+Span<const std::byte> AsBytes(Span<T> s) noexcept
+{
+    return {reinterpret_cast<const std::byte*>(s.data()), s.size_bytes()};
+}
+template <typename T>
+Span<std::byte> AsWritableBytes(Span<T> s) noexcept
+{
+    return {reinterpret_cast<std::byte*>(s.data()), s.size_bytes()};
+}
+
+template <typename V>
+Span<const std::byte> MakeByteSpan(V&& v) noexcept
+{
+    return AsBytes(MakeSpan(std::forward<V>(v)));
+}
+template <typename V>
+Span<std::byte> MakeWritableByteSpan(V&& v) noexcept
+{
+    return AsWritableBytes(MakeSpan(std::forward<V>(v)));
+}
+
 // Helper functions to safely cast to unsigned char pointers.
 inline unsigned char* UCharCast(char* c) { return (unsigned char*)c; }
 inline unsigned char* UCharCast(unsigned char* c) { return c; }
 inline const unsigned char* UCharCast(const char* c) { return (unsigned char*)c; }
 inline const unsigned char* UCharCast(const unsigned char* c) { return c; }
+inline const unsigned char* UCharCast(const std::byte* c) { return reinterpret_cast<const unsigned char*>(c); }
 
 // Helper function to safely convert a Span to a Span<[const] unsigned char>.
 template <typename T> constexpr auto UCharSpanCast(Span<T> s) -> Span<typename std::remove_pointer<decltype(UCharCast(s.data()))>::type> { return {UCharCast(s.data()), s.size()}; }

src/streams.h

@@ -226,7 +226,7 @@ class CDataStream
         : nType{nTypeIn},
           nVersion{nVersionIn} {}
 
-    explicit CDataStream(Span<const uint8_t> sp, int nTypeIn, int nVersionIn)
+    explicit CDataStream(Span<const value_type> sp, int nTypeIn, int nVersionIn)
         : vch(sp.data(), sp.data() + sp.size()),
           nType{nTypeIn},
           nVersion{nVersionIn} {}
@@ -254,17 +254,17 @@ class CDataStream
     iterator end()                                   { return vch.end(); }
     size_type size() const                           { return vch.size() - nReadPos; }
     bool empty() const                               { return vch.size() == nReadPos; }
-    void resize(size_type n, value_type c=0)         { vch.resize(n + nReadPos, c); }
+    void resize(size_type n, value_type c = value_type{}) { vch.resize(n + nReadPos, c); }
     void reserve(size_type n)                        { vch.reserve(n + nReadPos); }
     const_reference operator[](size_type pos) const  { return vch[pos + nReadPos]; }
     reference operator[](size_type pos)              { return vch[pos + nReadPos]; }
     void clear()                                     { vch.clear(); nReadPos = 0; }
-    iterator insert(iterator it, const uint8_t x) { return vch.insert(it, x); }
-    void insert(iterator it, size_type n, const uint8_t x) { vch.insert(it, n, x); }
+    iterator insert(iterator it, const value_type x) { return vch.insert(it, x); }
+    void insert(iterator it, size_type n, const value_type x) { vch.insert(it, n, x); }
     value_type* data()                               { return vch.data() + nReadPos; }
     const value_type* data() const                   { return vch.data() + nReadPos; }
 
-    void insert(iterator it, std::vector<uint8_t>::const_iterator first, std::vector<uint8_t>::const_iterator last)
+    void insert(iterator it, std::vector<value_type>::const_iterator first, std::vector<value_type>::const_iterator last)
     {
         if (last == first) return;
         assert(last - first > 0);
@@ -278,7 +278,7 @@ class CDataStream
             vch.insert(it, first, last);
     }
 
-    void insert(iterator it, const char* first, const char* last)
+    void insert(iterator it, const value_type* first, const value_type* last)
     {
         if (last == first) return;
         assert(last - first > 0);

src/test/base64_tests.cpp

@@ -23,6 +23,16 @@ BOOST_AUTO_TEST_CASE(base64_testvectors)
         BOOST_CHECK_EQUAL(strDec, vstrIn[i]);
     }
 
+    {
+        const std::vector<uint8_t> in_u{0xff, 0x01, 0xff};
+        const std::vector<std::byte> in_b{std::byte{0xff}, std::byte{0x01}, std::byte{0xff}};
+        const std::string in_s{"\xff\x01\xff"};
+        const std::string out_exp{"/wH/"};
+        BOOST_CHECK_EQUAL(EncodeBase64(in_u), out_exp);
+        BOOST_CHECK_EQUAL(EncodeBase64(in_b), out_exp);
+        BOOST_CHECK_EQUAL(EncodeBase64(in_s), out_exp);
+    }
+
     // Decoding strings with embedded NUL characters should fail
     bool failure;
     (void)DecodeBase64("invalid\0"s, &failure);

src/test/streams_tests.cpp

@@ -172,7 +172,7 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
     ds.Xor(key);
     BOOST_CHECK_EQUAL(
             std::string(expected_xor.begin(), expected_xor.end()),
-            std::string(ds.begin(), ds.end()));
+            ds.str());
 
     in.push_back('\x0f');
     in.push_back('\xf0');
@@ -189,7 +189,7 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
     ds.Xor(key);
     BOOST_CHECK_EQUAL(
             std::string(expected_xor.begin(), expected_xor.end()),
-            std::string(ds.begin(), ds.end()));
+            ds.str());
 
     // Multi character key
 
@@ -210,7 +210,7 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
     ds.Xor(key);
     BOOST_CHECK_EQUAL(
             std::string(expected_xor.begin(), expected_xor.end()),
-            std::string(ds.begin(), ds.end()));
+            ds.str());
 }
 
 BOOST_AUTO_TEST_CASE(streams_buffered_file)

src/test/util_tests.cpp

@@ -151,12 +151,25 @@ BOOST_AUTO_TEST_CASE(util_HexStr)
         HexStr(Span<const unsigned char>(ParseHex_expected, ParseHex_expected)),
         "");
 
-    std::vector<unsigned char> ParseHex_vec(ParseHex_expected, ParseHex_expected + 5);
+    {
+        const std::vector<char> in_s{ParseHex_expected, ParseHex_expected + 5};
+        const Span<const uint8_t> in_u{MakeUCharSpan(in_s)};
+        const Span<const std::byte> in_b{MakeByteSpan(in_s)};
+        const std::string out_exp{"04678afdb0"};
+
+        BOOST_CHECK_EQUAL(HexStr(in_u), out_exp);
+        BOOST_CHECK_EQUAL(HexStr(in_s), out_exp);
+        BOOST_CHECK_EQUAL(HexStr(in_b), out_exp);
+    }
+}
 
-    BOOST_CHECK_EQUAL(
-        HexStr(ParseHex_vec),
-        "04678afdb0"
-    );
+BOOST_AUTO_TEST_CASE(span_write_bytes)
+{
+    std::array mut_arr{uint8_t{0xaa}, uint8_t{0xbb}};
+    const auto mut_bytes{MakeWritableByteSpan(mut_arr)};
+    mut_bytes[1] = std::byte{0x11};
+    BOOST_CHECK_EQUAL(mut_arr.at(0), 0xaa);
+    BOOST_CHECK_EQUAL(mut_arr.at(1), 0x11);
 }
 
 BOOST_AUTO_TEST_CASE(util_Join)

src/util/strencodings.cpp

@@ -139,11 +139,6 @@ std::string EncodeBase64(Span<const unsigned char> input)
     return str;
 }
 
-std::string EncodeBase64(const std::string& str)
-{
-    return EncodeBase64(MakeUCharSpan(str));
-}
-
 std::vector<unsigned char> DecodeBase64(const char* p, bool* pf_invalid)
 {
     static const int decode64_table[256] =

src/util/strencodings.h

@@ -67,7 +67,8 @@ bool IsHexNumber(const std::string& str);
 std::vector<unsigned char> DecodeBase64(const char* p, bool* pf_invalid = nullptr);
 std::string DecodeBase64(const std::string& str, bool* pf_invalid = nullptr);
 std::string EncodeBase64(Span<const unsigned char> input);
-std::string EncodeBase64(const std::string& str);
+inline std::string EncodeBase64(Span<const std::byte> input) { return EncodeBase64(MakeUCharSpan(input)); }
+inline std::string EncodeBase64(const std::string& str) { return EncodeBase64(MakeUCharSpan(str)); }
 std::vector<unsigned char> DecodeBase32(const char* p, bool* pf_invalid = nullptr);
 std::string DecodeBase32(const std::string& str, bool* pf_invalid = nullptr);
 
@@ -206,6 +207,7 @@ std::optional<T> ToIntegral(const std::string& str)
  */
 std::string HexStr(const Span<const uint8_t> s);
 inline std::string HexStr(const Span<const char> s) { return HexStr(MakeUCharSpan(s)); }
+inline std::string HexStr(const Span<const std::byte> s) { return HexStr(MakeUCharSpan(s)); }
 
 /**
  * Format a paragraph of text to a fixed width, adding spaces for