bitcoin-cli: Add -ipcconnect option

ryanofsky commented at 1:55 pm on April 17, 2025: contributor

This implements an idea from sipa in #28722 (comment) 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.

This feature can be tested with:

0build/bin/bitcoin-node -regtest -ipcbind=unix -debug=ipc
1build/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.

This PR is part of the process separation project.

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.

d81b499bfc

bitcoin-cli: Add -ipcconnect option

This implements an idea from Pieter Wuille <pieter@wuille.net>
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.

0394da2fec

DrahtBot commented at 1:55 pm on April 17, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32297.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	laanwj, jlest01, pinheadmz

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32061 (Replace libevent with our own HTTP and socket-handling implementation by pinheadmz)
#30437 (multiprocess: add bitcoin-mine test program by ryanofsky)
#29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)
#29409 (multiprocess: Add capnp wrapper for Chain interface by ryanofsky)
#28710 (Remove the legacy wallet and BDB dependency by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

The following typos/grammar issues were found in added lines:

Extra “unix’” in the –ipcconnect help text
Replace
“…node.sock unix’ but fall back to TCP…”
with
“…node.sock’ but fall back to TCP…”
Double negative in JSONErrorReply comment
Replace
“…when a request cannot not be parsed,…”
with
“…when a request cannot be parsed,…”

ryanofsky force-pushed on Apr 17, 2025

DrahtBot added the label CI failed on Apr 17, 2025

laanwj added the label RPC/REST/ZMQ on Apr 17, 2025

in src/bitcoin-cli.cpp:985 in df40f46571 outdated

974+        } else {
975+            throw CConnectionFailed("uri-encode failed");
976+        }
977+    }
978+
979+    std::string username{gArgs.GetArg("-rpcuser", "")};

laanwj commented at 2:34 pm on April 17, 2025:

Passing -rpcconnect and -ipcconnect at the same time should probably result in an error. (at least, the logic of trying IPC first then RPC if that fails is a bit confusing to me, as they’re completely different protocols)

ryanofsky commented at 5:22 pm on April 18, 2025:

re: #32297 (review)

Passing -rpcconnect and -ipcconnect at the same time should probably result in an error. (at least, the logic of trying IPC first then RPC if that fails is a bit confusing to me, as they’re completely different protocols)

Thanks, this makes sense and also makes sense to skip IPC if -rpcconnect is specified. Both these changes are implemented now

laanwj commented at 2:40 pm on April 17, 2025: member

Concept ACK, it’d be useful to have an utility that can talk to the node through IPC and making bitcoin-cli double for that prevents code duplication.

jlest01 commented at 6:28 pm on April 17, 2025: none

Concept ACK, as it can make communication between the CLI and the node simpler and configuration-free.

pinheadmz commented at 6:58 pm on April 17, 2025: member

concept ACK, cool idea and will review. Does this close #5029 (adding unix sockets to the bitcoind rpc api) or does it only apply to bitcoin-node? Or is IPC protocol different from RPC and must be defined in the client?

ryanofsky commented at 7:16 pm on April 17, 2025: contributor

concept ACK, cool idea and will review. Does this close #5029 (adding unix sockets to the bitcoind rpc api) or does it only apply to bitcoin-node? Or is IPC protocol different from RPC and must be defined in the client?

It uses capnproto, so it may or may not achieve goals of #5029 depending on what they are. This PR does make it possible to call RPC methods over a unix socket, but in order to do that you need to use capnproto which is not available everywhere. For example there wouldn’t be an easy way for our python test framework to use the socket directly unless it depended on https://github.com/capnproto/pycapnp. So another solution that uses a another protocol or just uses http over the socket might be preferable to this approach. But the first commit here could be useful for supporting other approaches as well.

test: add interface_ipc_cli.py testing bitcoin-cli -ipcconnect

Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>

bd29de3896

ryanofsky force-pushed on Apr 18, 2025

ryanofsky commented at 5:26 pm on April 18, 2025: contributor

Updated df40f46571bc7b8f0a85a437130e99bad5b0de63 -> 538521a37c3788ce2a0ba1bda76844a25cfd3e06 (pr/ipc-cli.2 -> pr/ipc-cli.3, compare) handling -rpcconnect option, adding a test, making various cleanups, adding comments, and fixing CI failures in non-ipc builds Updated 538521a37c3788ce2a0ba1bda76844a25cfd3e06 -> bd29de3896911317fc6dc0010c33b91ebf5b9f16 (pr/ipc-cli.3 -> pr/ipc-cli.4, compare) fixing compile error in intermediate commit, https://github.com/bitcoin/bitcoin/actions/runs/14538988101/job/40793056407?pr=32297, fixing functional test failure when socket path is too long https://cirrus-ci.com/task/6103052337283072, and adding new test to cover “bitcoin-cli was not built with IPC support” error

ryanofsky force-pushed on Apr 18, 2025

ryanofsky marked this as ready for review on Apr 18, 2025

DrahtBot removed the label CI failed on Apr 18, 2025

bitcoin-cli: Add -ipcconnect option #32297

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)