Building on #10102, this adds an -ipcconnect option to bitcoin-gui that connects the GUI to an existing bitcoin-node process already running in the background instead of spawning a new bitcoin-node process. This allows the GUI to be started and stopped independently of the node. By default with this change, bitcoin-gui will check if a <datadir>/sockets/node.sock socket exists and try to connect to that. If that doesn’t work, it will spawn a new node process and start up the same way it did before this PR.
The default bitcoin-gui connect option is -ipcconnect=auto, which tries to connect if possible as described above, and spawns a new bitcoin-node process if not possible. Other supported options are -noipcconnect to never connect to an existing node and always spawn a new one, -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.
With this PR, basic functionality works and gui instances can connect and disconnect from a running node. But there are rough edges: If a gui process doesn’t shut down cleanly, the node can see unhandled IpcExceptions, and if node command line options are passed to bitcoin-gui and bitcoin-gui connects to an exiting bitcoin-node process instead of spawning a new one, the node options will be silently ignored.
These changes require multiprocess support and this PR has no effect unless bitcoin is configured with --enable-multiprocess as described in doc/multiprocess.md
#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.
laanwj
commented at 5:30 pm on July 9, 2020:
member
Concept ACK, I’ve only reviewed the option help yet but -ipcconnect SGTM as option name, it reminds of -rpcconnect of the -cli client.
in
src/bitcoin-wallet.cpp:34
in
70aaca9ebeoutdated
29@@ -28,15 +30,18 @@ static void SetupWalletToolArgs()
30 gArgs.AddArg("-wallet=<wallet-name>", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS);
31 gArgs.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
32 gArgs.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
33+ if (include_ipc) {
34+ gArgs.AddArg("-ipcconnect=<address>, -noipcconnect", "Connect to bitcoin-node process in the background to perform online operations. Valid <address> values are 'auto' to try connecting to default socket in <datadir>/sockets/node.sock, but proceed offline if it isn't available, 'unix' to connect to the default socket and fail if it isn't available, 'unix:<socket path>' to connect to a socket at a nonstandard path. Default value: auto", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);
I think this is the only case of multiple options separated by , inside the first argument of ArgsManager::AddArg. I don’t know if it’s strictly wrong, as it’s not currently parsed in any way, but maybe the idea is to do so in the future. @MarcoFalke?
I think this is the only case of multiple options separated by , inside the first argument of ArgsManager::AddArg. I don’t know if it’s strictly wrong, as it’s not currently parsed in any way, but maybe the idea is to do so in the future. @MarcoFalke?
Thanks, dropped nonstandard formatting and updated description in base PR #19460. No reason to separately list -no prefix.
DrahtBot added the label
Needs rebase
on Jul 11, 2020
ryanofsky referenced this in commit
6a48a575d1
on Jul 14, 2020
ryanofsky force-pushed
on Jul 14, 2020
ryanofsky
commented at 2:18 pm on July 14, 2020:
contributor
Rebased 70aaca9ebecb802f7da4babd27a79a0551225475 -> 4041990fcba01a5c6eb22b827b2220f2bd8ee666 (pr/ipc-gui.1 -> pr/ipc-gui.2, compare) on top of #19460 pr/ipc-connect.3
Rebased 4041990fcba01a5c6eb22b827b2220f2bd8ee666 -> 0714770d6bf52fbf46a50aff298df8ffdf4eb3ff (pr/ipc-gui.2 -> pr/ipc-gui.3, compare) on top of #19460 pr/ipc-connect.4
Rebased 878e11049c97564f81018381b88bf314ce4c4f01 -> 5c05c38c1a6185ee18eaf74948cf28fd1560abf4 (pr/ipc-gui.4 -> pr/ipc-gui.5, compare) on top of #19460 pr/ipc-connect.7
Rebased 5c05c38c1a6185ee18eaf74948cf28fd1560abf4 -> 2b9fbc08cc9f27dd092798b75afa1e2318e749c9 (pr/ipc-gui.5 -> pr/ipc-gui.6, compare) on top of #19460 pr/ipc-connect.8
Rebased 2b9fbc08cc9f27dd092798b75afa1e2318e749c9 -> 6f39b4689aea86283b43add4bdebda11dee4d73e (pr/ipc-gui.6 -> pr/ipc-gui.7, compare) on top of #19460 pr/ipc-connect.9
Rebased 6f39b4689aea86283b43add4bdebda11dee4d73e -> e2e2a06bf91cc321eb7e6cab2c19cfc4984dfe1c (pr/ipc-gui.7 -> pr/ipc-gui.8, compare) on top of #19460 pr/ipc-connect.10
Rebased e2e2a06bf91cc321eb7e6cab2c19cfc4984dfe1c -> 137225d7ba437d7d14e122a3eb35512ef8648041 (pr/ipc-gui.8 -> pr/ipc-gui.9, compare) on top of #19460 pr/ipc-connect.14
Rebased 137225d7ba437d7d14e122a3eb35512ef8648041 -> 871891d38cb3a4b73fa2016de70990f221662d90 (pr/ipc-gui.9 -> pr/ipc-gui.10, compare) on top of #19460 pr/ipc-connect.15
Rebased 871891d38cb3a4b73fa2016de70990f221662d90 -> 4b045d994b82bb24d0cab1efd5488575b9e7851d (pr/ipc-gui.10 -> pr/ipc-gui.11, compare) on top of #19460 pr/ipc-connect.16 due to conflict with bitcoin-core/gui#233
Rebased 4b045d994b82bb24d0cab1efd5488575b9e7851d -> 7cb12b4bc81c7e8978830e427f9d36b8590d9848 (pr/ipc-gui.11 -> pr/ipc-gui.12, compare) on top of #19460 pr/ipc-connect.19
Rebased 7cb12b4bc81c7e8978830e427f9d36b8590d9848 -> e7dfe88e2ab772c4eed939dd5bda53c42827cac8 (pr/ipc-gui.12 -> pr/ipc-gui.13, compare) on top of #19460 pr/ipc-connect.21
Rebased e7dfe88e2ab772c4eed939dd5bda53c42827cac8 -> 4dc8af0440073813101b85360482713c7abc6949 (pr/ipc-gui.13 -> pr/ipc-gui.14, compare) on top of #19460 pr/ipc-connect.22
Rebased 4dc8af0440073813101b85360482713c7abc6949 -> c997272eb0a6c805f098c920d8210221f770672f (pr/ipc-gui.14 -> pr/ipc-gui.15, compare) on top of #19460 pr/ipc-connect.24 due to conflict with bitcoin-core/gui#381
Rebased c997272eb0a6c805f098c920d8210221f770672f -> e23cf8171fdd6e1187731dd2393fcd64469ffd80 (pr/ipc-gui.15 -> pr/ipc-gui.16, compare) on top of #19460 pr/ipc-connect.25 due to conflict with bitcoin-core/gui#434
Rebased e23cf8171fdd6e1187731dd2393fcd64469ffd80 -> f2f33658dddd45477e43bcad749833f6fd336bb2 (pr/ipc-gui.16 -> pr/ipc-gui.17, compare) on top of #19460 pr/ipc-connect.26
Rebased f2f33658dddd45477e43bcad749833f6fd336bb2 -> f8d4a15805296edb365602a37875637610fe77c7 (pr/ipc-gui.17 -> pr/ipc-gui.18, compare) on top of #19460 pr/ipc-connect.28
ryanofsky referenced this in commit
f3ace3d21f
on Jul 14, 2020
ryanofsky force-pushed
on Jul 14, 2020
DrahtBot removed the label
Needs rebase
on Jul 14, 2020
ryanofsky referenced this in commit
5adf00eb1e
on Jul 16, 2020
ryanofsky referenced this in commit
551c50873d
on Jul 17, 2020
DrahtBot added the label
Needs rebase
on Jul 23, 2020
ryanofsky referenced this in commit
1ac160cdfe
on Aug 3, 2020
ryanofsky referenced this in commit
134153fc41
on Aug 7, 2020
ryanofsky referenced this in commit
427c8da711
on Aug 17, 2020
ryanofsky referenced this in commit
d4bc442797
on Aug 17, 2020
ryanofsky referenced this in commit
e133631625
on Aug 26, 2020
maflcko referenced this in commit
93ab136a33
on Aug 26, 2020
sidhujag referenced this in commit
797191d45b
on Aug 26, 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
meshcollider
commented at 7:50 pm on September 10, 2020:
contributor
Concept ACK
ajtowns removed the label
Build system
on Sep 17, 2020
ajtowns removed the label
Docs
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
ajtowns removed the label
Wallet
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
janus referenced this in commit
ed6e35e3be
on Dec 7, 2020
ryanofsky force-pushed
on Dec 12, 2020
DrahtBot removed the label
Needs rebase
on Dec 12, 2020
Fabcien referenced this in commit
c2a9184653
on Dec 14, 2020
DrahtBot added the label
Needs rebase
on Dec 16, 2020
ryanofsky force-pushed
on Feb 4, 2021
DrahtBot removed the label
Needs rebase
on Feb 4, 2021
DrahtBot added the label
Needs rebase
on Feb 4, 2021
ryanofsky force-pushed
on Feb 22, 2021
DrahtBot removed the label
Needs rebase
on Feb 22, 2021
DrahtBot added the label
Needs rebase
on Mar 2, 2021
ryanofsky force-pushed
on Mar 10, 2021
DrahtBot removed the label
Needs rebase
on Mar 10, 2021
DrahtBot added the label
Needs rebase
on Mar 11, 2021
ryanofsky force-pushed
on Apr 11, 2021
DrahtBot removed the label
Needs rebase
on Apr 11, 2021
laanwj referenced this in commit
ac219dcbcc
on Apr 27, 2021
DrahtBot added the label
Needs rebase
on Apr 27, 2021
Sjors
commented at 2:22 pm on April 27, 2021:
member
Needs rebase 🎉
What was the magic incantation again to tunnel a unix socket over SSH? I’ll try if I can run the GUI on a different machine than the node.
ryanofsky force-pushed
on Jun 2, 2021
DrahtBot removed the label
Needs rebase
on Jun 2, 2021
DrahtBot added the label
Needs rebase
on Jun 9, 2021
ryanofsky force-pushed
on Jun 18, 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 Aug 18, 2021
DrahtBot removed the label
Needs rebase
on Aug 18, 2021
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 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
ClaraBara22 approved
ClaraBara22
commented at 6:38 pm on March 21, 2022:
none
Thank you
ryanofsky force-pushed
on Sep 27, 2022
DrahtBot removed the label
Needs rebase
on Sep 27, 2022
DrahtBot added the label
Needs rebase
on Dec 6, 2022
ryanofsky force-pushed
on Feb 11, 2023
DrahtBot removed the label
Needs rebase
on Feb 11, 2023
Sjors
commented at 11:38 am on February 11, 2023:
member
What was the magic incantation again to tunnel a unix socket over SSH? I’ll try if I can run the GUI on a different machine than the node.
On an Ubuntu 22.10 machine I ran src/bitcoin-node -ipcbind=unix:///tmp/node.sock. I then opened an SSH connection: ssh -L/tmp/remote-node.sock:/tmp/node.sock ubuntu. And then on the mac I connected the GUI: src/bitcoin-gui -ipconnect=unix:///tmp/remote-node.sock
I connected just after the node caught up on old blocks. It had the most recent block, but the logs show it as 0.999998% finished. The UI displayed a nonsensical status for that:
Once a new block came in a few minutes later, the sync dialog was correctly dismissed. Also, when I started the node again and it had to sync a bunch of blocks, the progress dialog did work correctly. Except that the ETA was always 0 seconds.
Other than that, it seems to work. Cool stuff!
When I exit the GUI and after that stop bitcoind the latter crashes:
0^C2023-02-11T12:02:40Z [rpc] Interrupting HTTP RPC server
12023-02-11T12:02:40Z [rpc] Interrupting RPC
22023-02-11T12:02:40Z tor: Thread interrupt
32023-02-11T12:02:40Z Shutdown: In progress...
42023-02-11T12:02:40Z addcon thread exit
52023-02-11T12:02:40Z [rpc] Stopping HTTP RPC server
62023-02-11T12:02:40Z torcontrol thread exit
72023-02-11T12:02:40Z [rpc] Stopping RPC
82023-02-11T12:02:40Z [rpc] RPC stopped.
9terminate called after throwing an instance of 'ipc::Exception'
10 what(): kj::Exception: kj/async-io-unix.c++:304: disconnected: ::read(fd, buffer, maxBytes): Connection reset by peer
11stack: 7ff9e59af780 5604c599569c 5604c594b36612Aborted (core dumped)
DrahtBot added the label
Needs rebase
on Feb 17, 2023
ryanofsky force-pushed
on Mar 1, 2023
DrahtBot removed the label
Needs rebase
on Mar 1, 2023
DrahtBot added the label
Needs rebase
on Mar 11, 2023
ryanofsky force-pushed
on May 3, 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.
Add `-ipcconnect` option to `bitcoin-gui` to allow connecting gui to an
existing node instead of starting a new node process, so it is possible to
start and stop the gui independently of the node.
This change doesn't add an -ipcbind option to the bitcoin-wallet (which would
allow the gui to control wallet processes without going through the node), but
this would be a logical extension to add in the future.
5f91aa3d8b
DrahtBot removed the label
Needs rebase
on May 3, 2023
DrahtBot added the label
CI failed
on May 4, 2023
ryanofsky force-pushed
on May 4, 2023
DrahtBot removed the label
CI failed
on May 4, 2023
Sjors
commented at 10:33 am on May 5, 2023:
member
When I exit the GUI and after that stop bitcoind the latter crashes.
This still happens on 5f91aa3d8b47cee97e625c3b5689a37dbd941eff. Same setup as above: node and wallet running on Ubuntu 23.04 with a GUI client connecting from macOS using a unix socket of SSH.
But now I’ve notice this happens even if you don’t connect to it, i.e. starting without -ipcbind.
After letting the chain sync (a few days worth of blocks), I shut it down. At that point the log (on Ubuntu):
(update: it’s unrelated to the commits in this PR, I added a comment on #10102 (comment))
Sjors
commented at 11:41 am on May 5, 2023:
member
I’m able to produce another crash that may be related to this PR. When using an external signer that’s connected to the Ubuntu machine, and trying to spend with it from the macOs GUI. After I click Sign on device, and hit “send”:
0Assertion failed: (!complete), function sendButtonClicked, file sendcoinsdialog.cpp, line 525.
1zsh: abort src/bitcoin-gui -ipcconnect=unix:///tmp/remote-node.sock
It also crashes if I create a PSBT (options -> wallet -> enable psbt controls) instead, just a different line
0Assertion failed: (!complete), function sendButtonClicked, file sendcoinsdialog.cpp, line 509.
Both nodes were configured with have --enable-external-signer. I’m able to call enumeratesigners from the console.
These asserts happen after calling fillPSBT(without signing (This prevents an external from being called prematurely). complete can’t be possibly be true, hence the assert.
ryanofsky
commented at 2:44 pm on May 5, 2023:
contributor
Thanks Sjors, I’m trying to reproduce the shutdown error but didn’t succeed yet. Will try again with a mainnet node. I didn’t look into the external signer bug yet, but maybe that is more straightforward. In both cases running with -debug=ipc could make it clearer what’s going on, especially in the shutdown case. Probably what is happening in that case is that some thread is trying make an IPC call after the socket is closed, so an ipc::Exception is raised which is uncaught. Fix might be synchronizing the IPC call & socket close, or just catching the exception and handling it
DrahtBot added the label
CI failed
on May 14, 2023
DrahtBot removed the label
CI failed
on May 21, 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
knst referenced this in commit
66412c24e7
on Jan 16, 2024
knst referenced this in commit
b64c59cc28
on Jan 17, 2024
knst referenced this in commit
1eeef51600
on Jan 19, 2024
knst referenced this in commit
485672a8b7
on Jan 20, 2024
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.
knst referenced this in commit
51cae7a51d
on Jan 24, 2024
knst referenced this in commit
f9710847a6
on Jan 27, 2024
knst referenced this in commit
d59de00282
on Jan 27, 2024
PastaPastaPasta referenced this in commit
e269fa44c5
on Jan 31, 2024
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
