From 5cd0717a54ce7a2065b29d90717aa2eb1c5e302d Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 1 Jun 2023 10:27:33 -0400 Subject: streams: Drop confusing DataStream::Serialize method and << operator DataStream Serialize method has surprising behavior because it just serializes raw bytes without a length prefix. When you serialize a string or vector, a length prefix is serialized before the raw object contents so the object can be unambiguously deserialized later. But DataStreams don't support deserializing at all and just dump the raw bytes. Having this inconsistency is not necessary and could be confusing (see https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1212315030) so this PR just drops the DataStream::Serialize method. --- src/net_processing.cpp | 4 ++-- src/streams.h | 8 -------- src/wallet/dump.cpp | 6 +----- src/wallet/wallet.cpp | 4 +--- 4 files changed, 4 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e723d4657d..51bcaf6d73 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3418,8 +3418,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // If the peer is old enough to have the old alert system, send it the final alert. if (greatest_common_version <= 70012) { - DataStream finalAlert{ParseHex("60010000000000000000000000ffffff7f00000000ffffff7ffeffff7f01ffffff7f00000000ffffff7f00ffffff7f002f555247454e543a20416c657274206b657920636f6d70726f6d697365642c2075706772616465207265717569726564004630440220653febd6410f470f6bae11cad19c48413becb1ac2c17f908fd0fd53bdc3abd5202206d0e9c96fe88d4a0f01ed9dedae2b6f9e00da94cad0fecaae66ecf689bf71b50")}; - m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make("alert", finalAlert)); + const auto finalAlert{ParseHex("60010000000000000000000000ffffff7f00000000ffffff7ffeffff7f01ffffff7f00000000ffffff7f00ffffff7f002f555247454e543a20416c657274206b657920636f6d70726f6d697365642c2075706772616465207265717569726564004630440220653febd6410f470f6bae11cad19c48413becb1ac2c17f908fd0fd53bdc3abd5202206d0e9c96fe88d4a0f01ed9dedae2b6f9e00da94cad0fecaae66ecf689bf71b50")}; + m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make("alert", Span{finalAlert})); } // Feeler connections exist only to verify if address is online. diff --git a/src/streams.h b/src/streams.h index e346aa0a3f..03df20b5db 100644 --- a/src/streams.h +++ b/src/streams.h @@ -293,14 +293,6 @@ public: vch.insert(vch.end(), src.begin(), src.end()); } - template - void Serialize(Stream& s) const - { - // Special case: stream << stream concatenates like stream += stream - if (!vch.empty()) - s.write(MakeByteSpan(vch)); - } - template DataStream& operator<<(const T& obj) { diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index 865af0bb23..44c93eed7c 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -255,11 +255,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: std::vector k = ParseHex(key); std::vector v = ParseHex(value); - - DataStream ss_key{k}; - DataStream ss_value{v}; - - if (!batch->Write(ss_key, ss_value)) { + if (!batch->Write(Span{k}, Span{v})) { error = strprintf(_("Error: Unable to write record to new wallet")); ret = false; break; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 63ace5c47f..dce99b94bd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3875,9 +3875,7 @@ bool CWallet::MigrateToSQLite(bilingual_str& error) bool began = batch->TxnBegin(); assert(began); // This is a critical error, the new db could not be written to. The original db exists as a backup, but we should not continue execution. for (const auto& [key, value] : records) { - DataStream ss_key{key}; - DataStream ss_value{value}; - if (!batch->Write(ss_key, ss_value)) { + if (!batch->Write(MakeUCharSpan(key), MakeUCharSpan(value))) { batch->TxnAbort(); m_database->Close(); fs::remove(m_database->Filename()); -- cgit v1.2.3