Merge bitcoin/bitcoin#23653: Generalize/simplify VectorReader into SpanReader

2c35a93 Generalize/simplify VectorReader into SpanReader (Pieter Wuille)

Pull request description:

  Originally written for #21590 (safegcd-based MuHash inverses), but then found a better way that removed the need for it, so I'm submitting it independently.

ACKs for top commit:
  MarcoFalke:
    re-ACK 2c35a93 🖨
  shaavan:
    ACK 2c35a93

Tree-SHA512: 959e3251e0cfe20e13a50639b617c9dc2a561d613a0884d983c93d15dacb6d2305d760aa933d18ba055cef8a1651a344bcb6b3f93051ecf26d3f2efc5779efa4

Author: MarcoFalke

src/blockfilter.cpp

@@ -81,7 +81,7 @@ GCSFilter::GCSFilter(const Params& params)
 GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter)
     : m_params(params), m_encoded(std::move(encoded_filter))
 {
-    VectorReader stream(GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0);
+    SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0};
 
     uint64_t N = ReadCompactSize(stream);
     m_N = static_cast<uint32_t>(N);
@@ -92,7 +92,7 @@ GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_fi
 
     // Verify that the encoded filter contains exactly N elements. If it has too much or too little
     // data, a std::ios_base::failure exception will be raised.
-    BitStreamReader<VectorReader> bitreader(stream);
+    BitStreamReader<SpanReader> bitreader{stream};
     for (uint64_t i = 0; i < m_N; ++i) {
         GolombRiceDecode(bitreader, m_params.m_P);
     }
@@ -133,13 +133,13 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements)
 
 bool GCSFilter::MatchInternal(const uint64_t* element_hashes, size_t size) const
 {
-    VectorReader stream(GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0);
+    SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0};
 
     // Seek forward by size of N
     uint64_t N = ReadCompactSize(stream);
     assert(N == m_N);
 
-    BitStreamReader<VectorReader> bitreader(stream);
+    BitStreamReader<SpanReader> bitreader{stream};
 
     uint64_t value = 0;
     size_t hashes_index = 0;

src/signet.cpp

