foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Code Issues Activity

streams: Drop confusing DataStream::Serialize method and << operator

Browse source 
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.
This commit is contained in:
Ryan Ofsky 2023-06-01 10:27:33 -04:00
parent 3a83d4417b
commit 5cd0717a54
4 changed files with 4 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/net_processing.cpp
View file

@ -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.

8
src/streams.h
View file

@ -293,14 +293,6 @@ public:
vch.insert(vch.end(), src.begin(), src.end());
}
template<typename Stream>
void Serialize(Stream& s) const
{
// Special case: stream << stream concatenates like stream += stream
if (!vch.empty())
s.write(MakeByteSpan(vch));
}
template<typename T>
DataStream& operator<<(const T& obj)
{

6
src/wallet/dump.cpp
View file

@ -255,11 +255,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs::
std::vector<unsigned char> k = ParseHex(key);
std::vector<unsigned char> 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;

4
src/wallet/wallet.cpp
View file

@ -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());