Merge bitcoin#24253: Remove broken and unused CDataStream methods

fa1b227 Remove broken and unused CDataStream methods (MarcoFalke)
faee5f8 test: Create fresh CDataStream each time (MarcoFalke)
fa71114 test: Inline expected_xor (MarcoFalke)

Pull request description:

  The `insert` and `erase` methods have many issues:

  * They are unused
  * They are confusing and hard to read, as they implement "special cases" for optimization, that isn't needed
  * They are broken (See bitcoin#24231)
  * Fixing them leads to mingw compile errors (See bitcoin#24231 (comment))

  Fix all issues by removing them

ACKs for top commit:
  laanwj:
    Code review ACK fa1b227

Tree-SHA512: 9d9e5d42e6ffc5ae82bdb67cfb5b50b45977ae674acee6ff99092560aebf2fc7e4584ded614e190db0663226fa198e34350517cd7ee57d518de22e9568bc349a

Author: laanwj

src/streams.h

@@ -240,76 +240,9 @@ class CDataStream
     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 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<value_type>::const_iterator first, std::vector<value_type>::const_iterator last)
-    {
-        if (last == first) return;
-        assert(last - first > 0);
-        if (it == vch.begin() + nReadPos && (unsigned int)(last - first) <= nReadPos)
-        {
-            // special case for inserting at the front when there's room
-            nReadPos -= (last - first);
-            memcpy(&vch[nReadPos], &first[0], last - first);
-        }
-        else
-            vch.insert(it, first, last);
-    }
-
-    void insert(iterator it, const value_type* first, const value_type* last)
-    {
-        if (last == first) return;
-        assert(last - first > 0);
-        if (it == vch.begin() + nReadPos && (unsigned int)(last - first) <= nReadPos)
-        {
-            // special case for inserting at the front when there's room
-            nReadPos -= (last - first);
-            memcpy(&vch[nReadPos], &first[0], last - first);
-        }
-        else
-            vch.insert(it, first, last);
-    }
-
-    iterator erase(iterator it)
-    {
-        if (it == vch.begin() + nReadPos)
-        {
-            // special case for erasing from the front
-            if (++nReadPos >= vch.size())
-            {
-                // whenever we reach the end, we take the opportunity to clear the buffer
-                nReadPos = 0;
-                return vch.erase(vch.begin(), vch.end());
-            }
-            return vch.begin() + nReadPos;
-        }
-        else
-            return vch.erase(it);
-    }
-
-    iterator erase(iterator first, iterator last)
-    {
-        if (first == vch.begin() + nReadPos)
-        {
-            // special case for erasing from the front
-            if (last == vch.end())
-            {
-                nReadPos = 0;
-                return vch.erase(vch.begin(), vch.end());
-            }
-            else
-            {
-                nReadPos = (last - vch.begin());
-                return last;
-            }
-        }
-        else
-            return vch.erase(first, last);
-    }
-
     inline void Compact()
     {
         vch.erase(vch.begin(), vch.begin() + nReadPos);

src/test/serialize_tests.cpp

@@ -215,51 +215,6 @@ BOOST_AUTO_TEST_CASE(noncanonical)
     BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException);
 }
 
