Merge bitcoin/bitcoin#28921: multiprocess: Add basic type conversion hooks

6acec6b9ff multiprocess: Add type conversion code for UniValue types (Ryan Ofsky)
0cc74fce72 multiprocess: Add type conversion code for serializable types (Ryan Ofsky)
4aaee23921 test: add ipc test to test multiprocess type conversion code (Ryan Ofsky)

Pull request description:

  Add type conversion hooks to allow `UniValue` objects, and objects that have `CDataStream` `Serialize` and `Unserialize` methods to be used as arguments and return values in Cap'nProto interface methods. Also add unit test to verify the hooks are working and data can be round-tripped correctly.

  The non-test code in this PR was previously part of #10102 and has been split off for easier review, but the test code is new.

  ---

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).

ACKs for top commit:
  achow101:
    ACK 6acec6b9ff
  dergoegge:
    reACK 6acec6b9ff

Tree-SHA512: 5d2cbc5215d488b876d34420adf91205dabf09b736183dcc85aa86255e3804c2bac5bab6792dacd585ef99a1d92cf29c8afb3eb65e4d953abc7ffe41994340c6

build_msvc/test_bitcoin/test_bitcoin.vcxproj

@@ -10,7 +10,7 @@
   </PropertyGroup>
   <ItemGroup>
     <ClCompile Include="..\..\src\test\*_properties.cpp" />
-    <ClCompile Include="..\..\src\test\*_tests.cpp" />
+    <ClCompile Include="..\..\src\test\*_tests.cpp" Exclude="..\..\src\test\ipc_tests.cpp" />
     <ClCompile Include="..\..\src\test\gen\*_gen.cpp" />
     <ClCompile Include="..\..\src\test\main.cpp" />
     <ClCompile Include="..\..\src\test\util\*.cpp" />

src/Makefile.am

@@ -1090,6 +1090,7 @@ ipc/capnp/libbitcoin_ipc_a-protocol.$(OBJEXT): $(libbitcoin_ipc_mpgen_input:=.h)
 if BUILD_MULTIPROCESS
 LIBBITCOIN_IPC=libbitcoin_ipc.a
 libbitcoin_ipc_a_SOURCES = \
+  ipc/capnp/common-types.h \
   ipc/capnp/context.h \
   ipc/capnp/init-types.h \
   ipc/capnp/protocol.cpp \

src/Makefile.test.include

@@ -216,6 +216,39 @@ BITCOIN_TEST_SUITE += \
   wallet/test/init_test_fixture.h
 endif # ENABLE_WALLET
 
+if BUILD_MULTIPROCESS
+# Add boost ipc_tests definition to BITCOIN_TESTS
+BITCOIN_TESTS += test/ipc_tests.cpp
+
+# Build ipc_test code in a separate library so it can be compiled with custom
+# LIBMULTIPROCESS_CFLAGS without those flags affecting other tests
+LIBBITCOIN_IPC_TEST=libbitcoin_ipc_test.a
+EXTRA_LIBRARIES += $(LIBBITCOIN_IPC_TEST)
+libbitcoin_ipc_test_a_SOURCES = \
+  test/ipc_test.cpp \
+  test/ipc_test.h
+libbitcoin_ipc_test_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
+libbitcoin_ipc_test_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) $(LIBMULTIPROCESS_CFLAGS)
+
+# Generate various .c++/.h files from the ipc_test.capnp file
+include $(MPGEN_PREFIX)/include/mpgen.mk
+EXTRA_DIST += test/ipc_test.capnp
+libbitcoin_ipc_test_mpgen_output = \
+  test/ipc_test.capnp.c++ \
+  test/ipc_test.capnp.h \
+  test/ipc_test.capnp.proxy-client.c++ \
+  test/ipc_test.capnp.proxy-server.c++ \
+  test/ipc_test.capnp.proxy-types.c++ \
+  test/ipc_test.capnp.proxy-types.h \
+  test/ipc_test.capnp.proxy.h
+nodist_libbitcoin_ipc_test_a_SOURCES = $(libbitcoin_ipc_test_mpgen_output)
+CLEANFILES += $(libbitcoin_ipc_test_mpgen_output)
+endif
+
+# Explicitly list dependencies on generated headers as described in
+# https://www.gnu.org/software/automake/manual/html_node/Built-Sources-Example.html#Recording-Dependencies-manually
+test/libbitcoin_ipc_test_a-ipc_test.$(OBJEXT): test/ipc_test.capnp.h
+
 test_test_bitcoin_SOURCES = $(BITCOIN_TEST_SUITE) $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES)
 test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(TESTDEFS) $(BOOST_CPPFLAGS) $(EVENT_CFLAGS)
 test_test_bitcoin_LDADD = $(LIBTEST_UTIL)
@@ -223,6 +256,9 @@ if ENABLE_WALLET
 test_test_bitcoin_LDADD += $(LIBBITCOIN_WALLET)
 test_test_bitcoin_CPPFLAGS += $(BDB_CPPFLAGS)
 endif
