Add bitcoin wrapper executable (interfaces)

https://github.com/bitcoin/bitcoin/pull/31375

Host: ryanofsky  -  PR author: ryanofsky

The PR branch HEAD was 81c0b9edfe533afbb2f4dda56142afdedffdb347 at the time of this review club meeting.

Notes

Motivation & context

  • For years Bitcoin Core has shipped five separate user‑facing binaries. The upcoming multiprocess work would add at least two more (bitcoin‑node, bitcoin‑gui). Reviewers feared an explosion of filenames and user confusion.
  • The PR introduces a single command‑line front‑end called bitcoin that does no consensus or wallet work itself – it simply chooses and exec()’s the appropriate helper binary:

    bitcoin sub‑command Traditional binary Multiprocess binary (-m)
    bitcoin gui … bitcoin‑qt bitcoin‑gui
    bitcoin daemon … bitcoind bitcoin‑node
    bitcoin rpc … bitcoin‑cli -named … bitcoin‑cli -named …
    bitcoin wallet … bitcoin‑wallet bitcoin-wallet
    bitcoin tx … bitcoin‑tx bitcoin-tx

    The bitcoin wrapper therefore accomplishes the “side‑binaries + unified entry point” idea discussed in issue #30983.

New util helpers

  • util::ExecVp() – thin, cross‑platform execvp replacement.
    • POSIX: directly forwards to execvp.
    • Windows: builds a quoted & escaped command line that CommandLineToArgvW in the child process will parse identically to POSIX argv rules.
    • Escaping rules follow the MSVCRT specification: backslashes are doubled only when they precede a quote, and every internal quote is back‑slash‑escaped.
  • util::GetExePath() – attempts to resolve argv[0] into the executable file path.
    • On Unix: uses either the literal argv[0] (if it contains a slash) or searches each element of $PATH until a regular file is found.
    • On Windows: uses GetModuleFileNameW(nullptr, …).

Wrapper lookup logic (ExecCommand)

  1. Determine the directory of the wrapper itself (resolves symlinks).
  2. Try possible candidate paths for the target binary, in descending priority:
    • libexec dir – ${prefix}/libexec/<target> if wrapper is in ${prefix}/bin/
    • Windows installer “daemon” sub‑dir ${wrapper_dir}/daemon/<target>
    • Sibling${wrapper_dir}/<target>
    • Finally, rely on the system PATH only if the wrapper itself was invoked via PATH search (mitigates accidentally running an old system bitcoind while testing a local build).
  3. Call util::ExecVp() with each candidate, moving onto the next candidate if it returns ENOENT (“No such file or directory”) and raising an exception if a different error is returned or if there is no next candidate.

Build‑system & test changes

  • CMake option BUILD_BITCOIN_BIN (ON by default) builds/installs the wrapper.
  • Functional test framework understands BITCOIN_CMD="bitcoin -m" so the entire suite can be driven through the new CLI.
  • CI jobs for the multiprocess build now export that variable.
  • Static‑analysis suppression: the wrapper intentionally contains no FORTIFY functions; security-check.py is taught to ignore it.

Documentation updates

Numerous docs now mention that bitcoin rpc, bitcoin daemon, etc. are synonyms for the traditional commands, improving discoverability for new users while remaining fully backwards‑compatible.


