From fa4a9c0f4334678fb80358ead667807bf2a0a153 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 11 Sep 2023 14:58:22 +0000 Subject: [PATCH 1/3] Remove unused GetType() from OverrideStream, CVectorWriter, SpanReader GetType() is never called, so it is completely unused and can be removed. --- src/blockfilter.cpp | 9 ++--- src/net.cpp | 2 +- src/netmessagemaker.h | 2 +- src/psbt.h | 26 +++++++------- src/signet.cpp | 4 +-- src/streams.h | 23 ++++--------- src/test/fuzz/golomb_rice.cpp | 6 ++-- .../fuzz/script_assets_test_minimizer.cpp | 4 +-- src/test/script_tests.cpp | 6 ++-- src/test/streams_tests.cpp | 34 +++++++++---------- src/wallet/test/wallet_tests.cpp | 4 +-- 11 files changed, 53 insertions(+), 67 deletions(-) diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp index 985a81f522..dd3824fb1c 100644 --- a/src/blockfilter.cpp +++ b/src/blockfilter.cpp @@ -16,9 +16,6 @@ #include #include -/// SerType used to serialize parameters in GCS filter encoding. -static constexpr int GCS_SER_TYPE = SER_NETWORK; - /// Protocol version used to serialize parameters in GCS filter encoding. static constexpr int GCS_SER_VERSION = 0; @@ -52,7 +49,7 @@ GCSFilter::GCSFilter(const Params& params) GCSFilter::GCSFilter(const Params& params, std::vector encoded_filter, bool skip_decode_check) : m_params(params), m_encoded(std::move(encoded_filter)) { - SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded}; + SpanReader stream{GCS_SER_VERSION, m_encoded}; uint64_t N = ReadCompactSize(stream); m_N = static_cast(N); @@ -84,7 +81,7 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements) } m_F = static_cast(m_N) * static_cast(m_params.m_M); - CVectorWriter stream(GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0); + CVectorWriter stream(GCS_SER_VERSION, m_encoded, 0); WriteCompactSize(stream, m_N); @@ -106,7 +103,7 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements) bool GCSFilter::MatchInternal(const uint64_t* element_hashes, size_t size) const { - SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded}; + SpanReader stream{GCS_SER_VERSION, m_encoded}; // Seek forward by size of N uint64_t N = ReadCompactSize(stream); diff --git a/src/net.cpp b/src/net.cpp index ef1135bb8c..29d66a1d1a 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -859,7 +859,7 @@ bool V1Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept // serialize header m_header_to_send.clear(); - CVectorWriter{SER_NETWORK, INIT_PROTO_VERSION, m_header_to_send, 0, hdr}; + CVectorWriter{INIT_PROTO_VERSION, m_header_to_send, 0, hdr}; // update state m_message_to_send = std::move(msg); diff --git a/src/netmessagemaker.h b/src/netmessagemaker.h index 89fb4758f9..a121183aab 100644 --- a/src/netmessagemaker.h +++ b/src/netmessagemaker.h @@ -19,7 +19,7 @@ public: { CSerializedNetMsg msg; msg.m_type = std::move(msg_type); - CVectorWriter{ SER_NETWORK, nFlags | nVersion, msg.data, 0, std::forward(args)... }; + CVectorWriter{nFlags | nVersion, msg.data, 0, std::forward(args)...}; return msg; } diff --git a/src/psbt.h b/src/psbt.h index 9464b10268..48e0453084 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -226,7 +226,7 @@ struct PSBTInput // Write the utxo if (non_witness_utxo) { SerializeToVector(s, CompactSizeWriter(PSBT_IN_NON_WITNESS_UTXO)); - OverrideStream os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS); + OverrideStream os{&s, s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS}; SerializeToVector(os, non_witness_utxo); } if (!witness_utxo.IsNull()) { @@ -315,7 +315,7 @@ struct PSBTInput const auto& [leaf_hashes, origin] = leaf_origin; SerializeToVector(s, PSBT_IN_TAP_BIP32_DERIVATION, xonly); std::vector value; - CVectorWriter s_value(s.GetType(), s.GetVersion(), value, 0); + CVectorWriter s_value{s.GetVersion(), value, 0}; s_value << leaf_hashes; SerializeKeyOrigin(s_value, origin); s << value; @@ -381,7 +381,7 @@ struct PSBTInput } // Type is compact size uint at beginning of key - SpanReader skey(s.GetType(), s.GetVersion(), key); + SpanReader skey{s.GetVersion(), key}; uint64_t type = ReadCompactSize(skey); // Do stuff based on type @@ -394,7 +394,7 @@ struct PSBTInput throw std::ios_base::failure("Non-witness utxo key is more than one byte type"); } // Set the stream to unserialize with witness since this is always a valid network transaction - OverrideStream os(&s, s.GetType(), s.GetVersion() & ~SERIALIZE_TRANSACTION_NO_WITNESS); + OverrideStream os{&s, s.GetVersion() & ~SERIALIZE_TRANSACTION_NO_WITNESS}; UnserializeFromVector(os, non_witness_utxo); break; } @@ -590,7 +590,7 @@ struct PSBTInput } else if (key.size() != 65) { throw std::ios_base::failure("Input Taproot script signature key is not 65 bytes"); } - SpanReader s_key(s.GetType(), s.GetVersion(), Span{key}.subspan(1)); + SpanReader s_key{s.GetVersion(), Span{key}.subspan(1)}; XOnlyPubKey xonly; uint256 hash; s_key >> xonly; @@ -632,7 +632,7 @@ struct PSBTInput } else if (key.size() != 33) { throw std::ios_base::failure("Input Taproot BIP32 keypath key is not at 33 bytes"); } - SpanReader s_key(s.GetType(), s.GetVersion(), Span{key}.subspan(1)); + SpanReader s_key{s.GetVersion(), Span{key}.subspan(1)}; XOnlyPubKey xonly; s_key >> xonly; std::set leaf_hashes; @@ -757,7 +757,7 @@ struct PSBTOutput if (!m_tap_tree.empty()) { SerializeToVector(s, PSBT_OUT_TAP_TREE); std::vector value; - CVectorWriter s_value(s.GetType(), s.GetVersion(), value, 0); + CVectorWriter s_value{s.GetVersion(), value, 0}; for (const auto& [depth, leaf_ver, script] : m_tap_tree) { s_value << depth; s_value << leaf_ver; @@ -771,7 +771,7 @@ struct PSBTOutput const auto& [leaf_hashes, origin] = leaf; SerializeToVector(s, PSBT_OUT_TAP_BIP32_DERIVATION, xonly); std::vector value; - CVectorWriter s_value(s.GetType(), s.GetVersion(), value, 0); + CVectorWriter s_value{s.GetVersion(), value, 0}; s_value << leaf_hashes; SerializeKeyOrigin(s_value, origin); s << value; @@ -807,7 +807,7 @@ struct PSBTOutput } // Type is compact size uint at beginning of key - SpanReader skey(s.GetType(), s.GetVersion(), key); + SpanReader skey{s.GetVersion(), key}; uint64_t type = ReadCompactSize(skey); // Do stuff based on type @@ -856,7 +856,7 @@ struct PSBTOutput } std::vector tree_v; s >> tree_v; - SpanReader s_tree(s.GetType(), s.GetVersion(), tree_v); + SpanReader s_tree{s.GetVersion(), tree_v}; if (s_tree.empty()) { throw std::ios_base::failure("Output Taproot tree must not be empty"); } @@ -984,7 +984,7 @@ struct PartiallySignedTransaction SerializeToVector(s, CompactSizeWriter(PSBT_GLOBAL_UNSIGNED_TX)); // Write serialized tx to a stream - OverrideStream os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS); + OverrideStream os{&s, s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS}; SerializeToVector(os, *tx); // Write xpubs @@ -1061,7 +1061,7 @@ struct PartiallySignedTransaction } // Type is compact size uint at beginning of key - SpanReader skey(s.GetType(), s.GetVersion(), key); + SpanReader skey{s.GetVersion(), key}; uint64_t type = ReadCompactSize(skey); // Do stuff based on type @@ -1075,7 +1075,7 @@ struct PartiallySignedTransaction } CMutableTransaction mtx; // Set the stream to serialize with non-witness since this should always be non-witness - OverrideStream os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS); + OverrideStream os{&s, s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS}; UnserializeFromVector(os, mtx); tx = std::move(mtx); // Make sure that all scriptSigs and scriptWitnesses are empty diff --git a/src/signet.cpp b/src/signet.cpp index 21b289b637..ef0faaa5f8 100644 --- a/src/signet.cpp +++ b/src/signet.cpp @@ -98,7 +98,7 @@ std::optional SignetTxs::Create(const CBlock& block, const CScript& c // no signet solution -- allow this to support OP_TRUE as trivial block challenge } else { try { - SpanReader v{SER_NETWORK, INIT_PROTO_VERSION, signet_solution}; + SpanReader v{INIT_PROTO_VERSION, signet_solution}; v >> tx_spending.vin[0].scriptSig; v >> tx_spending.vin[0].scriptWitness.stack; if (!v.empty()) return std::nullopt; // extraneous data encountered @@ -109,7 +109,7 @@ std::optional SignetTxs::Create(const CBlock& block, const CScript& c uint256 signet_merkle = ComputeModifiedMerkleRoot(modified_cb, block); std::vector block_data; - CVectorWriter writer(SER_NETWORK, INIT_PROTO_VERSION, block_data, 0); + CVectorWriter writer{INIT_PROTO_VERSION, block_data, 0}; writer << block.nVersion; writer << block.hashPrevBlock; writer << signet_merkle; diff --git a/src/streams.h b/src/streams.h index f9a817c9b6..2b9a5023bf 100644 --- a/src/streams.h +++ b/src/streams.h @@ -50,11 +50,10 @@ class OverrideStream { Stream* stream; - const int nType; const int nVersion; public: - OverrideStream(Stream* stream_, int nType_, int nVersion_) : stream(stream_), nType(nType_), nVersion(nVersion_) {} + OverrideStream(Stream* stream_, int nVersion_) : stream{stream_}, nVersion{nVersion_} {} template OverrideStream& operator<<(const T& obj) @@ -81,7 +80,6 @@ public: } int GetVersion() const { return nVersion; } - int GetType() const { return nType; } size_t size() const { return stream->size(); } void ignore(size_t size) { return stream->ignore(size); } }; @@ -95,13 +93,12 @@ class CVectorWriter public: /* - * @param[in] nTypeIn Serialization Type * @param[in] nVersionIn Serialization Version (including any flags) * @param[in] vchDataIn Referenced byte vector to overwrite/append * @param[in] nPosIn Starting position. Vector index where writes should start. The vector will initially * grow as necessary to max(nPosIn, vec.size()). So to append, use vec.size(). */ - CVectorWriter(int nTypeIn, int nVersionIn, std::vector& vchDataIn, size_t nPosIn) : nType(nTypeIn), nVersion(nVersionIn), vchData(vchDataIn), nPos(nPosIn) + CVectorWriter(int nVersionIn, std::vector& vchDataIn, size_t nPosIn) : nVersion{nVersionIn}, vchData{vchDataIn}, nPos{nPosIn} { if(nPos > vchData.size()) vchData.resize(nPos); @@ -111,7 +108,7 @@ class CVectorWriter * @param[in] args A list of items to serialize starting at nPosIn. */ template - CVectorWriter(int nTypeIn, int nVersionIn, std::vector& vchDataIn, size_t nPosIn, Args&&... args) : CVectorWriter(nTypeIn, nVersionIn, vchDataIn, nPosIn) + CVectorWriter(int nVersionIn, std::vector& vchDataIn, size_t nPosIn, Args&&... args) : CVectorWriter{nVersionIn, vchDataIn, nPosIn} { ::SerializeMany(*this, std::forward(args)...); } @@ -137,12 +134,8 @@ class CVectorWriter { return nVersion; } - int GetType() const - { - return nType; - } + private: - const int nType; const int nVersion; std::vector& vchData; size_t nPos; @@ -153,19 +146,16 @@ private: class SpanReader { private: - const int m_type; const int m_version; Span m_data; public: - /** - * @param[in] type Serialization Type * @param[in] version Serialization Version (including any flags) * @param[in] data Referenced byte vector to overwrite/append */ - SpanReader(int type, int version, Span data) - : m_type(type), m_version(version), m_data(data) {} + SpanReader(int version, Span data) + : m_version{version}, m_data{data} {} template SpanReader& operator>>(T&& obj) @@ -175,7 +165,6 @@ public: } int GetVersion() const { return m_version; } - int GetType() const { return m_type; } size_t size() const { return m_data.size(); } bool empty() const { return m_data.empty(); } diff --git a/src/test/fuzz/golomb_rice.cpp b/src/test/fuzz/golomb_rice.cpp index e006653ca9..f3073c5c97 100644 --- a/src/test/fuzz/golomb_rice.cpp +++ b/src/test/fuzz/golomb_rice.cpp @@ -51,7 +51,7 @@ FUZZ_TARGET(golomb_rice) for (int i = 0; i < n; ++i) { elements.insert(ConsumeRandomLengthByteVector(fuzzed_data_provider, 16)); } - CVectorWriter stream(SER_NETWORK, 0, golomb_rice_data, 0); + CVectorWriter stream{0, golomb_rice_data, 0}; WriteCompactSize(stream, static_cast(elements.size())); BitStreamWriter bitwriter(stream); if (!elements.empty()) { @@ -68,7 +68,7 @@ FUZZ_TARGET(golomb_rice) std::vector decoded_deltas; { - SpanReader stream{SER_NETWORK, 0, golomb_rice_data}; + SpanReader stream{0, golomb_rice_data}; BitStreamReader bitreader{stream}; const uint32_t n = static_cast(ReadCompactSize(stream)); for (uint32_t i = 0; i < n; ++i) { @@ -80,7 +80,7 @@ FUZZ_TARGET(golomb_rice) { const std::vector random_bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider, 1024); - SpanReader stream{SER_NETWORK, 0, random_bytes}; + SpanReader stream{0, random_bytes}; uint32_t n; try { n = static_cast(ReadCompactSize(stream)); diff --git a/src/test/fuzz/script_assets_test_minimizer.cpp b/src/test/fuzz/script_assets_test_minimizer.cpp index 7862be2f21..66c862a6f9 100644 --- a/src/test/fuzz/script_assets_test_minimizer.cpp +++ b/src/test/fuzz/script_assets_test_minimizer.cpp @@ -54,7 +54,7 @@ CMutableTransaction TxFromHex(const std::string& str) { CMutableTransaction tx; try { - SpanReader{SER_DISK, SERIALIZE_TRANSACTION_NO_WITNESS, CheckedParseHex(str)} >> tx; + SpanReader{SERIALIZE_TRANSACTION_NO_WITNESS, CheckedParseHex(str)} >> tx; } catch (const std::ios_base::failure&) { throw std::runtime_error("Tx deserialization failure"); } @@ -68,7 +68,7 @@ std::vector TxOutsFromJSON(const UniValue& univalue) for (size_t i = 0; i < univalue.size(); ++i) { CTxOut txout; try { - SpanReader{SER_DISK, 0, CheckedParseHex(univalue[i].get_str())} >> txout; + SpanReader{0, CheckedParseHex(univalue[i].get_str())} >> txout; } catch (const std::ios_base::failure&) { throw std::runtime_error("Prevout invalid format"); } diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index d63bfb9603..94656b229e 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1470,7 +1470,7 @@ BOOST_AUTO_TEST_CASE(script_HasValidOps) static CMutableTransaction TxFromHex(const std::string& str) { CMutableTransaction tx; - SpanReader{SER_DISK, SERIALIZE_TRANSACTION_NO_WITNESS, ParseHex(str)} >> tx; + SpanReader{SERIALIZE_TRANSACTION_NO_WITNESS, ParseHex(str)} >> tx; return tx; } @@ -1480,7 +1480,7 @@ static std::vector TxOutsFromJSON(const UniValue& univalue) std::vector prevouts; for (size_t i = 0; i < univalue.size(); ++i) { CTxOut txout; - SpanReader{SER_DISK, 0, ParseHex(univalue[i].get_str())} >> txout; + SpanReader{0, ParseHex(univalue[i].get_str())} >> txout; prevouts.push_back(std::move(txout)); } return prevouts; @@ -1751,7 +1751,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; - SpanReader{SER_NETWORK, PROTOCOL_VERSION, txhex} >> tx; + SpanReader{PROTOCOL_VERSION, txhex} >> tx; std::vector utxos; for (const auto& utxo_spent : vec["given"]["utxosSpent"].getValues()) { auto script_bytes = ParseHex(utxo_spent["scriptPubKey"].get_str()); diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 99740ee779..aca38c747f 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -74,49 +74,49 @@ BOOST_AUTO_TEST_CASE(streams_vector_writer) // point should yield the same results, even if the first test grew the // vector. - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 0, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 0, a, b}; BOOST_CHECK((vch == std::vector{{1, 2}})); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 0, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 0, a, b}; BOOST_CHECK((vch == std::vector{{1, 2}})); vch.clear(); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 1, 2}})); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 1, 2}})); vch.clear(); vch.resize(5, 0); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 1, 2, 0}})); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 1, 2, 0}})); vch.clear(); vch.resize(4, 0); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 3, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 3, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 0, 1, 2}})); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 3, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 3, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 0, 1, 2}})); vch.clear(); vch.resize(4, 0); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 4, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 4, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 0, 0, 1, 2}})); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 4, a, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 4, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 0, 0, 1, 2}})); vch.clear(); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 0, bytes); + CVectorWriter{INIT_PROTO_VERSION, vch, 0, bytes}; BOOST_CHECK((vch == std::vector{{3, 4, 5, 6}})); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 0, bytes); + CVectorWriter{INIT_PROTO_VERSION, vch, 0, bytes}; BOOST_CHECK((vch == std::vector{{3, 4, 5, 6}})); vch.clear(); vch.resize(4, 8); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, bytes, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, bytes, b}; BOOST_CHECK((vch == std::vector{{8, 8, 1, 3, 4, 5, 6, 2}})); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, bytes, b); + CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, bytes, b}; BOOST_CHECK((vch == std::vector{{8, 8, 1, 3, 4, 5, 6, 2}})); vch.clear(); } @@ -125,7 +125,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) { std::vector vch = {1, 255, 3, 4, 5, 6}; - SpanReader reader{SER_NETWORK, INIT_PROTO_VERSION, vch}; + SpanReader reader{INIT_PROTO_VERSION, vch}; BOOST_CHECK_EQUAL(reader.size(), 6U); BOOST_CHECK(!reader.empty()); @@ -155,7 +155,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. - SpanReader new_reader{SER_NETWORK, INIT_PROTO_VERSION, vch}; + SpanReader new_reader{INIT_PROTO_VERSION, vch}; new_reader >> d; BOOST_CHECK_EQUAL(d, 67370753); // 1,255,3,4 in little-endian base-256 BOOST_CHECK_EQUAL(new_reader.size(), 2U); @@ -169,7 +169,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) BOOST_AUTO_TEST_CASE(streams_vector_reader_rvalue) { std::vector data{0x82, 0xa7, 0x31}; - SpanReader reader{SER_NETWORK, INIT_PROTO_VERSION, data}; + SpanReader reader{INIT_PROTO_VERSION, data}; uint32_t varint = 0; // Deserialize into r-value reader >> VARINT(varint); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 5c297d76e4..ab17f51f8e 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -752,14 +752,14 @@ bool malformed_descriptor(std::ios_base::failure e) BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup) { std::vector malformed_record; - CVectorWriter vw(0, 0, malformed_record, 0); + CVectorWriter vw{0, malformed_record, 0}; vw << std::string("notadescriptor"); vw << uint64_t{0}; vw << int32_t{0}; vw << int32_t{0}; vw << int32_t{1}; - SpanReader vr{0, 0, malformed_record}; + SpanReader vr{0, malformed_record}; WalletDescriptor w_desc; BOOST_CHECK_EXCEPTION(vr >> w_desc, std::ios_base::failure, malformed_descriptor); } From fa72f09d6ff8ee204f331a69d3f5e825223c9e11 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 11 Sep 2023 15:47:52 +0200 Subject: [PATCH 2/3] Remove CHashWriter type The type is only ever set, but never read via GetType(), so remove it. Also, remove SerializeHash to avoid silent merge conflicts and use the already existing GetHash() boilerplate consistently. --- src/hash.h | 13 +------------ src/primitives/block.cpp | 2 +- src/primitives/transaction.cpp | 6 +++--- src/test/hash_tests.cpp | 2 +- src/test/serialize_tests.cpp | 2 +- src/test/sighash_tests.cpp | 2 +- 6 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/hash.h b/src/hash.h index f2b627ff4f..d355b703ff 100644 --- a/src/hash.h +++ b/src/hash.h @@ -149,13 +149,11 @@ public: class CHashWriter : public HashWriter { private: - const int nType; const int nVersion; public: - CHashWriter(int nTypeIn, int nVersionIn) : nType(nTypeIn), nVersion(nVersionIn) {} + CHashWriter(int nVersionIn) : nVersion{nVersionIn} {} - int GetType() const { return nType; } int GetVersion() const { return nVersion; } template @@ -223,15 +221,6 @@ public: } }; -/** Compute the 256-bit hash of an object's serialization. */ -template -uint256 SerializeHash(const T& obj, int nType=SER_GETHASH, int nVersion=PROTOCOL_VERSION) -{ - CHashWriter ss(nType, nVersion); - ss << obj; - return ss.GetHash(); -} - /** Single-SHA256 a 32-byte input (represented as uint256). */ [[nodiscard]] uint256 SHA256Uint256(const uint256& input); diff --git a/src/primitives/block.cpp b/src/primitives/block.cpp index 50a30cb511..3d21708820 100644 --- a/src/primitives/block.cpp +++ b/src/primitives/block.cpp @@ -10,7 +10,7 @@ uint256 CBlockHeader::GetHash() const { - return SerializeHash(*this); + return (CHashWriter{PROTOCOL_VERSION} << *this).GetHash(); } std::string CBlock::ToString() const diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 3060746909..2c913bf432 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -67,12 +67,12 @@ CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), uint256 CMutableTransaction::GetHash() const { - return SerializeHash(*this, SER_GETHASH, SERIALIZE_TRANSACTION_NO_WITNESS); + return (CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash(); } uint256 CTransaction::ComputeHash() const { - return SerializeHash(*this, SER_GETHASH, SERIALIZE_TRANSACTION_NO_WITNESS); + return (CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash(); } uint256 CTransaction::ComputeWitnessHash() const @@ -80,7 +80,7 @@ uint256 CTransaction::ComputeWitnessHash() const if (!HasWitness()) { return hash; } - return SerializeHash(*this, SER_GETHASH, 0); + return (CHashWriter{0} << *this).GetHash(); } CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {} diff --git a/src/test/hash_tests.cpp b/src/test/hash_tests.cpp index a990797ca7..54afcef989 100644 --- a/src/test/hash_tests.cpp +++ b/src/test/hash_tests.cpp @@ -122,7 +122,7 @@ BOOST_AUTO_TEST_CASE(siphash) (uint64_t(x+4)<<32)|(uint64_t(x+5)<<40)|(uint64_t(x+6)<<48)|(uint64_t(x+7)<<56)); } - CHashWriter ss(SER_DISK, CLIENT_VERSION); + CHashWriter ss{CLIENT_VERSION}; CMutableTransaction tx; // Note these tests were originally written with tx.nVersion=1 // and the test would be affected by default tx version bumps if not fixed. diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 2f2bb6698c..d18d2623b1 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -176,7 +176,7 @@ BOOST_AUTO_TEST_CASE(vector_bool) std::vector vec2{1, 0, 0, 1, 1, 1, 0, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 0, 1, 1, 1, 1, 0, 1, 0, 0, 1}; BOOST_CHECK(vec1 == std::vector(vec2.begin(), vec2.end())); - BOOST_CHECK(SerializeHash(vec1) == SerializeHash(vec2)); + BOOST_CHECK((HashWriter{} << vec1).GetHash() == (HashWriter{} << vec2).GetHash()); } BOOST_AUTO_TEST_CASE(noncanonical) diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index d1c0e1349e..178b16772b 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -78,7 +78,7 @@ uint256 static SignatureHashOld(CScript scriptCode, const CTransaction& txTo, un } // Serialize and hash - CHashWriter ss(SER_GETHASH, SERIALIZE_TRANSACTION_NO_WITNESS); + CHashWriter ss{SERIALIZE_TRANSACTION_NO_WITNESS}; ss << txTmp << nHashType; return ss.GetHash(); } From fac29a0ab19fda457b55d7a0a37c5cd3d9680f82 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 11 Sep 2023 16:12:34 +0000 Subject: [PATCH 3/3] Remove SER_GETHASH, hard-code client version in CKeyPool serialize It was never set, so it can be removed along with any code reading it. --- src/serialize.h | 1 - src/wallet/scriptpubkeyman.h | 10 ++-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/serialize.h b/src/serialize.h index 1ad8ac4373..e53ff9fa4c 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -131,7 +131,6 @@ enum // primary actions SER_NETWORK = (1 << 0), SER_DISK = (1 << 1), - SER_GETHASH = (1 << 2), }; /** diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index ec7b017720..3722c1ae1f 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -123,20 +123,14 @@ public: template void Serialize(Stream& s) const { - int nVersion = s.GetVersion(); - if (!(s.GetType() & SER_GETHASH)) { - s << nVersion; - } + s << int{259900}; // Unused field, writes the highest client version ever written s << nTime << vchPubKey << fInternal << m_pre_split; } template void Unserialize(Stream& s) { - int nVersion = s.GetVersion(); - if (!(s.GetType() & SER_GETHASH)) { - s >> nVersion; - } + s >> int{}; // Discard unused field s >> nTime >> vchPubKey; try { s >> fInternal;