From ea98a42640b9ff77a462e55887025ddd1da54727 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 17 Apr 2025 09:34:36 -0400 Subject: [PATCH 1/3] refactor: Add ExecuteHTTPRPC function Add ExecuteHTTPRPC to provide a way to execute an HTTP request without relying on HTTPRequest and libevent types. Behavior is not changing in any way, this is just moving code. This commit may be easiest to review using git's --color-moved option. --- src/httprpc.cpp | 143 ++++++++++++++++++++++++--------------------- src/httprpc.h | 7 +++ src/rpc/protocol.h | 2 +- 3 files changed, 84 insertions(+), 68 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 57893702b8b..de93393722f 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -79,13 +79,15 @@ static std::vector> g_rpcauth; static std::map> g_rpc_whitelist; static bool g_rpc_whitelist_default = false; -static void JSONErrorReply(HTTPRequest* req, UniValue objError, const JSONRPCRequest& jreq) +static UniValue JSONErrorReply(UniValue objError, const JSONRPCRequest& jreq, HTTPStatusCode& nStatus) { - // Sending HTTP errors is a legacy JSON-RPC behavior. + // HTTP errors should never be returned if JSON-RPC v2 was requested. This + // function should only be called when a v1 request fails or when a request + // cannot be parsed, so the version is unknown. Assume(jreq.m_json_version != JSONRPCVersion::V2); // Send error reply from json-rpc error object - int nStatus = HTTP_INTERNAL_SERVER_ERROR; + nStatus = HTTP_INTERNAL_SERVER_ERROR; int code = objError.find_value("code").getInt(); if (code == RPC_INVALID_REQUEST) @@ -93,10 +95,7 @@ static void JSONErrorReply(HTTPRequest* req, UniValue objError, const JSONRPCReq else if (code == RPC_METHOD_NOT_FOUND) nStatus = HTTP_NOT_FOUND; - std::string strReply = JSONRPCReplyObj(NullUniValue, std::move(objError), jreq.id, jreq.m_json_version).write() + "\n"; - - req->WriteHeader("Content-Type", "application/json"); - req->WriteReply(nStatus, strReply); + return JSONRPCReplyObj(NullUniValue, std::move(objError), jreq.id, jreq.m_json_version); } //This function checks username and password against -rpcauth @@ -153,60 +152,23 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna return multiUserAuthorized(strUserPass); } -static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) +UniValue ExecuteHTTPRPC(const UniValue& valRequest, JSONRPCRequest& jreq, HTTPStatusCode& status) { - // JSONRPC handles only POST - if (req->GetRequestMethod() != HTTPRequest::POST) { - req->WriteReply(HTTP_BAD_METHOD, "JSONRPC server handles only POST requests"); - return false; - } - // Check authorization - std::pair authHeader = req->GetHeader("authorization"); - if (!authHeader.first) { - req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA); - req->WriteReply(HTTP_UNAUTHORIZED); - return false; - } - - JSONRPCRequest jreq; - jreq.context = context; - jreq.peerAddr = req->GetPeer().ToStringAddrPort(); - if (!RPCAuthorized(authHeader.second, jreq.authUser)) { - LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", jreq.peerAddr); - - /* Deter brute-forcing - If this results in a DoS the user really - shouldn't have their RPC port exposed. */ - UninterruptibleSleep(std::chrono::milliseconds{250}); - - req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA); - req->WriteReply(HTTP_UNAUTHORIZED); - return false; - } - + status = HTTP_OK; try { - // Parse request - UniValue valRequest; - if (!valRequest.read(req->ReadBody())) - throw JSONRPCError(RPC_PARSE_ERROR, "Parse error"); - - // Set the URI - jreq.URI = req->GetURI(); - - UniValue reply; bool user_has_whitelist = g_rpc_whitelist.count(jreq.authUser); if (!user_has_whitelist && g_rpc_whitelist_default) { LogPrintf("RPC User %s not allowed to call any methods\n", jreq.authUser); - req->WriteReply(HTTP_FORBIDDEN); - return false; + status = HTTP_FORBIDDEN; + return {}; // singleton request } else if (valRequest.isObject()) { jreq.parse(valRequest); if (user_has_whitelist && !g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) { LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, jreq.strMethod); - req->WriteReply(HTTP_FORBIDDEN); - return false; + status = HTTP_FORBIDDEN; + return {}; } // Legacy 1.0/1.1 behavior is for failed requests to throw @@ -214,14 +176,13 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) // 2.0 behavior is to catch exceptions and return HTTP success with // RPC errors, as long as there is not an actual HTTP server error. const bool catch_errors{jreq.m_json_version == JSONRPCVersion::V2}; - reply = JSONRPCExec(jreq, catch_errors); - + UniValue reply{JSONRPCExec(jreq, catch_errors)}; if (jreq.IsNotification()) { // Even though we do execute notifications, we do not respond to them - req->WriteReply(HTTP_NO_CONTENT); - return true; + status = HTTP_NO_CONTENT; + return {}; } - + return reply; // array of requests } else if (valRequest.isArray()) { // Check authorization for each request's method @@ -235,15 +196,15 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) std::string strMethod = request.find_value("method").get_str(); if (!g_rpc_whitelist[jreq.authUser].count(strMethod)) { LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, strMethod); - req->WriteReply(HTTP_FORBIDDEN); - return false; + status = HTTP_FORBIDDEN; + return {}; } } } } // Execute each request - reply = UniValue::VARR; + UniValue reply = UniValue::VARR; for (size_t i{0}; i < valRequest.size(); ++i) { // Batches never throw HTTP errors, they are always just included // in "HTTP OK" responses. Notifications never get any response. @@ -270,23 +231,71 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) // empty response in this case to favor being backwards compatible // over complying with the JSON-RPC 2.0 spec in this case. if (reply.size() == 0 && valRequest.size() > 0) { - req->WriteReply(HTTP_NO_CONTENT); - return true; + status = HTTP_NO_CONTENT; + return {}; } + return reply; } else throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error"); - - req->WriteHeader("Content-Type", "application/json"); - req->WriteReply(HTTP_OK, reply.write() + "\n"); } catch (UniValue& e) { - JSONErrorReply(req, std::move(e), jreq); - return false; + return JSONErrorReply(std::move(e), jreq, status); } catch (const std::exception& e) { - JSONErrorReply(req, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq); + return JSONErrorReply(JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq, status); + } +} + +static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) +{ + // JSONRPC handles only POST + if (req->GetRequestMethod() != HTTPRequest::POST) { + req->WriteReply(HTTP_BAD_METHOD, "JSONRPC server handles only POST requests"); return false; } - return true; + // Check authorization + std::pair authHeader = req->GetHeader("authorization"); + if (!authHeader.first) { + req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA); + req->WriteReply(HTTP_UNAUTHORIZED); + return false; + } + + JSONRPCRequest jreq; + jreq.context = context; + jreq.peerAddr = req->GetPeer().ToStringAddrPort(); + jreq.URI = req->GetURI(); + if (!RPCAuthorized(authHeader.second, jreq.authUser)) { + LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", jreq.peerAddr); + + /* Deter brute-forcing + If this results in a DoS the user really + shouldn't have their RPC port exposed. */ + UninterruptibleSleep(std::chrono::milliseconds{250}); + + req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA); + req->WriteReply(HTTP_UNAUTHORIZED); + return false; + } + + // Generate reply + HTTPStatusCode status; + UniValue reply; + UniValue request; + if (request.read(req->ReadBody())) { + reply = ExecuteHTTPRPC(request, jreq, status); + } else { + reply = JSONErrorReply(JSONRPCError(RPC_PARSE_ERROR, "Parse error"), jreq, status); + } + + // Write reply + if (reply.isNull()) { + // Error case or no-content notification reply. + req->WriteReply(status); + } else { + req->WriteHeader("Content-Type", "application/json"); + req->WriteReply(status, reply.write() + "\n"); + } + return status < 400; } static bool InitRPCAuthentication() diff --git a/src/httprpc.h b/src/httprpc.h index 404d13083fc..cd7eacf6c1d 100644 --- a/src/httprpc.h +++ b/src/httprpc.h @@ -7,6 +7,10 @@ #include +class JSONRPCRequest; +class UniValue; +enum HTTPStatusCode : int; + /** Start HTTP RPC subsystem. * Precondition; HTTP and RPC has been started. */ @@ -19,6 +23,9 @@ void InterruptHTTPRPC(); */ void StopHTTPRPC(); +/** Execute a single HTTP request, containing one or more JSONRPC requests. */ +UniValue ExecuteHTTPRPC(const UniValue& valRequest, JSONRPCRequest& jreq, HTTPStatusCode& status); + /** Start HTTP REST subsystem. * Precondition; HTTP and RPC has been started. */ diff --git a/src/rpc/protocol.h b/src/rpc/protocol.h index 0574335c964..ef31d6e1a73 100644 --- a/src/rpc/protocol.h +++ b/src/rpc/protocol.h @@ -7,7 +7,7 @@ #define BITCOIN_RPC_PROTOCOL_H //! HTTP status codes -enum HTTPStatusCode +enum HTTPStatusCode : int { HTTP_OK = 200, HTTP_NO_CONTENT = 204, From 113b46d31075c66cecbe4482aed362d50cdec28c Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 17 Apr 2025 09:40:30 -0400 Subject: [PATCH 2/3] bitcoin-cli: Add -ipcconnect option This implements an idea from Pieter Wuille https://github.com/bitcoin/bitcoin/issues/28722#issuecomment-2807026958 to allow `bitcoin-cli` to connect to the node via IPC instead of TCP, if the `ENABLE_IPC` cmake option is enabled and the node has been started with `-ipcbind`. The feature can be tested with: build/bin/bitcoin-node -regtest -ipcbind=unix -debug=ipc build/bin/bitcoin-cli -regtest -ipcconnect=unix -getinfo The `-ipconnect` parameter can also be omitted, since this change also makes `bitcoin-cli` prefer IPC over HTTP by default, and falling back to HTTP if an IPC connection can't be established. --- src/CMakeLists.txt | 6 ++++ src/bitcoin-cli.cpp | 72 ++++++++++++++++++++++++++++++-------- src/init/basic-ipc.cpp | 25 +++++++++++++ src/init/basic.cpp | 12 +++++++ src/init/bitcoin-gui.cpp | 2 ++ src/init/bitcoin-node.cpp | 2 ++ src/interfaces/init.h | 21 +++++++++++ src/interfaces/rpc.h | 31 ++++++++++++++++ src/ipc/CMakeLists.txt | 1 + src/ipc/capnp/init-types.h | 1 + src/ipc/capnp/init.capnp | 2 ++ src/ipc/capnp/rpc-types.h | 12 +++++++ src/ipc/capnp/rpc.capnp | 17 +++++++++ src/ipc/interfaces.cpp | 5 ++- src/node/interfaces.cpp | 22 ++++++++++++ 15 files changed, 216 insertions(+), 15 deletions(-) create mode 100644 src/init/basic-ipc.cpp create mode 100644 src/init/basic.cpp create mode 100644 src/interfaces/rpc.h create mode 100644 src/ipc/capnp/rpc-types.h create mode 100644 src/ipc/capnp/rpc.capnp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index dac8872080a..21c7e4ca3c5 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -391,6 +391,12 @@ target_link_libraries(bitcoin_cli # Bitcoin Core RPC client if(BUILD_CLI) add_executable(bitcoin-cli bitcoin-cli.cpp) + if(ENABLE_IPC) + target_sources(bitcoin-cli PRIVATE init/basic-ipc.cpp) + target_link_libraries(bitcoin-cli bitcoin_ipc) + else() + target_sources(bitcoin-cli PRIVATE init/basic.cpp) + endif() add_windows_resources(bitcoin-cli bitcoin-cli-res.rc) target_link_libraries(bitcoin-cli core_interface diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 79410b73035..17211d9808c 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -11,6 +11,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -108,6 +111,7 @@ static void SetupCliArgs(ArgsManager& argsman) argsman.AddArg("-stdin", "Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-stdinrpcpass", "Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password. When combined with -stdinwalletpassphrase, -stdinrpcpass consumes the first line, and -stdinwalletpassphrase consumes the second.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-stdinwalletpassphrase", "Read wallet passphrase from standard input as a single line. When combined with -stdin, the first line from standard input is used for the wallet passphrase.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-ipcconnect=
", "Connect to bitcoin-node through IPC socket instead of TCP socket to execute requests. Valid
values are 'auto' to try to connect to default socket path at /node.sock' but fall back to TCP if it is not available, 'unix' to connect to the default socket and fail if it isn't available, or 'unix:' to connect to a socket at a nonstandard path. -noipcconnect can be specified to avoid attempting to use IPC at all. Default value: auto", ArgsManager::ALLOW_ANY, OptionsCategory::IPC); } std::optional RpcWalletName(const ArgsManager& args) @@ -775,7 +779,40 @@ struct DefaultRequestHandler : BaseRequestHandler { } }; -static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector& args, const std::optional& rpcwallet = {}) +static std::optional CallIPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector& args, const std::string& endpoint, const std::string& username) +{ + auto ipcconnect{gArgs.GetArg("-ipcconnect", "auto")}; + if (ipcconnect == "0") return {}; // Do not attempt IPC if -ipcconnect is disabled. + if (gArgs.IsArgSet("-rpcconnect") && !gArgs.IsArgNegated("-rpcconnect")) { + if (ipcconnect == "auto") return {}; // Use HTTP if -ipcconnect=auto is set and -rpcconnect is enabled. + throw std::runtime_error("-rpcconnect and -ipcconnect options cannot both be enabled"); + } + + std::unique_ptr local_init{interfaces::MakeBasicInit("bitcoin-cli")}; + if (!local_init || !local_init->ipc()) { + if (ipcconnect == "auto") return {}; // Use HTTP if -ipcconnect=auto is set and there is no IPC support. + throw std::runtime_error("bitcoin-cli was not built with IPC support"); + } + + std::unique_ptr node_init; + try { + node_init = local_init->ipc()->connectAddress(ipcconnect); + if (!node_init) return {}; // Fall back to HTTP if -ipcconnect=auto connect failed. + } catch (const std::exception& e) { + // Catch connect error if -ipcconnect=unix was specified + throw std::runtime_error{strprintf("%s\n\n" + "Probably bitcoin-node is not running or not listening on a unix socket. Can be started with:\n\n" + " bitcoin-node -chain=%s -ipcbind=unix", e.what(), gArgs.GetChainTypeString())}; + } + + std::unique_ptr rpc{node_init->makeRpc()}; + assert(rpc); + UniValue request{rh->PrepareRequest(strMethod, args)}; + UniValue reply{rpc->executeRpc(std::move(request), endpoint, username)}; + return rh->ProcessReply(reply); +} + +static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector& args, const std::string& endpoint, const std::string& username) { std::string host; // In preference order, we choose the following for the port: @@ -856,7 +893,7 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co failedToGetAuthCookie = true; } } else { - strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", ""); + strRPCUserColonPass = username + ":" + gArgs.GetArg("-rpcpassword", ""); } struct evkeyvalq* output_headers = evhttp_request_get_output_headers(req.get()); @@ -872,17 +909,7 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co assert(output_buffer); evbuffer_add(output_buffer, strRequest.data(), strRequest.size()); - // check if we should use a special wallet endpoint - std::string endpoint = "/"; - if (rpcwallet) { - char* encodedURI = evhttp_uriencode(rpcwallet->data(), rpcwallet->size(), false); - if (encodedURI) { - endpoint = "/wallet/" + std::string(encodedURI); - free(encodedURI); - } else { - throw CConnectionFailed("uri-encode failed"); - } - } + int r = evhttp_make_request(evcon.get(), req.get(), EVHTTP_REQ_POST, endpoint.c_str()); req.release(); // ownership moved to evcon in above call if (r != 0) { @@ -943,9 +970,26 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str const int timeout = gArgs.GetIntArg("-rpcwaittimeout", DEFAULT_WAIT_CLIENT_TIMEOUT); const auto deadline{std::chrono::steady_clock::now() + 1s * timeout}; + // check if we should use a special wallet endpoint + std::string endpoint = "/"; + if (rpcwallet) { + char* encodedURI = evhttp_uriencode(rpcwallet->data(), rpcwallet->size(), false); + if (encodedURI) { + endpoint = "/wallet/" + std::string(encodedURI); + free(encodedURI); + } else { + throw CConnectionFailed("uri-encode failed"); + } + } + + std::string username{gArgs.GetArg("-rpcuser", "")}; + if (auto response{CallIPC(rh, strMethod, args, endpoint, username)}) { + return *response; + } + do { try { - response = CallRPC(rh, strMethod, args, rpcwallet); + response = CallRPC(rh, strMethod, args, endpoint, username); if (fWait) { const UniValue& error = response.find_value("error"); if (!error.isNull() && error["code"].getInt() == RPC_IN_WARMUP) { diff --git a/src/init/basic-ipc.cpp b/src/init/basic-ipc.cpp new file mode 100644 index 00000000000..d17559e002b --- /dev/null +++ b/src/init/basic-ipc.cpp @@ -0,0 +1,25 @@ +// Copyright (c) 2025 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 + +namespace init { +namespace { +class BitcoinBasicInit : public interfaces::Init +{ +public: + BitcoinBasicInit(const char* exe_name, const char* process_argv0) : m_ipc(interfaces::MakeIpc(exe_name, process_argv0, *this)) {} + interfaces::Ipc* ipc() override { return m_ipc.get(); } + std::unique_ptr m_ipc; +}; +} // namespace +} // namespace init + +namespace interfaces { +std::unique_ptr MakeBasicInit(const char* exe_name, const char* process_argv0) +{ + return std::make_unique(exe_name, process_argv0); +} +} // namespace interfaces diff --git a/src/init/basic.cpp b/src/init/basic.cpp new file mode 100644 index 00000000000..c714ddfd042 --- /dev/null +++ b/src/init/basic.cpp @@ -0,0 +1,12 @@ +// Copyright (c) 2025 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 + +namespace interfaces { +std::unique_ptr MakeBasicInit(const char* exe_name, const char* process_argv0) +{ + return std::make_unique(); +} +} // namespace interfaces diff --git a/src/init/bitcoin-gui.cpp b/src/init/bitcoin-gui.cpp index eae30bc995a..a671cb9edca 100644 --- a/src/init/bitcoin-gui.cpp +++ b/src/init/bitcoin-gui.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -33,6 +34,7 @@ public: return MakeWalletLoader(chain, *Assert(m_node.args)); } std::unique_ptr makeEcho() override { return interfaces::MakeEcho(); } + std::unique_ptr makeRpc() override { return interfaces::MakeRpc(m_node); } interfaces::Ipc* ipc() override { return m_ipc.get(); } // bitcoin-gui accepts -ipcbind option even though it does not use it // directly. It just returns true here to accept the option because diff --git a/src/init/bitcoin-node.cpp b/src/init/bitcoin-node.cpp index 3f8c50b8d66..9d33b4475df 100644 --- a/src/init/bitcoin-node.cpp +++ b/src/init/bitcoin-node.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +37,7 @@ public: return MakeWalletLoader(chain, *Assert(m_node.args)); } std::unique_ptr makeEcho() override { return interfaces::MakeEcho(); } + std::unique_ptr makeRpc() override { return interfaces::MakeRpc(m_node); } interfaces::Ipc* ipc() override { return m_ipc.get(); } bool canListenIpc() override { return true; } node::NodeContext& m_node; diff --git a/src/interfaces/init.h b/src/interfaces/init.h index b496ada05f4..264ea5a7d87 100644 --- a/src/interfaces/init.h +++ b/src/interfaces/init.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -36,6 +37,7 @@ public: virtual std::unique_ptr makeMining() { return nullptr; } virtual std::unique_ptr makeWalletLoader(Chain& chain) { return nullptr; } virtual std::unique_ptr makeEcho() { return nullptr; } + virtual std::unique_ptr makeRpc() { return nullptr; } virtual Ipc* ipc() { return nullptr; } virtual bool canListenIpc() { return false; } }; @@ -53,6 +55,25 @@ std::unique_ptr MakeWalletInit(int argc, char* argv[], int& exit_status); //! Return implementation of Init interface for the gui process. std::unique_ptr MakeGuiInit(int argc, char* argv[]); + +//! Return implementation of Init interface for a basic IPC client that doesn't +//! provide any IPC services itself. +//! +//! When an IPC client connects to a socket or spawns a process, it gets a pointer +//! to an Init object allowing it to create objects and threads on the remote +//! side of the IPC connection. But the client also needs to provide a local Init +//! object to allow the remote side of the connection to create objects and +//! threads on this side. This function just returns a basic Init object +//! allowing remote connections to only create local threads, not other objects +//! (because its Init::make* methods return null.) +//! +//! @param exe_name Current executable name, which is just passed to the IPC +//! system and used for logging. +//! +//! @param process_argv0 Optional string containing argv[0] value passed to +//! main(). This is passed to the IPC system and used to locate binaries by +//! relative path if subprocesses are spawned. +std::unique_ptr MakeBasicInit(const char* exe_name, const char* process_argv0=""); } // namespace interfaces #endif // BITCOIN_INTERFACES_INIT_H diff --git a/src/interfaces/rpc.h b/src/interfaces/rpc.h new file mode 100644 index 00000000000..6fb68047480 --- /dev/null +++ b/src/interfaces/rpc.h @@ -0,0 +1,31 @@ +// Copyright (c) 2025 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_INTERFACES_RPC_H +#define BITCOIN_INTERFACES_RPC_H + +#include +#include + +class UniValue; + +namespace node { +struct NodeContext; +} // namespace node + +namespace interfaces { +//! Interface giving clients ability to emulate RPC calls. +class Rpc +{ +public: + virtual ~Rpc() = default; + virtual UniValue executeRpc(UniValue request, std::string url, std::string user) = 0; +}; + +//! Return implementation of Rpc interface. +std::unique_ptr MakeRpc(node::NodeContext& node); + +} // namespace interfaces + +#endif // BITCOIN_INTERFACES_RPC_H diff --git a/src/ipc/CMakeLists.txt b/src/ipc/CMakeLists.txt index 904d72f56e1..44e68db60d5 100644 --- a/src/ipc/CMakeLists.txt +++ b/src/ipc/CMakeLists.txt @@ -14,6 +14,7 @@ target_capnp_sources(bitcoin_ipc ${PROJECT_SOURCE_DIR} capnp/echo.capnp capnp/init.capnp capnp/mining.capnp + capnp/rpc.capnp ) target_link_libraries(bitcoin_ipc diff --git a/src/ipc/capnp/init-types.h b/src/ipc/capnp/init-types.h index c3ddca27c04..981147f8c1b 100644 --- a/src/ipc/capnp/init-types.h +++ b/src/ipc/capnp/init-types.h @@ -7,5 +7,6 @@ #include #include +#include #endif // BITCOIN_IPC_CAPNP_INIT_TYPES_H diff --git a/src/ipc/capnp/init.capnp b/src/ipc/capnp/init.capnp index 1001ee53368..5d76f046135 100644 --- a/src/ipc/capnp/init.capnp +++ b/src/ipc/capnp/init.capnp @@ -15,9 +15,11 @@ $Proxy.includeTypes("ipc/capnp/init-types.h"); using Echo = import "echo.capnp"; using Mining = import "mining.capnp"; +using Rpc = import "rpc.capnp"; interface Init $Proxy.wrap("interfaces::Init") { construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :Proxy.ThreadMap); makeEcho @1 (context :Proxy.Context) -> (result :Echo.Echo); makeMining @2 (context :Proxy.Context) -> (result :Mining.Mining); + makeRpc @3 (context :Proxy.Context) -> (result :Rpc.Rpc); } diff --git a/src/ipc/capnp/rpc-types.h b/src/ipc/capnp/rpc-types.h new file mode 100644 index 00000000000..4d385dee312 --- /dev/null +++ b/src/ipc/capnp/rpc-types.h @@ -0,0 +1,12 @@ +// Copyright (c) 2025 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_RPC_TYPES_H +#define BITCOIN_IPC_CAPNP_RPC_TYPES_H + +#include +#include +#include + +#endif // BITCOIN_IPC_CAPNP_RPC_TYPES_H diff --git a/src/ipc/capnp/rpc.capnp b/src/ipc/capnp/rpc.capnp new file mode 100644 index 00000000000..c831424a046 --- /dev/null +++ b/src/ipc/capnp/rpc.capnp @@ -0,0 +1,17 @@ +# Copyright (c) 2025 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +@0x9c3505dc45e146ac; + +using Cxx = import "/capnp/c++.capnp"; +$Cxx.namespace("ipc::capnp::messages"); + +using Common = import "common.capnp"; +using Proxy = import "/mp/proxy.capnp"; +$Proxy.include("interfaces/rpc.h"); +$Proxy.includeTypes("ipc/capnp/rpc-types.h"); + +interface Rpc $Proxy.wrap("interfaces::Rpc") { + executeRpc @0 (context :Proxy.Context, request :Text, uri :Text, user :Text) -> (result :Text); +} diff --git a/src/ipc/interfaces.cpp b/src/ipc/interfaces.cpp index 33555f05d4c..9eb40fa419f 100644 --- a/src/ipc/interfaces.cpp +++ b/src/ipc/interfaces.cpp @@ -71,10 +71,13 @@ public: fd = m_process->connect(gArgs.GetDataDirNet(), "bitcoin-node", address); } catch (const std::system_error& e) { // If connection type is auto and socket path isn't accepting connections, or doesn't exist, catch the error and return null; - if (e.code() == std::errc::connection_refused || e.code() == std::errc::no_such_file_or_directory) { + if (e.code() == std::errc::connection_refused || e.code() == std::errc::no_such_file_or_directory || e.code() == std::errc::not_a_directory) { return nullptr; } throw; + } catch (const std::invalid_argument&) { + // Catch 'Unix address path "..." exceeded maximum socket path length' error + return nullptr; } } else { fd = m_process->connect(gArgs.GetDataDirNet(), "bitcoin-node", address); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 8aec2758f8b..aef576f6c07 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -12,12 +12,14 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -80,6 +82,7 @@ using interfaces::Handler; using interfaces::MakeSignalHandler; using interfaces::Mining; using interfaces::Node; +using interfaces::Rpc; using interfaces::WalletLoader; using node::BlockAssembler; using node::BlockWaitOptions; @@ -1115,6 +1118,24 @@ public: KernelNotifications& notifications() { return *Assert(m_node.notifications); } NodeContext& m_node; }; + +class RpcImpl : public Rpc +{ +public: + explicit RpcImpl(NodeContext& node) : m_node(node) {} + + UniValue executeRpc(UniValue request, std::string uri, std::string user) override + { + JSONRPCRequest req; + req.context = &m_node; + req.URI = std::move(uri); + req.authUser = std::move(user); + HTTPStatusCode status; + return ExecuteHTTPRPC(request, req, status); + } + + NodeContext& m_node; +}; } // namespace } // namespace node @@ -1122,4 +1143,5 @@ namespace interfaces { std::unique_ptr MakeNode(node::NodeContext& context) { return std::make_unique(context); } std::unique_ptr MakeChain(node::NodeContext& context) { return std::make_unique(context); } std::unique_ptr MakeMining(node::NodeContext& context) { return std::make_unique(context); } +std::unique_ptr MakeRpc(node::NodeContext& context) { return std::make_unique(context); } } // namespace interfaces From 9ff5bc75659e0314f42c1477fc0c37ac0ad132fa Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 18 Apr 2025 11:37:21 -0400 Subject: [PATCH 3/3] test: add interface_ipc_cli.py testing bitcoin-cli -ipcconnect Co-authored-by: Sjors Provoost --- test/CMakeLists.txt | 1 + test/config.ini.in | 1 + test/functional/interface_bitcoin_cli.py | 9 +++ test/functional/interface_ipc_cli.py | 81 +++++++++++++++++++ .../test_framework/test_framework.py | 17 +++- test/functional/test_runner.py | 1 + 6 files changed, 107 insertions(+), 3 deletions(-) create mode 100755 test/functional/interface_ipc_cli.py diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 2c23dd41100..a58b310adda 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -26,6 +26,7 @@ function(create_test_config) set_configure_variable(WITH_ZMQ ENABLE_ZMQ) set_configure_variable(ENABLE_EXTERNAL_SIGNER ENABLE_EXTERNAL_SIGNER) set_configure_variable(WITH_USDT ENABLE_USDT_TRACEPOINTS) + set_configure_variable(ENABLE_IPC ENABLE_IPC) configure_file(config.ini.in config.ini USE_SOURCE_PERMISSIONS @ONLY) endfunction() diff --git a/test/config.ini.in b/test/config.ini.in index bd4bd30d47e..0db80acf793 100644 --- a/test/config.ini.in +++ b/test/config.ini.in @@ -26,3 +26,4 @@ RPCAUTH=@abs_top_srcdir@/share/rpcauth/rpcauth.py @ENABLE_ZMQ_TRUE@ENABLE_ZMQ=true @ENABLE_EXTERNAL_SIGNER_TRUE@ENABLE_EXTERNAL_SIGNER=true @ENABLE_USDT_TRACEPOINTS_TRUE@ENABLE_USDT_TRACEPOINTS=true +@ENABLE_IPC_TRUE@ENABLE_IPC=true diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 2618c12e9f5..044af74aaca 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -6,6 +6,7 @@ from decimal import Decimal import re +import subprocess from test_framework.blocktools import COINBASE_MATURITY from test_framework.netutil import test_ipv6_local @@ -400,6 +401,14 @@ class TestBitcoinCli(BitcoinTestFramework): self.log.info("Test that only one of -addrinfo, -generate, -getinfo, -netinfo may be specified at a time") assert_raises_process_error(1, "Only one of -getinfo, -netinfo may be specified", self.nodes[0].cli('-getinfo', '-netinfo').send_cli) + if not self.is_ipc_enabled(): + self.log.info("Test bitcoin-cli -ipcconnect triggers error if not built with IPC support") + args = [self.binary_paths.bitcoincli, "-ipcconnect=unix", "-getinfo"] + result = subprocess.run(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True) + assert_equal(result.stdout, "error: bitcoin-cli was not built with IPC support\n") + assert_equal(result.stderr, None) + assert_equal(result.returncode, 1) + if __name__ == '__main__': TestBitcoinCli(__file__).main() diff --git a/test/functional/interface_ipc_cli.py b/test/functional/interface_ipc_cli.py new file mode 100755 index 00000000000..63184d1c851 --- /dev/null +++ b/test/functional/interface_ipc_cli.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python3 +# Copyright (c) 2025 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test IPC with bitcoin-cli""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + rpc_port +) + +import subprocess +import tempfile + +class TestBitcoinIpcCli(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_ipc() + + def setup_nodes(self): + # Always run IPC node binary + self.binary_paths.bitcoind = self.binary_paths.bitcoin_node + + # Work around default CI path exceeding maximum socket path length. + # On Linux sun_path is 108 bytes, on macOS it's only 104. Includes + # null terminator. + self.socket_path = self.options.tmpdir + "/node0/regtest/node.sock" + if len(self.socket_path.encode('utf-8')) < 104: + self.extra_args = [["-ipcbind=unix"]] + self.default_socket = True + else: + self.socket_path = tempfile.mktemp() + self.extra_args = [[f"-ipcbind=unix:{self.socket_path}"]] + self.default_socket = False + super().setup_nodes() + + def test_cli(self, args, error=None): + # Intentionally set wrong RPC password so only IPC not HTTP connections work + args = [self.binary_paths.bitcoincli, f"-datadir={self.nodes[0].datadir_path}", "-rpcpassword=wrong"] + args + ["echo", "foo"] + result = subprocess.run(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True) + if error: + assert_equal(result.stdout, error) + else: + assert_equal(result.stdout, '[\n "foo"\n]\n') + assert_equal(result.stderr, None) + assert_equal(result.returncode, 1 if error else 0) + + def run_test(self): + if not self.default_socket: + self.log.info("Skipping a few checks because temporary directory path is too long") + + http_auth_error = "error: Authorization failed: Incorrect rpcuser or rpcpassword\n" + http_connect_error = f"error: timeout on transient error: Could not connect to the server 127.0.0.1:{rpc_port(self.nodes[0].index)}\n\nMake sure the bitcoind server is running and that you are connecting to the correct RPC port.\nUse \"bitcoin-cli -help\" for more info.\n" + http_ipc_error = "error: -rpcconnect and -ipcconnect options cannot both be enabled\n" + ipc_connect_error = "error: Connection refused\n\nProbably bitcoin-node is not running or not listening on a unix socket. Can be started with:\n\n bitcoin-node -chain=regtest -ipcbind=unix\n" + + for started in (True, False): + auto_error = None if started else http_connect_error + http_error = http_auth_error if started else http_connect_error + ipc_error = None if started else ipc_connect_error + + if self.default_socket: + self.test_cli([], auto_error) + self.test_cli(["-rpcconnect=127.0.0.1"], http_error) + self.test_cli(["-ipcconnect=auto"], auto_error) + self.test_cli(["-ipcconnect=auto", "-rpcconnect=127.0.0.1"], http_error) + self.test_cli(["-ipcconnect=unix"], ipc_error) + + self.test_cli([f"-ipcconnect=unix:{self.socket_path}"], ipc_error) + self.test_cli(["-noipcconnect"], http_error) + self.test_cli(["-ipcconnect=unix", "-rpcconnect=127.0.0.1"], http_ipc_error) + + self.stop_node(0) + + +if __name__ == '__main__': + TestBitcoinIpcCli(__file__).main() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 659d62530d8..03524e68f34 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -298,13 +298,15 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): "bitcoin-chainstate": ("bitcoinchainstate", "BITCOINCHAINSTATE"), "bitcoin-wallet": ("bitcoinwallet", "BITCOINWALLET"), } - for binary, [attribute_name, env_variable_name] in binaries.items(): - default_filename = os.path.join( + def binary_path(binary): + return os.path.join( self.config["environment"]["BUILDDIR"], "bin", binary + self.config["environment"]["EXEEXT"], ) - setattr(paths, attribute_name, os.getenv(env_variable_name, default=default_filename)) + for binary, [attribute_name, env_variable_name] in binaries.items(): + setattr(paths, attribute_name, os.getenv(env_variable_name) or binary_path(binary)) + paths.bitcoin_node = binary_path("bitcoin-node") return paths def get_binaries(self, bin_dir=None): @@ -1037,6 +1039,11 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): if not self.is_cli_compiled(): raise SkipTest("bitcoin-cli has not been compiled.") + def skip_if_no_ipc(self): + """Skip the running test if ipc is not enabled.""" + if not self.is_ipc_enabled(): + raise SkipTest("ipc is not enabled.") + def skip_if_no_previous_releases(self): """Skip the running test if previous releases are not available.""" if not self.has_previous_releases(): @@ -1099,5 +1106,9 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): """Checks whether the wallet module was compiled with BDB support.""" return self.config["components"].getboolean("USE_BDB") + def is_ipc_enabled(self): + """Checks whether ipc is enabled.""" + return self.config["components"].getboolean("ENABLE_IPC") + def has_blockfile(self, node, filenum: str): return (node.blocks_path/ f"blk{filenum}.dat").is_file() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 3df1644622c..af8770bda06 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -408,6 +408,7 @@ BASE_SCRIPTS = [ 'rpc_help.py', 'p2p_handshake.py', 'p2p_handshake.py --v2transport', + 'interface_ipc_cli.py', 'feature_dirsymlinks.py', 'feature_help.py', 'feature_shutdown.py',