Skip to content
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.

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

multiprocess: Add bitcoin-wallet -ipcconnect option #19460

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

Conversation

ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Jul 7, 2020

This is a draft PR because it is based on #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.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 7, 2020

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

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:

  • #27711 (kernel: Remove shutdown from kernel library by TheCharlatan)
  • #27636 (kernel: Remove util/system from kernel library, interface_ui from validation. by TheCharlatan)
  • #27632 (Raise on invalid -debug and -loglevel config options by jonatack)
  • #27576 (kernel: Remove args, chainparams, chainparamsbase from kernel library by TheCharlatan)
  • #27375 (net: support unix domain sockets for -proxy and -onion by pinheadmz)
  • #26606 (wallet: Implement independent BDB parser by achow101)
  • #26596 (wallet: Migrate legacy wallets to descriptor wallets without requiring BDB by achow101)
  • #25273 (wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction 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.

ryanofsky and others added 13 commits May 3, 2023 17:49
Spawn node subprocess instead of running node code internally
Spawn wallet subprocess instead of running wallet code internally
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.
Add `-ipcbind` option to `bitcoin-node`, and an `-ipcconnect` option to
`bitcoin-wallet` to allow running a node that listens on an IPC socket and
accepts connections from wallet processes.

Example usage:

    src/bitcoin-node -regtest -debug -ipcbind=unix
    src/bitcoin-wallet -regtest -ipcconnect=unix info

`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.
@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

@DrahtBot
Copy link
Contributor

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@achow101 achow101 marked this pull request as draft September 20, 2023 16:09
@ryanofsky ryanofsky mentioned this pull request Oct 24, 2023
49 tasks
@DrahtBot
Copy link
Contributor

⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?

  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@DrahtBot
Copy link
Contributor

⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?

  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

None yet

6 participants