ipc: support per-address max-connections options on -ipcbind #35037

enirox001 commented at 1:44 PM on April 9, 2026: contributor

The branch extends -ipcbind to accept :max-connections=<n> options, for example -ipcbind=unix::max-connections=8 or -ipcbind=unix:/custom/path:max-connections=8, instead of introducing a separate global -ipcmaxconnections option.

This keeps the connection limit attached to each individual IPC listener instead of introducing a global -ipcmaxconnections option (https://github.com/bitcoin/bitcoin/pull/34978). A global option would not allow different limits for different -ipcbind addresses, and the inline option format can be extended later with other per-listener settings.

DrahtBot commented at 1:44 PM on April 9, 2026: 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/35037.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	ryanofsky, kevkevinpal, ViniciusCestarii, Sjors

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35730 (http: limit connected HTTPRemoteClients by pinheadmz)
#35084 (ipc: Add nonunix platform support by ryanofsky)
#34978 (init: reserve file descriptors for IPC connections by enirox001)
#31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
#30437 (ipc: add bitcoin-mine test program by ryanofsky)
#19461 (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
#19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
#17783 (common: Disallow calling IsArgSet() on ALLOW_LIST options by ryanofsky)
#17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
#17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
#17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)
#10102 (Multiprocess bitcoin by ryanofsky)

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 places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

check_bind("unix::max-connections=8", "unix:", 8, "") in src/ipc/test/ipc_tests.cpp
check_bind("unix:path.sock:max-connections=8", "unix:path.sock", 8, "") in src/ipc/test/ipc_tests.cpp
check_bind("unix::max-connections=1", "unix:", 1, "") in src/ipc/test/ipc_tests.cpp
check_bind("unix::max-connections=2147483647", "unix:", 2147483647ULL, "") in src/ipc/test/ipc_tests.cpp
check_bind("unix:path.sock:max-connections=8,unknown=1", "", 0, "Unknown socket option 'unknown'") in src/ipc/test/ipc_tests.cpp
check_bind("unix::max-connections=8,", "", 0, "Empty socket option") in src/ipc/test/ipc_tests.cpp
check_bind("unix::max-connections=0", "", 0, "max-connections must be at least 1") in src/ipc/test/ipc_tests.cpp
check_bind("unix::max-connections=-1", "", 0, "max-connections must be at least 1") in src/ipc/test/ipc_tests.cpp
check_bind("unix::max-connections=-9223372036854775808", "", 0, "max-connections must be at least 1") in src/ipc/test/ipc_tests.cpp
check_bind("unix::max-connections=2147483648", "", 0, "max-connections must be at most 2147483647") in src/ipc/test/ipc_tests.cpp
check_bind("unix::max-connections=9223372036854775808", "", 0, "Invalid max-connections value '9223372036854775808'") in src/ipc/test/ipc_tests.cpp
check_bind("unix::max-connections=", "", 0, "Missing value for max-connections option") in src/ipc/test/ipc_tests.cpp

enirox001 renamed this:
~~04 26 ipcbind max connections draft~~
ipc: support per-address max-connections options on -ipcbind
on Apr 9, 2026

enirox001 force-pushed on Apr 9, 2026

DrahtBot added the label CI failed on Apr 9, 2026

enirox001 force-pushed on Apr 9, 2026

ryanofsky commented at 2:40 PM on April 9, 2026: contributor

Concept ACK. Looks like suggestions from #35036 (comment) apply here too (sorry, saw that PR first in my inbox).

IMO this is a better alternative to adding a new -ipcmaxconnections option like #34978, and I'd prefer this approach but both approaches seem reasonable. Main reason I prefer this approach is IPC is a pretty generic feature, so it seems useful to be able to set different limits for different purposes (like GUIs vs mining clients).

enirox001 force-pushed on Apr 9, 2026

enirox001 commented at 3:50 PM on April 9, 2026: contributor

Thanks for the review @ryanofsky

I pushed updates based on your review here as well

the bind-address type is now shared
parsing moved behind the interfaces::Ipc boundary
the parser handles trailing socket options
listenAddress() now takes the bind-address struct directly

in test/functional/interface_ipc_init.py:29 in aadc51fb5f outdated

  24 | +        super().setup_nodes()
  25 | +        self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-ipcbind=")]
  26 | +        self.ipc_tmpdir = tempfile.TemporaryDirectory(prefix="btc-ipc-init-")
  27 | +        self.ipcbind_path = Path(self.ipc_tmpdir.name) / "ipcinit.sock"
  28 | +
  29 | +    def test_ipcbind_max_connections(self):

ViniciusCestarii commented at 5:21 PM on April 9, 2026:

Same suggestion I made on #34978. It would be good to have a functional test case where max-connections is not specified, to verify the default value is actually applied correctly

enirox001 commented at 11:49 AM on April 20, 2026:

Thanks for the suggestion, applied in the latest push

kevkevinpal commented at 3:43 PM on April 13, 2026: contributor

Concept ACK

I agree with Ryanofsky. I think this approach seems more reasonable than #34978, even though both can work and both make sense this seems cleaner. I think it makes sense to go with this approach instead of adding the -ipcmaxconnections option.

fanquake commented at 1:02 PM on April 14, 2026: member

cc @Sjors

enirox001 force-pushed on Apr 20, 2026

enirox001 force-pushed on Apr 21, 2026

enirox001 commented at 9:07 AM on April 21, 2026: contributor

Had a peer review with @Eunovo a few days ago, and I pushed a few follow-up changes to make the branch a bit easier to review:

Updated ParseBindAddress() to return util::Result, and adjusted the callers/tests accordingly.
Extended parser coverage to include 0, large valid values, and negative/overflow cases.
Cleaned up the commit structure so the branch is split into a subtree update, per-address integration, and tests.
Fixed the IPC limit test by correcting the async lifetime issue that could trigger disconnect errors.

The remaining subtree lint failure is expected for this draft, since the branch carries the libmultiprocess subtree update ahead of the upstream merge.

enirox001 force-pushed on May 12, 2026

DrahtBot added the label Needs rebase on May 22, 2026

enirox001 marked this as ready for review on Jun 25, 2026

enirox001 force-pushed on Jun 26, 2026

enirox001 commented at 11:17 AM on June 26, 2026: contributor

Rebased against master to resolve some merge conflicts. This PR should now be ready for review

DrahtBot removed the label Needs rebase on Jun 26, 2026

in src/ipc/libmultiprocess/include/mp/proxy-io.h:870 in b194acc106

 871 | +    auto* ptr = state->listener.get();
 872 |      loop.m_task_set->add(ptr->accept().then(
 873 | -        [&loop, &init, listener = kj::mv(listener)](kj::Own<kj::AsyncIoStream>&& stream) mutable {
 874 | -            _Serve<InitInterface>(loop, kj::mv(stream), init);
 875 | -            _Listen<InitInterface>(loop, kj::mv(listener), init);
 876 | +        [&loop, &init, state](kj::Own<kj::AsyncIoStream>&& stream) mutable {

ViniciusCestarii commented at 2:28 PM on June 26, 2026:

In Squashed 'src/ipc/libmultiprocess/' changes from 3edbe8f67c..8511c68f8 b194acc106b50af2a97e5475845b46f31a5d91f3

nit: this mutable keyword is no longer necessary

        [&loop, &init, state](kj::Own<kj::AsyncIoStream>&& stream) {

enirox001 commented at 2:08 PM on June 29, 2026:

Thanks, updated this in the upstream libmultiprocess PR https://github.com/bitcoin-core/libmultiprocess/pull/269 and squashed into this PR.

in src/interfaces/ipc.h:109 in ddfc34b65a

 104 | +                if (value.empty()) {
 105 | +                    return util::Error{Untranslated("Missing value for max-connections option")};
 106 | +                }
 107 | +                int64_t parsed_limit{-1};
 108 | +                const auto [last, ec] = std::from_chars(value.data(), value.data() + value.size(), parsed_limit);
 109 | +                if (ec != std::errc{} || last != value.data() + value.size() || parsed_limit < 0) {

ViniciusCestarii commented at 5:32 PM on June 26, 2026:

In ipc: add per-address max-connections parsing and support for -ipcbind ddfc34b65ab8c38e33eac05794246dab614a7f01

Shouldn't the lower bound check be parsed_limit < 1? I can't see why someone would use -ipcbind with max-connections=0 to bind something that doesn't accept any connections.

I believe it would be worth to add an upper bound check too because large values flow into the int FD reservation and overflow it: -ipcbind=unix::max-connections=200000000000 aborts the node with Assertion 'min_fd >= 0' failed.

Also it'd be worth splitting the range check from the parse check so out-of-range values like -1 and 0 get a specific message ("max-connections must be at least 1") and too big values for an int get ("max-connections must be at most [upper bound]") instead of a generic "Invalid max-connections value".

enirox001 commented at 2:11 PM on June 29, 2026:

Yes, looking at this, it isn't proper that the lower bound is 0 and the upper bound is not checked.

Changed this to 1 for the lower bound and added an upper bound check.

Also, split the range check; this is not only good for the message specificity, but also makes it more readable

Thanks

in src/ipc/test/ipc_tests.cpp:65 in ddfc34b65a outdated

  60 | +    check_bind("unix:path.sock:max-connections=8,unknown=1", "", 0, "Unknown socket option 'unknown'");
  61 | +    check_bind("unix:max-connections=8", "", 0, "Missing unix socket path before socket options; use unix::<options> for the default path");
  62 | +    check_bind("unix::max-connections=-1", "", 0, "Invalid max-connections value '-1'");
  63 | +    check_bind("unix::max-connections=-9223372036854775808", "", 0, "Invalid max-connections value '-9223372036854775808'");
  64 | +    check_bind("unix::max-connections=9223372036854775808", "", 0, "Invalid max-connections value '9223372036854775808'");
  65 | +    check_bind("unix::max-connections=", "", 0, "Missing value for max-connections option");

ViniciusCestarii commented at 7:28 PM on June 26, 2026:

In ipc: add per-address max-connections parsing and support for -ipcbind ddfc34b65ab8c38e33eac05794246dab614a7f01

There could be a test for the error "Empty socket option":

check_bind("unix::max-connections=8,", "", 0, "Empty socket option");

enirox001 commented at 2:11 PM on June 29, 2026:

Done, added

in test/functional/interface_ipc_init.py:42 in 5d66053ff0

  37 | +            self.restart_node(0, extra_args=[f"-ipcbind=unix:{self.ipcbind_path}"])
  38 | +
  39 | +        ipcbind_arg = f"-ipcbind=unix:{self.ipcbind_path}:max-connections={{}}"
  40 | +
  41 | +        with node.assert_debug_log([
  42 | +            "file descriptors available",

ViniciusCestarii commented at 7:43 PM on June 26, 2026:

In ipc: test per-address connection limiting over unix sockets 5d66053ff08f3d64a81f7bb99462a9a5ce475ada

nit: checking for "file descriptors available" seems unnecessary.

enirox001 commented at 2:12 PM on June 29, 2026:

Yes, this is unecessary, i think i added this while testing and forgot to remove it.

Removed. Thanks

ViniciusCestarii commented at 7:49 PM on June 26, 2026: contributor

Concept ACK, this is a nice feature. Left a few comments

enirox001 force-pushed on Jun 29, 2026

enirox001 commented at 2:35 PM on June 29, 2026: contributor

Thanks for the review @ViniciusCestarii

Made the changes suggested in the latest push

Sjors commented at 9:09 AM on June 30, 2026: member

Concept ACK.

I would be could to explain in the PR description that the alternative, -ipcmaxconnections, won't let us set the limit per binding. That itself doesn't seem very useful now, but it might be when we expand IPC functionality. The -ipcbind=<address>:max-connections=8 option can later be expanded to include a whitelist of interfaces / methods (like -rpcwhitelist), unix group name and chmod flags.

It's bit confusing that the PR description describes this as a "Follow to #34978", even though it doesn't build on it.

The subtree update is done incorrectly, which is presumably why the linter fails. I also prefer to do these in separate pull requests, unless changes are exclusively related to the change here, or it's obvious from the PR title that it involves such an update - neither is the case here.

ddd2e84346b10877c713bb7d383391c1ce2d860c could be split between introducing a default limit per connection and then the second commit that makes it configurable.

ParseBindAddress is rather large. The name implies it's reusable for p2p / rpc bindings , but this PR doesn't do that, and I don't think there's much demand for it. It could still be justified based on the ability to expand it. I don't think the implementation needs to be in the header. Maybe add node/ipc_args.{h,cpp}.

The new default DEFAULT_MAX_CONNECTIONS limit (and how to override it) needs a release note.

Sjors commented at 9:29 AM on June 30, 2026: member

Also it looks like the subtree is based on https://github.com/bitcoin-core/libmultiprocess/pull/269, which is still open. So this PR should be draft.

DrahtBot marked this as a draft on Jun 30, 2026

enirox001 commented at 11:52 AM on July 2, 2026: contributor

Thanks for the review @Sjors

I agree with your point regarding the subtree update. I'm moving this to draft pending the merge of the standalone subtree PR. Once that lands, I will rebase this and apply all of your feedback

DrahtBot added the label Needs rebase on Jul 7, 2026

fanquake referenced this in commit b6becf3534 on Jul 10, 2026

Kino1994 referenced this in commit 75e5bbe4c5 on Jul 10, 2026

enirox001 force-pushed on Jul 11, 2026

DrahtBot removed the label Needs rebase on Jul 11, 2026

enirox001 force-pushed on Jul 11, 2026

ipc: add default connection limit for ipcbind listeners

Reserve file descriptors for each configured -ipcbind listener
and its default accepted connection slots during initialization.

Pass the default listener limit through interfaces::Ipc and ipc::Protocol
so each IPC listener stops accepting new clients after reaching the cap
and resumes after a disconnect.

3b9075a0b6

ipc: support per-address max-connections on ipcbind

Parse optional max-connections socket options from -ipcbind values
in node IPC argument handling.

Pass the parsed per-listener limit through interfaces::Ipc and ipc::Protocol,
and validate each configured limit before adding it to IPC file descriptor
reservations.

b5c72a4c42

ipc: test per-address connection limiting over unix sockets

Add IPC coverage for local listener capacity over a unix socket.

Start a listener with max_connections=1, verify the first client
is served immediately, check that a second client remains blocked
while the first connection stays open, and confirm the second
client becomes usable after the first disconnects.

2b3c6ddd6c

doc: add ipcbind max-connections release note c6d2cd8d0c

enirox001 force-pushed on Jul 11, 2026

enirox001 commented at 7:58 PM on July 11, 2026: contributor

Subtree PR adding the max connections PR has been merged and added to core https://github.com/bitcoin/bitcoin/pull/35684

enirox001 commented at 8:03 PM on July 11, 2026: contributor

Concept ACK.

I would be could to explain in the PR description that the alternative, -ipcmaxconnections, won't let us set the limit per binding. That itself doesn't seem very useful now, but it might be when we expand IPC functionality. The -ipcbind=<address>:max-connections=8 option can later be expanded to include a whitelist of interfaces / methods (like -rpcwhitelist), unix group name and chmod flags.

It's bit confusing that the PR description describes this as a "Follow to #34978", even though it doesn't build on it.

Updated the PR description to mention the alternative approach as why this approach was taken instead. Also removed mention of the followup as it does not build on it

The subtree update is done incorrectly, which is presumably why the linter fails. I also prefer to do these in separate pull requests, unless changes are exclusively related to the change here, or it's obvious from the PR title that it involves such an update - neither is the case here.

ddd2e84 could be split between introducing a default limit per connection and then the second commit that makes it configurable.

Split this to introduce the default limit per connection and another commit that makes it configurable

ParseBindAddress is rather large. The name implies it's reusable for p2p / rpc bindings , but this PR doesn't do that, and I don't think there's much demand for it. It could still be justified based on the ability to expand it. I don't think the implementation needs to be in the header. Maybe add node/ipc_args.{h,cpp}.

The name did imply more applications than what it was intended to be used for. I have renamed it to ParseIpcBindAddress and moved into into the new node/ipc_args.{h,cpp} files

The new default DEFAULT_MAX_CONNECTIONS limit (and how to override it) needs a release note.

Added the release notes in the latest commit

enirox001 commented at 8:04 PM on July 11, 2026: contributor

Once again moving this out of draft. Should be ready for review now. Thanks

enirox001 marked this as ready for review on Jul 11, 2026

DrahtBot removed the label CI failed on Jul 11, 2026

in src/init.cpp:1071 in 3b9075a0b6

1066 |          return InitError(Untranslated("-maxconnections must be greater or equal than zero"));
1067 |      }
1068 | +    // Reserve one listening socket per -ipcbind address plus its accepted
1069 | +    // connection slots. Unlike P2P, IPC has no default listener.
1070 | +    const size_t ipc_bind{args.GetArgs("-ipcbind").size()};
1071 | +    if (ipc_bind > 0 && interfaces::Ipc::DEFAULT_MAX_CONNECTIONS > (MAX_IPC_FDS - ipc_bind) / ipc_bind) {

ryanofsky commented at 9:06 PM on July 15, 2026:

In commit "ipc: add default connection limit for ipcbind listeners" (3b9075a0b67fbf4c6bd4e87579200252b7323565)

I think this would be more readable if the comparison was simplified to if (ipc_max_connections > MAX_IPC_FDS - ipc_bind). Also would consider renaming ipc_bind to ipc_addresses.

in src/init.cpp:751 in b5c72a4c42

 747 | @@ -747,7 +748,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
 748 |      argsman.AddArg("-rpcworkqueue=<n>", strprintf("Set the maximum depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
 749 |      argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
 750 |      if (can_listen_ipc) {
 751 | -        argsman.AddArg("-ipcbind=<address>", "Bind to Unix socket address and listen for incoming connections. Valid address values are \"unix\" to listen on the default path, <datadir>/node.sock, or \"unix:/custom/path\" to specify a custom path. Each configured listener reserves " + ToString(interfaces::Ipc::DEFAULT_MAX_CONNECTIONS) + " accepted connections. Can be specified multiple times to listen on multiple paths. Default behavior is not to listen on any path. If relative paths are specified, they are interpreted relative to the network data directory. If paths include any parent directory components and the parent directories do not exist, they will be created. Enabling this gives local processes that can access the socket unauthenticated RPC access, so it's important to choose a path with secure permissions if customizing this.", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);
 752 | +        argsman.AddArg("-ipcbind=<address>", "Bind to Unix socket address and listen for incoming connections. Valid address values are \"unix\" to listen on the default path, <datadir>/node.sock, or \"unix:/custom/path\" to specify a custom path. Append socat-style socket options like \":max-connections=<n>\" to set per-address listener options, for example \"unix::max-connections=8\" or \"unix:/custom/path:max-connections=8\". If no max-connections option is specified, " + ToString(interfaces::Ipc::DEFAULT_MAX_CONNECTIONS) + " accepted connections will be reserved per listener. Can be specified multiple times to listen on multiple paths. Default behavior is not to listen on any path. If relative paths are specified, they are interpreted relative to the network data directory. If paths include any parent directory components and the parent directories do not exist, they will be created. Enabling this gives local processes that can access the socket unauthenticated RPC access, so it's important to choose a path with secure permissions if customizing this.", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);

ryanofsky commented at 9:42 PM on July 15, 2026:

In commit "ipc: support per-address max-connections on ipcbind" (b5c72a4c4288c17188f34a689a6fbe578b523768)

I think it would be good to change the colon to a comma here (:max-connections=<n> -> ,max-connections=<n>) because this should work better with windows paths which include colons (C:\Bitcoin\node.sock). This would also be more similar to socat syntax.

in src/interfaces/ipc.h:77 in 3b9075a0b6

  71 | @@ -68,8 +72,9 @@ class Ipc
  72 |      virtual std::unique_ptr<Init> connectAddress(std::string& address) = 0;
  73 |  
  74 |      //! Listen on a socket address exposing this process's init interface to
  75 | -    //! clients. Throws an exception if there was an error.
  76 | -    virtual void listenAddress(std::string& address) = 0;
  77 | +    //! clients, accepting at most max_connections simultaneous client
  78 | +    //! connections. Throws an exception if there was an error.
  79 | +    virtual void listenAddress(std::string& address, size_t max_connections) = 0;

ryanofsky commented at 9:54 PM on July 15, 2026:

In commit "ipc: add default connection limit for ipcbind listeners" (3b9075a0b67fbf4c6bd4e87579200252b7323565)

I think it would be nice if this took an const node::IpcBindAddress& so this interface could be future proof if new options are added.

in src/node/ipc_args.h:15 in b5c72a4c42

  10 | +#include <cstddef>
  11 | +#include <string>
  12 | +
  13 | +namespace node {
  14 | +
  15 | +struct IpcBindAddress {

ryanofsky commented at 9:59 PM on July 15, 2026:

In commit "ipc: support per-address max-connections on ipcbind" (b5c72a4c4288c17188f34a689a6fbe578b523768)

I think it probably makes more sense for this to live inside the IPC library than the node library. Would suggest renaming node::IpcBindAddress to ipc::ListenAddress and defining it inside an src/ipc/types.h file for external consumption similar to src/node/types.h and src/wallet/types.h.

Similarly, would suggest changing node::ParseIpcBindAddress to an interfaces::Ipc::parseListenAddress method.

Would also suggest moving interfaces::Ipc::DEFAULT_MAX_CONNECTIONS to ipc::DEFAULT_MAX_CONNECTIONS in src/ipc/types.h using it to initialize the struct field size_t max_connections{DEFAULT_MAX_CONNECTIONS}

These changes would make it easier to add more options in the future without needing to change the interfaces::Ipc method signatures, and I think would organize code better.

ryanofsky commented at 10:09 PM on July 15, 2026: contributor

Code review c6d2cd8d0c64ad9723f02d05e3924374a5b76314. I plan to review more but this looks like a nicely implemented change. I left a few suggestions below, none are too important or blocking, except I do really think the : options separator should be replaced by , to avoid problems with windows paths