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
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.
DrahtBot added the label
Needs rebase
on Jul 11, 2020
ryanofsky force-pushed
on Jul 14, 2020
ryanofsky
commented at 1:55 pm on July 14, 2020:
contributor
DrahtBot removed the label
Needs rebase
on Jul 14, 2020
DrahtBot added the label
Needs rebase
on Jul 30, 2020
ryanofsky force-pushed
on Aug 12, 2020
DrahtBot removed the label
Needs rebase
on Aug 12, 2020
DrahtBot added the label
Needs rebase
on Aug 13, 2020
ryanofsky force-pushed
on Aug 28, 2020
DrahtBot removed the label
Needs rebase
on Aug 28, 2020
DrahtBot added the label
Needs rebase
on Aug 31, 2020
ajtowns removed the label
Docs
on Sep 17, 2020
ajtowns removed the label
GUI
on Sep 17, 2020
ajtowns removed the label
P2P
on Sep 17, 2020
ajtowns removed the label
RPC/REST/ZMQ
on Sep 17, 2020
ajtowns removed the label
Tests
on Sep 17, 2020
ryanofsky force-pushed
on Sep 28, 2020
DrahtBot removed the label
Needs rebase
on Sep 28, 2020
ryanofsky force-pushed
on Oct 2, 2020
DrahtBot added the label
Needs rebase
on Oct 27, 2020
ryanofsky force-pushed
on Nov 25, 2020
DrahtBot removed the label
Needs rebase
on Nov 25, 2020
DrahtBot added the label
Needs rebase
on Dec 2, 2020
ryanofsky force-pushed
on Dec 11, 2020
DrahtBot removed the label
Needs rebase
on Dec 11, 2020
DrahtBot added the label
Needs rebase
on Dec 16, 2020
ryanofsky force-pushed
on Dec 18, 2020
DrahtBot removed the label
Needs rebase
on Dec 18, 2020
jonasschnelli
commented at 8:02 am on December 18, 2020:
contributor
Impressive change. Again!
Conceptual I’m not yet convinced.
My understanding is/was that bitcoin-wallet is an offline wallet exploring and manipulation tool without the requirement of a node or online connectivity.
Things that require a node, should IMO go into wallet RPC calls.
But my conceptual understanding might be old and rusty (happy to get convinced).
Things like bitcoin-wallet shell would be nice though. I guess we would just need to make sure the IPC/node option is completely optional and thus the offline functionality (for things that can work offline) is still guaranteed.
Or is this also an attempt to decouple the wallet from the node (in the long run)?
DrahtBot added the label
Needs rebase
on Dec 18, 2020
ryanofsky
commented at 11:34 am on December 18, 2020:
contributor
My understanding is/was that bitcoin-wallet is an offline wallet exploring and manipulation tool without the requirement of a node or online connectivity.
To address any concern here, there’s no thought of changing this. This PR gives bitcoin-wallet tool ability to do online things as well as offline things. It doesn’t require a node or take away the ability to do offline things. Even if you require separate bitcoin-wallet-online-stuff and bitcoin-wallet-offline-only binaries, it only needs build changes not code changes (a new automake entry and src/init/bitcoin-wallet-offline-only.cpp stub)
DrahtBot removed the label
Needs rebase
on Dec 18, 2020
DrahtBot added the label
Needs rebase
on Jan 7, 2021
ryanofsky force-pushed
on Feb 1, 2021
DrahtBot removed the label
Needs rebase
on Feb 1, 2021
ryanofsky force-pushed
on Feb 4, 2021
DrahtBot added the label
Needs rebase
on Feb 4, 2021
ryanofsky force-pushed
on Feb 21, 2021
DrahtBot removed the label
Needs rebase
on Feb 21, 2021
DrahtBot added the label
Needs rebase
on Mar 2, 2021
ryanofsky force-pushed
on Mar 9, 2021
DrahtBot removed the label
Needs rebase
on Mar 9, 2021
DrahtBot added the label
Needs rebase
on Mar 11, 2021
ryanofsky force-pushed
on Mar 24, 2021
DrahtBot removed the label
Needs rebase
on Mar 25, 2021
ryanofsky force-pushed
on Mar 29, 2021
DrahtBot added the label
Needs rebase
on Mar 30, 2021
ryanofsky force-pushed
on Apr 10, 2021
DrahtBot removed the label
Needs rebase
on Apr 10, 2021
RonSherfey changes_requested
RonSherfey
commented at 3:24 pm on April 11, 2021:
none
Requested changes
laanwj referenced this in commit
ac219dcbcc
on Apr 27, 2021
DrahtBot added the label
Needs rebase
on Apr 27, 2021
ryanofsky force-pushed
on May 24, 2021
DrahtBot removed the label
Needs rebase
on May 24, 2021
RonSherfey approved
RonSherfey
commented at 8:40 am on May 31, 2021:
none
approved changes
ryanofsky force-pushed
on Jun 2, 2021
DrahtBot added the label
Needs rebase
on Jun 9, 2021
ryanofsky force-pushed
on Jun 17, 2021
DrahtBot removed the label
Needs rebase
on Jun 18, 2021
DrahtBot added the label
Needs rebase
on Jun 23, 2021
ryanofsky force-pushed
on Jun 23, 2021
DrahtBot removed the label
Needs rebase
on Jun 23, 2021
DrahtBot added the label
Needs rebase
on Jun 24, 2021
ryanofsky force-pushed
on Aug 18, 2021
DrahtBot removed the label
Needs rebase
on Aug 18, 2021
RonSherfey approved
RonSherfey
commented at 3:53 pm on August 22, 2021:
none
Approved changes
DrahtBot added the label
Needs rebase
on Sep 24, 2021
ryanofsky force-pushed
on Oct 6, 2021
DrahtBot removed the label
Needs rebase
on Oct 6, 2021
DrahtBot added the label
Needs rebase
on Oct 15, 2021
ryanofsky force-pushed
on Nov 1, 2021
DrahtBot removed the label
Needs rebase
on Nov 1, 2021
DrahtBot added the label
Needs rebase
on Nov 10, 2021
ryanofsky force-pushed
on Jan 11, 2022
DrahtBot removed the label
Needs rebase
on Jan 11, 2022
DrahtBot added the label
Needs rebase
on Jan 11, 2022
ryanofsky force-pushed
on Jan 13, 2022
DrahtBot removed the label
Needs rebase
on Jan 13, 2022
DrahtBot added the label
Needs rebase
on Jan 31, 2022
uvhw referenced this in commit
47d44ccc3e
on Feb 14, 2022
ryanofsky force-pushed
on Sep 21, 2022
ryanofsky force-pushed
on Sep 21, 2022
DrahtBot removed the label
Needs rebase
on Sep 21, 2022
ryanofsky force-pushed
on Sep 21, 2022
DrahtBot added the label
Needs rebase
on Sep 25, 2022
ryanofsky force-pushed
on Sep 26, 2022
DrahtBot removed the label
Needs rebase
on Sep 26, 2022
DrahtBot added the label
Needs rebase
on Dec 6, 2022
ryanofsky force-pushed
on Feb 10, 2023
DrahtBot removed the label
Needs rebase
on Feb 10, 2023
ryanofsky force-pushed
on Feb 10, 2023
DrahtBot added the label
Needs rebase
on Feb 17, 2023
ryanofsky force-pushed
on Feb 28, 2023
DrahtBot removed the label
Needs rebase
on Feb 28, 2023
DrahtBot added the label
Needs rebase
on Mar 11, 2023
ryanofsky force-pushed
on May 2, 2023
DrahtBot removed the label
Needs rebase
on May 2, 2023
doc: fix broken doc/design/multiprocess.md links after #2435225a94f30a5
Add SpanReader ignore method
Needed to deserialize some types from spans like CScripts
8e245d1b15
Fix const virtual method that breaks multiprocess support13274fca38
Change getUnspentOutput return type to avoid multiprocess segfault
Coin serialize method segfaults if IsSpent condition is true. This caused
multiprocess code to segfault when serializing the Coin& output argument to of
the Node::getUnspentOutput method if the coin was not found. Segfault could be
triggered by double clicking and viewing transaction details in the GUI
transaction list.
Fix this by replacing Coin& output argument with optional<Coin> return value to
avoid trying to serializing spent coins.
3a10f7130a
node: Add schedulerMockForward method so mockscheduler RPC can work across processes
Needed to fix new wallet_groups.py and wallet_resendwallettransactions.py tests
with multiprocess bitcoin-node executable.
291875605d
Increase feature_block.py and feature_taproot.py timeouts
Needed because BlockConnected notifications are a lot slower with the wallet
running in separate process.
52b407bcf1
Add util::Result workaround to be compatible with libmultiprocess2a31d3555a
Add capnp serialization code for bitcoin typesf942c69780
Add capnp wrapper for Handler interfaced314829582
Add capnp wrapper for Chain interface21fd995bd9
Add capnp wrapper for Wallet interface5fd4f202e6
Add capnp wrapper for Node interface2623d083ba
Make bitcoin-gui spawn a bitcoin-node process
Spawn node subprocess instead of running node code internally
6b3e636294
Make bitcoin-node spawn a bitcoin-wallet process
Spawn wallet subprocess instead of running wallet code internally
8ce9a36c58
multiprocess: Add debug.log .wallet/.gui suffixes
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.
06380b771a
doc: Multiprocess misc doc and comment updatescebfd371ed
multiprocess: Add -ipcconnect and -ipcbind options
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.
bbefbd3c19
ryanofsky force-pushed
on May 4, 2023
DrahtBot added the label
Needs rebase
on May 30, 2023
DrahtBot
commented at 4:04 pm on May 30, 2023:
contributor
🐙 This pull request conflicts with the target branch and needs rebase.
DrahtBot
commented at 1:45 am on August 28, 2023:
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 marked this as a draft
on Sep 20, 2023
DrahtBot
commented at 0:52 am on January 22, 2024:
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
commented at 1:06 am on April 20, 2024:
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.
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2024-07-01 13:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