Questions

  1. Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK? What was your review approach?

  2. Review approach – did you test the wrapper? Did you try both monolithic (bitcoin daemon) and multiprocess (bitcoin -m daemon) modes? (requires -DENABLE_IPC=ON cmake option). Attempt to run one of the strace or dtrace tracing commands suggested in bitcoin.cpp? Any cross‑platform checks?

  3. From issue #30983, four packaging strategies were listed. Which specific drawbacks of the “side‑binaries” approach does this PR address?

  4. In util::ExecVp() (Windows branch) why is a second std::vector escaped_args needed instead of modifying argv in‑place?

  5. Walk through the escaping algorithm in util::ExecVp for the argument C:\Program Files\Bitcoin\bitcoin-qt. What exact string is passed to _execvp()?

  6. GetExePath() does not use readlink("/proc/self/exe") on Linux even though it would be more direct. What advantages does the current implementation have? What corner cases might it miss?

  7. In ExecCommand, explain the purpose of the fallback_os_search Boolean. Under what circumstances is it better to avoid letting the OS search for the binary on the PATH?

  8. The wrapper searches ${prefix}/libexec only when it detects that it is running from an installed bin/ directory. Why not always search libexec?

  9. The functional test layer now conditionally prepends bitcoin -m to every command. How does this interact with backwards‑compatibility testing where older releases are run in the same test suite?

  10. The PR adds an exemption in security-check.py because the wrapper contains no fortified glibc calls. Why does it not contain them, and would adding a trivial printf to bitcoin.cpp break reproducible builds under the current rules?

  11. Discuss an alternative design: linking a static table of sub‑commands to absolute paths at build time instead of computing them at run time. What trade‑offs (deployment, relocatability, reproducibility) influenced the chosen design?

  12. Suppose a user installs only bitcoin (wrapper) and forgets to install bitcoin-cli. Describe the failure mode when they run bitcoin rpc getblockcount. Would it be better for the wrapper to pre‑check the availability of the target binary?

  13. (Forward‑looking) Once bitcoin-gui actually spawns bitcoin-node automatically (after #10102 lands), what additional command‑line options or UX changes might the wrapper need?

  14. Typing bitcoin --version prints wrapper metadata, not bitcoind’s or bitcoin‑qt’s. Is that the right UX? Propose a mechanism for the wrapper to forward --version and --help to the underlying sub‑command when one is specified (e.g. bitcoin --version daemon).

  15. The wrapper is agnostic to options such as -ipcbind passed down to bitcoin‑node. Should the wrapper eventually enforce a policy (e.g. refuse to forward -ipcconnect unless -m is given)? What might go wrong if a user mixes monolithic binaries with IPC flags?

  16. BITCOIN_CMD="bitcoin -m" is parsed with shlex; spaces inside quotes are preserved. Should the framework use an explicit list instead of shell parsing?

  17. Would it ever make sense to ship only the wrapper in bin/ and relocate all other executables to libexec/ to tidy PATH?

Meeting Log

  117:00 <ryanofsky> #startmeeting
  217:00 <corebot> ryanofsky: Meeting started at 2025-05-07T17:00+0000
  317:00 <corebot> ryanofsky: Current chairs: ryanofsky
  417:00 <corebot> ryanofsky: Useful commands: #action #info #idea #link #topic #motion #vote #close #endmeeting
  517:00 <corebot> ryanofsky: See also: https://hcoop-meetbot.readthedocs.io/en/stable/
  617:00 <corebot> ryanofsky: Participants should now identify themselves with '#here' or with an alias like '#here FirstLast'
  717:00 <abubakarsadiq> hi
  817:00 <ryanofsky> hi
  917:00 <hodlinator> hi
 1017:00 <stickies-v> hi
 1117:00 <kevkevin> hi
 1217:00 <emzy> hi
 1317:00 <ryanofsky> Welcome to the bitcoin review consortium! Today up for discussion is https://bitcoincore.reviews/31375 #31375
 1417:00 <corebot> https://github.com/bitcoin/bitcoin/issues/31375 | multiprocess: Add bitcoin wrapper executable by ryanofsky · Pull Request #31375 · bitcoin/bitcoin · GitHub
 1517:01 <ryanofsky> I'll squash the first two questions and ask if anybody looked at the code or tested the new command? What was your approach? Any feedback or questions you have before we begin?
 1617:01 <pseudoramdom> hi
 1717:01 <monlovesmango> hey
 1817:02 <hodlinator> feels like it's a good change even without multiprocess
 1917:02 <abubakarsadiq> I read the notes, reviewed the notes, tested the functionality it run smoothly, and performed a light code review recently.
 2017:02 <monlovesmango> concept ack, looked through some of the code and did very light testing
 2117:02 <hodlinator> just in general to increase discoverability
 2217:02 <stickies-v> looked at most of the code and spun up a signet node in both mono and multi mode, all felt very ergonomic!
 2317:03 <stickies-v> we must add tons of parsing logic to allow for `bitcoin -mh` though, forcing me to type `bitcoin -m -h` is unacceptable
 2417:03 <kevkevin> very briefly looked through the PR
 2517:03 <brunoer__> hi
 2617:03 <abubakarsadiq> Q: how will this wrapper approach fixes the ambiguity of the wallet command after https://github.com/bitcoin/bitcoin/pull/10102 what will happen to current bitcoin-wallet executable
 2717:04 <ryanofsky> stickies-v, that seems like a good thing to note for a future improvement
 2817:04 <pseudoramdom> liking the new subcommand ergonomics. Is this only when multiprocess is enabled?
 2917:05 <emzy> I found out the "bitcoin util" also works. But is not listet at "bitcoin --help"
 3017:05 <ryanofsky> abubakarsadiq, current code in #10102 just adds new IPC functionality to current executable. Different directions could be taken though. I don't think it shoudl matter too much to users if they are using the wrapper
 3117:05 <corebot> https://github.com/bitcoin/bitcoin/issues/10102 | Multiprocess bitcoin by ryanofsky · Pull Request #10102 · bitcoin/bitcoin · GitHub
 3217:05 <hodlinator> emzy: needs "bitcoin -h -a" to show I think.
 3317:06 <ryanofsky> pseudoramdom, in current PR new binary is not tied to IPC / multiprocess options, it's available regardless
 3417:06 <monlovesmango> oh when I built it with -DENABLE_IPC=ON the counter was going to 101%
 3517:06 <hodlinator> ah, no it doesn't show util even then.
 3617:06 <emzy> hodlinator: that shows more but also no util
 3717:06 <ryanofsky> emzy, good find, I didn't know that. intention was to punt on bitcoin-util and not support it for now
 3817:07 <ryanofsky> i think it might be better to support `bitcoin grind` directly instead of requiring `bitcoin util grind` but that requires argsmanager changes
 3917:08 <ryanofsky> monlovesmango, i've definitely seen that cmake 101% progress thing, not sure what causes it
 4017:08 <ryanofsky> next question from the list is: 3. From issue #30983, four packaging strategies were listed. Which specific drawbacks of the “side‑binaries” approach does this PR address?
 4117:08 <corebot> https://github.com/bitcoin/bitcoin/issues/30983 | RFC: Multiprocess binaries and packaging options · Issue #30983 · bitcoin/bitcoin · GitHub
 4217:10 <stickies-v> there's only one con listed for the side-binaries approach so i'm gonna go with #1 of 1: confusing
 4317:10 <corebot> https://github.com/bitcoin/bitcoin/issues/1 | JSON-RPC support for mobile devices ("ultra-lightweight" clients) · Issue #1 · bitcoin/bitcoin · GitHub
 4417:10 <monlovesmango> remove the need for adding more binaries for the user to choose from?
 4517:10 <abubakarsadiq> I think having multiple binaries in a single release. Instead of requiring users to call individual binaries, the binaries will be placed in the `libexec` directory and wrapped under the bitcoin command. users dont have to deal with the new binaries they should just know the commands
 4617:11 <stickies-v> it's easier for the user to find what they need when they can go through a single binary that helps them find others
 4717:11 <ryanofsky> yeah exactly, there's only one thing listed there and the main goal is to avoid confusion if multiprocess support is added
 4817:11 <ryanofsky> Another thing it provides is forward compatability. Wrapper lets us rename binaries, consolidate them, replace them without changing external interface.
 4917:12 <ryanofsky> 4. In util::ExecVp() (Windows branch) why is a second std::vector escaped_args needed instead of modifying argv in‑place?
 5017:12 <stickies-v> and maybe in the future we can add some fuzzy search too for when you're not quite sure exactly what it was clled?
 5117:13 <abubakarsadiq> @ryanofsky is it okay to update an array while iterating through it?
 5217:14 <ryanofsky> It can be ok to update an array but in this case the array members are const `char *const argv[]`
 5317:14 <ryanofsky> relevant code is https://github.com/bitcoin/bitcoin/commit/9ac787c7a85c3d2ff407bf149b982fc347537b12
 5417:15 <ryanofsky> 5. Walk through the escaping algorithm in util::ExecVp for the argument C:\Program Files\Bitcoin\bitcoin-qt. What exact string is passed to _execvp()?
 5517:17 <abubakarsadiq> 1. The algorithm first checks if the argument contains spaces, tabs, or quotes.
 5617:17 <abubakarsadiq> 2. Quotes are added at the beginning and end of the argument.
 5717:17 <abubakarsadiq> 3. Backslashes (\) followed by quotes are doubled to ensure proper parsing.
 5817:17 <abubakarsadiq> 4. Any standalone quotes are escaped with a backslash.The resulting string passed to _execvp() is:"C:\\Program Files\\Bitcoin\\bitcoin-qt"
 5917:17 <ryanofsky> stickies-v, added your CLI improvement suggestions to the list in the description
 6017:18 <ryanofsky> Yes that's close but backslashes only need to be escaped if followed by quotes. So in the example the only change made is to add quotes around the argument. Backslashes don't need to be escaped there
 6117:18 <monlovesmango> is there any possibility single quotes are used? or is there validation upstream?
 6217:19 <abubakarsadiq> I see thanks
 6317:19 <ryanofsky> monlovesmango, in general this depends on your shell. Single quotes are allowed there
 6417:20 <ryanofsky> The only escaping this PR is doing is on windows where you have escape the argv[] array in a quirky way that the microsoft C runtime expects
 6517:21 <ryanofsky> otherwise the argv passed to execvp will not match the argv received by the program which is executed
 6617:21 <monlovesmango> I guess my question is do single quotes need to be handled?
 6717:21 <ryanofsky> monlovesmango, nope just because the windows internal argv parsing doesn't care about them
 6817:21 <monlovesmango> ryanofsky: makes sense
 6917:22 <monlovesmango> thanks
 7017:22 <ryanofsky> No problem!
 7117:22 <ryanofsky> 6. GetExePath() does not use readlink("/proc/self/exe") on Linux even though it would be more direct. What advantages does the current implementation have? What corner cases might it miss?
 7217:24 <hodlinator> We may be running on another non-windows system that doesn't have proc-fs?
 7317:25 <ryanofsky> Yeah I think that's the only possible advantage, otherwise /proc/self/exe would probably be more reliable and direct
 7417:26 <ryanofsky> I'm not sure what situation is on macos or bsds
 7517:26 <ryanofsky> In ExecCommand, explain the purpose of the fallback_os_search Boolean. Under what circumstances is it better to avoid letting the OS search for the binary on the PATH?
 7617:26 <ryanofsky> ^^^ That was question 7
 7717:27 <ryanofsky> Link to relevant commit: https://github.com/bitcoin/bitcoin/commit/f2c003c927557f97dafa263e6cbb90a4e3421842
 7817:28 <hodlinator> It might be that one hasn't built all target executables in one's local build?
 7917:29 <ryanofsky> hodlinator, yeah this behavior was just added to avoid confusing developers
 8017:29 <abubakarsadiq> When the wrapper executable is invoked using a specific path to prevent unintentionally use of a binary in PATH instead of the intended local binary.
 8117:29 <monlovesmango> if executiable is invoked from path we don't want to fall back to operating system
 8217:29 <stickies-v> i think generally the wrapper and the individual binaries are all going to be shipped and compiled together, so searching locally is more robust?
 8317:30 <ryanofsky> stickies-v, that seems like a good point. maybe it would make sense to avoid searching PATH altogether
 8417:31 <ryanofsky> I think when I first implemented it, I didn't want to rely on GetExePath working perfectly, and wanted to take advantage of OS native ability find binaries
 8517:32 <ryanofsky> but then Sjors pointed out searching PATH could be confusing for developers if they didnt' build everythign, so narrowed the use of PATH. but could make sense to drop it altogether
 8617:32 <stickies-v> not the same thing, but i stopped searching the system path with py-bitcoinkernel because it was leading to too much confusion and errors, so i allowed providing an explicit path env var instead which is hard to abuse
 8717:34 <emzy> I think using "bitcoin rpc --version" is helpful to figure out what you actualy running. Would help to also show the path of the binary.
 8817:34 <ryanofsky> Yeah I think main reason for using system path is I'd like wrapper to "just work" and avoid being unreliable
 8917:35 <hodlinator> Agree it may be a bit too magical.
 9017:36 <ryanofsky> "bitcoin rpc --version doesn't seem to show paths when I try, but could make sense to add a verbose / debug option maybe to show paths?
 9117:37 <ryanofsky> 8. The wrapper searches ${prefix}/libexec only when it detects that it is running from an installed bin/ directory. Why not always search libexec?
 9217:37 <ryanofsky> This is the same commit https://github.com/bitcoin/bitcoin/commit/f2c003c927557f97dafa263e6cbb90a4e3421842 line 189
 9317:40 <ryanofsky> My answer to this was that wrapper should be conservative about what paths it tries to execute, and encourage standard PREFIX/{bin,libexec} layouts, not encourage packagers to create nonstandard layouts or work when binaries arranged in unexpected ways.
 9417:41 <ryanofsky> 9. The functional test layer now conditionally prepends bitcoin -m to every command. How does this interact with backwards‑compatibility testing where older releases are run in the same test suite?
 9517:41 <stickies-v> oh, is this because bitcoin core can also be shipped through package managers? because with our own build system we know it'll always be stored in the bin,libexec dirs?
 9617:42 <hodlinator> Re 9: https://github.com/bitcoin/bitcoin/pull/31375/commits/ccacae70ed050f37f5d00362152ba31036818691
 9717:44 <ryanofsky> stickies-v, yeah the code isn't just trying to be conservative abotu what it executes. It will execute binaries in paths explicitly listed on PATH and in places that seem to match expected layout, but avoid executing things in novel situations
 9817:46 <stickies-v> that sounds like a good approach to me, and not something i realized we had to think of
 9917:46 <ryanofsky> Re 9: The Binaries._argv method in commit hodlinator linked to https://github.com/bitcoin/bitcoin/commit/ccacae70ed050f37f5d00362152ba31036818691 will ignore the wrapper executable entirely when testing previous releases because bin_dir will be set in that case
10017:46 <ryanofsky> In future if bitcoin wrapper executable becomes part of previous releases and we want to test it, this code will need to be updated.
10117:47 <stickies-v> so on the one hand we need to be conservative in what we execute, but on the other hand there may be package managers that (for some reason, idk) are unable to store binaries in the {bin,libexec} dirs?
10217:48 <ryanofsky> stickies-v, I don't think we need to be conservative, I just thought it would be a good starting point. I don't think package manager will need to choose divergent layouts probably, but we can find out and adapt
10317:49 <stickies-v> 👍
10417:49 <hodlinator> So a test must explicitly call add_nodes() and pass in old versions, which will result in the old binaries being resolved.. and this resolution-logic will need to be updated to support the wrapper in the future.
10517:49 <ryanofsky> Note: we won't have time to get to all questions listed so if any in particular someone wants to talk about (or some specific topic) feel free to suggest
10617:50 <emzy> I'm in general concerned about searching for binaries to execute. There are many possible security problems. Better have it hardcoded to one path as libexec.
10717:51 <ryanofsky> hodlinator, yes that sounds right. Test framework code right now just assumes all previous releases don't have a wrapper binary to call. So if we want to write new tests calling wrapper binaries in old releases something like that needs to change
10817:51 <ryanofsky> emzy, I think that is what current PR does. It hardcodes to libexec and uses the operating systems normal mechanism to search the PATH
10917:52 <hodlinator> Being flexible with PATH might enable attackers to put malicious binaries there to be resolved?
11017:52 <emzy> The user/attacker can change the PATH.
11117:53 <stickies-v> I was wondering why on windows we search the "daemon" dir - that's unrelated to our "bitcoin daemon" bin, right? Is this just a windows convention?
11217:53 <emzy> At first glance that's not a problem. But whow knows.
11317:54 <abubakarsadiq> emzy: if the attacker can change the PATH; he can do worse than just putting malicious binaries no?
11417:54 <ryanofsky> stickies-v, yeah not sure why that directory exists on windows, it just seems to be a quick of the installer
11517:55 <ryanofsky> abubakarsadiq, yeah I think that's the general think that makes me not worried about executing binaries on the PATH. but maybe we could be conservative and never do that, it's reasonable thing to consider
11617:56 <hodlinator> yeah, better to add it later if needed.
11717:56 <ryanofsky> 10. The PR adds an exemption in security-check.py because the wrapper contains no fortified glibc calls. Why does it not contain them, and would adding a trivial printf to bitcoin.cpp break reproducible builds under the current rules?
11817:57 <emzy> abubakarsadiq: That's right. It needs some combination. Like some software allows to change (only) PATH. Looks like no problem at first.
11917:58 <ryanofsky> I think that'll be the last question
12017:58 <abubakarsadiq> yep unanswered yet :)
12117:59 <ryanofsky> I think the answer adding is adding new calls should not break the build. If we add new calls they should be fortified based on build options.
12217:59 <emzy> I worked in IT security. So I'm always (too) concerned. ;)
12318:00 <abubakarsadiq> emzy: most of the times it's a good thing to.
12418:00 <ryanofsky> Adding new calls should just allow the security-check.py exception to be removed. The exception is needed because it is checking for fortified symbols but the wrapper binary is so simple it doesn't contain any
12518:01 <ryanofsky> Yeah it's good to be looking at security very closely in this realm
12618:01 <ryanofsky> Thanks everybody for participating!
12718:01 <ryanofsky> #endmeeting