RFC: multiprocess binaries in 29.0

ryanofsky commented at 5:25 pm on January 29, 2025: contributor

I want to reopen topic of including multiprocess binaries in 29.0. I’m fine with any decision on this issue, but I was rereading the previous irc discussion and it struck me as abstract and process-oriented, detached from practical reality, and likely based on misperceptions of the actual change being discussed.

I think it would be helpful if people who had concerns about including bitcoin-node and bitcoin-gui binaries in a release (assuming they are not in bin/ and not on the PATH) could clarify what their concerns are in practical terms. For example,

Is the primary concern that the new -ipcbind option might not work well, or might cause instability?
Or is it that even not specifying -ipcbind at all could cause problems in some way?
Or is it not technical, but about signaling, that if we released an experimental feature, it could cause backlash later if the feature had to be changed or removed?
Or is adding opt-in experimental features just always bad, because there should only be a single standard for when features are ready. Users should not be told a default-disabled feature is “experimental” and be able to choose to enable it and help improve the feature.

I’d like to know more about specific concerns (from @darosior @fanquake @theuni @sipa @stickies-v or anyone else) not to have a debate about them but to know what I / we should be working on and prioritizing to address them. I would appreciate concrete input here and would like to know what, specifically, people are collectively or individually worried about.

For clarity, I do want to describe the actual release change being proposed in (#30975 + eb41f92c0889708045ce0a30c6d0f8d43cecd6d5 + #31679) and the steps Sjors and I have taken to reduce technical risks of this change:

The change introduces two new bitcoin-node and bitcoin-gui binaries that behave the same as existing bitcoind and bitcoin-qt binaries, and even contain the same compiled objects, they are just linked differently.
The new binaries have only two differences in behavior from existing binaries: (1) They introduce a new -ipcbind option disabled by default, that, if specified, will listen on a unix socket and expose a small mining interface. And (2) they they have an echoipc RPC method implementation that will test spawning and communicating with a subprocess. There are no other differences. The new binaries behave exactly the same as the existing ones unless you are actively using new features. #10102 would create bigger differences but #10102 is not targeted for 29.0.
The new mining interface has smallest possible implementation, sharing as much code as possible with RPC’s, and exposing as many features as it is possible to expose with RPC’s.
The new binaries have two new dependencies: capnproto and libmultiprocess. Since there is risk in adding new dependencies, we are creating separate binaries specifically so nobody who does not actually want these dependencies will have them.
To further isolate the new dependencies, no code outside of the src/ipc/ directory and libbitcoin_ipc.a library includes them or links against them.

Because #30975 + eb41f92c0889708045ce0a30c6d0f8d43cecd6d5 adds a feature that is isolated in the ways described above and disabled by default, it seems very low risk to me. It actually seems less risky than the average PR.

This issue is part of the process separation project.

ryanofsky commented at 5:27 pm on January 29, 2025: contributor

Responding to fanquake’s post a few minutes before the meeting which was very useful and constructive, but also one sided and I think did not result in a balanced discussion at the meeting.

Given that merging this PR for 29.x is going to be discussed further, I wanted to leave a few general thoughts here.

The mining interface isn’t finished yet?

Looking at #31098, there are still un-merged changes in the “To complete the interface list” (also unclear if there are more PRs missing from that list). It seems premature to do this if the thing that is meant to be utilising it isn’t even finished.

#31283 introducing waitNext() is the only unmerged change, but it is critical and multiprocess binaries should not be released without it. It implements a feature that’s had a lot of iteration and discussion (there and in a previous closed PR) and finally became ready for review 3 weeks ago.

Development process

Big new features / changes are done early after a branch off, so they have plenty of time to sit, be tested, devs can adjust etc. If this is merged now, it’s close to the last minute, with basically no real testing from inside the project.

This is very vague to me. I don’t have a conception what “sitting” and “adjusting” mean in the context of #30975 because for vast majority of users #30975 has no effect. It seems less risky than the PRs we merge every day which contain code changes in actual binaries used by normal users.

The only people affected by #30975 should be people going out of their way to use the new binaries. So maybe the idea is that these people would be better off without the binaries? But in any case I think it would good to be more clear and specific when discussing this.

Known bugs / test failures:

I don’t think we should be starting to ship / turning something on by default, when there are multiple known intermittent test failures, or other outstanding issues. i.e

Agree, but nothing is turned on by default. [EDIT: nothing is turned on by default for users. but fanquake was probably referring to the default for new CI variants here so his statement is correct.] The intermittent test failures you are referencing were reported one time, 3 months ago in #31151. At that time I debugged the issue as much as I could given the available information and waited to see if the issue would ever happen again. Then there was another report, last Tuesday, which led me to try to reproduce the failure myself, which I was able to do and posted a fix Thursday morning.

The other issues referenced are issues in dead code, or not related to this release. I honestly have not been focusing on them. Other parts of #31098 have seemed more critical.

Some of these are months old, without a fix. That’s just opting the project into random CI failures.

Given what i described above, I don’t think that is an accurate characterization of what happened or what is likely to happen.

Build issues:

Basic usage of building libmultiprocess yourself, on the current Ubuntu LTS release (24.04), for use with Core, doesn’t currently work:

Thanks for reporting this. I made an issue for it https://github.com/chaincodelabs/libmultiprocess/issues/132 which is currently fixed.

Having to work around this was also mentioned here: #10102 (comment).

Thanks, I misread that part of the comment. I thought it it was refering to an older issue https://github.com/chaincodelabs/libmultiprocess/pull/119 which was fixed and did not realize it was referring to a new issue.

We can likely work around this ourselves, but something this basic should be fixed before requiring developers to use this library.

This was easy to fix. I’m sorry I missed the first report.

Along with reporting & upstreaming fixes to what are now new (hard) dependencies for us.

Fair enough but just to be clear this is a bug specifically in a debian patch of capnproto. Capnproto does not actually have a bug, and if you install an unpatched capnproto on a debian system you will not see this problem.

Vendoring libmultiprocess

I think if we are going to do this, we should be vendoring libmultiprocess the same as secp256k1 or leveldb. Otherwise the dev experience is awkward, most will likely be building without, or might be forced to use depends (see above) etc.

The default should (properly) change

WITH_MULTIPROCESS=ON should become the default, and a missing library should be an error (unless WITH_MULTIPROCESS=OFF is passed). Changing the default in depends is not enough, as we just end up with this awkward middle ground, where devs or outside builders are likely no-longer even building/testing what we are actually shipping to users. @theuni said something similar in the IRC meeting, and and upped the ante with “It’d be more consistent with our process to turn it on by default at the beginning of a release cycle, have all devs dogfood it for 6 months, then discuss shipping it once we’re all on the same page.”

Honestly this sounds like a great process to have, and I wish somebody mentioned these ideas when we were talking about this release months ago, because I was able to implement vendoring pretty easily in just a few hours (with a huge amount of help from chatgpt deciphering the depends makefile and figuring out git subtree commands) in https://github.com/chaincodelabs/libmultiprocess/pull/136 and #31741. Even now, building multiprocess binary is enabled in the “dev-mode” preset, and flipping the non-dev-mode default will be easy once a vendoring PR is merged.

I do think the discussion about process is detached from the reality of this PR though. I think if the vendoring was present earlier and cmake option was enabled by default it would have increased dogfooding, but slightly and not meaningfully. Because unless developers actively use the new binaries with the -ipcbind flag or `echoipc`` features they are just testing the build or just testing binaries linked against the libbitcoin_ipc.a library without actually running any of code in that library.

(Longer term) Packaging has not been decided on

#31375 I don’t think delaying figuring that out before we start using/shipping it, is a good idea. I think it’s exactly the kind of thing we should have a rought/good idea about, before making substantial change to what the project is doing.

This is another claim that doesn’t seem accurate to me. I don’t think anything is undecided. #31375 just needs normal review.

A a new (core) dependency with a bus factor of 1

Libmultiprocess was written, and is currently maintained by Russ. It is unclear how many of the other developers in this project have ever (seriously) looked at the code. My understanding is it might be 1 or 2 others. For something that is going to become a core part of the binaries we ship, and external interfaces we provide, it’d be great if that wasn’t the case.

It seems pretty obvious to me that an unreleased feature will have a small bus factor, and the way to fix that is to release the feature and see if it gains adoption. There will be a transition period. During that period if I get hit by bus, this will be sad, but course of action should be fairly straightforward. You can announce, “Sadly, the IPC code in bitcoin core has lost its most active maintainer, and as a result, unless a new volunteer steps up and is able to handle issues that arise, IPC features are planned to be deprecated and disabled by default in the next release, and removed in the release after that.” Then, if IPC features are being used for useful things, someone from within the project or outside will step up. And if someone does not step up, that fact itself would be telling and a sign that IPC features are not presently valuable or worth the costs and the right choice is to remove them.

Is this a new external API?

Is the mining interface exposed on bitcoin-node now considered a (stable) external API? If this ships, what sort of guarantees are we giving to consumers?

I think it could be declared stable now and it would be easy to be backwards compatible. But when this was last discussed at coredev there didn’t seem to be a strong appetite to declare it stable. I think any decision here would be fine.

Who will be harmed by waiting an extra release or two cycle for things to stabilize?

It’d be good to have a better idea of where the demand is coming from, such that we need to ship this now.

I think I would be harmed by getting less feedback on IPC and multiprocess features than I would if there was a release. I think the mining API would also be worse off for not getting more usage and testing. Maybe these aren’t the biggest concerns because the practical risks of #30975 are too great. I would like to see the risks of #30975 clearely articulated though. I tried to steelman them at top of this issue, but it would be better to hear from people who actually think these risks are significant.

Sjors commented at 5:51 pm on January 29, 2025: member

Sorry about changing the title and scope of #30975 from under you right before you opened this. It now only changes the CI to run multiprocess everywhere and avoids enabling it by default (including it in the release, since this follows what’s in depends). But the latter of course is what the main discussion is about.

darosior commented at 7:15 pm on January 29, 2025: member

I’m not sure i’m the most legitimate to give my opinion here, but since you asked me explicitly i will share.

First of all why is the default position that we should ship a new binary on short notice right before feature freeze, and people who are not comfortable with this should argue against it? It seems backward to me. The default should be business as usual and if people want to introduce new binaries for the project to release, they should be arguing for it.

If you start from this default position i think the feedback you received is objectively more than reasonable:

What is the rush?
Shouldn’t we actually finish the project before releasing binaries enabling it?
Shouldn’t we get more people to run, let alone test, this before releasing binaries enabling it?

Sure, deciding as a project what binaries we should release is “process”. But i don’t think this feedback is “detached from reality”.

not to have a debate about them but to know what I / we should be working on and prioritizing to address them.

Unfortunately it seems to me what’s needed isn’t really more work for you, but more reviews of multiprocess from other contributors. More review from key people would allow to merge this work, then enable it and eventually ship it.

concrete input here and [..] what, specifically, people are [..] worried about.

Concretely, i am personally “worried about” shipping binaries that have not received enough review and testing from other contributors. “Worry” may not be the right word, let’s say i don’t think it’s a good idea.

Ultimately what @Sjors wants from Bitcoin Core releasing binaries, which i understand, is to break the chicken-and-egg problem of getting adoption of Stratum v2. The project is asking for users and users won’t adopt it until they have the “Bitcoin Core seal of approval”. And given the money they have on the line, it makes total sense!

But just releasing under-reviewed and under-tested binaries under the Bitcoin Core organization is most likely not going to have the intended effect, and erode trust in the said “Bitcoin Core seal of approval”. What these companies want is quality assurance, there is no getting around proper review of the multiprocess project and proper testing of binaries to be shipped.

ryanofsky commented at 7:44 pm on January 29, 2025: contributor

Just to clarify it sounds like your opinion has nothing to to with ipc or process separation a mining interface. You are just opposed as a matter of principle to releasing any new set of binaries including any experimental feature? This is a new pov to me if I am understanding it correctly so thanks for expressing

darosior commented at 9:03 pm on January 29, 2025: member

Yes, i would say the same about another feature in the same stage of development.

Sjors commented at 8:23 am on January 30, 2025: member

First of all why is the default position that we should ship a new binary on short notice right before feature freeze

#30975 was opened late September and from the start had a clear intention of shipping these binaries in a release (I only reduced the scope yesterday). This goes back to several discussions in the months before that, including at core dev (the minutes aren’t precise, so don’t take them as gospel).

So it wasn’t short notice. However, over the months review was limited to a small group of people. It wasn’t until last week that it was discussed again in a larger setting. At that point we were getting pretty close to the release, so it does make sense to me to wait for the next one. @darosior the chicking-egg problem has two stages:

Getting people to test Sv2
Getting bigger companies to use it

The “seal of approval” argument is important for (2), but we’re not there yet.

In the shorter run I’m more worried about (1). I think the main barrier to increased testing is that Stratum v2 has many moving parts. It makes deployment difficult and also just causes cognitive overload.

The Getting Started guide now includes an instruction to download self-sign a bitcoind binary that I ship. This adds extra confusion.

I would like to see that instruction change to “make sure you have Bitcoin Core v29/30+ and add -ipcconnect=unix to your config”. It can be accompanied by a warning that it’s experimental.

Because Stratum v2 has so many moving parts it makes sense to package it in something like Umbrel or startOS. But this is made difficult by the need to manage two separate Bitcoin Core installations, one of which from a non-official source and unsigned.

Since you can only run one instance of Bitcoin Core at a time, it means that when they’re running my fork to mine, that also has access to any wallet they might use.

A selling point for DATUM is its lower complexity, just one binary that works with Bitcoin Core (or Knots). And indeed they already have Umbrel integration: https://apps.umbrel.com/app/datum

I could imagine SRI re-implementing my Template Provider https://github.com/Sjors/bitcoin/pull/48 in rust and then bundeling it with the Job Declarator Client and Translator roles into a single binary that would be just as easy to deploy on Umbrel. But not until we ship a release that allows this thing to connect via IPC.

ryanofsky commented at 2:25 pm on January 30, 2025: contributor

Thanks again for clarifying your concerns @darosior. Hopefully Sjors post added background and answered the specific questions you asked. I just wanted to respond to two things:

First of all why is the default position that we should ship a new binary on short notice right before feature freeze

I think you can be assured that this is not the case. As a rule controversial PRs do not get merged. I’m just trying to get to the bottom of why #31756 + eb41f92c0889708045ce0a30c6d0f8d43cecd6d5 was controversial because with all the precautions we took listed in the issue description, it seemed to me less risky than the average PR, not more risky.

Sure, deciding as a project what binaries we should release is “process”. But i don’t think this feedback is “detached from reality”.

Talking about process is fine. My problem with the IRC discussion last week is that it was only talking about process without connecting it to specific and real possible outcomes. That’s what I meant by detached from reality.

Of course not everybody perceives the exact same reality. Your reference to a “seal of approval” seems to be describing a reality where features are developed, dogfooded internally, and then either ready for external use and included in a release, or not ready, and released/not released is the only distinction we should make.

In my reality, there is a vast spectrum of software readiness, and we should have more than a simple released/not released binary state. I think there should be a released but experimental state, where we can share features with an opt-in set of users before recommending them to be used more widely. I think this is particularly necessary for a mining API feature where we need external feedback and testing to make it stable and to even know if it is working.

There is also an older tradition in open source I believe in where users and developers can work together to test and improve features, and not all software deployment has to follow corporate model where software is delivered as finished product and users cannot to choose to have a level of participation. (This tradition may be becoming obsolete though, I admit.)

hebasto commented at 3:04 pm on January 30, 2025: member

I think there should be a released but experimental state…

FWIW, libsecp256k1 did release “experimental” modules.

sipa commented at 4:20 pm on January 30, 2025: member

I’m wondering if everyone in this discussion is using the term “experimental” with the same connotation. I feel like in the past we have fairly regularly had “experimental” features, in the sense that we didn’t want to commit to this feature being a stable interface, but not in the sense that it had a lower bar of review (for the purpose of code quality, review, …).

ryanofsky commented at 4:53 pm on January 30, 2025: contributor

I can’t speak for anyone else but I am using “experimental” to mean that the feature is new and has not had as much testing as other features because it is new. Therefore, it is not recommended for widespread use, but is recommended for use by people who want to experiment and help improve it.

I feel like in the past we have fairly regularly had “experimental” features, in the sense that we didn’t want to commit to this feature being a stable interface

Right this is not really what I am talking about, but it could be a minor factor. Even though the -ipcbind feature is exposing an API, it’s a very small API, and it is pretty easy to be backwards compatible.

it had a lower bar of review (for the purpose of code quality, review, …).

If it’s had a lower bar of review, this shouldn’t have happened. The code has been reviewed, and review quality varies. Obviously I am biased but think the code quality is also good.

I think the bigger issues here are the testing and deployment issues fanquake brought up. The code has just not been used by many people and not been tested in different environments. I think #31741 will help a little bit with this, but the amount it will help is very small unless bitcoin core developers are actively turning on the -ipcbind option and installing software that will connect to the socket. I think the best way we will be able to test this feature is by getting it into the hands of people who want to use it and fixing problems that are found. I think releasing separate binaries allows us to do this in a safe way without affecting other users.

stickies-v commented at 9:58 pm on January 31, 2025: contributor

Thank you for your effort in approaching this issue in a structural way, @ryanofsky . I’m not advocating for or against shipping multiprocess in 29.0 - I could be okay with either, I haven’t reviewed the code enough to have an opinion on that yet. My concerns quite closely align with darosior’s write-up. It appears more review and dog-fooding could be helpful, and I just haven’t seen an urgent reason to ship this with 29.0, so I’m erring on the side of caution.

Directionally, I think I’m very aligned with your view on how we’re using and shipping multiprocess, and I’m excited about the work that has been done in the past months, so I definitely don’t mean to halt the project’s progress. I think we need to have sufficient internal agreement on the interface (and review wrt quality), but once we have that I’m definitely okay with shipping experimental binaries the interface of which may change or be retracted entirely. I absolutely support a collaborative process between users and developers.

but the amount it will help is very small unless bitcoin core developers are actively turning on the -ipcbind option and installing software that will connect to the socket.

Maybe, and we shouldn’t stall this to infinity, but I think we should at least give that a try? There seem to be a lot of ideas floating around about building things with ipc, and I think encouraging those efforts and gathering feedback from it would be a worthwhile effort? It’s important not to lose momentum, but I’m not convinced that this momentum will be materially affected by whether or not we ship binaries?

RFC: multiprocess binaries in 29.0 #31756

The mining interface isn’t finished yet?

Development process

Known bugs / test failures:

Build issues:

Vendoring libmultiprocess

The default should (properly) change

(Longer term) Packaging has not been decided on

A a new (core) dependency with a bus factor of 1

Is this a new external API?

Who will be harmed by waiting an extra release or two cycle for things to stabilize?