@@ -98,7 +98,7 @@ std::optional<SignetTxs> SignetTxs::Create(const CBlock& block, const CScript& c
         // no signet solution -- allow this to support OP_TRUE as trivial block challenge
     } else {
         try {
-            VectorReader v(SER_NETWORK, INIT_PROTO_VERSION, signet_solution, 0);
+            SpanReader v{SER_NETWORK, INIT_PROTO_VERSION, signet_solution, 0};
             v >> tx_spending.vin[0].scriptSig;
             v >> tx_spending.vin[0].scriptWitness.stack;
             if (!v.empty()) return std::nullopt; // extraneous data encountered

src/streams.h

@@ -128,15 +128,14 @@ class CVectorWriter
     size_t nPos;
 };
 
-/** Minimal stream for reading from an existing vector by reference
+/** Minimal stream for reading from an existing byte array by Span.
  */
-class VectorReader
+class SpanReader
 {
 private:
     const int m_type;
     const int m_version;
-    const std::vector<unsigned char>& m_data;
-    size_t m_pos = 0;
+    Span<const unsigned char> m_data;
 
 public:
 
@@ -146,28 +145,29 @@ class VectorReader
      * @param[in]  data Referenced byte vector to overwrite/append
      * @param[in]  pos Starting position. Vector index where reads should start.
      */
-    VectorReader(int type, int version, const std::vector<unsigned char>& data, size_t pos)
-        : m_type(type), m_version(version), m_data(data), m_pos(pos)
+    SpanReader(int type, int version, Span<const unsigned char> data, size_t pos)
+        : m_type(type), m_version(version), m_data(data)
     {
-        if (m_pos > m_data.size()) {
-            throw std::ios_base::failure("VectorReader(...): end of data (m_pos > m_data.size())");
+        if (pos > m_data.size()) {
+            throw std::ios_base::failure("SpanReader(...): end of data (pos > m_data.size())");
         }
+        data = data.subspan(pos);
     }
 
     /**
      * (other params same as above)
      * @param[in]  args  A list of items to deserialize starting at pos.
      */
     template <typename... Args>
-    VectorReader(int type, int version, const std::vector<unsigned char>& data, size_t pos,
+    SpanReader(int type, int version, Span<const unsigned char> data, size_t pos,
                   Args&&... args)
-        : VectorReader(type, version, data, pos)
+        : SpanReader(type, version, data, pos)
     {
         ::UnserializeMany(*this, std::forward<Args>(args)...);
     }
 
     template<typename T>
-    VectorReader& operator>>(T&& obj)
+    SpanReader& operator>>(T&& obj)
     {
         // Unserialize from this stream
         ::Unserialize(*this, obj);
@@ -177,8 +177,8 @@ class VectorReader
     int GetVersion() const { return m_version; }
     int GetType() const { return m_type; }
 
-    size_t size() const { return m_data.size() - m_pos; }
-    bool empty() const { return m_data.size() == m_pos; }
+    size_t size() const { return m_data.size(); }
+    bool empty() const { return m_data.empty(); }
 
     void read(char* dst, size_t n)
     {
@@ -187,12 +187,11 @@ class VectorReader
         }
 
         // Read from the beginning of the buffer
-        size_t pos_next = m_pos + n;
-        if (pos_next > m_data.size()) {
-            throw std::ios_base::failure("VectorReader::read(): end of data");
+        if (n > m_data.size()) {
+            throw std::ios_base::failure("SpanReader::read(): end of data");
         }
-        memcpy(dst, m_data.data() + m_pos, n);
-        m_pos = pos_next;
+        memcpy(dst, m_data.data(), n);
+        m_data = m_data.subspan(n);
     }
 };
 

src/test/fuzz/golomb_rice.cpp

@@ -82,8 +82,8 @@ FUZZ_TARGET(golomb_rice)
 
     std::vector<uint64_t> decoded_deltas;
     {
-        VectorReader stream{SER_NETWORK, 0, golomb_rice_data, 0};
-        BitStreamReader<VectorReader> bitreader(stream);
+        SpanReader stream{SER_NETWORK, 0, golomb_rice_data, 0};
+        BitStreamReader<SpanReader> bitreader{stream};
         const uint32_t n = static_cast<uint32_t>(ReadCompactSize(stream));
         for (uint32_t i = 0; i < n; ++i) {
             decoded_deltas.push_back(GolombRiceDecode(bitreader, BASIC_FILTER_P));
@@ -94,14 +94,14 @@ FUZZ_TARGET(golomb_rice)
 
     {
         const std::vector<uint8_t> random_bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider, 1024);
-        VectorReader stream{SER_NETWORK, 0, random_bytes, 0};
+        SpanReader stream{SER_NETWORK, 0, random_bytes, 0};
         uint32_t n;
         try {
             n = static_cast<uint32_t>(ReadCompactSize(stream));
         } catch (const std::ios_base::failure&) {
             return;
         }
-        BitStreamReader<VectorReader> bitreader(stream);
+        BitStreamReader<SpanReader> bitreader{stream};
         for (uint32_t i = 0; i < std::min<uint32_t>(n, 1024); ++i) {
             try {
                 (void)GolombRiceDecode(bitreader, BASIC_FILTER_P);

src/test/fuzz/script_assets_test_minimizer.cpp

@@ -54,7 +54,7 @@ CMutableTransaction TxFromHex(const std::string& str)
 {
     CMutableTransaction tx;
     try {
-        VectorReader(SER_DISK, SERIALIZE_TRANSACTION_NO_WITNESS, CheckedParseHex(str), 0) >> tx;
+        SpanReader{SER_DISK, SERIALIZE_TRANSACTION_NO_WITNESS, CheckedParseHex(str), 0} >> tx;
     } catch (const std::ios_base::failure&) {
         throw std::runtime_error("Tx deserialization failure");
     }
@@ -68,7 +68,7 @@ std::vector<CTxOut> TxOutsFromJSON(const UniValue& univalue)
     for (size_t i = 0; i < univalue.size(); ++i) {
         CTxOut txout;
         try {
-            VectorReader(SER_DISK, 0, CheckedParseHex(univalue[i].get_str()), 0) >> txout;
+            SpanReader{SER_DISK, 0, CheckedParseHex(univalue[i].get_str()), 0} >> txout;
         } catch (const std::ios_base::failure&) {
             throw std::runtime_error("Prevout invalid format");
         }

src/test/script_tests.cpp

@@ -1473,7 +1473,7 @@ BOOST_AUTO_TEST_CASE(script_HasValidOps)
 static CMutableTransaction TxFromHex(const std::string& str)
 {
     CMutableTransaction tx;
-    VectorReader(SER_DISK, SERIALIZE_TRANSACTION_NO_WITNESS, ParseHex(str), 0) >> tx;
+    SpanReader{SER_DISK, SERIALIZE_TRANSACTION_NO_WITNESS, ParseHex(str), 0} >> tx;
     return tx;
 }
 
@@ -1483,7 +1483,7 @@ static std::vector<CTxOut> TxOutsFromJSON(const UniValue& univalue)
     std::vector<CTxOut> prevouts;
     for (size_t i = 0; i < univalue.size(); ++i) {
         CTxOut txout;
-        VectorReader(SER_DISK, 0, ParseHex(univalue[i].get_str()), 0) >> txout;
+        SpanReader{SER_DISK, 0, ParseHex(univalue[i].get_str()), 0} >> txout;
         prevouts.push_back(std::move(txout));
     }
     return prevouts;
@@ -1754,7 +1754,7 @@ BOOST_AUTO_TEST_CASE(bip341_keypath_test_vectors)
     for (const auto& vec : vectors.getValues()) {
         auto txhex = ParseHex(vec["given"]["rawUnsignedTx"].get_str());
         CMutableTransaction tx;
-        VectorReader(SER_NETWORK, PROTOCOL_VERSION, txhex, 0) >> tx;
+        SpanReader{SER_NETWORK, PROTOCOL_VERSION, txhex, 0} >> tx;
         std::vector<CTxOut> utxos;
         for (const auto& utxo_spent : vec["given"]["utxosSpent"].getValues()) {
             auto script_bytes = ParseHex(utxo_spent["scriptPubKey"].get_str());

src/test/streams_tests.cpp

@@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader)
 {
     std::vector<unsigned char> vch = {1, 255, 3, 4, 5, 6};
 
-    VectorReader reader(SER_NETWORK, INIT_PROTO_VERSION, vch, 0);
+    SpanReader reader{SER_NETWORK, INIT_PROTO_VERSION, vch, 0};
     BOOST_CHECK_EQUAL(reader.size(), 6U);
     BOOST_CHECK(!reader.empty());
 
@@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader)
     BOOST_CHECK_THROW(reader >> d, std::ios_base::failure);
 
     // Read a 4 bytes as a signed int from the beginning of the buffer.
-    VectorReader new_reader(SER_NETWORK, INIT_PROTO_VERSION, vch, 0);
+    SpanReader new_reader{SER_NETWORK, INIT_PROTO_VERSION, vch, 0};
     new_reader >> d;
     BOOST_CHECK_EQUAL(d, 67370753); // 1,255,3,4 in little-endian base-256
     BOOST_CHECK_EQUAL(new_reader.size(), 2U);
@@ -115,7 +115,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader)
 BOOST_AUTO_TEST_CASE(streams_vector_reader_rvalue)
 {
     std::vector<uint8_t> data{0x82, 0xa7, 0x31};
-    VectorReader reader(SER_NETWORK, INIT_PROTO_VERSION, data, /* pos= */ 0);
+    SpanReader reader{SER_NETWORK, INIT_PROTO_VERSION, data, /* pos= */ 0};
     uint32_t varint = 0;
     // Deserialize into r-value
     reader >> VARINT(varint);

src/wallet/test/wallet_tests.cpp

@@ -682,7 +682,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup)
     vw << (int32_t)0;
     vw << (int32_t)1;
 
-    VectorReader vr(0, 0, malformed_record, 0);
+    SpanReader vr{0, 0, malformed_record, 0};
     WalletDescriptor w_desc;
     BOOST_CHECK_EXCEPTION(vr >> w_desc, std::ios_base::failure, malformed_descriptor);
 }