ipc: Serialize null CTransactionRef as empty Data

Reference and rationale:
bitcoin/bitcoin#34020 (comment)

Empty `Data` can be a valid representation for some types, so this behavior
should not be generalized.

In `List(Data)` contexts, Cap'n Proto's current C++ API does not let us
distinguish null vs empty `Data`.

Given those constraints, this commit is narrowly scoped: interpret empty `Data`
as null only for `CTransactionRef`-like pointer paths where empty serialization
is known to be safe. The implementation avoids making generic pointer-type
assumptions and introduces a targeted has-field customization path.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

Author: Sjors

include/mp/proxy-types.h

@@ -23,9 +23,10 @@ class ValueField
     ValueField(Value&& value) : m_value(value) {}
     Value& m_value;
 
+    const Value& get() const { return m_value; }
     Value& get() { return m_value; }
     Value& init() { return m_value; }
-    bool has() { return true; }
+    bool has() const { return true; }
 };
 
 template <typename Accessor, typename Struct>
@@ -166,10 +167,52 @@ struct ReadDestUpdate
     Value& m_value;
 };
 
-template <typename... LocalTypes, typename... Args>
-decltype(auto) ReadField(TypeList<LocalTypes...>, Args&&... args)
+//! Return whether to read a C++ value from a Cap'n Proto field. Returning
+//! false can be useful to interpret certain Cap'n Proto field values as null
+//! C++ values when initializing nullable C++ std::optional / std::unique_ptr /
+//! std::shared_ptr types.
+//!
+//! For example, when reading from a `List(Data)` field into a
+//! `std::vector<std::shared_ptr<const CTransaction>>` value, it's useful to be
+//! able to interpret empty `Data` values as null pointers. This is useful
+//! because the Cap'n Proto C++ API does not currently provide a way to
+//! distinguish between null and empty Data values in a List[*], so we need to
+//! choose some Data value to represent null if we want to allow passing null
+//! pointers. Since no CTransaction is ever serialized as empty Data, it's safe
+//! to use empty Data values to represeent null pointers.
+//!
+//! [*] The Cap'n Proto wire format actually does distinguish between null and
+//! empty Data values inside Lists, and the C++ API does allow distinguishing
+//! between null and empty Data values in other contexts, just not the List
+//! context, so this limitation could be removed in the future.
+//!
+//! Design note: CustomHasField() and CustomHasValue() are inverses of each
+//! other.  CustomHasField() allows leaving Cap'n Proto fields unset when C++
+//! types have certain values, and CustomHasValue() allows leaving C++ values
+//! unset when Cap'n Proto fields have certain values. But internally the
+//! functions get called in different ways. This is because in C++, unlike in
+//! Cap'n Proto not every C++ type is default constructible, and it may be
+//! impossible to leave certain C++ values unset. For example if a C++ method
+//! requires function parameters, there's no way to call the function
+//! constructing values for each of the parameters. Similarly there's no way to
+//! add values to C++ vectors or maps without initializing those values.  This
+//! is not the case in Cap'n Proto where all values are optional and it's
+//! possible to skip initializing parameters and list elements.
+//!
+//! Because of this difference, CustomHasValue() works universally and can be
+//! used to disable BuildField() calls in every contexts, while CustomHasField()
+//! can only be used to disable ReadField() calls in certain contexts like
+//! std::optional and pointer contexts.
+template <typename... LocalTypes, typename Input>
+bool CustomHasField(TypeList<LocalTypes...>, InvokeContext& invoke_context, const Input& input)
+{
+    return input.has();
+}
+
+template <typename... LocalTypes, typename Input, typename... Args>
+decltype(auto) ReadField(TypeList<LocalTypes...>, InvokeContext& invoke_context, Input&& input, Args&&... args)
 {
-    return CustomReadField(TypeList<RemoveCvRef<LocalTypes>...>(), Priority<2>(), std::forward<Args>(args)...);
+    return CustomReadField(TypeList<RemoveCvRef<LocalTypes>...>(), Priority<2>(), invoke_context, std::forward<Input>(input), std::forward<Args>(args)...);
 }
 
 template <typename LocalType, typename Input>
@@ -190,6 +233,13 @@ void ThrowField(TypeList<std::exception>, InvokeContext& invoke_context, Input&&
     throw std::runtime_error(std::string(CharCast(data.begin()), data.size()));
 }
 
