Merge bitcoin/bitcoin#33774: cmake: Move IPC tests to ipc/test

866bbb98fd cmake, test: Improve locality of `bitcoin_ipc_test` library description (Hennadii Stepanov)
ae2e438b25 cmake: Move IPC tests to `ipc/test` (Hennadii Stepanov)

Pull request description:

  This PR follows up on https://github.com/bitcoin/bitcoin/pull/33445 and:
  1. Organizes the IPC tests in the same way as the wallet tests.
  2. Removes no longer needed `src/test/.clang-tidy.in`.

  See the previous discussion:
  - https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2379651340
  - https://github.com/bitcoin/bitcoin/pull/33445#pullrequestreview-3411868329

  Additionally, the locality of the `bitcoin_ipc_test` build target description has been improved.

ACKs for top commit:
  Sjors:
    ACK 866bbb98fd
  janb84:
    ACK 866bbb98fd
  ryanofsky:
    Code review ACK 866bbb98fd, just adding back the suggested comment, and also fixing bad include arguments passed to target_capnp_sources. It would probably be a little better if the include fix was done in an earlier commit, since it's not really related to the other changes in the last commit, but would also be ok to make both changes at the same time.

Tree-SHA512: ed7cc817ccb88595d8516978bff0ea2560048d35b3f548e7913aec7d58b8d6ac550e230e992c527fb747bef175580be92dc4df6342e4485f3a9870dba0a25cba

src/ipc/CMakeLists.txt

@@ -9,7 +9,7 @@ add_library(bitcoin_ipc STATIC EXCLUDE_FROM_ALL
   process.cpp
 )
 
-target_capnp_sources(bitcoin_ipc ${PROJECT_SOURCE_DIR}
+target_capnp_sources(bitcoin_ipc ${CMAKE_CURRENT_SOURCE_DIR}
   capnp/common.capnp
   capnp/echo.capnp
   capnp/init.capnp
@@ -22,4 +22,27 @@ target_link_libraries(bitcoin_ipc
     univalue
 )
 
+if(BUILD_TESTS)
+  # bitcoin_ipc_test library target is defined here in src/ipc/CMakeLists.txt
+  # instead of src/ipc/test/CMakeLists.txt so capnp files in src/ipc/test/ are able to
+  # reference capnp files in src/ipc/capnp/ by relative path. The Cap'n Proto
+  # compiler only allows importing by relative path when the importing and
+  # imported files are underneath the same compilation source prefix, so the
+  # source prefix must be src/ipc, not src/ipc/test/
+  add_library(bitcoin_ipc_test STATIC EXCLUDE_FROM_ALL
+    test/ipc_test.cpp
+  )
+  target_capnp_sources(bitcoin_ipc_test ${CMAKE_CURRENT_SOURCE_DIR}
+    test/ipc_test.capnp
+  )
+  add_dependencies(bitcoin_ipc_test bitcoin_ipc_headers)
+
+  target_link_libraries(bitcoin_ipc_test
+    PRIVATE
+      core_interface
+      univalue
+      Boost::headers
+  )
+endif()
+
 configure_file(.clang-tidy.in .clang-tidy USE_SOURCE_PERMISSIONS COPYONLY)

src/ipc/test/CMakeLists.txt

@@ -0,0 +1,12 @@
+# Copyright (c) 2023-present The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or https://opensource.org/license/mit/.
+
+# Do not use generator expressions in test sources because the
+# SOURCES property is processed to gather test suite macros.
+target_sources(test_bitcoin
+  PRIVATE
+    ipc_tests.cpp
+)
+
+target_link_libraries(test_bitcoin bitcoin_ipc_test bitcoin_ipc)

src/ipc/test/ipc_test.capnp

@@ -0,0 +1,23 @@
+# 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("ipc/test/ipc_test.h");
+$Proxy.includeTypes("ipc/test/ipc_test_types.h");
+
+using Mining = import "../capnp/mining.capnp";
+
+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);
+    passTransaction @3 (arg :Data) -> (result :Data);
+    passVectorChar @4 (arg :Data) -> (result :Data);
+    passScript @5 (arg :Data) -> (result :Data);
+}

src/ipc/test/ipc_test.cpp

