diff --git a/src/addrman.cpp b/src/addrman.cpp index eb9c3725507..d0b820ee651 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -173,7 +173,7 @@ void AddrManImpl::Serialize(Stream& s_) const */ // Always serialize in the latest version (FILE_FORMAT). - ParamsStream s{CAddress::V2_DISK, s_}; + ParamsStream s{s_, CAddress::V2_DISK}; s << static_cast(FILE_FORMAT); @@ -238,7 +238,7 @@ void AddrManImpl::Unserialize(Stream& s_) s_ >> Using>(format); const auto ser_params = (format >= Format::V3_BIP155 ? CAddress::V2_DISK : CAddress::V1_DISK); - ParamsStream s{ser_params, s_}; + ParamsStream s{s_, ser_params}; uint8_t compat; s >> compat; diff --git a/src/net.cpp b/src/net.cpp index ad1e4646678..472b8e517e6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -196,8 +196,7 @@ static std::vector ConvertSeeds(const std::vector &vSeedsIn) const auto one_week{7 * 24h}; std::vector vSeedsOut; FastRandomContext rng; - DataStream underlying_stream{vSeedsIn}; - ParamsStream s{CAddress::V2_NETWORK, underlying_stream}; + ParamsStream s{DataStream{vSeedsIn}, CAddress::V2_NETWORK}; while (!s.eof()) { CService endpoint; s >> endpoint; diff --git a/src/netaddress.h b/src/netaddress.h index c63bd4b4e5c..ea2d14336e4 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -237,7 +237,7 @@ public: template void Serialize(Stream& s) const { - if (s.GetParams().enc == Encoding::V2) { + if (s.template GetParams().enc == Encoding::V2) { SerializeV2Stream(s); } else { SerializeV1Stream(s); @@ -250,7 +250,7 @@ public: template void Unserialize(Stream& s) { - if (s.GetParams().enc == Encoding::V2) { + if (s.template GetParams().enc == Encoding::V2) { UnserializeV2Stream(s); } else { UnserializeV1Stream(s); diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index ccbeb3ec49b..976542cfae6 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -326,7 +326,7 @@ public: template inline void Serialize(Stream& s) const { - SerializeTransaction(*this, s, s.GetParams()); + SerializeTransaction(*this, s, s.template GetParams()); } /** This deserializing constructor is provided instead of an Unserialize method. @@ -334,7 +334,7 @@ public: template CTransaction(deserialize_type, const TransactionSerParams& params, Stream& s) : CTransaction(CMutableTransaction(deserialize, params, s)) {} template - CTransaction(deserialize_type, ParamsStream& s) : CTransaction(CMutableTransaction(deserialize, s)) {} + CTransaction(deserialize_type, Stream& s) : CTransaction(CMutableTransaction(deserialize, s)) {} bool IsNull() const { return vin.empty() && vout.empty(); @@ -386,12 +386,12 @@ struct CMutableTransaction template inline void Serialize(Stream& s) const { - SerializeTransaction(*this, s, s.GetParams()); + SerializeTransaction(*this, s, s.template GetParams()); } template inline void Unserialize(Stream& s) { - UnserializeTransaction(*this, s, s.GetParams()); + UnserializeTransaction(*this, s, s.template GetParams()); } template @@ -400,7 +400,7 @@ struct CMutableTransaction } template - CMutableTransaction(deserialize_type, ParamsStream& s) { + CMutableTransaction(deserialize_type, Stream& s) { Unserialize(s); } diff --git a/src/protocol.h b/src/protocol.h index 243cd23e6e6..fba08c6f55d 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -375,9 +375,10 @@ public: static constexpr SerParams V1_DISK{{CNetAddr::Encoding::V1}, Format::Disk}; static constexpr SerParams V2_DISK{{CNetAddr::Encoding::V2}, Format::Disk}; - SERIALIZE_METHODS_PARAMS(CAddress, obj, SerParams, params) + SERIALIZE_METHODS(CAddress, obj) { bool use_v2; + auto& params = SER_PARAMS(SerParams); if (params.fmt == Format::Disk) { // In the disk serialization format, the encoding (v1 or v2) is determined by a flag version // that's part of the serialization itself. ADDRV2_FORMAT in the stream version only determines diff --git a/src/serialize.h b/src/serialize.h index 2f13fba5825..35519056a5d 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -182,9 +182,8 @@ const Out& AsBase(const In& x) static void SerializationOps(Type& obj, Stream& s, Operation ser_action) /** - * Variant of FORMATTER_METHODS that supports a declared parameter type. - * - * If a formatter has a declared parameter type, it must be invoked directly or + * Formatter methods can retrieve parameters attached to a stream using the + * SER_PARAMS(type) macro as long as the stream is created directly or * indirectly with a parameter of that type. This permits making serialization * depend on run-time context in a type-safe way. * @@ -192,7 +191,8 @@ const Out& AsBase(const In& x) * struct BarParameter { bool fancy; ... }; * struct Bar { ... }; * struct FooFormatter { - * FORMATTER_METHODS(Bar, obj, BarParameter, param) { + * FORMATTER_METHODS(Bar, obj) { + * auto& param = SER_PARAMS(BarParameter); * if (param.fancy) { * READWRITE(VARINT(obj.value)); * } else { @@ -214,13 +214,7 @@ const Out& AsBase(const In& x) * Compilation will fail in any context where serialization is invoked but * no parameter of a type convertible to BarParameter is provided. */ -#define FORMATTER_METHODS_PARAMS(cls, obj, paramcls, paramobj) \ - template \ - static void Ser(Stream& s, const cls& obj) { SerializationOps(obj, s, ActionSerialize{}, s.GetParams()); } \ - template \ - static void Unser(Stream& s, cls& obj) { SerializationOps(obj, s, ActionUnserialize{}, s.GetParams()); } \ - template \ - static void SerializationOps(Type& obj, Stream& s, Operation ser_action, const paramcls& paramobj) +#define SER_PARAMS(type) (s.template GetParams()) #define BASE_SERIALIZE_METHODS(cls) \ template \ @@ -247,15 +241,6 @@ const Out& AsBase(const In& x) BASE_SERIALIZE_METHODS(cls) \ FORMATTER_METHODS(cls, obj) -/** - * Variant of SERIALIZE_METHODS that supports a declared parameter type. - * - * See FORMATTER_METHODS_PARAMS for more information on parameters. - */ -#define SERIALIZE_METHODS_PARAMS(cls, obj, paramcls, paramobj) \ - BASE_SERIALIZE_METHODS(cls) \ - FORMATTER_METHODS_PARAMS(cls, obj, paramcls, paramobj) - // Templates for serializing to anything that looks like a stream, // i.e. anything that supports .read(Span) and .write(Span) // @@ -1118,27 +1103,85 @@ size_t GetSerializeSize(const T& t) return (SizeComputer() << t).size(); } -/** Wrapper that overrides the GetParams() function of a stream (and hides GetVersion/GetType). */ -template +//! Check if type contains a stream by seeing if has a GetStream() method. +template +concept ContainsStream = requires(T t) { t.GetStream(); }; + +/** Wrapper that overrides the GetParams() function of a stream. */ +template class ParamsStream { const Params& m_params; - SubStream& m_substream; // private to avoid leaking version/type into serialization code that shouldn't see it + // If ParamsStream constructor is passed an lvalue argument, Substream will + // be a reference type, and m_substream will reference that argument. + // Otherwise m_substream will be a substream instance and move from the + // argument. Letting ParamsStream contain a substream instance instead of + // just a reference is useful to make the ParamsStream object self contained + // and let it do cleanup when destroyed, for example by closing files if + // SubStream is a file stream. + SubStream m_substream; public: - ParamsStream(const Params& params LIFETIMEBOUND, SubStream& substream LIFETIMEBOUND) : m_params{params}, m_substream{substream} {} + ParamsStream(SubStream&& substream, const Params& params LIFETIMEBOUND) : m_params{params}, m_substream{std::forward(substream)} {} + + template + ParamsStream(NestedSubstream&& s, const Params1& params1 LIFETIMEBOUND, const Params2& params2 LIFETIMEBOUND, const NestedParams&... params LIFETIMEBOUND) + : ParamsStream{::ParamsStream{std::forward(s), params2, params...}, params1} {} + template ParamsStream& operator<<(const U& obj) { ::Serialize(*this, obj); return *this; } template ParamsStream& operator>>(U&& obj) { ::Unserialize(*this, obj); return *this; } - void write(Span src) { m_substream.write(src); } - void read(Span dst) { m_substream.read(dst); } - void ignore(size_t num) { m_substream.ignore(num); } - bool eof() const { return m_substream.eof(); } - size_t size() const { return m_substream.size(); } - const Params& GetParams() const { return m_params; } - int GetVersion() = delete; // Deprecated with Params usage - int GetType() = delete; // Deprecated with Params usage + void write(Span src) { GetStream().write(src); } + void read(Span dst) { GetStream().read(dst); } + void ignore(size_t num) { GetStream().ignore(num); } + bool eof() const { return GetStream().eof(); } + size_t size() const { return GetStream().size(); } + + //! Get reference to stream parameters. + template + const auto& GetParams() const + { + if constexpr (std::is_convertible_v) { + return m_params; + } else { + return m_substream.template GetParams

