Skip to content

multiprocess: Add bitcoin-wallet -ipcconnect option #19460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
ryanofsky wants to merge 19 commits into
base: master
Choose a base branch
from
Draft

multiprocess: Add bitcoin-wallet -ipcconnect option #19460

ryanofsky wants to merge 19 commits into from
+2,388 −110

Conversation

@ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Jul 7, 2020 •
edited
Loading

This is based on #29409 + #10102. The non-base commits are:

Building on #10102, this adds an -ipcconnect option to bitcoin-wallet and an -ipcbind option to bitcoin-node (both enabled by default in multiprocess builds) so bitcoin node will listen on a <datadir>/sockets/node.sock unix socket, and bitcoin-wallet will connect to it.

The idea is that bitcoin-wallet can be extended in the future to have some online functionality. For example, there could be a bitcoin-wallet sync command that will update balances and sync latest transactions to an unloaded wallet, or a bitcoin-wallet serve subcommand that loads a wallet and serves RPC requests, or a bitcoin-wallet shell subcommand that allows running RPC methods interactively like the GUI console, or just general support for bitcoin-wallet <rpc method> <rpc params> invocations suggested #13926 (comment).

This PR is small and doesn't do much. The only visible change is that bitcoin-wallet now checks whether a node socket exists on startup and prints "Connected to IPC address" if it can connect it it.

The default bitcoin-wallet connect option is -ipcconnect=auto, which connects if possible as described above, and proceeds offline if not possible. Other supported options are -noipcconnect to disable ipc, -ipcconnect to require a connection and fail if it can't be established, and -ipcconnect=unix:<socket> to require a connection and use a custom socket path.

These changes require multiprocess support and this PR has no effect unless bitcoin is configured with --enable-multiprocess as described in doc/multiprocess.md

This PR is part of the process separation project.

@ryanofsky ryanofsky mentioned this pull request Jul 7, 2020
Draft
@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 7, 2020 •
edited
Loading

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/19460.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #33585 (cmake: Use builtin support for .manifest files by purpleKarrot)
  • #33034 (wallet: Store transactions in a separate sqlite table by achow101)
  • #32895 (wallet: Prepare for future upgrades by recording versions of last client to open and decrypt by achow101)
  • #32685 (wallet: Allow read-only database access for info and dump commands by PeterWrighten)
  • #32636 (Split CWallet::Create() into CreateNew and LoadExisting by davidgumberg)
  • #32387 (ipc: add windows support by ryanofsky)
  • #32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)
  • #32138 (wallet, rpc: remove settxfee and paytxfee by polespinasa)
  • #31672 (rpc: add cpu_load to getpeerinfo by vasild)
  • #27052 (test: rpc: add last block announcement time to getpeerinfo result by LarryRuane)
  • #25665 (refactor: Add util::Result failure types and ability to merge result values 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.

This was referenced Jul 7, 2020
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Closed
Merged
Closed
Closed
Closed
Closed
Merged
@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 20, 2025

🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/actions/runs/19547951188/job/55972518109
LLM reason (✨ experimental): Subtree lint check failed: a subtree directory was touched without a proper subtree merge.

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@DrahtBot DrahtBot mentioned this pull request Dec 15, 2025
Draft
1 task
ryanofsky and others added 19 commits January 6, 2026 05:45
@ryanofsky
Add capnp serialization code for bitcoin types
7a247cd 
- Add capnp ToBlob, ToArray, Wrap, Serialize, and Unserialize helper functions
- Add support for std::chrono::seconds capnp serialization
- Add support for util::Result capnp serialization
@ryanofsky
Add capnp wrapper for Handler interface
7f4cd21
@ryanofsky
Add capnp wrapper for Chain interface
ea89449
@ryanofsky
multiprocess: Expose Chain interface
b7b6d6c 
Expose Chain interface to external processes spawning or connecting to
bitcoin-node.
@ryanofsky
Merge branch 'pr/ipc-chain' into pr/ipc
eee530a
@ryanofsky
test: Increase feature_block.py and feature_taproot.py timeouts
93f4714 
Needed because BlockConnected notifications are a lot slower with the wallet
running in separate process.
@ryanofsky
test: Fix multiprocess test for unclean shutdown on kill
9c2f8ce
@ryanofsky
util: Add util::Result workaround to be compatible with libmultiprocess
3582e4a 
Make default constructor more generic so it doesn't only work with void types.
@ryanofsky
multiprocess: Add capnp serialization code for bitcoin types
f1d2d87
@ryanofsky
interfaces, refactor: Change WalletLoader::restoreWallet parameter order
65e4390 
Libmultiprocess requires output parameters to be ordered after input parameter,
so move warnings parameter last. Problematic order was introduced in
4ec2d18 from
bitcoin-core/gui#877
@ryanofsky
multiprocess: Add capnp wrapper for Wallet interface
0e43669
@ryanofsky
multiprocess: Add capnp wrapper for Node interface
0381cc5
@ryanofsky
multiprocess: Make bitcoin-gui spawn a bitcoin-node process
88f3246 
Spawn node subprocess instead of running node code internally
@ryanofsky
multiprocess: Make bitcoin-node spawn a bitcoin-wallet process
aebc163 
Spawn wallet subprocess instead of running wallet code internally
@ryanofsky
multiprocess: Add debug.log .wallet/.gui suffixes
c5b9fe5 
Add .wallet/.gui suffixes to log files created by bitcoin-gui and
bitcoin-wallet processes so they don't clash with bitcoin-node log file.
@ryanofsky
doc: Multiprocess misc doc and comment updates
a73f70f
@ryanofsky
combine_logs: Handle multiprocess wallet log files
2503f57
@ryanofsky
Merge branch 'pr/ipc' into pr/ipc-connect
acd3878
@ryanofsky
multiprocess: Add bitcoin-wallet -ipcconnect option
5124e76 
Add `-ipcconnect` option to `bitcoin-wallet` to allow connecting to a bitcoin
node process over IPC.  The `bitcoin-wallet` tool doesn't really do anything with its
connection to the node yet, but it could potentially run or serve RPCs that
require being online.

Example usage:

    src/bitcoin-node -regtest -debug -ipcbind=unix
    src/bitcoin-wallet -regtest -ipcconnect=unix info
@DrahtBot DrahtBot removed the CI failed label Jan 7, 2026
@DrahtBot DrahtBot mentioned this pull request Jan 8, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@RonSherfey RonSherfey RonSherfey approved these changes

@Korosiua1206 Korosiua1206 Korosiua1206 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Build system Utils/log/libs Wallet

Projects

Process Separation
Status: In progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@ryanofsky @DrahtBot @jonasschnelli @maflcko @RonSherfey @Korosiua1206 @ajtowns @hebasto