+//! Return whether to write a C++ value into a Cap'n Proto field. Returning
+//! false can be useful to map certain C++ values to unset Cap'n Proto fields.
+//!
+//! For example the bitcoin `Coin` class asserts false when a spent coin is
+//! serialized. But some C++ methods return these coins, so there needs to be a
+//! way to represent them in Cap'n Proto and a null Data field is a convenient
+//! representation.
 template <typename... Values>
 bool CustomHasValue(InvokeContext& invoke_context, const Values&... value)
 {
@@ -372,7 +422,7 @@ struct ClientException
         void handleField(InvokeContext& invoke_context, Results& results, ParamList)
         {
             StructField<Accessor, Results> input(results);
-            if (input.has()) {
+            if (CustomHasField(TypeList<Exception>(), invoke_context, input)) {
                 ThrowField(TypeList<Exception>(), invoke_context, input);
             }
         }

include/mp/type-function.h

@@ -48,13 +48,13 @@ struct ProxyCallFn
 };
 
 template <typename FnR, typename... FnParams, typename Input, typename ReadDest>
-decltype(auto) CustomReadField(TypeList<std::function<FnR(FnParams...)>>,
+decltype(auto) CustomReadField(TypeList<std::function<FnR(FnParams...)>> types,
     Priority<1>,
     InvokeContext& invoke_context,
     Input&& input,
     ReadDest&& read_dest)
 {
-    if (input.has()) {
+    if (CustomHasField(types, invoke_context, input)) {
         using Interface = typename Decay<decltype(input.get())>::Calls;
         auto client = std::make_shared<ProxyClient<Interface>>(
             input.get(), &invoke_context.connection, /* destroy_connection= */ false);

include/mp/type-interface.h

@@ -85,7 +85,7 @@ decltype(auto) CustomReadField(TypeList<std::unique_ptr<LocalType>>,
     typename Decay<decltype(input.get())>::Calls* enable = nullptr)
 {
     using Interface = typename Decay<decltype(input.get())>::Calls;
-    if (input.has()) {
+    if (CustomHasField(TypeList<LocalType>(), invoke_context, input)) {
         return read_dest.construct(
                                    CustomMakeProxyClient<Interface, LocalType>(invoke_context, std::move(input.get())));
     }
@@ -101,7 +101,7 @@ decltype(auto) CustomReadField(TypeList<std::shared_ptr<LocalType>>,
     typename Decay<decltype(input.get())>::Calls* enable = nullptr)
 {
     using Interface = typename Decay<decltype(input.get())>::Calls;
-    if (input.has()) {
+    if (CustomHasField(TypeList<LocalType>(), invoke_context, input)) {
         return read_dest.construct(
             CustomMakeProxyClient<Interface, LocalType>(invoke_context, std::move(input.get())));
     }

include/mp/type-message.h

@@ -23,13 +23,13 @@ void CustomBuildField(TypeList<LocalType>, Priority<2>, InvokeContext& invoke_co
 //! overloads. Defining a CustomReadMessage overload is simpler than defining a
 //! CustomReadField overload because it only requires defining a normal
 //! function, not a template function, but less flexible.
-template <typename LocalType, typename Reader, typename ReadDest>
-decltype(auto) CustomReadField(TypeList<LocalType>, Priority<2>, InvokeContext& invoke_context, Reader&& reader,
+template <typename LocalType, typename Input, typename ReadDest>
+decltype(auto) CustomReadField(TypeList<LocalType>, Priority<2>, InvokeContext& invoke_context, Input&& input,
                                ReadDest&& read_dest,
-                               decltype(CustomReadMessage(invoke_context, reader.get(),
+                               decltype(CustomReadMessage(invoke_context, input.get(),
                                                           std::declval<LocalType&>()))* enable = nullptr)
 {
-    return read_dest.update([&](auto& value) { if (reader.has()) CustomReadMessage(invoke_context, reader.get(), value); });
+    return read_dest.update([&](auto& value) { if (CustomHasField(TypeList<LocalType>(), invoke_context, input)) CustomReadMessage(invoke_context, input.get(), value); });
 }
 
 //! Helper for CustomPassField below. Call Accessor::init method if it has one,

include/mp/type-optional.h

@@ -30,7 +30,7 @@ decltype(auto) CustomReadField(TypeList<std::optional<LocalType>>,
     ReadDest&& read_dest)
 {
     return read_dest.update([&](auto& value) {
-        if (!input.has()) {
+        if (!CustomHasField(TypeList<LocalType>(), invoke_context, input)) {
             value.reset();
         } else if (value) {
             ReadField(TypeList<LocalType>(), invoke_context, input, ReadDestUpdate(*value));

include/mp/type-pointer.h

@@ -50,7 +50,7 @@ decltype(auto) CustomReadField(TypeList<std::shared_ptr<LocalType>>,
     ReadDest&& read_dest)
 {
     return read_dest.update([&](auto& value) {
-        if (!input.has()) {
+        if (!CustomHasField(TypeList<LocalType>(), invoke_context, input)) {
             value.reset();
         } else if (value) {
             ReadField(TypeList<LocalType>(), invoke_context, input, ReadDestUpdate(*value));
@@ -72,7 +72,7 @@ decltype(auto) CustomReadField(TypeList<std::shared_ptr<const LocalType>>,
     ReadDest&& read_dest)
 {
     return read_dest.update([&](auto& value) {
-        if (!input.has()) {
+        if (!CustomHasField(TypeList<LocalType>(), invoke_context, input)) {
             value.reset();
             return;
         }