From 4aaee239211a5287fbc361c0eb158b105ae8c8db Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 20 Nov 2023 14:54:37 -0500 Subject: [PATCH 1/3] test: add ipc test to test multiprocess type conversion code Add unit test to test IPC method calls and type conversion between bitcoin c++ types and capnproto messages. Right now there are custom type hooks in bitcoin IPC code, so the test is simple, but in upcoming commits, code will be added to convert bitcoin types to capnproto messages, and the test will be expanded. --- build_msvc/test_bitcoin/test_bitcoin.vcxproj | 2 +- src/Makefile.test.include | 36 +++++++++++++ src/test/.gitignore | 2 + src/test/ipc_test.capnp | 15 ++++++ src/test/ipc_test.cpp | 57 ++++++++++++++++++++ src/test/ipc_test.h | 18 +++++++ src/test/ipc_tests.cpp | 13 +++++ 7 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 src/test/.gitignore create mode 100644 src/test/ipc_test.capnp create mode 100644 src/test/ipc_test.cpp create mode 100644 src/test/ipc_test.h create mode 100644 src/test/ipc_tests.cpp diff --git a/build_msvc/test_bitcoin/test_bitcoin.vcxproj b/build_msvc/test_bitcoin/test_bitcoin.vcxproj index de836bc01d5..b5aa58057fd 100644 --- a/build_msvc/test_bitcoin/test_bitcoin.vcxproj +++ b/build_msvc/test_bitcoin/test_bitcoin.vcxproj @@ -10,7 +10,7 @@ - + diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 416a11b0c01..b8afc47bd4e 100644 --- a/src/Makefile.test.include +++ b/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) diff --git a/src/test/.gitignore b/src/test/.gitignore new file mode 100644 index 00000000000..036df1430c5 --- /dev/null +++ b/src/test/.gitignore @@ -0,0 +1,2 @@ +# capnp generated files +*.capnp.* diff --git a/src/test/ipc_test.capnp b/src/test/ipc_test.capnp new file mode 100644 index 00000000000..f8473a4dec3 --- /dev/null +++ b/src/test/ipc_test.capnp @@ -0,0 +1,15 @@ +# 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"); + +interface FooInterface $Proxy.wrap("FooImplementation") { + add @0 (a :Int32, b :Int32) -> (result :Int32); +} diff --git a/src/test/ipc_test.cpp b/src/test/ipc_test.cpp new file mode 100644 index 00000000000..b84255f68b6 --- /dev/null +++ b/src/test/ipc_test.cpp @@ -0,0 +1,57 @@ +// 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 +#include +#include +#include +#include + +#include +#include +#include +#include + +#include + +//! 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>> foo_promise; + std::function 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(loop, kj::mv(pipe.ends[0])); + auto foo_client = std::make_unique>( + connection_client->m_rpc_system.bootstrap(mp::ServerVatId().vat_id).castAs(), + 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(loop, kj::mv(pipe.ends[1]), [&](mp::Connection& connection) { + auto foo_server = kj::heap>(std::make_shared(), connection); + return capnp::Capability::Client(kj::mv(foo_server)); + }); + connection_server->onDisconnect([&] { connection_server.reset(); }); + loop.loop(); + }); + std::unique_ptr> 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); + + // Test cleanup: disconnect pipe and join thread + disconnect_client(); + thread.join(); +} diff --git a/src/test/ipc_test.h b/src/test/ipc_test.h new file mode 100644 index 00000000000..61c85b5a47a --- /dev/null +++ b/src/test/ipc_test.h @@ -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. + +#ifndef BITCOIN_TEST_IPC_TEST_H +#define BITCOIN_TEST_IPC_TEST_H + +#include + +class FooImplementation +{ +public: + int add(int a, int b) { return a + b; } +}; + +void IpcTest(); + +#endif // BITCOIN_TEST_IPC_TEST_H diff --git a/src/test/ipc_tests.cpp b/src/test/ipc_tests.cpp new file mode 100644 index 00000000000..6e144b0f418 --- /dev/null +++ b/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 +#include + +BOOST_AUTO_TEST_SUITE(ipc_tests) +BOOST_AUTO_TEST_CASE(ipc_tests) +{ + IpcTest(); +} +BOOST_AUTO_TEST_SUITE_END() From 0cc74fce72e0c79849109ee5d7afe707991b3512 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 20 Nov 2023 15:49:55 -0500 Subject: [PATCH 2/3] multiprocess: Add type conversion code for serializable types Allow any C++ object that has Serialize and Unserialize methods and can be serialized to a bitcoin CDataStream to be converted to a capnproto Data field and passed as arguments or return values to capnproto methods using the Data type. Extend IPC unit test to cover this and verify the serialization happens correctly. --- src/Makefile.am | 1 + src/ipc/capnp/common-types.h | 89 ++++++++++++++++++++++++++++++++++++ src/test/ipc_test.capnp | 2 + src/test/ipc_test.cpp | 6 ++- src/test/ipc_test.h | 1 + 5 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 src/ipc/capnp/common-types.h diff --git a/src/Makefile.am b/src/Makefile.am index 99b2184cf25..746c4df677e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1096,6 +1096,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 \ diff --git a/src/ipc/capnp/common-types.h b/src/ipc/capnp/common-types.h new file mode 100644 index 00000000000..d1343c40dd1 --- /dev/null +++ b/src/ipc/capnp/common-types.h @@ -0,0 +1,89 @@ +// 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 +#include + +#include +#include +#include +#include + +namespace ipc { +namespace capnp { +//! Use SFINAE to define Serializeable trait which is true if type T has a +//! Serialize(stream) method, false otherwise. +template +struct Serializable { +private: + template + static std::true_type test(decltype(std::declval().Serialize(std::declval()))*); + template + static std::false_type test(...); + +public: + static constexpr bool value = decltype(test(nullptr))::value; +}; + +//! Use SFINAE to define Unserializeable trait which is true if type T has +//! an Unserialize(stream) method, false otherwise. +template +struct Unserializable { +private: + template + static std::true_type test(decltype(std::declval().Unserialize(std::declval()))*); + template + static std::false_type test(...); + +public: + static constexpr bool value = decltype(test(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 +void CustomBuildField( + TypeList, 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::value && + std::is_same_v>>>* 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 +decltype(auto) +CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, Input&& input, ReadDest&& read_dest, + std::enable_if_t::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); + }); +} +} // namespace mp + +#endif // BITCOIN_IPC_CAPNP_COMMON_TYPES_H diff --git a/src/test/ipc_test.capnp b/src/test/ipc_test.capnp index f8473a4dec3..7b970e2affe 100644 --- a/src/test/ipc_test.capnp +++ b/src/test/ipc_test.capnp @@ -9,7 +9,9 @@ $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); } diff --git a/src/test/ipc_test.cpp b/src/test/ipc_test.cpp index b84255f68b6..f835859705d 100644 --- a/src/test/ipc_test.cpp +++ b/src/test/ipc_test.cpp @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include #include +#include #include #include #include @@ -51,6 +51,10 @@ void IpcTest() // 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); + // Test cleanup: disconnect pipe and join thread disconnect_client(); thread.join(); diff --git a/src/test/ipc_test.h b/src/test/ipc_test.h index 61c85b5a47a..f100ae8c5d2 100644 --- a/src/test/ipc_test.h +++ b/src/test/ipc_test.h @@ -11,6 +11,7 @@ class FooImplementation { public: int add(int a, int b) { return a + b; } + COutPoint passOutPoint(COutPoint o) { return o; } }; void IpcTest(); From 6acec6b9ff02b91de132bb1575d75908a8a2d27b Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 20 Nov 2023 15:49:55 -0500 Subject: [PATCH 3/3] multiprocess: Add type conversion code for UniValue types Extend IPC unit test to cover this and verify the serialization happens correctly. --- src/ipc/capnp/common-types.h | 19 +++++++++++++++++++ src/test/ipc_test.capnp | 1 + src/test/ipc_test.cpp | 6 ++++++ src/test/ipc_test.h | 2 ++ 4 files changed, 28 insertions(+) diff --git a/src/ipc/capnp/common-types.h b/src/ipc/capnp/common-types.h index d1343c40dd1..39e368491b4 100644 --- a/src/ipc/capnp/common-types.h +++ b/src/ipc/capnp/common-types.h @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -84,6 +85,24 @@ CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, value.Unserialize(stream); }); } + +template +void CustomBuildField(TypeList, 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 +decltype(auto) CustomReadField(TypeList, 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 diff --git a/src/test/ipc_test.capnp b/src/test/ipc_test.capnp index 7b970e2affe..55a3dc26839 100644 --- a/src/test/ipc_test.capnp +++ b/src/test/ipc_test.capnp @@ -14,4 +14,5 @@ $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); } diff --git a/src/test/ipc_test.cpp b/src/test/ipc_test.cpp index f835859705d..ce4edaceb08 100644 --- a/src/test/ipc_test.cpp +++ b/src/test/ipc_test.cpp @@ -55,6 +55,12 @@ void IpcTest() 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(); diff --git a/src/test/ipc_test.h b/src/test/ipc_test.h index f100ae8c5d2..bcfcc2125c6 100644 --- a/src/test/ipc_test.h +++ b/src/test/ipc_test.h @@ -6,12 +6,14 @@ #define BITCOIN_TEST_IPC_TEST_H #include +#include 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();