@@ -0,0 +1,179 @@
+// 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 <interfaces/init.h>
+#include <ipc/capnp/protocol.h>
+#include <ipc/process.h>
+#include <ipc/protocol.h>
+#include <logging.h>
+#include <mp/proxy-types.h>
+#include <ipc/test/ipc_test.capnp.h>
+#include <ipc/test/ipc_test.capnp.proxy.h>
+#include <ipc/test/ipc_test.h>
+#include <tinyformat.h>
+#include <validation.h>
+
+#include <future>
+#include <thread>
+#include <kj/common.h>
+#include <kj/memory.h>
+#include <kj/test.h>
+#include <stdexcept>
+
+#include <boost/test/unit_test.hpp>
+
+//! Remote init class.
+class TestInit : public interfaces::Init
+{
+public:
+    std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
+};
+
+//! Generate a temporary path with temp_directory_path and mkstemp
+static std::string TempPath(std::string_view pattern)
+{
+    std::string temp{fs::PathToString(fs::path{fs::temp_directory_path()} / fs::PathFromString(std::string{pattern}))};
+    temp.push_back('\0');
+    int fd{mkstemp(temp.data())};
+    BOOST_CHECK_GE(fd, 0);
+    BOOST_CHECK_EQUAL(close(fd), 0);
+    temp.resize(temp.size() - 1);
+    fs::remove(fs::PathFromString(temp));
+    return temp;
+}
+
+//! 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 IpcPipeTest()
+{
+    // Setup: create FooImplementation object and listen for FooInterface requests
+    std::promise<std::unique_ptr<mp::ProxyClient<gen::FooInterface>>> foo_promise;
+    std::thread thread([&]() {
+        mp::EventLoop loop("IpcPipeTest", [](bool raise, const std::string& log) { LogInfo("LOG%i: %s", 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= */ true);
+        connection_client.release();
+        foo_promise.set_value(std::move(foo_client));
+
+        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());
+
+    CMutableTransaction mtx;
+    mtx.version = 2;
+    mtx.nLockTime = 3;
+    mtx.vin.emplace_back(txout1);
+    mtx.vout.emplace_back(COIN, CScript());
+    CTransactionRef tx1{MakeTransactionRef(mtx)};
+    CTransactionRef tx2{foo->passTransaction(tx1)};
+    BOOST_CHECK(*Assert(tx1) == *Assert(tx2));
+
+    std::vector<char> vec1{'H', 'e', 'l', 'l', 'o'};
+    std::vector<char> vec2{foo->passVectorChar(vec1)};
+    BOOST_CHECK_EQUAL(std::string_view(vec1.begin(), vec1.end()), std::string_view(vec2.begin(), vec2.end()));
+
+    auto script1{CScript() << OP_11};
+    auto script2{foo->passScript(script1)};
+    BOOST_CHECK_EQUAL(HexStr(script1), HexStr(script2));
+
+    // Test cleanup: disconnect and join thread
+    foo.reset();
+    thread.join();
+}
+
+//! Test ipc::Protocol connect() and serve() methods connecting over a socketpair.
+void IpcSocketPairTest()
+{
+    int fds[2];
+    BOOST_CHECK_EQUAL(socketpair(AF_UNIX, SOCK_STREAM, 0, fds), 0);
+    std::unique_ptr<interfaces::Init> init{std::make_unique<TestInit>()};
+    std::unique_ptr<ipc::Protocol> protocol{ipc::capnp::MakeCapnpProtocol()};
+    std::promise<void> promise;
+    std::thread thread([&]() {
+        protocol->serve(fds[0], "test-serve", *init, [&] { promise.set_value(); });
+    });
+    promise.get_future().wait();
+    std::unique_ptr<interfaces::Init> remote_init{protocol->connect(fds[1], "test-connect")};
+    std::unique_ptr<interfaces::Echo> remote_echo{remote_init->makeEcho()};
+    BOOST_CHECK_EQUAL(remote_echo->echo("echo test"), "echo test");
+    remote_echo.reset();
+    remote_init.reset();
+    thread.join();
+}
+
+//! Test ipc::Process bind() and connect() methods connecting over a unix socket.
+void IpcSocketTest(const fs::path& datadir)
+{
+    std::unique_ptr<interfaces::Init> init{std::make_unique<TestInit>()};
+    std::unique_ptr<ipc::Protocol> protocol{ipc::capnp::MakeCapnpProtocol()};
+    std::unique_ptr<ipc::Process> process{ipc::MakeProcess()};
+
+    std::string invalid_bind{"invalid:"};
+    BOOST_CHECK_THROW(process->bind(datadir, "test_bitcoin", invalid_bind), std::invalid_argument);
+    BOOST_CHECK_THROW(process->connect(datadir, "test_bitcoin", invalid_bind), std::invalid_argument);
+
+    auto bind_and_listen{[&](const std::string& bind_address) {
+        std::string address{bind_address};
+        int serve_fd = process->bind(datadir, "test_bitcoin", address);
+        BOOST_CHECK_GE(serve_fd, 0);
+        BOOST_CHECK_EQUAL(address, bind_address);
+        protocol->listen(serve_fd, "test-serve", *init);
+    }};
+
+    auto connect_and_test{[&](const std::string& connect_address) {
+        std::string address{connect_address};
+        int connect_fd{process->connect(datadir, "test_bitcoin", address)};
+        BOOST_CHECK_EQUAL(address, connect_address);
+        std::unique_ptr<interfaces::Init> remote_init{protocol->connect(connect_fd, "test-connect")};
+        std::unique_ptr<interfaces::Echo> remote_echo{remote_init->makeEcho()};
+        BOOST_CHECK_EQUAL(remote_echo->echo("echo test"), "echo test");
+    }};
+
+    // Need to specify explicit socket addresses outside the data directory, because the data
+    // directory path is so long that the default socket address and any other
+    // addresses in the data directory would fail with errors like:
+    //   Address 'unix' path '"/tmp/test_common_Bitcoin Core/ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff/test_bitcoin.sock"' exceeded maximum socket path length
+    std::vector<std::string> addresses{
+        strprintf("unix:%s", TempPath("bitcoin_sock0_XXXXXX")),
+        strprintf("unix:%s", TempPath("bitcoin_sock1_XXXXXX")),
+    };
+
+    // Bind and listen on multiple addresses
+    for (const auto& address : addresses) {
+        bind_and_listen(address);
+    }
+
+    // Connect and test each address multiple times.
+    for (int i : {0, 1, 0, 0, 1}) {
+        connect_and_test(addresses[i]);
+    }
+}

src/ipc/test/ipc_test.h

@@ -0,0 +1,30 @@
+// 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_TEST_IPC_TEST_H
+#define BITCOIN_IPC_TEST_IPC_TEST_H
+
+#include <primitives/transaction.h>
+#include <script/script.h>
+#include <univalue.h>
+#include <util/fs.h>
+#include <validation.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; }
+    CTransactionRef passTransaction(CTransactionRef t) { return t; }
+    std::vector<char> passVectorChar(std::vector<char> v) { return v; }
+    BlockValidationState passBlockState(BlockValidationState s) { return s; }
+    CScript passScript(CScript s) { return s; }
+};
+
+void IpcPipeTest();
+void IpcSocketPairTest();
+void IpcSocketTest(const fs::path& datadir);
+
+#endif // BITCOIN_IPC_TEST_IPC_TEST_H

src/ipc/test/ipc_test_types.h

@@ -0,0 +1,12 @@
+// Copyright (c) 2024 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_TEST_IPC_TEST_TYPES_H
+#define BITCOIN_IPC_TEST_IPC_TEST_TYPES_H
+
+#include <ipc/capnp/common-types.h>
+#include <ipc/capnp/mining-types.h>
+#include <ipc/test/ipc_test.capnp.h>
+
+#endif // BITCOIN_IPC_TEST_IPC_TEST_TYPES_H

src/ipc/test/ipc_tests.cpp

@@ -0,0 +1,42 @@
+// 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 <ipc/process.h>
+#include <ipc/test/ipc_test.h>
+
+#include <test/util/setup_common.h>
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(ipc_tests, BasicTestingSetup)
+BOOST_AUTO_TEST_CASE(ipc_tests)
+{
+    IpcPipeTest();
+    IpcSocketPairTest();
+    IpcSocketTest(m_args.GetDataDirNet());
+}
+
+// Test address parsing.
+BOOST_AUTO_TEST_CASE(parse_address_test)
+{
+    std::unique_ptr<ipc::Process> process{ipc::MakeProcess()};
+    fs::path datadir{"/var/empty/notexist"};
+    auto check_notexist{[](const std::system_error& e) { return e.code() == std::errc::no_such_file_or_directory; }};
+    auto check_address{[&](std::string address, std::string expect_address, std::string expect_error) {
+        if (expect_error.empty()) {
+            BOOST_CHECK_EXCEPTION(process->connect(datadir, "test_bitcoin", address), std::system_error, check_notexist);
+        } else {
+            BOOST_CHECK_EXCEPTION(process->connect(datadir, "test_bitcoin", address), std::invalid_argument, HasReason(expect_error));
+        }
+        BOOST_CHECK_EQUAL(address, expect_address);
+    }};
+    check_address("unix", "unix:/var/empty/notexist/test_bitcoin.sock", "");
+    check_address("unix:", "unix:/var/empty/notexist/test_bitcoin.sock", "");
+    check_address("unix:path.sock", "unix:/var/empty/notexist/path.sock", "");
+    check_address("unix:0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000.sock",
+                  "unix:/var/empty/notexist/0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000.sock",
+                  "Unix address path \"/var/empty/notexist/0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000.sock\" exceeded maximum socket path length");
+    check_address("invalid", "invalid", "Unrecognized address 'invalid'");
+}
+
+BOOST_AUTO_TEST_SUITE_END()