search

Sjors / bitcoin

Bitcoin Core integration/staging tree
https://bitcoin.org/en/download
MIT License
6 stars 10 forks source link

Stratum v2 Template Provider via IPC interface (multiprocess) #48

Open Sjors opened 2 months ago

Sjors commented 2 months ago

Based on #49 and https://github.com/bitcoin/bitcoin/pull/30437. This is an alternative approach to https://github.com/bitcoin/bitcoin/pull/29432. Rather than integrating the Template Provider directly into Bitcoin Core, we make it an entirely separate application that communicates via IPC, see design/multiprocess.md.

Usage

Compile as usual, multiprocess is enabled by default.

Alternatively get a recent binary, see (unofficial, by me) releases ending in "ICP".

Start node:

build/src/bitcoin-node -chain=testnet4 -ipcbind=unix

Start the Template Provider:

build/src/bitcoin-mine -chain=testnet4 -debug=sv2 -loglevel=sv2:trace

See general Stratum v2 usage instructions in https://github.com/bitcoin/bitcoin/pull/29432.

Rationale, PRs this builds on

There are multiple ways to create such an application. It could be built from scratch in Rust, using bits and pieces of the Stratum Reference Implementation SRI. It could be a c++ application built from scratch, perhaps using some libraries from Bitcoin Core.

But this PR goes about in a slightly different way. Similar to Elements or Knots it's a set of patches on top of the full Bitcoin Core codebase. This allows me to leverage the build system, subclass Transport and use various other useful bits.

This lets me reuse almost all code from the integrated Template Provider #29432. Both build direct on:

This PR relies on the interfaces changes proposed in:

And the build changes proposed in:

These interface changes are also used by #29432, but there they run in the same process.

I modified the guix script to only produce bitcoin-node (for now) and bitcoin-mine binaries.

Goal

The goal is to drop bitcoin-node from the release here. Ideally users would install Bitcoin Core in the manner they're familiar with. They would then install bitcoin-mine separately and it should just work(tm), at most having to add a line to bitcoin.conf to turn IPC on.

This won't work until we ship a release with multiprocess and the mining interface enabled, ideally in bitcoind and bitcoin-qt rather than in seperate bitcoin-node and bitcoin-gui binaries. See https://github.com/bitcoin/bitcoin/pull/30437 for progress towards that.

