bitcoin

bitcoin copied to clipboard

This is a draft PR because it is based on #29409 + #10102. The non-base commits are:

• 34b714020e88 multiprocess: Add bitcoin-gui -ipcconnect option

---

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

---

This PR is part of the process separation project.

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.

Type	Reviewers
Concept ACK	laanwj, meshcollider

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #30510 (multiprocess: Add IPC wrapper for Mining interface by ryanofsky)
• #30509 (multiprocess: Add -ipcbind option to bitcoin-node by ryanofsky)
• #30454 (build: Introduce CMake-based build system by hebasto)
• #30437 (multiprocess: add bitcoin-mine test program by ryanofsky)
• #30409 (Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals by Sjors)
• #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
• #30148 (cli: restrict multiple exclusive argument usage in bitcoin-cli by naiyoma)
• #29641 (scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint by maflcko)
• #29432 (Stratum v2 Template Provider (take 3) by Sjors)
• #27052 (test: rpc: add last block announcement time to getpeerinfo result by LarryRuane)
• #26022 (Add util::ResultPtr class by ryanofsky)
• #25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
• #25665 (refactor: Add util::Result failure values, multiple error and warning messages by ryanofsky)
• #22693 (RPC/Wallet: Add "use_txids" to output of getaddressinfo by luke-jr)

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.

Concept ACK, I've only reviewed the option help yet but -ipcconnect SGTM as option name, it reminds of -rpcconnect of the -cli client.

Concept ACK

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.

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:

^C2023-02-11T12:02:40Z [rpc] Interrupting HTTP RPC server
2023-02-11T12:02:40Z [rpc] Interrupting RPC
2023-02-11T12:02:40Z tor: Thread interrupt
2023-02-11T12:02:40Z Shutdown: In progress...
2023-02-11T12:02:40Z addcon thread exit
2023-02-11T12:02:40Z [rpc] Stopping HTTP RPC server
2023-02-11T12:02:40Z torcontrol thread exit
2023-02-11T12:02:40Z [rpc] Stopping RPC
2023-02-11T12:02:40Z [rpc] RPC stopped.
terminate called after throwing an instance of 'ipc::Exception'
  what():  kj::Exception: kj/async-io-unix.c++:304: disconnected: ::read(fd, buffer, maxBytes): Connection reset by peer
stack: 7ff9e59af780 5604c599569c 5604c594b366
Aborted (core dumped)

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):

2023-05-05T10:30:24Z UpdateTip: new best=000000000000000000010ed2e719320bb8bdea58868cf0bee4897b33b7c20c14 height=788138 version=0x32b96000 log2_work=94.158122 tx=831696670 date='2023-05-03T20:58:19Z' progress=0.999404 cache=515.2MiB(4072240txo)
2023-05-05T10:30:29Z Synchronizing blockheaders, height: 788359 (~100.00%)
^C2023-05-05T10:30:42Z [rpc] Interrupting HTTP RPC server
2023-05-05T10:30:42Z [rpc] Interrupting RPC
2023-05-05T10:30:42Z tor: Thread interrupt
2023-05-05T10:30:42Z torcontrol thread exit
2023-05-05T10:30:42Z addcon thread exit
2023-05-05T10:30:42Z Shutdown: In progress...
2023-05-05T10:30:42Z opencon thread exit
2023-05-05T10:30:42Z [rpc] Stopping HTTP RPC server
2023-05-05T10:30:42Z [rpc] Stopping RPC
2023-05-05T10:30:42Z [rpc] RPC stopped.
terminate called after throwing an instance of 'ipc::Exception'
  what():  kj::Exception: kj/async-io-unix.c++:498: disconnected: ::read(fd, buffer, maxBytes): Connection reset by peer
stack: 7fb82ba25644 7fb82ba34eab 7fb82b9c41b5 7fb82ba1bdb0 7fb82bb95720 7fb82bb8ec20 7fb82bb92df0 7fb82bbf6b30 7fb82bbf1220 7fb82bbe6cc0 7fb82bbe2800 7fb82bbd25d0 7fb82bbd3084 7fb82bbcf610 564eea2664d0 564eea26ad60
Aborted (core dumped)

I then have to restart the node and sync again.

(update: it's unrelated to the commits in this PR, I added a comment on https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-1536075360)

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":

Assertion failed: (!complete), function sendButtonClicked, file sendcoinsdialog.cpp, line 525.
zsh: 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

Assertion 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.

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

🚧 At least one of the CI tasks failed. _{Debug: https://github.com/bitcoin/bitcoin/runs/27980688205}

Hints

Make sure to run all tests locally, according to the documentation.

The failure may 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.