doc: Add multiprocess design doc #28978

pull ryanofsky wants to merge 1 commits into bitcoin:master from ryanofsky:pr/ipcdoc changing 2 files +285 −59
  1. ryanofsky commented at 9:26 PM on November 30, 2023: contributor

    Add multiprocess design doc and existing multiprocess documentation into design and usage sections.

    Links to rendered markdown:

    https://github.com/ryanofsky/bitcoin/blob/pr/ipcdoc/doc/design/multiprocess.md https://github.com/ryanofsky/bitcoin/blob/pr/ipcdoc/doc/multiprocess.md


    This PR is part of the process separation project.

  2. DrahtBot commented at 9:27 PM on November 30, 2023: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK TheCharlatan, fjahr, stickies-v, achow101

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

  3. DrahtBot added the label Docs on Nov 30, 2023
  4. in doc/multiprocess.md:33 in 66e19e7659 outdated
      28 | +Alternately, you can install [Cap'n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) packages on your system, and just run `./configure --enable-multiprocess` without using the depends system. The configure script will be able to locate the installed packages via [pkg-config](https://www.freedesktop.org/wiki/Software/pkg-config/). See [Installation](https://github.com/chaincodelabs/libmultiprocess/blob/master/doc/install.md) section of the libmultiprocess readme for install steps. See [build-unix.md](build-unix.md) and [build-osx.md](build-osx.md) for information about installing dependencies in general.
      29 | +
      30 | +## Usage
      31 | +
      32 | +`bitcoin-node` is a drop-in replacement for `bitcoind`, and `bitcoin-gui` is a drop-in replacement for `bitcoin-qt`, and there are no differences in use or external behavior between the new and old executables. But internally after [#10102](https://github.com/bitcoin/bitcoin/pull/10102), `bitcoin-gui` will spawn a `bitcoin-node` process to run P2P and RPC code, communicating with it across a socket pair, and `bitcoin-node` will spawn `bitcoin-wallet` to run wallet code, also communicating over a socket pair. This will let node, wallet, and GUI code run in separate address spaces for better isolation, and allow future improvements like being able to start and stop components independently on different machines and environments.
      33 | +[#19160](https://github.com/bitcoin/bitcoin/pull/19160) adds a new `bitcoin-node` `-ipcbind` option and an `bitcoind-wallet` `-ipcconnect` option to allow new wallet processes to connect to an existing node process.
    


    maflcko commented at 8:17 AM on December 1, 2023:
    [#19160](https://github.com/bitcoin/bitcoin/pull/19160) added a new `bitcoin-node` `-ipcbind` option and an `bitcoind-wallet` `-ipcconnect` option to allow new wallet processes to connect to an existing node process.
    

    The pull is merged?


    ryanofsky commented at 12:24 AM on December 2, 2023:

    re: #28978 (review)

    The pull is merged?

    Good catch, this was supposed to point to #19460

  5. in doc/multiprocess.md:34 in 66e19e7659 outdated
      29 | +
      30 | +## Usage
      31 | +
      32 | +`bitcoin-node` is a drop-in replacement for `bitcoind`, and `bitcoin-gui` is a drop-in replacement for `bitcoin-qt`, and there are no differences in use or external behavior between the new and old executables. But internally after [#10102](https://github.com/bitcoin/bitcoin/pull/10102), `bitcoin-gui` will spawn a `bitcoin-node` process to run P2P and RPC code, communicating with it across a socket pair, and `bitcoin-node` will spawn `bitcoin-wallet` to run wallet code, also communicating over a socket pair. This will let node, wallet, and GUI code run in separate address spaces for better isolation, and allow future improvements like being able to start and stop components independently on different machines and environments.
      33 | +[#19160](https://github.com/bitcoin/bitcoin/pull/19160) adds a new `bitcoin-node` `-ipcbind` option and an `bitcoind-wallet` `-ipcconnect` option to allow new wallet processes to connect to an existing node process.
      34 | +[#19161](https://github.com/bitcoin/bitcoin/pull/19161) adds a new `bitcoin-gui` `-ipcconnect` option to allow new GUI processes to connect to an existing node process.
    


    maflcko commented at 8:17 AM on December 1, 2023:
    [#19161](https://github.com/bitcoin/bitcoin/pull/???) adds a new `bitcoin-gui` `-ipcconnect` option to allow new GUI processes to connect to an existing node process.
    

    wrong number?


    ryanofsky commented at 12:24 AM on December 2, 2023:

    re: #28978 (review)

    wrong number?

    Thanks, this one was supposed to point to #19461

  6. maflcko approved
  7. ryanofsky force-pushed on Dec 2, 2023
  8. ryanofsky commented at 1:08 AM on December 2, 2023: contributor

    Updated 66e19e7659dc19dac6b0cc2318efd00a4d2f5240 -> f566245147003648099f961306be82ea32ea47ae (pr/ipcdoc.6 -> pr/ipcdoc.7, compare) fixing broken links and making small edits.

  9. in doc/design/multiprocess.md:214 in f566245147 outdated
     217 | +
     218 | +- Separating indexes from `bitcoin-node`, and running indexing code in separate processes.
     219 | +- Enabling wallet processes to listen for JSON-RPC requests on their own ports instead of needing the node process to listen and forward requests to them.
     220 | +- Automatically generating `.capnp` files from C++ interface definitions (see [Interface Definition Maintenance](#interface-definition-maintenance))
     221 | +- Simplifying and stabilizing interfaces (see [Interface Stability](#interface-stability))
     222 | +- Adding sandbox features, restricting subprocess access to resources and data (see [https://eklitzke.org/multiprocess-bitcoin](https://eklitzke.org/multiprocess-bitcoin)).
    


    ariard commented at 2:53 AM on December 4, 2023:
    • Generating server / client code in different langages (e.g C++ -> Rust or Rust -> C++) from a common set of capnp files and with compatible interfaces.

    Tested capnp rust code generation a while back works well, though never been as a far as being able to bridge process written in different langages.


    ryanofsky commented at 10:41 PM on December 4, 2023:

    re: #28978 (review)

    • Generating server / client code in different langages (e.g C++ -> Rust or Rust -> C++) from a common set of capnp files and with compatible interfaces.

    Yes, this significant and worth mentioning. Added some information about this and a link the work you mentioned.

  10. ryanofsky force-pushed on Dec 4, 2023
  11. ryanofsky commented at 10:45 PM on December 4, 2023: contributor

    Updated f566245147003648099f961306be82ea32ea47ae -> 837c53d14f24924cdcb2cfd8b18915882dc3b620 (pr/ipcdoc.7 -> pr/ipcdoc.8, compare) adding suggested future enhancement and making a few other tweaks

  12. in doc/design/multiprocess.md:96 in 837c53d14f outdated
     163 | +## Design Considerations
     164 | +
     165 | +### Selection of Cap’n Proto
     166 | +The choice to use [Cap’n Proto](https://capnproto.org/) for IPC was primarily influenced by its support for passing object references and managing object lifetimes, which would have to be implemented manually with a framework that only supported plain requests and responses like [gRPC](https://grpc.io/). The support is especially helpful for passing callback objects like `std::function` and supporting bidirectional communication.
     167 | +
     168 | +The choice to use an RPC framework at all instead of a custom protocol was neccesitated by the size of Bitcoin Core internal interfaces which consist of around 150 methods that pass complex data structures and are called in complicated ways (in parallel, from callbacks that can be nested and stored). Writing a custom protocol to wrap these complicated interfaces would be too much work, akin to writing a new RPC framework.
    


    stickies-v commented at 4:17 PM on December 12, 2023:

    typo nit

    The choice to use an RPC framework at all instead of a custom protocol was necessitated by the size of Bitcoin Core internal interfaces which consist of around 150 methods that pass complex data structures and are called in complicated ways (in parallel, from callbacks that can be nested and stored). Writing a custom protocol to wrap these complicated interfaces would be too much work, akin to writing a new RPC framework.
    

    ryanofsky commented at 12:48 PM on December 19, 2023:

    re: #28978 (review)

    typo nit

    Thanks, fixed

  13. in doc/design/multiprocess.md:128 in 837c53d14f outdated
     195 | +
     196 | +1. **Initiation in bitcoin-wallet**
     197 | +   - The wallet process calls the `getBlockHash` method on a `Chain` object. This method is defined as a virtual function in [`src/interfaces/chain.h`](../../src/interfaces/chain.h).
     198 | +
     199 | +2. **Translation to Cap’n Proto RPC**
     200 | +   - The call to `getBlockHash` is translated into a Cap’n Proto RPC call. This translation is handled by code automatically generated by the `mpgen` tool, based on the [`chain.capnp`](../../src/ipc/capnp/chain.capnp)` file in [`src/ipc/capnp/`](../../src/ipc/capnp/).
    


    stickies-v commented at 4:18 PM on December 12, 2023:

    nit: rogue backtick

       - The call to `getBlockHash` is translated into a Cap’n Proto RPC call. This translation is handled by code automatically generated by the `mpgen` tool, based on the [`chain.capnp`](../../src/ipc/capnp/chain.capnp) file in [`src/ipc/capnp/`](../../src/ipc/capnp/).
    

    ryanofsky commented at 12:48 PM on December 19, 2023:

    re: #28978 (review)

    nit: rogue backtick

    Thanks, fixed

  14. in doc/design/multiprocess.md:136 in 837c53d14f outdated
     203 | +   - The generated `Chain` subclass in `bitcoin-wallet` populates a Cap’n Proto request with the `height` parameter.
     204 | +   - The request is sent to the `bitcoin-node` process.
     205 | +
     206 | +4. **Handling in bitcoin-node**
     207 | +   - Upon receiving the request, the `bitcoin-node` process reads the `height` value from the request.
     208 | +   - It then calls the `getBlockHash` method on its local `Chain` object with the provided `height`.
    


    stickies-v commented at 4:21 PM on December 12, 2023:

    Steps 2 and 3 highlight what's handled by generated code, for consistency that might be helpful in steps 4&5 too?


    ryanofsky commented at 12:48 PM on December 19, 2023:

    re: #28978 (review)

    Steps 2 and 3 highlight what's handled by generated code, for consistency that might be helpful in steps 4&5 too?

    Thanks, added more details to this section so it now mentions the generated client and server classes and references the generated code consistently.

  15. stickies-v commented at 5:45 PM on December 12, 2023: contributor

    Approach ACK. I'm still working on my understanding of multiprocess but this document was very helpful already in its current state, no major suggestions at this point.

  16. in doc/design/multiprocess.md:39 in 837c53d14f outdated
     106 | +
     107 | +## Proposed Architecture
     108 | +
     109 | +The new architecture divides the existing code into three specialized executables:
     110 | +
     111 | +- `bitcoin-node`: Manages the node, indexes, and JSON-RPC server.
    


    fjahr commented at 10:23 PM on December 14, 2023:

    nit

    - `bitcoin-node`: Manages the P2P node, indexes, and JSON-RPC server.
    

    ryanofsky commented at 12:48 PM on December 19, 2023:

    re: #28978 (review)

    nit

    Useful clarification, added

  17. in doc/design/multiprocess.md:29 in 837c53d14f outdated
      96 | +  - [References](#references)
      97 | +- [Acknowledgements](#acknowledgements)
      98 | +
      99 | +## Introduction
     100 | +
     101 | +The Bitcoin Core software, central to the Bitcoin network, has historically employed a monolithic architecture. The existing design has integrated functionality like P2P network operations, wallet management, and a GUI into a single executable. While effective, it has limitations in flexibility, security, and scalability. This project introduces changes that transition Bitcoin Core to a more modular architecture. It aims to enhance security, improve usability, and facilitate maintenance and development of the software.
    


    fjahr commented at 10:26 PM on December 14, 2023:

    nit suggestions:

    • 'central to the bitcoin network' isn't really relevant to this doc
    • I hear people complain that big changes are always a security risk in the short term all the time
    The Bitcoin Core software has historically employed a monolithic architecture. The existing design has integrated functionality like P2P network operations, wallet management, and a GUI into a single executable. While effective, it has limitations in flexibility, security, and scalability. This project introduces changes that transition Bitcoin Core to a more modular architecture. It aims to enhance security, improve usability, and facilitate maintenance and development of the software in the long run.
    

    ryanofsky commented at 12:47 PM on December 19, 2023:

    re: #28978 (review)

    nit suggestions:

    • 'central to the bitcoin network' isn't really relevant to this doc

    • I hear people complain that big changes are always a security risk in the short term all the time

    Yes, definitely agree and took the suggestion.

  18. in doc/design/multiprocess.md:217 in 837c53d14f outdated
     220 | +- Automatically generating `.capnp` files from C++ interface definitions (see [Interface Definition Maintenance](#interface-definition-maintenance)).
     221 | +- Simplifying and stabilizing interfaces (see [Interface Stability](#interface-stability)).
     222 | +- Adding sandbox features, restricting subprocess access to resources and data (see [https://eklitzke.org/multiprocess-bitcoin](https://eklitzke.org/multiprocess-bitcoin)).
     223 | +- Using Cap'n Proto's support for [other languages](https://capnproto.org/otherlang.html), such as [Rust](https://github.com/capnproto/capnproto-rust), to allow code written in other languages to call Bitcoin Core C++ code, and vice versa (see [How to rustify libmultiprocess? #56](https://github.com/chaincodelabs/libmultiprocess/issues/56)).
     224 | +
     225 | +## Conclusion
    


    fjahr commented at 4:09 PM on December 15, 2023:

    nit: This just repeats what is said in the Introduction part and isn't really needed for the purpose of this doc IMO


    ryanofsky commented at 12:49 PM on December 19, 2023:

    re: #28978 (review)

    nit: This just repeats what is said in the Introduction part and isn't really needed for the purpose of this doc IMO

    Agree, cut it down. I do think having a 1 sentence summary is useful to provide a little orientation at the end so didn't remove it entirely.

  19. in doc/design/multiprocess.md:84 in 837c53d14f outdated
     151 | +- **Bootstrapping IPC Connections**: It provides functions for starting new IPC connections, specifically binding generated client and server classes for an initial `interfaces::Init` interface (defined in [`src/interfaces/init.h`](../../src/interfaces/init.h)) to a UNIX socket. This initial interface has methods returning other interfaces that different Bitcoin-Core modules use to communicate after the bootstrapping phase.
     152 | +- **Asynchronous I/O and Thread Management**: The library is also responsible for managing I/O and threading. Particularly, it ensures that IPC requests never block each other and that new threads on either side of a connection can always make client calls. It also manages worker threads on the server side of calls, ensuring that calls from the same client thread always execute on the same server thread (to avoid locking issues and support nested callbacks).
     153 | +
     154 | +### Type Hooks in [`src/ipc/capnp/*-types.h`](../../src/ipc/capnp/)
     155 | +- **Custom Type Conversions**: In [`src/ipc/capnp/*-types.h`](../../src/ipc/capnp/), multiple function overloads of two `libmultiprocess` C++ functions, `mp::CustomReadField` and `mp::CustomBuildFields`, are defined. These functions are used for customizing the conversion of specific C++ types to and from Cap’n Proto types.
     156 | +- **Handling Special Cases**: The `mpgen` tool and `libmultiprocess` library can convert most C++ types to and from Cap’n Proto types automatically, including interface types, primitive C++ types, standard C++ types like `std::vector`, `std::set`, `std::map`, `std::tuple`, and `std::function`, as well as simple C++ structs that consist of afforementioned types and whose fields correspond 1:1 with Cap’n Proto struct fields. For other types, `*-types.h` files provide custom code to convert between C++ and Cap’n Proto data representations.
    


    fjahr commented at 4:16 PM on December 15, 2023:

    nit: typo

    - **Handling Special Cases**: The `mpgen` tool and `libmultiprocess` library can convert most C++ types to and from Cap’n Proto types automatically, including interface types, primitive C++ types, standard C++ types like `std::vector`, `std::set`, `std::map`, `std::tuple`, and `std::function`, as well as simple C++ structs that consist of aforementioned types and whose fields correspond 1:1 with Cap’n Proto struct fields. For other types, `*-types.h` files provide custom code to convert between C++ and Cap’n Proto data representations.
    

    ryanofsky commented at 12:48 PM on December 19, 2023:

    re: #28978 (review)

    nit: typo

    Thanks, fixed

  20. in doc/design/multiprocess.md:104 in 837c53d14f outdated
     171 | +
     172 | +The IPC mechanism is deliberately isolated from the rest of the codebase so less code has to be concerned with IPC.
     173 | +
     174 | +Building Bitcoin Core with IPC support is optional, and the same code can be compiled to either run in the same process or separate processes. The build system also ensures Cap’n Proto library headers can only be used within the [`src/ipc/capnp/`](../../src/ipc/capnp/) directory, not in other parts of the codebase.
     175 | +
     176 | +The libmultiprocess runtime is designed to place as few constraints as possible on IPC interfaces and make IPC calls act like normal function calls. Method arguments, return values, and exceptions are automatically serialized and sent between processes. Object references and `std::function` arguments are automatically tracked and mapped to allow invoked code to call back into invoking code at any time. And there is a 1:1 threading model where any every client thread has a corresponding server thread responsible for executing incoming calls from that thread (there can be multiple calls from the same thread due to callbacks), without blocking, and holding the same thread-local variables and locks so behavior is the same whether IPC is used or not.
    


    fjahr commented at 4:18 PM on December 15, 2023:
    The libmultiprocess runtime is designed to place as few constraints as possible on IPC interfaces and make IPC calls act like normal function calls. Method arguments, return values, and exceptions are automatically serialized and sent between processes. Object references and `std::function` arguments are automatically tracked and mapped to allow invoked code to call back into invoking code at any time. And there is a 1:1 threading model where every client thread has a corresponding server thread responsible for executing incoming calls from that thread (there can be multiple calls from the same thread due to callbacks), without blocking, and holding the same thread-local variables and locks so behavior is the same whether IPC is used or not.
    

    ryanofsky commented at 12:48 PM on December 19, 2023:

    re: #28978 (review)

    Thanks, fixed

  21. in doc/design/multiprocess.md:110 in 837c53d14f outdated
     177 | +
     178 | +### Interface Definition Maintenance
     179 | +
     180 | +The choice to maintain interface definitions and C++ type mappings as `.capnp` files in the [`src/ipc/capnp/`](../../src/ipc/capnp/) was mostly done for convenience, and probably something that could be improved in the future.
     181 | +
     182 | +In the current design, class names, method names, and parameter names are duplicated between C++ interfaces in [`src/interfaces/`](../../src/interfaces/) and Cap’n Proto files in [`src/ipc/capnp/`](../../src/ipc/capnp/). While this keeps C++ interface headers simple and free of references to IPC, it is a maintence burden because it means inconsistencies between C++ declarations and Cap’n Proto declarations will result in compile errors. (Static type checking ensures these are not runtime errors.)
    


    fjahr commented at 4:18 PM on December 15, 2023:
    In the current design, class names, method names, and parameter names are duplicated between C++ interfaces in [`src/interfaces/`](../../src/interfaces/) and Cap’n Proto files in [`src/ipc/capnp/`](../../src/ipc/capnp/). While this keeps C++ interface headers simple and free of references to IPC, it is a maintenance burden because it means inconsistencies between C++ declarations and Cap’n Proto declarations will result in compile errors. (Static type checking ensures these are not runtime errors.)
    

    ryanofsky commented at 12:48 PM on December 19, 2023:

    re: #28978 (review)

    Thanks, fixed

  22. in doc/multiprocess.md:15 in 837c53d14f outdated
      10 | +
      11 | +The `-debug=ipc` command line option can be used to see requests and responses between processes.
      12 | +
      13 | +## Installation
      14 | +
      15 | +The multiprocess feature requires [Cap'n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) as dependencies. A simple way to get starting using it without installing these dependencies manually is to use the [depends system](../depends) with the `MULTIPROCESS=1` [dependency option](../depends#dependency-options) passed to make:
    


    fjahr commented at 4:21 PM on December 15, 2023:
    The multiprocess feature requires [Cap'n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) as dependencies. A simple way to get started using it without installing these dependencies manually is to use the [depends system](../depends) with the `MULTIPROCESS=1` [dependency option](../depends#dependency-options) passed to make:
    

    ryanofsky commented at 12:49 PM on December 19, 2023:

    re: #28978 (review)

    Thanks, fixed

  23. in doc/design/multiprocess.md:159 in 837c53d14f outdated
     189 | +
     190 | +## Example Use Cases and Flows
     191 | +
     192 | +### Retrieving a Block Hash
     193 | +
     194 | +Let’s walk through an example where the `bitcoin-wallet` process requests the hash of a block at a specific height from the `bitcoin-node` process. This example demonstrates the practical application of the IPC mechanism, specifically the interplay between C++ method calls and Cap’n Proto-generated RPC calls.
    


    fjahr commented at 4:38 PM on December 15, 2023:

    Maybe mention that his is based on code that is not merged yet, for example chain.capnp is linked below but it doesn't exist on master yet.


    ryanofsky commented at 12:48 PM on December 19, 2023:

    re: #28978 (review)

    Maybe mention that his is based on code that is not merged yet, for example chain.capnp is linked below but it doesn't exist on master yet.

    Oops, this is not good. I changed the link to point to the branch version for now since I think seeing the interface definition can be helpful.

  24. fjahr commented at 4:48 PM on December 15, 2023: contributor

    ACK 837c53d14f24924cdcb2cfd8b18915882dc3b620

    For me this can already be merged as is but I also have a list of nits if you retouch. I have made myself familiar multiprocess a while ago but not looked at it recently. For me this was a nice refresher and I definitely think it's helpful for newcomers.

    A few more ideas of what might be interesting to add, but they are not essential and may also be done in a follow-up or just be ignored:

    • Could mention potential security implications (or lack thereof) of using capnp and libmultiprocess (at least as long as it's not maintained under the bitcoin-core org.
    • Similar to the usage example there could be an example of how a developer may go about changing something in the interfaces, mentioning common pitfalls etc.
    • If possible maybe a visualization/diagram how the pieces of the architecture fit together could be helpful to grasp the high level quicker.
  25. DrahtBot requested review from stickies-v on Dec 15, 2023
  26. ryanofsky force-pushed on Dec 19, 2023
  27. ryanofsky commented at 4:29 PM on December 19, 2023: contributor

    Updated 837c53d14f24924cdcb2cfd8b18915882dc3b620 -> fd98beea07b0488be6d3b6e7e09568331640785e (pr/ipcdoc.8 -> pr/ipcdoc.9, compare) with suggested changes. Updated fd98beea07b0488be6d3b6e7e09568331640785e -> c80bd16c826d564086006cff67b7b9ade9b9f38c (pr/ipcdoc.9 -> pr/ipcdoc.10, compare) with more edits and tweaks.

    re: #28978#pullrequestreview-1777912181

    Approach ACK. I'm still working on my understanding of multiprocess but this document was very helpful already in its current state, no major suggestions at this point.

    Thank you, applied your suggestions, and improved the example flow section based on your feedback.

    re: #28978#pullrequestreview-1782842554

    ACK 837c53d

    For me this can already be merged as is but I also have a list of nits if you retouch. I have made myself familiar multiprocess a while ago but not looked at it recently. For me this was a nice refresher and I definitely think it's helpful for newcomers.

    A few more ideas of what might be interesting to add, but they are not essential and may also be done in a follow-up or just be ignored:

    • Could mention potential security implications (or lack thereof) of using capnp and libmultiprocess (at least as long as it's not maintained under the bitcoin-core org.

    Great idea, added security consideration section addressing these things.

    • Similar to the usage example there could be an example of how a developer may go about changing something in the interfaces, mentioning common pitfalls etc.

    I think developer notes, particularly the Internal interface guidelines section will be a good place for this type of information. I added another link to it in the Interface Definition Maintenance section here. Probably the developer notes will need to have a little more information added, but that depends on more code being merged so should probably be left for future PRs.

    • If possible maybe a visualization/diagram how the pieces of the architecture fit together could be helpful to grasp the high level quicker.

    Yeah some visualizations would probably help. I added 3 simple diagrams, showing process/socket connections, code generation, and a call diagram, so hopefully these are helpful.

  28. ryanofsky force-pushed on Dec 19, 2023
  29. in doc/design/multiprocess.md:204 in c80bd16c82 outdated
     258 | +
     259 | +5. **Response and Return**
     260 | +   - The `getBlockHash` method of the generated `Chain` client subclass in `bitcoin-wallet` which sent the request now receives the response.
     261 | +   - It extracts the block hash value from the response, and returns it to the original caller.
     262 | +
     263 | +<table><tr><td>
    


    0xB10C commented at 7:36 PM on December 20, 2023:
    </td></tr></table>
    

    I think you meant to close/end the table here. Otherwise this looks a bit weird rendered on GitHub.


    ryanofsky commented at 4:20 AM on December 27, 2023:

    I think you meant to close/end the table here. Otherwise this looks a bit weird rendered on GitHub.

    Good catch should be fixed now

  30. doc: Add multiprocess design doc
    Also split up existing multiprocess documentation into design and usage
    sections
    91dc48c148
  31. ryanofsky force-pushed on Dec 27, 2023
  32. ryanofsky commented at 4:33 AM on December 27, 2023: contributor

    Updated c80bd16c826d564086006cff67b7b9ade9b9f38c -> 91dc48c14825a9075a57c1eefda202b83b6346ba (pr/ipcdoc.10 -> pr/ipcdoc.11, compare) fixing table tag and reference heading.

    I think this PR gets another ACK it could be merged to add the new documentation, and improvements could be implemented in a followup PR.

  33. TheCharlatan approved
  34. TheCharlatan commented at 7:57 AM on December 27, 2023: contributor

    ACK 91dc48c14825a9075a57c1eefda202b83b6346ba

  35. DrahtBot requested review from fjahr on Dec 27, 2023
  36. fjahr commented at 8:19 PM on December 28, 2023: contributor

    ACK 91dc48c14825a9075a57c1eefda202b83b6346ba

  37. DrahtBot removed review request from fjahr on Dec 28, 2023
  38. in doc/multiprocess.md:20 in 91dc48c148
      15 | +The multiprocess feature requires [Cap'n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) as dependencies. A simple way to get started using it without installing these dependencies manually is to use the [depends system](../depends) with the `MULTIPROCESS=1` [dependency option](../depends#dependency-options) passed to make:
      16 | +
      17 | +```
      18 | +cd <BITCOIN_SOURCE_DIRECTORY>
      19 | +make -C depends NO_QT=1 MULTIPROCESS=1
      20 | +CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure
    


    stickies-v commented at 12:44 PM on January 2, 2024:

    nit: Perhaps useful to parameterize the host platform to highlight it's configurable? E.g.

    HOST_PLATFORM="x86_64-pc-linux-gnu"
    cd <BITCOIN_SOURCE_DIRECTORY>
    make -C depends NO_QT=1 MULTIPROCESS=1
    CONFIG_SITE=$PWD/depends/$HOST_PLATFORM/share/config.site ./configure
    

    ryanofsky commented at 10:30 PM on January 11, 2024:

    re: #28978 (review)

    nit: Perhaps useful to parameterize the host platform to highlight it's configurable? E.g.

    Thanks I split it into a separate variable and clarified how it could be set in #10102.

  39. in doc/design/multiprocess.md:84 in 91dc48c148
      92 | +
      93 | +### C++ Client Subclasses in Generated Code
      94 | +- In the generated code, we have C++ client subclasses that inherit from the abstract classes in [`src/interfaces/`](../../src/interfaces/). These subclasses are the workhorses of the IPC mechanism.
      95 | +- They implement all the methods of the interface, marshalling arguments into a structured format, sending them as requests to the IPC server via a UNIX socket, and handling the responses.
      96 | +- These subclasses effectively mask the complexity of IPC, presenting a familiar C++ interface to developers.
      97 | +- Internally, the client subclasses generated by the `mpgen` tool wrap [client classes generated by Cap'n Proto](https://capnproto.org/cxxrpc.html#clients), and use them to send IPC requests.
    


    stickies-v commented at 1:03 PM on January 2, 2024:

    Perhaps useful to elaborate on what the mpgen wrapping adds to the Cap'n Proto output?


    ryanofsky commented at 10:30 PM on January 11, 2024:

    re: #28978 (review)

    Perhaps useful to elaborate on what the mpgen wrapping adds to the Cap'n Proto output?

    Great suggestion, done in #10102. The main difference is mpgen clients subclasses have a high-level blocking interface, while capnp client subclasses require you to do asynchronous and pass arguments and return values in capnp message format.

  40. in doc/design/multiprocess.md:145 in 91dc48c148
     159 | +
     160 | +In the current design, class names, method names, and parameter names are duplicated between C++ interfaces in [`src/interfaces/`](../../src/interfaces/) and Cap’n Proto files in [`src/ipc/capnp/`](../../src/ipc/capnp/). While this keeps C++ interface headers simple and free of references to IPC, it is a maintenance burden because it means inconsistencies between C++ declarations and Cap’n Proto declarations will result in compile errors. (Static type checking ensures these are not runtime errors.)
     161 | +
     162 | +An alternate approach could use custom [C++ Attributes](https://en.cppreference.com/w/cpp/language/attributes) embedded in interface declarations to automatically generate `.capnp` files from C++ headers. This has not been pursued because parsing C++ headers is more complicated than parsing Cap’n Proto interface definitions, especially portably on multiple platforms.
     163 | +
     164 | +In the meantime, the developer guide [Internal interface guidelines](developer-notes.md#internal-interface-guidelines) can provide guidance on keeping interfaces consistent and functional and avoiding compile errors.
    


    stickies-v commented at 1:34 PM on January 2, 2024:

    dead link

    In the meantime, the developer guide [Internal interface guidelines](../developer-notes.md#internal-interface-guidelines) can provide guidance on keeping interfaces consistent and functional and avoiding compile errors.
    

    ryanofsky commented at 10:30 PM on January 11, 2024:

    re: #28978 (review)

    dead link

    Thanks, fixed in #10102

  41. in doc/design/multiprocess.md:200 in 91dc48c148
     254 | +
     255 | +4. **Handling in bitcoin-node**
     256 | +   - Upon receiving the request, the Cap'n Proto dispatching code in the `bitcoin-node` process calls the `getBlockHash` method of the `Chain` [server class](#c-server-classes-in-generated-code).
     257 | +   - The server class is automatically generated by the `mpgen` tool from the [`chain.capnp`](https://github.com/ryanofsky/bitcoin/blob/pr/ipc/src/ipc/capnp/chain.capnp) file in [`src/ipc/capnp/`](../../src/ipc/capnp/).
     258 | +   - The `getBlockHash` method of the generated `Chain` server subclass in `bitcoin-wallet` receives a Cap’n Proto request object with the `height` parameter, and calls the `getBlockHash` method on its local `Chain` object with the provided `height`.
     259 | +   - When the call returns, it encapsulates the return value in a Cap’n Proto response, which it sends back to the `bitcoin-wallet` process,
    


    stickies-v commented at 1:54 PM on January 2, 2024:
       - When the call returns, it encapsulates the return value in a Cap’n Proto response, which it sends back to the `bitcoin-wallet` process.
    

    ryanofsky commented at 10:30 PM on January 11, 2024:

    re: #28978 (review)

    Thanks, fixed in #10102

  42. in doc/design/multiprocess.md:235 in 91dc48c148
     289 | +
     290 | +- **Cap’n Proto interface**: A set of methods defined in Cap’n Proto to facilitate structured communication between different software components.
     291 | +
     292 | +- **Cap’n Proto struct**: A structured data format used in Cap’n Proto, similar to structs in C++, for organizing and transporting data across different processes.
     293 | +
     294 | +- **client class (in generated code)**: A C++ class generated from a Cap’n Proto interface which inherits from a Bitcoin core abstract class, and implements each virtual method to send IPC requests to another process. (see also [components section](#c-client-subclasses-in-generated-code))
    


    stickies-v commented at 2:02 PM on January 2, 2024:

    nit

    - **client class (in generated code)**: A C++ class generated from a Cap’n Proto interface which inherits from a Bitcoin Core abstract class, and implements each virtual method to send IPC requests to another process. (see also [components section](#c-client-subclasses-in-generated-code))
    

    ryanofsky commented at 10:30 PM on January 11, 2024:

    re: #28978 (review)

    nit

    Thanks, fixed in #10102

  43. stickies-v approved
  44. stickies-v commented at 2:05 PM on January 2, 2024: contributor

    ACK 91dc48c14825a9075a57c1eefda202b83b6346ba - left a couple of improvements but agreed that iterating in future PRs is better.

  45. achow101 commented at 3:43 PM on January 2, 2024: member

    ACK 91dc48c14825a9075a57c1eefda202b83b6346ba

  46. achow101 merged this on Jan 2, 2024
  47. achow101 closed this on Jan 2, 2024

  48. jamesob commented at 3:48 PM on January 5, 2024: contributor

    Post-merge ACK. This doc is very helpful, thanks @ryanofsky!

  49. ryanofsky referenced this in commit ee4b9138c8 on Jan 11, 2024
  50. ryanofsky commented at 11:25 PM on January 11, 2024: contributor

    re: #28978#pullrequestreview-1800375604

    left a couple of improvements but agreed that iterating in future PRs is better.

    Thanks for the close reading and suggestions. I implemented them for now in the combined PR #10102 commit https://github.com/bitcoin/bitcoin/pull/10102/commits/ee4b9138c837f6dc6b8f063b0df27573736d6578 and will split it out into another PR at some point.

  51. willcl-ark referenced this in commit c96a84679d on Apr 25, 2024
  52. willcl-ark referenced this in commit 8fbab4475a on May 1, 2024
  53. willcl-ark referenced this in commit bc87a774a5 on May 2, 2024
  54. ryanofsky referenced this in commit 5aaaef09b2 on Jun 11, 2024
  55. ryanofsky referenced this in commit ebcbba1a7d on Jun 11, 2024
  56. ryanofsky referenced this in commit b031746518 on Jun 11, 2024
  57. ryanofsky referenced this in commit 95e6c68cc0 on Jun 11, 2024
  58. ryanofsky referenced this in commit 10e17a0d29 on Jul 11, 2024
  59. ryanofsky referenced this in commit a82f438a26 on Jul 26, 2024
  60. ryanofsky referenced this in commit dee0f699f1 on Jul 26, 2024
  61. willcl-ark referenced this in commit 44312a849e on Aug 7, 2024
  62. ryanofsky referenced this in commit c538ec69f2 on Aug 8, 2024
  63. ryanofsky referenced this in commit 51ebedab82 on Sep 6, 2024
  64. ryanofsky referenced this in commit 083b886bc6 on Sep 6, 2024
  65. ryanofsky referenced this in commit 61c7ad5681 on Sep 9, 2024
  66. ryanofsky referenced this in commit a289a55102 on Sep 10, 2024
  67. ryanofsky referenced this in commit 9c98c42a01 on Sep 10, 2024
  68. Sjors referenced this in commit e4880f1c70 on Sep 10, 2024
  69. Sjors referenced this in commit a4ca632595 on Sep 11, 2024
  70. Sjors referenced this in commit e92d8d1111 on Sep 11, 2024
  71. ryanofsky referenced this in commit 1be749c771 on Sep 12, 2024
  72. ryanofsky referenced this in commit b95bb21796 on Sep 12, 2024
  73. ryanofsky referenced this in commit f9245e4bb8 on Sep 17, 2024
  74. ryanofsky referenced this in commit 56b4e675b6 on Sep 17, 2024
  75. ryanofsky referenced this in commit cf01bfacb3 on Sep 19, 2024
  76. ryanofsky referenced this in commit 54611e99f5 on Sep 19, 2024
  77. ryanofsky referenced this in commit d59f5a31a1 on Sep 19, 2024
  78. ryanofsky referenced this in commit db83545702 on Sep 19, 2024
  79. Sjors referenced this in commit 4184127bb7 on Sep 19, 2024
  80. Sjors referenced this in commit 542d3565ff on Sep 19, 2024
  81. Sjors referenced this in commit dcf681adb1 on Sep 20, 2024
  82. ryanofsky referenced this in commit 44e7b856c3 on Sep 24, 2024
  83. ryanofsky referenced this in commit 3499c2ca4b on Sep 24, 2024
  84. ryanofsky referenced this in commit fbed31494a on Sep 24, 2024
  85. ryanofsky referenced this in commit 1a33281766 on Sep 24, 2024
  86. m3dwards referenced this in commit 412cafb59e on Oct 3, 2024
  87. bitcoin locked this on Jan 10, 2025

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-22 18:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me