It might eventually be feasible to complete separate the codebase. A first step towards that would be to turn libbitcoin_net into a library (see #47). However for the time being I'd rather rebase on Bitcoin Core, which I'm familiar with anyway, than maintain an entire (guix) build system myself.

Sjors commented 2 months ago

@ryanofsky I slightly mangled e3736422b87e9f5b67d058fc278fe0838e073e07 because of https://github.com/bitcoin/bitcoin/pull/30437/commits/4e1a4342f3b2a857e3211587cd14e36704197483#r1681193702. I'll clean that up later.

Sjors commented 2 months ago

Incorporated the changes from https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2233717971 and cleaned up commit history.

Sjors commented 2 months ago

@ryanofsky do you have a commit somewhere that adds libmultiprocess to the guix build?

Sjors commented 2 months ago

Getting a cryptic error: '__NVCC___WORKAROUND_GUARD' is not defined error for the multiprocess job: https://cirrus-ci.com/task/4940413969104896?logs=ci#L2225

Somehow related to the Template Provider depending on boost/multi_index via its use of the mempool.

I can't make sense of the personality(old_personality | ADDR_NO_RANDOMIZE) error thrown by MSAN and TSAN, but it's possible that's related to my self-hosted CI setup and/or the modification in 33196682f15caf6fb54d05ed4c53a0d711a91241.

ryanofsky commented 2 months ago

re: https://github.com/Sjors/bitcoin/pull/48#issuecomment-2235989334

@ryanofsky do you have a commit somewhere that adds libmultiprocess to the guix build?

I don't, since I haven't really experimented with the guix build. Maybe simplest approach to start with would enable the multiprocess build option and build and package all the multiprocess executables including bitcoin-tp and bitcoin-node. I know in the longer run you want to have separate releases as described in https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2226847354.

re: https://github.com/Sjors/bitcoin/pull/48#issuecomment-2236061272

Getting a cryptic error: '__NVCC___WORKAROUND_GUARD' is not defined error for the multiprocess job: https://cirrus-ci.com/task/4940413969104896?logs=ci#L2225

Somehow related to the Template Provider depending on boost/multi_index via its use of the mempool.

This is a -Wundef error caused by https://github.com/bitcoin/bitcoin/pull/29876 that happens when a boost header is included from a source file that is compiled with -I /path/to/boost/headers rather than -isystem /path/to/boost/headers. The fix for this is probably:

-bitcoin_mine_CPPFLAGS = $(bitcoin_bin_cppflags)
+bitcoin_mine_CPPFLAGS = $(bitcoin_bin_cppflags) $(BOOST_CPPFLAGS)

to add the right -isystem option.

I can't make sense of the personality(old_personality | ADDR_NO_RANDOMIZE) error thrown by MSAN and TSAN, but it's possible that's related to my self-hosted CI setup and/or the modification in 3319668.

Not sure about this error. Link seems to be https://cirrus-ci.com/task/4759864851824640?logs=ci#L2486

Sjors commented 2 months ago

I'm getting the MSAN / TSAN error inconsistently on my self-hosted CI. Does not seem related to this PR. Trying to reproduce in isolation in #51.

ryanofsky commented 2 months ago

I have not bothered to enable / test Windows builds for bitcoin-miner. If anyone deploys more than 1 petahash with a Windows machine, let me know and I'll look into it. (specifically I'm assuming unix sockets wouldn't work?)

I haven't tested on windows either, but I think it does actually support unix sockets (https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/), or if that doesn't work, it should not a require a big code change to support windows named pipes.

Sjors commented 1 month ago

Rebased on the latest prerequisite PRs. Not thoroughly tested.

Sjors commented 1 month ago

I dropped some changes that explicitly disable windows. That said it's untested, and Guix can't build the multiprocess for that platform atm: MULTIPROCESS=1 HOSTS='x86_64-w64-mingw32' ./contrib/guix/guix-build will complain it can't #include <sys/resource.h>.

Sjors commented 3 weeks ago

Rebased for testing https://github.com/bitcoin/bitcoin/pull/30510.

The switch to cmake invalidated a bunch of autotools changes in f852619ee767f89f32ede05ca683028ba36c2241, 50b13e50e351236c0349f9552f9d8eb1b6e9e366, d009f681e2300201c679500553fc6dc81cff0f68, 347ecfb6c81dd07487103ddcde9908824b0ad9f5, 2444500e4e02f48eb84e76a326dedff01e9e58cf, 958fe0e6982cbcf8a46585c2eee9c06614c6b09f and 58a8e0e73ea22150c0d2f92a6d465a6bcb14cb92. For now I just dropped them, but I'll have to bring some back later.

It's a bit of a mess now. E.g. building fails with:

In file included from /Users/sjors/dev/bitcoin-sv2-ipc/src/bitcoin-mine.cpp:17:
In file included from /Users/sjors/dev/bitcoin-sv2-ipc/src/node/sv2_template_provider.h:11:
In file included from /Users/sjors/dev/bitcoin-sv2-ipc/src/node/miner.h:12:
/Users/sjors/dev/bitcoin-sv2-ipc/src/txmempool.h:26:10: fatal error: 'boost/multi_index/hashed_index.hpp' file not found
#include <boost/multi_index/hashed_index.hpp>

Which is something I previously fixed for autotools: https://github.com/Sjors/bitcoin/pull/48#issuecomment-2237477471

ryanofsky commented 3 weeks ago

Which is something I previously fixed for autotools: #48 (comment)

You may be able to fix that by adding Boost::headers or bitcoin_node to target_link_libraries(bitcoin-mine ...)

Sjors commented 3 weeks ago

@ryanofsky I ended up moving #include <node/miner.h> to the tests, which is the only place that needs it. Changed in #49.

I also moved TemplateProvider from node to common to avoid having to pull in bitcoin_node. It could probably be its own lib, but I'd like to see something like #47 happen first.

Tested locally by connecting SRI and mining a testnet4 block.

I noticed on macOS 14.6.1 that bitcoin-node -testnet4 -ipcbind=unix listens on unix:/Users/sjors/Library/Application Support/Bitcoin/testnet4/node.sock, as expected. But bitcoin-mine -testnet4 (or -chain=testnet4) won't connect unless I give it that path. It's also ignoring the default sv2 listening port for testnet4. So there's probably something wrong with chainparam network selection

Sjors commented 3 weeks ago

I set -DENABLE_WALLET=OFF for more CI machines.

I also added or WITH_MULTIPROCESS to find_package(Libevent, though not sure if that's needed.

Guix depends (cmake) build currently fails:

MULTIPROCESS=1 HOSTS="x86_64-linux-gnu ..." ./contrib/guix/guix-build

...

make: Entering directory '/bitcoin/depends'
copying packages: native_libmultiprocess native_capnp boost libevent systemtap libmultiprocess capnp
to: /bitcoin/depends/x86_64-linux-gnu
make: Leaving directory '/bitcoin/depends'
-- The CXX compiler identification is GNU 12.4.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /home/guix/.guix-profile/bin/x86_64-linux-gnu-g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at /bitcoin/depends/x86_64-linux-gnu/native/lib/cmake/Libmultiprocess/LibmultiprocessConfig.cmake:46 (include):
  include could not find requested file:

    /bitcoin/depends/x86_64-linux-gnu/native/lib/cmake/Libmultiprocess/LibTargets.cmake
Call Stack (most recent call first):
  CMakeLists.txt:172 (find_package)

This PR stack is a bit behind master, so maybe it's already fixed. cc @ryanofsky does this look familiar?

Update: it does build on master, but when I rebase the whole 61 commit stack onto master @ 0c1e5072781f487b110f6ae6532ac04cf3bb9dd9 I get the same error. Perhaps there's an implicit dependency somewhere on one of the things I removed in guix: only build Template Provider?

Update: no, the error starts happening once I put MULTIPROCESS=1 in contrib/guix/libexec/build.sh.

Update: looks like it's a combination of https://github.com/bitcoin/bitcoin/pull/30510/commits/9378b30774d40c8a093c69d2b1807c9eea32455a#r1757005855 plus something that was fixed in recent master (the error goes away if I add if(BUILD_TESTS) and rebase on master).

ryanofsky commented 3 weeks ago

/bitcoin/depends/x86_64-linux-gnu/native/lib/cmake/Libmultiprocess/LibTargets.cmake

I'll take a look at your branch but this error stands out because this this file is not expected to exist. When cross compiling, libmultiprocess library targets LibTargets should not be built, installed or needed in the native build, only in the cross compiled build. That's why bitcoin/CMakeLists.txt writes:

  find_package(Libmultiprocess COMPONENTS Lib)
  find_package(LibmultiprocessNative COMPONENTS Bin NAMES Libmultiprocess)

And only looks for the "Bin" component not the "Lib" component in the native build. If the native find_package line did include "Lib" you would expect to see the error above.

ryanofsky commented 3 weeks ago

I guess looking at CMakeLists.txt in the sv2-ipc branch I don't see any changes to the find_package calls that could cause this. And I also don't think it's possible that 9378b30774d40c8a093c69d2b1807c9eea32455a could affect that find_package call at all, but maybe changing the BUILD_TESTS options could. I wonder if you were able to find a workaround for the problem, or if not if you are still saying the same native LibTargets error currently.

Sjors commented 3 weeks ago

Since the problem went away once I rebased on master, and applied the BUILD_TESTS fix, I haven't looked for another workaround. I'll just try again after the next rebase round.

Sjors commented 2 weeks ago

Since the problem went away once I rebased on master, and applied the BUILD_TESTS fix, I haven't looked for another workaround. I'll just try again after the next rebase round.

I probably did something wrong in the earlier testing, because the issue is still there after a full rebase.

It's unrelated though: I tracked it down to the introduction of cmake, see #30931.

Sjors commented 3 days ago

Rebased after #30510.

Sjors commented 2 days ago

Still a few things left to clean up, but this now produces a working bitcoin-mine for x86_64-darwin.

I pushed a commit to https://github.com/bitcoin/bitcoin/pull/30975, because some of the CI failures don't seem related to sv2.