Merge bitcoin/bitcoin#28929: serialization: Support for multiple parameters

8d491ae serialization: Add ParamsStream GetStream() method (Ryan Ofsky)
951203b net: Simplify ParamsStream usage (Ryan Ofsky)
e6794e4 serialization: Accept multiple parameters in ParamsStream constructor (Ryan Ofsky)
cb28849 serialization: Reverse ParamsStream constructor order (Ryan Ofsky)
83436d1 serialization: Drop unnecessary ParamsStream references (Ryan Ofsky)
84502b7 serialization: Drop references to GetVersion/GetType (Ryan Ofsky)
f3a2b52 serialization: Support for multiple parameters (Ryan Ofsky)

Pull request description:

  Currently it is only possible to attach one serialization parameter to a stream at a time. For example, it is not possible to set a parameter controlling the transaction format and a parameter controlling the address format at the same time because one parameter will override the other.

  This limitation is inconvenient for multiprocess code since it is not possible to create just one type of stream and serialize any object to it. Instead it is necessary to create different streams for different object types, which requires extra boilerplate and makes using the new parameter fields a lot more awkward than the older version and type fields.

  Fix this problem by allowing an unlimited number of serialization stream parameters to be set, and allowing them to be requested by type. Later parameters will still override earlier parameters, but only if they have the same type.

  For an example of different ways multiple parameters can be set, see the new [`with_params_multi`](https://github.com/bitcoin/bitcoin/blob/40f505583f4edeb2859aeb70da20c6374d331a4f/src/test/serialize_tests.cpp#L394-L410) unit test.

  This change requires replacing the `stream.GetParams()` method with a `stream.GetParams<T>()` method in order for serialization code to retrieve the desired parameters. The change is more verbose, but probably a good thing for readability because previously it could be difficult to know what type the `GetParams()` method would return, and now it is more obvious.

  ---

  This PR is part of the [process separation project](bitcoin/bitcoin#28722).

ACKs for top commit:
  maflcko:
    ACK 8d491ae 🔵
  sipa:
    utACK 8d491ae
  TheCharlatan:
    ACK 8d491ae

Tree-SHA512: 40b7041ee01c0372b1f86f7fd6f3b6af56ef24a6383f91ffcedd04d388e63407006457bf7ed056b0e37b4dec9ffd5ca006cb8192e488ea2c64678567e38d4647

Author: achow101

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<uint8_t>(FILE_FORMAT);
 
@@ -238,7 +238,7 @@ void AddrManImpl::Unserialize(Stream& s_)
     s_ >> Using<CustomUintFormatter<1>>(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;

src/net.cpp

@@ -196,8 +196,7 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn)
     const auto one_week{7 * 24h};
     std::vector<CAddress> 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;

src/netaddress.h

@@ -237,7 +237,7 @@ class CNetAddr
     template <typename Stream>
     void Serialize(Stream& s) const
     {
-        if (s.GetParams().enc == Encoding::V2) {
+        if (s.template GetParams<SerParams>().enc == Encoding::V2) {
             SerializeV2Stream(s);
         } else {
             SerializeV1Stream(s);
@@ -250,7 +250,7 @@ class CNetAddr
     template <typename Stream>
     void Unserialize(Stream& s)
     {
-        if (s.GetParams().enc == Encoding::V2) {
+        if (s.template GetParams<SerParams>().enc == Encoding::V2) {
             UnserializeV2Stream(s);
         } else {
             UnserializeV1Stream(s);

src/primitives/transaction.h

@@ -326,15 +326,15 @@ class CTransaction
 
     template <typename Stream>
     inline void Serialize(Stream& s) const {
-        SerializeTransaction(*this, s, s.GetParams());
+        SerializeTransaction(*this, s, s.template GetParams<TransactionSerParams>());
     }
 
     /** This deserializing constructor is provided instead of an Unserialize method.
      *  Unserialize is not possible, since it would require overwriting const fields. */
     template <typename Stream>
     CTransaction(deserialize_type, const TransactionSerParams& params, Stream& s) : CTransaction(CMutableTransaction(deserialize, params, s)) {}
     template <typename Stream>
-    CTransaction(deserialize_type, ParamsStream<TransactionSerParams,Stream>& 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 <typename Stream>
     inline void Serialize(Stream& s) const {
-        SerializeTransaction(*this, s, s.GetParams());
+        SerializeTransaction(*this, s, s.template GetParams<TransactionSerParams>());
     }
 
     template <typename Stream>
     inline void Unserialize(Stream& s) {
-        UnserializeTransaction(*this, s, s.GetParams());
+        UnserializeTransaction(*this, s, s.template GetParams<TransactionSerParams>());
     }
 
     template <typename Stream>
@@ -400,7 +400,7 @@ struct CMutableTransaction
     }
 
     template <typename Stream>
-    CMutableTransaction(deserialize_type, ParamsStream<TransactionSerParams,Stream>& s) {
+    CMutableTransaction(deserialize_type, Stream& s) {
         Unserialize(s);
     }
 

src/protocol.h

@@ -375,9 +375,10 @@ class CAddress : public CService
     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

src/serialize.h

@@ -182,17 +182,17 @@ 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.
  *
  * Example use:
  *   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 <typename Stream>                                                                                 \
-    static void Ser(Stream& s, const cls& obj) { SerializationOps(obj, s, ActionSerialize{}, s.GetParams()); } \
-    template <typename Stream>                                                                                 \
-    static void Unser(Stream& s, cls& obj) { SerializationOps(obj, s, ActionUnserialize{}, s.GetParams()); }   \
-    template <typename Stream, typename Type, typename Operation>                                              \
-    static void SerializationOps(Type& obj, Stream& s, Operation ser_action, const paramcls& paramobj)
+#define SER_PARAMS(type) (s.template GetParams<type>())
 
 #define BASE_SERIALIZE_METHODS(cls)                                                                 \
     template <typename Stream>                                                                      \
@@ -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<std::byte>) and .write(Span<const std::byte>)
 //
@@ -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 <typename Params, typename SubStream>
+//! Check if type contains a stream by seeing if has a GetStream() method.
+template<typename T>
+concept ContainsStream = requires(T t) { t.GetStream(); };
+
+/** Wrapper that overrides the GetParams() function of a stream. */
+template <typename SubStream, typename Params>
 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>(substream)} {}
+
+    template <typename NestedSubstream, typename Params1, typename Params2, typename... NestedParams>
+    ParamsStream(NestedSubstream&& s, const Params1& params1 LIFETIMEBOUND, const Params2& params2 LIFETIMEBOUND, const NestedParams&... params LIFETIMEBOUND)
+        : ParamsStream{::ParamsStream{std::forward<NestedSubstream>(s), params2, params...}, params1} {}
+
     template <typename U> ParamsStream& operator<<(const U& obj) { ::Serialize(*this, obj); return *this; }
     template <typename U> ParamsStream& operator>>(U&& obj) { ::Unserialize(*this, obj); return *this; }
-    void write(Span<const std::byte> src) { m_substream.write(src); }
-    void read(Span<std::byte> 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<const std::byte> src) { GetStream().write(src); }
+    void read(Span<std::byte> 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 <typename P>
+    const auto& GetParams() const
+    {
+        if constexpr (std::is_convertible_v<Params, P>) {
+            return m_params;
+        } else {
+            return m_substream.template GetParams<P>();
+        }
+    }
+
+    //! Get reference to underlying stream.
+    auto& GetStream()
+    {
+        if constexpr (ContainsStream<SubStream>) {
+            return m_substream.GetStream();
+        } else {
+            return m_substream;
+        }
+    }
+    const auto& GetStream() const
+    {
+        if constexpr (ContainsStream<SubStream>) {
+            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 <typename Substream, typename Params>
+ParamsStream(Substream&&, const Params&) -> ParamsStream<Substream, Params>;
+
+/**
+ * Template deduction guide for multiple params arguments that creates a nested
+ * ParamsStream.
+ */
+template <typename Substream, typename Params1, typename Params2, typename... Params>
+ParamsStream(Substream&& s, const Params1& params1, const Params2& params2, const Params&... params) ->
+    ParamsStream<decltype(ParamsStream{std::forward<Substream>(s), params2, params...}), Params1>;
+
 /** Wrapper that serializes objects with the specified parameters. */
 template <typename Params, typename T>
 class ParamsWrapper
@@ -1152,13 +1195,13 @@ class ParamsWrapper
     template <typename Stream>
     void Serialize(Stream& s) const
     {
-        ParamsStream ss{m_params, s};
+        ParamsStream ss{s, m_params};
         ::Serialize(ss, m_object);
     }
     template <typename Stream>
     void Unserialize(Stream& s)
     {
-        ParamsStream ss{m_params, s};
+        ParamsStream ss{s, m_params};
         ::Unserialize(ss, m_object);
     }
 };
@@ -1176,7 +1219,7 @@ class ParamsWrapper
     /**                                                                                  \
      * 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 <typename T>                                                                \
     auto operator()(T&& t) const                                                         \