+if BUILD_MULTIPROCESS
+test_test_bitcoin_LDADD += $(LIBBITCOIN_IPC_TEST) $(LIBMULTIPROCESS_LIBS)
+endif
 
 test_test_bitcoin_LDADD += $(LIBBITCOIN_NODE) $(LIBBITCOIN_CLI) $(LIBBITCOIN_COMMON) $(LIBBITCOIN_UTIL) $(LIBBITCOIN_CONSENSUS) $(LIBBITCOIN_CRYPTO) $(LIBUNIVALUE) \
   $(LIBLEVELDB) $(LIBMEMENV) $(LIBSECP256K1) $(EVENT_LIBS) $(EVENT_PTHREADS_LIBS) $(MINISKETCH_LIBS)

src/ipc/capnp/common-types.h

@@ -0,0 +1,108 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_IPC_CAPNP_COMMON_TYPES_H
+#define BITCOIN_IPC_CAPNP_COMMON_TYPES_H
+
+#include <clientversion.h>
+#include <streams.h>
+#include <univalue.h>
+
+#include <cstddef>
+#include <mp/proxy-types.h>
+#include <type_traits>
+#include <utility>
+
+namespace ipc {
+namespace capnp {
+//! Use SFINAE to define Serializeable<T> trait which is true if type T has a
+//! Serialize(stream) method, false otherwise.
+template <typename T>
+struct Serializable {
+private:
+    template <typename C>
+    static std::true_type test(decltype(std::declval<C>().Serialize(std::declval<std::nullptr_t&>()))*);
+    template <typename>
+    static std::false_type test(...);
+
+public:
+    static constexpr bool value = decltype(test<T>(nullptr))::value;
+};
+
+//! Use SFINAE to define Unserializeable<T> trait which is true if type T has
+//! an Unserialize(stream) method, false otherwise.
+template <typename T>
+struct Unserializable {
+private:
+    template <typename C>
+    static std::true_type test(decltype(std::declval<C>().Unserialize(std::declval<std::nullptr_t&>()))*);
+    template <typename>
+    static std::false_type test(...);
+
+public:
+    static constexpr bool value = decltype(test<T>(nullptr))::value;
+};
+} // namespace capnp
+} // namespace ipc
+
+//! Functions to serialize / deserialize common bitcoin types.
+namespace mp {
+//! Overload multiprocess library's CustomBuildField hook to allow any
+//! serializable object to be stored in a capnproto Data field or passed to a
+//! canproto interface. Use Priority<1> so this hook has medium priority, and
+//! higher priority hooks could take precedence over this one.
+template <typename LocalType, typename Value, typename Output>
+void CustomBuildField(
+    TypeList<LocalType>, Priority<1>, InvokeContext& invoke_context, Value&& value, Output&& output,
+    // Enable if serializeable and if LocalType is not cv or reference
+    // qualified. If LocalType is cv or reference qualified, it is important to
+    // fall back to lower-priority Priority<0> implementation of this function
+    // that strips cv references, to prevent this CustomBuildField overload from
+    // taking precedence over more narrow overloads for specific LocalTypes.
+    std::enable_if_t<ipc::capnp::Serializable<LocalType>::value &&
+                     std::is_same_v<LocalType, std::remove_cv_t<std::remove_reference_t<LocalType>>>>* enable = nullptr)
+{
+    DataStream stream;
+    value.Serialize(stream);
+    auto result = output.init(stream.size());
+    memcpy(result.begin(), stream.data(), stream.size());
+}
+
+//! Overload multiprocess library's CustomReadField hook to allow any object
+//! with an Unserialize method to be read from a capnproto Data field or
+//! returned from canproto interface. Use Priority<1> so this hook has medium
+//! priority, and higher priority hooks could take precedence over this one.
+template <typename LocalType, typename Input, typename ReadDest>
+decltype(auto)
+CustomReadField(TypeList<LocalType>, Priority<1>, InvokeContext& invoke_context, Input&& input, ReadDest&& read_dest,
+                std::enable_if_t<ipc::capnp::Unserializable<LocalType>::value>* enable = nullptr)
+{
+    return read_dest.update([&](auto& value) {
+        if (!input.has()) return;
+        auto data = input.get();
+        SpanReader stream({data.begin(), data.end()});
+        value.Unserialize(stream);
+    });
+}
+
+template <typename Value, typename Output>
+void CustomBuildField(TypeList<UniValue>, Priority<1>, InvokeContext& invoke_context, Value&& value, Output&& output)
+{
+    std::string str = value.write();
+    auto result = output.init(str.size());
+    memcpy(result.begin(), str.data(), str.size());
+}
+
+template <typename Input, typename ReadDest>
+decltype(auto) CustomReadField(TypeList<UniValue>, Priority<1>, InvokeContext& invoke_context, Input&& input,
+                               ReadDest&& read_dest)
+{
+    return read_dest.update([&](auto& value) {
+        auto data = input.get();
+        value.read(std::string_view{data.begin(), data.size()});
+    });
+}
+} // namespace mp
+
+#endif // BITCOIN_IPC_CAPNP_COMMON_TYPES_H

