highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-08 13:49:35 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#30509: multiprocess: Add -ipcbind option to bitcoin-node

Browse Source 
30073e6b3a multiprocess: Add -ipcbind option to bitcoin-node (Russell Yanofsky)
73fe7d7230 multiprocess: Add unit tests for connect, serve, and listen functions (Ryan Ofsky)
955d4077aa multiprocess: Add IPC connectAddress and listenAddress methods (Russell Yanofsky)
4da20434d4 depends: Update libmultiprocess library for CustomMessage function and ThreadContext bugfix (Ryan Ofsky)

Pull request description:

  Add `-ipcbind` option to `bitcoin-node` to make it listen on a unix socket and accept connections from other processes. The default socket path is `<datadir>/node.sock`, but this can be customized.

  This option lets potential wallet, gui, index, and mining processes connect to the node and control it. See examples in #19460, #19461, and #30437.

  Motivation for this PR, in combination with #30510, is be able to release a bitcoin core node binary that can generate block templates for a separate Stratum v2 mining service, like the one being implemented in https://github.com/Sjors/bitcoin/pull/48, that connects over IPC.

  Other things to know about this PR:

  - While the `-ipcbind` option lets other processes to connect to the `bitcoin-node` process, the only thing they can actually do after connecting is call methods on the [`Init`](https://github.com/bitcoin/bitcoin/blob/master/src/ipc/capnp/init.capnp#L17-L20) interface which is currently very limited and doesn't do much. But PRs [#30510](https://github.com/bitcoin/bitcoin/pull/30510), [#29409](https://github.com/bitcoin/bitcoin/pull/29409), and [#10102](https://github.com/bitcoin/bitcoin/pull/10102) expand the `Init` interface to expose mining, wallet, and gui functionality respectively.

  - This PR is not needed for [#10102](https://github.com/bitcoin/bitcoin/pull/10102), which runs GUI, node, and wallet code in different processes, because [#10102](https://github.com/bitcoin/bitcoin/pull/10102) does not use unix sockets or allow outside processes to connect to existing processes. [#10102](https://github.com/bitcoin/bitcoin/pull/10102) lets parent and child processes communicate over internal socketpairs, not externally accessible sockets.

  ---

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

ACKs for top commit:
  achow101:
    ACK 30073e6b3a
  TheCharlatan:
    Re-ACK 30073e6b3a
  itornaza:
    Code review ACK 30073e6b3a

Tree-SHA512: 2b766e60535f57352e8afda9c3748a32acb5a57b2827371b48ba865fa9aa1df00f340732654f2e300c6823dbc6f3e14377fca87e4e959e613fe85a6d2312d9c8
This commit is contained in:
Ava Chow
2024-09-09 17:14:15 -04:00
parent 712a2b5453 30073e6b3a
commit df3f63ccfa
21 changed files with 359 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/test/CMakeLists.txt
View File

@@ -177,7 +177,7 @@ if(WITH_MULTIPROCESS)
PRIVATE
ipc_tests.cpp
)
target_link_libraries(test_bitcoin bitcoin_ipc_test)
target_link_libraries(test_bitcoin bitcoin_ipc_test bitcoin_ipc)
endif()
function(add_boost_test source_file)

2
src/test/fuzz/system.cpp
View File

@@ -59,7 +59,7 @@ FUZZ_TARGET(system, .init = initialize_system)
args_manager.SoftSetBoolArg(str_arg, f_value);
},
[&] {
const OptionsCategory options_category = fuzzed_data_provider.PickValueInArray<OptionsCategory>({OptionsCategory::OPTIONS, OptionsCategory::CONNECTION, OptionsCategory::WALLET, OptionsCategory::WALLET_DEBUG_TEST, OptionsCategory::ZMQ, OptionsCategory::DEBUG_TEST, OptionsCategory::CHAINPARAMS, OptionsCategory::NODE_RELAY, OptionsCategory::BLOCK_CREATION, OptionsCategory::RPC, OptionsCategory::GUI, OptionsCategory::COMMANDS, OptionsCategory::REGISTER_COMMANDS, OptionsCategory::CLI_COMMANDS, OptionsCategory::HIDDEN});
const OptionsCategory options_category = fuzzed_data_provider.PickValueInArray<OptionsCategory>({OptionsCategory::OPTIONS, OptionsCategory::CONNECTION, OptionsCategory::WALLET, OptionsCategory::WALLET_DEBUG_TEST, OptionsCategory::ZMQ, OptionsCategory::DEBUG_TEST, OptionsCategory::CHAINPARAMS, OptionsCategory::NODE_RELAY, OptionsCategory::BLOCK_CREATION, OptionsCategory::RPC, OptionsCategory::GUI, OptionsCategory::COMMANDS, OptionsCategory::REGISTER_COMMANDS, OptionsCategory::CLI_COMMANDS, OptionsCategory::IPC, OptionsCategory::HIDDEN});
// Avoid hitting:
// common/args.cpp:563: void ArgsManager::AddArg(const std::string &, const std::string &, unsigned int, const OptionsCategory &): Assertion `ret.second' failed.
const std::string argument_name = GetArgumentName(fuzzed_data_provider.ConsumeRandomLengthString(16));

99
src/test/ipc_test.cpp
View File

@@ -2,19 +2,46 @@
// 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 <test/ipc_test.capnp.h>
#include <test/ipc_test.capnp.proxy.h>
#include <test/ipc_test.h>
#include <tinyformat.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
@@ -23,13 +50,13 @@
//! 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()
void IpcPipeTest()
{
// 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); });
mp::EventLoop loop("IpcPipeTest", [](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]));
@@ -65,3 +92,71 @@ void IpcTest()
disconnect_client();
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]);
}
}

5
src/test/ipc_test.h
View File

@@ -7,6 +7,7 @@
#include <primitives/transaction.h>
#include <univalue.h>
#include <util/fs.h>
class FooImplementation
{
@@ -16,6 +17,8 @@ public:
UniValue passUniValue(UniValue v) { return v; }
};
void IpcTest();
void IpcPipeTest();
void IpcSocketPairTest();
void IpcSocketTest(const fs::path& datadir);
#endif // BITCOIN_TEST_IPC_TEST_H

33
src/test/ipc_tests.cpp
View File

@@ -2,12 +2,41 @@
// 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 <test/ipc_test.h>
#include <test/util/setup_common.h>
#include <boost/test/unit_test.hpp>
BOOST_AUTO_TEST_SUITE(ipc_tests)
BOOST_FIXTURE_TEST_SUITE(ipc_tests, BasicTestingSetup)
BOOST_AUTO_TEST_CASE(ipc_tests)
{
IpcTest();
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()