-BOOST_AUTO_TEST_CASE(insert_delete)
-{
-    constexpr auto B2I{[](std::byte b) { return std::to_integer<uint8_t>(b); }};
-
-    // Test inserting/deleting bytes.
-    CDataStream ss(SER_DISK, 0);
-    BOOST_CHECK_EQUAL(ss.size(), 0U);
-
-    ss.write(MakeByteSpan("\x00\x01\x02\xff").first(4));
-    BOOST_CHECK_EQUAL(ss.size(), 4U);
-
-    uint8_t c{11};
-
-    // Inserting at beginning/end/middle:
-    ss.insert(ss.begin(), std::byte{c});
-    BOOST_CHECK_EQUAL(ss.size(), 5U);
-    BOOST_CHECK_EQUAL(B2I(ss[0]), c);
-    BOOST_CHECK_EQUAL(B2I(ss[1]), 0);
-
-    ss.insert(ss.end(), std::byte{c});
-    BOOST_CHECK_EQUAL(ss.size(), 6U);
-    BOOST_CHECK_EQUAL(B2I(ss[4]), 0xff);
-    BOOST_CHECK_EQUAL(B2I(ss[5]), c);
-
-    ss.insert(ss.begin() + 2, std::byte{c});
-    BOOST_CHECK_EQUAL(ss.size(), 7U);
-    BOOST_CHECK_EQUAL(B2I(ss[2]), c);
-
-    // Delete at beginning/end/middle
-    ss.erase(ss.begin());
-    BOOST_CHECK_EQUAL(ss.size(), 6U);
-    BOOST_CHECK_EQUAL(B2I(ss[0]), 0);
-
-    ss.erase(ss.begin()+ss.size()-1);
-    BOOST_CHECK_EQUAL(ss.size(), 5U);
-    BOOST_CHECK_EQUAL(B2I(ss[4]), 0xff);
-
-    ss.erase(ss.begin()+1);
-    BOOST_CHECK_EQUAL(ss.size(), 4U);
-    BOOST_CHECK_EQUAL(B2I(ss[0]), 0);
-    BOOST_CHECK_EQUAL(B2I(ss[1]), 1);
-    BOOST_CHECK_EQUAL(B2I(ss[2]), 2);
-    BOOST_CHECK_EQUAL(B2I(ss[3]), 0xff);
-}
-
 BOOST_AUTO_TEST_CASE(class_methods)
 {
     int intval(100);

src/test/streams_tests.cpp

@@ -8,6 +8,8 @@
 
 #include <boost/test/unit_test.hpp>
 
+using namespace std::string_literals;
+
 BOOST_FIXTURE_TEST_SUITE(streams_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(streams_vector_writer)
@@ -162,46 +164,35 @@ BOOST_AUTO_TEST_CASE(bitstream_reader_writer)
 BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
 {
     std::vector<std::byte> in;
-    std::vector<char> expected_xor;
-    CDataStream ds(in, 0, 0);
 
     // Degenerate case
-    ds.Xor({0x00, 0x00});
-    BOOST_CHECK_EQUAL(
-            std::string(expected_xor.begin(), expected_xor.end()),
-            ds.str());
+    {
+        CDataStream ds{in, 0, 0};
+        ds.Xor({0x00, 0x00});
+        BOOST_CHECK_EQUAL(""s, ds.str());
+    }
 
     in.push_back(std::byte{0x0f});
     in.push_back(std::byte{0xf0});
-    expected_xor.push_back('\xf0');
-    expected_xor.push_back('\x0f');
 
     // Single character key
-
-    ds.clear();
-    ds.insert(ds.begin(), in.begin(), in.end());
-
-    ds.Xor({0xff});
-    BOOST_CHECK_EQUAL(
-            std::string(expected_xor.begin(), expected_xor.end()),
-            ds.str());
+    {
+        CDataStream ds{in, 0, 0};
+        ds.Xor({0xff});
+        BOOST_CHECK_EQUAL("\xf0\x0f"s, ds.str());
+    }
 
     // Multi character key
 
     in.clear();
-    expected_xor.clear();
     in.push_back(std::byte{0xf0});
     in.push_back(std::byte{0x0f});
-    expected_xor.push_back('\x0f');
-    expected_xor.push_back('\x00');
-
-    ds.clear();
-    ds.insert(ds.begin(), in.begin(), in.end());
 
-    ds.Xor({0xff, 0x0f});
-    BOOST_CHECK_EQUAL(
-            std::string(expected_xor.begin(), expected_xor.end()),
-            ds.str());
+    {
+        CDataStream ds{in, 0, 0};
+        ds.Xor({0xff, 0x0f});
+        BOOST_CHECK_EQUAL("\x0f\x00"s, ds.str());
+    }
 }
 
 BOOST_AUTO_TEST_CASE(streams_buffered_file)