src/test/.gitignore

@@ -0,0 +1,2 @@
+# capnp generated files
+*.capnp.*

src/test/ipc_test.capnp

@@ -0,0 +1,18 @@
+# Copyright (c) 2023 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+@0xd71b0fc8727fdf83;
+
+using Cxx = import "/capnp/c++.capnp";
+$Cxx.namespace("gen");
+
+using Proxy = import "/mp/proxy.capnp";
+$Proxy.include("test/ipc_test.h");
+$Proxy.includeTypes("ipc/capnp/common-types.h");
+
+interface FooInterface $Proxy.wrap("FooImplementation") {
+    add @0 (a :Int32, b :Int32) -> (result :Int32);
+    passOutPoint @1 (arg :Data) -> (result :Data);
+    passUniValue @2 (arg :Text) -> (result :Text);
+}

src/test/ipc_test.cpp

@@ -0,0 +1,67 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <logging.h>
+#include <mp/proxy-types.h>
+#include <test/ipc_test.capnp.h>
+#include <test/ipc_test.capnp.proxy.h>
+#include <test/ipc_test.h>
+
+#include <future>
+#include <kj/common.h>
+#include <kj/memory.h>
+#include <kj/test.h>
+
+#include <boost/test/unit_test.hpp>
+
+//! Unit test that tests execution of IPC calls without actually creating a
+//! separate process. This test is primarily intended to verify behavior of type
+//! conversion code that converts C++ objects to Cap'n Proto messages and vice
+//! versa.
+//!
+//! The test creates a thread which creates a FooImplementation object (defined
+//! in ipc_test.h) and a two-way pipe accepting IPC requests which call methods
+//! on the object through FooInterface (defined in ipc_test.capnp).
+void IpcTest()
+{
+    // Setup: create FooImplemention object and listen for FooInterface requests
+    std::promise<std::unique_ptr<mp::ProxyClient<gen::FooInterface>>> foo_promise;
+    std::function<void()> disconnect_client;
+    std::thread thread([&]() {
+        mp::EventLoop loop("IpcTest", [](bool raise, const std::string& log) { LogPrintf("LOG%i: %s\n", raise, log); });
+        auto pipe = loop.m_io_context.provider->newTwoWayPipe();
+
+        auto connection_client = std::make_unique<mp::Connection>(loop, kj::mv(pipe.ends[0]));
+        auto foo_client = std::make_unique<mp::ProxyClient<gen::FooInterface>>(
+            connection_client->m_rpc_system.bootstrap(mp::ServerVatId().vat_id).castAs<gen::FooInterface>(),
+            connection_client.get(), /* destroy_connection= */ false);
+        foo_promise.set_value(std::move(foo_client));
+        disconnect_client = [&] { loop.sync([&] { connection_client.reset(); }); };
+
+        auto connection_server = std::make_unique<mp::Connection>(loop, kj::mv(pipe.ends[1]), [&](mp::Connection& connection) {
+            auto foo_server = kj::heap<mp::ProxyServer<gen::FooInterface>>(std::make_shared<FooImplementation>(), connection);
+            return capnp::Capability::Client(kj::mv(foo_server));
+        });
+        connection_server->onDisconnect([&] { connection_server.reset(); });
+        loop.loop();
+    });
+    std::unique_ptr<mp::ProxyClient<gen::FooInterface>> foo{foo_promise.get_future().get()};
+
+    // Test: make sure arguments were sent and return value is received
+    BOOST_CHECK_EQUAL(foo->add(1, 2), 3);
+
+    COutPoint txout1{Txid::FromUint256(uint256{100}), 200};
+    COutPoint txout2{foo->passOutPoint(txout1)};
+    BOOST_CHECK(txout1 == txout2);
+
+    UniValue uni1{UniValue::VOBJ};
+    uni1.pushKV("i", 1);
+    uni1.pushKV("s", "two");
+    UniValue uni2{foo->passUniValue(uni1)};
+    BOOST_CHECK_EQUAL(uni1.write(), uni2.write());
+
+    // Test cleanup: disconnect pipe and join thread
+    disconnect_client();
+    thread.join();
+}

src/test/ipc_test.h

@@ -0,0 +1,21 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_TEST_IPC_TEST_H
+#define BITCOIN_TEST_IPC_TEST_H
+
+#include <primitives/transaction.h>
+#include <univalue.h>
+
+class FooImplementation
+{
+public:
+    int add(int a, int b) { return a + b; }
+    COutPoint passOutPoint(COutPoint o) { return o; }
+    UniValue passUniValue(UniValue v) { return v; }
+};
+
+void IpcTest();
+
+#endif // BITCOIN_TEST_IPC_TEST_H

src/test/ipc_tests.cpp

@@ -0,0 +1,13 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <test/ipc_test.h>
+#include <boost/test/unit_test.hpp>
+
+BOOST_AUTO_TEST_SUITE(ipc_tests)
+BOOST_AUTO_TEST_CASE(ipc_tests)
+{
+    IpcTest();
+}
+BOOST_AUTO_TEST_SUITE_END()