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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• Multiprocess bitcoin #10102 ([experimental] Multiprocess bitcoin by ryanofsky)

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.

@fanquake Thanks for the writeup and questions. I think we should go ahead and merge this and then reopen #16367, so that the questions can be discussed in the pull request that introduces the code changes, not the one that reverts them.

Thanks for writing that out @fanquake.

ACK on the revert for now with the understanding that this was simply merged too early, and with a few too many unanswered questions.

Thanks for writing up these questions.

1. What's the expectation for when this should/would be turned on by default? If this remained merged now, would it be on by 0.22? Or is the thinking an even later release?

I'm not going to push for this to be on by default. I want this to be an optional feature you can ignore if you're happy with current bitcoin usage, or use if you want to try new things like starting and stopping gui/node/wallet processes separately or running from different containers or devices.

2. How will Cores build/compatibility requirements change when we adopt this?

Requirements should not change because this is an optional feature.

If you download a source distribution and want to build ii with --enable-multiprocess, you will currently need a compiler that supports c++17. But this is a recent change that I thought was harmless and could be reversed if it's a problem. #16367 added a travis variant to make sure building in this configuration works and functional tests pass using the multiprocess binaries.

If we add decide to add new multiprocess binaries bitcoin-gui bitcoin-node and bitcoin-wallet to our binary distributions after #10102, these will work well on mac and linux, but I will need to replace fork/exec/pipe usages in libmultiprocess to make them useful or interesting to run on windows.

• libmultiprocess currently requires C++17 (although Russ has mentioned that it'd be possible to drop back to a C++11 if necessary).

Yes, that's all correct

• If the install method is clone and build, users would need autotools and cmake to build Core.

"users would need autotools and cmake to build Core" is only true if you are talking about building the new, optional, disabled-by-default multiprocess bitcoin-gui bitcoin-node and bitcoin-wallet binaries.

• I assume capnproto can be installed and "just works" for our purposes everywhere we'd expect anyone to be building Core?

I'm not assuming this. I'm assuming linux/mac/windows support is good but would not be surprised by build or other issues on Android, CloudABI, other less common IDEs or platforms people build bitcoin for. That's why capnp isn't a dependency unless you enable an optional feature.

• It was pointed out that building capnproto from source currently requires a newer compiler than what we do for building Core.

Only the latest capnp releases require c++14. The previous releases which I haven't used recently but should still work require c++11.

• It looks like there are already some back-compat patches being applied for the version of capnproto that ships with Ubuntu Bionic (18.04). Does libmultiprocess work with the older version of capnproto that ships with Ubuntu Xenial? Is the expectation that some/all users might have to build capnproto from source?

I don't know if it currently works with the xenial capnp package, but I started with 0.5.3 originally, and if it doesn't still work, any bugs should be fixable

1. What does integration actually look like:

• Will libmultiprocess end up being a submodule?
• How will it be versioned / tied to releases of Core?

Weak preference for not doing whatever we are doing with univalue and leveldb, because I get confused by the 30 different places you can submit univalue pull requests, and because I want to reduce barriers preventing good review. The context hash pinning in #16367 is nice because it's more straightforward to review a diff of two content hashes than it is to review diffs of diffs like seems to be required to review other dependency update PRs.

I would like to move the libmultiprocess repo from the chaincode org to bitcoin-core and to start signing tags for versioning.

Apart from that, I might be incorrect here, but it seems we've gotten some changes, such as adding a native_boost package, that I'd assume are now unnecessary given that libmultiprocess has dropped it's Boost usage? That was at least mentioned in 16367, although looking at later comments this is unclear, and it seems the Boost dependency might be coming back anyways?

I thought this was covered during #16367 review discussion, and I was thinking native_boost could be useful again, but it's basically just a handful of variable declarations. Will drop if / when I resubmit #16367

• Does the capnproto release schedule/upstream maintainence matter to us? Looking at the repo, it seems that there are at least a couple people calling for a new release (1, 2), and the current maintainer has mentioned that's he basically too busy at the moment. This might be irrelevant (and it seems that a release prep PR has been merged), but worth noting if this project is going to take on (by extension) a new dependency.

I'm only using basic features, and I believe the release from 5 years ago should still work fine. Also, CapnProto itself shouldn't be a hangup. I like it very much at this point, but if somebody doesn't like it, they should be able to ignore it and not use it. If someone wants to use a different protocol as an alternative or replacement, much of the work I've done here (interface declarations, process spawning code, build support) should be compatible or reusable for that.

ACK on the revert for now with the understanding that this was simply merged too early, and with a few too many unanswered questions.

Thanks for the questions. I'm sure there will be more asked, but I'll try to keep the number of unanswered ones close to zero