(); + } + } + + //! Get reference to underlying stream. + auto& GetStream() + { + if constexpr (ContainsStream) { + return m_substream.GetStream(); + } else { + return m_substream; + } + } + const auto& GetStream() const + { + if constexpr (ContainsStream) { + return m_substream.GetStream(); + } else { + return m_substream; + } + } }; +/** + * Explicit template deduction guide is required for single-parameter + * constructor so Substream&& is treated as a forwarding reference, and + * SubStream is deduced as reference type for lvalue arguments. + */ +template +ParamsStream(Substream&&, const Params&) -> ParamsStream; + +/** + * Template deduction guide for multiple params arguments that creates a nested + * ParamsStream. + */ +template +ParamsStream(Substream&& s, const Params1& params1, const Params2& params2, const Params&... params) -> + ParamsStream(s), params2, params...}), Params1>; + /** Wrapper that serializes objects with the specified parameters. */ template class ParamsWrapper @@ -1152,13 +1195,13 @@ public: template void Serialize(Stream& s) const { - ParamsStream ss{m_params, s}; + ParamsStream ss{s, m_params}; ::Serialize(ss, m_object); } template void Unserialize(Stream& s) { - ParamsStream ss{m_params, s}; + ParamsStream ss{s, m_params}; ::Unserialize(ss, m_object); } }; @@ -1176,7 +1219,7 @@ public: /** \ * Return a wrapper around t that (de)serializes it with specified parameter params. \ * \ - * See FORMATTER_METHODS_PARAMS for more information on serialization parameters. \ + * See SER_PARAMS for more information on serialization parameters. \ */ \ template \ auto operator()(T&& t) const \ diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index d75eb499b4a..b28e1b41966 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -15,6 +15,18 @@ BOOST_FIXTURE_TEST_SUITE(serialize_tests, BasicTestingSetup) +// For testing move-semantics, declare a version of datastream that can be moved +// but is not copyable. +class UncopyableStream : public DataStream +{ +public: + using DataStream::DataStream; + UncopyableStream(const UncopyableStream&) = delete; + UncopyableStream& operator=(const UncopyableStream&) = delete; + UncopyableStream(UncopyableStream&&) noexcept = default; + UncopyableStream& operator=(UncopyableStream&&) noexcept = default; +}; + class CSerializeMethodsTestSingle { protected: @@ -289,7 +301,7 @@ public: template void Serialize(Stream& s) const { - if (s.GetParams().m_base_format == BaseFormat::RAW) { + if (s.template GetParams().m_base_format == BaseFormat::RAW) { s << m_base_data; } else { s << Span{HexStr(Span{&m_base_data, 1})}; @@ -299,7 +311,7 @@ public: template void Unserialize(Stream& s) { - if (s.GetParams().m_base_format == BaseFormat::RAW) { + if (s.template GetParams().m_base_format == BaseFormat::RAW) { s >> m_base_data; } else { std::string hex{"aa"}; @@ -327,8 +339,9 @@ class Derived : public Base public: std::string m_derived_data; - SERIALIZE_METHODS_PARAMS(Derived, obj, DerivedAndBaseFormat, fmt) + SERIALIZE_METHODS(Derived, obj) { + auto& fmt = SER_PARAMS(DerivedAndBaseFormat); READWRITE(fmt.m_base_format(AsBase(obj))); if (ser_action.ForRead()) { @@ -343,6 +356,76 @@ public: } }; +struct OtherParam { + uint8_t param; + SER_PARAMS_OPFUNC +}; + +//! Checker for value of OtherParam. When being serialized, serializes the +//! param to the stream. When being unserialized, verifies the value in the +//! stream matches the param. +class OtherParamChecker +{ +public: + template + void Serialize(Stream& s) const + { + const uint8_t param = s.template GetParams().param; + s << param; + } + + template + void Unserialize(Stream& s) const + { + const uint8_t param = s.template GetParams().param; + uint8_t value; + s >> value; + BOOST_CHECK_EQUAL(value, param); + } +}; + +//! Test creating a stream with multiple parameters and making sure +//! serialization code requiring different parameters can retrieve them. Also +//! test that earlier parameters take precedence if the same parameter type is +//! specified twice. (Choice of whether earlier or later values take precedence +//! or multiple values of the same type are allowed was arbitrary, and just +//! decided based on what would require smallest amount of ugly C++ template +//! code. Intent of the test is to just ensure there is no unexpected behavior.) +BOOST_AUTO_TEST_CASE(with_params_multi) +{ + const OtherParam other_param_used{.param = 0x10}; + const OtherParam other_param_ignored{.param = 0x11}; + const OtherParam other_param_override{.param = 0x12}; + const OtherParamChecker check; + DataStream stream; + ParamsStream pstream{stream, RAW, other_param_used, other_param_ignored}; + + Base base1{0x20}; + pstream << base1 << check << other_param_override(check); + BOOST_CHECK_EQUAL(stream.str(), "\x20\x10\x12"); + + Base base2; + pstream >> base2 >> check >> other_param_override(check); + BOOST_CHECK_EQUAL(base2.m_base_data, 0x20); +} + +//! Test creating a ParamsStream that moves from a stream argument. +BOOST_AUTO_TEST_CASE(with_params_move) +{ + UncopyableStream stream{MakeByteSpan(std::string_view{"abc"})}; + ParamsStream pstream{std::move(stream), RAW, HEX, RAW}; + BOOST_CHECK_EQUAL(pstream.GetStream().str(), "abc"); + pstream.GetStream().clear(); + + Base base1{0x20}; + pstream << base1; + BOOST_CHECK_EQUAL(pstream.GetStream().str(), "\x20"); + + Base base2; + pstream >> base2; + BOOST_CHECK_EQUAL(base2.m_base_data, 0x20); +} + BOOST_AUTO_TEST_CASE(with_params_base) { Base b{0x0F};