#29409 ipc: Expose Chain interface to IPC clients
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #29409 ipc: Expose Chain interface to IPC clients | Ready | 0 (+2) | Mild | 2,359 + 33 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Exposes the node's `interfaces::Chain` over IPC using Cap'n Proto proxy wrappers, split into a stable `Chain` interface and an `UnstableChain` interface. The stable interface covers basic block lookups for external clients under Cap'n Proto backward-compatibility rules, while `UnstableChain` exposes the remaining methods for internal Bitcoin Core consumers. It also updates the `src/ipc/libmultiprocess` subtree and makes minor adjustments to `interfaces::Chain` method signatures to support code generation.
Problem: Processes connecting to `bitcoin-node` over IPC via `-ipcbind` cannot read chain data or follow chain notifications, blocking multiprocess wallet separation (#10102). Exposing the entire raw `interfaces::Chain` C++ interface to external IPC clients would freeze internal wallet APIs against future refactoring.
Evidence of importance:
- PR description: 'Processes connected to bitcoin-node over IPC (with -ipcbind) can use the Mining interface, but they can't currently use the Chain interface read chain data or follow chain events. Chain data could be useful to third-party clients and Bitcoin Core's own wallet could use this as well to run in a separate process (#10102).'
- PR description references #34981 ('multiprocess: expose existing interfaces or design new ones?') as the rationale for splitting into stable and unstable interfaces.
Scope: Carries an update to the src/ipc/libmultiprocess subtree accounting for the majority of changed files and commits, alongside new Cap'n Proto schema and wrapper files. Also includes minor signature adjustments in src/interfaces/chain.h (removing const qualification from two estimation methods and passing updateRwSetting callbacks by value).
Risk of a mistake: medium (wallet, build, other). Bugs in Cap'n Proto serialization or proxy wrappers could cause node crashes or memory errors when IPC requests are processed, though IPC is only enabled when configured via -ipcbind.
Changed since the previous assessment: The bundled libmultiprocess subtree update was refreshed with upstream changes including v14 platform support and CI updates, and a compiler flag was added to suppress -Wc++23-lambda-attributes warnings.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- reviewer: re-review following rebase and Chain/UnstableChain interface split 2026-10-15
Agreement
Mild
objection that needs a change was answered, no reply from the objector (xyzconstant: node aborts due to an unhandled remote exception if an external ChainNotifications client raises an error during notific)
Thread notes: The author rebased the branch to split the Chain Cap'n Proto interface into stable Chain and UnstableChain interfaces as discussed in #34981, updated libmultiprocess dependencies, and resolved a silent conflict with #36669.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| cbergqvist (contributor) | suggestion, maintenance | remove unused include <any> | fixed · fix pushed · author confirmed it was leftover and removed it | answered | yes | 2024-02-27: 'Builds fine without `#include <any>` using `g++` on Linux?' Status from: 2024-02-28: ryanofsky: 'Good catch, I think this must have been left over from a previous version of this file. Removed now.' |
| ariard (contributor) | suggestion, approach | parameterize Wrap with any CAddress version to test alternative crypto primitives | answered · author explained current hardcoded serialization is simplest but customized formats can be added later | answered | yes | 2024-03-07: 'i think `Wrap` could be parameterized with any `CAddress` version.' Status from: 2024-07-26: ryanofsky: 'For now though the easiest thing is for the wrapped stream to just hardcode parameters that serialize all the data.' |
| sedited (member) | question, approach | why is custom functionality introduced for findBlock and how to validate changes without tests | answered · author explained recursive structs require custom capnp translation code | answered | yes | 2024-07-05: 'why is so much custom functionality introduced for `findBlock`?' Status from: 2024-07-08: ryanofsky: 'Chain::findBlock and the handful of other Chain methods taking FoundBlock input/output arguments need custom code becaus' |
| sedited (member) | question, interface | whether UniValue in interfaces is intended to be refactored eventually | answered · author explained UniValue usage in RPC forwarding, settings, and ban map | answered | yes | 2024-07-26: 'Is the intention to eventually refactor this?' Status from: 2024-07-26: ryanofsky: 'In general there are lots of ways current interfaces are not ideal and can be improved over time' |
| josibake (member) | suggestion, maintenance | update the PR description to mention the dependency on PR 30509 | answered · author updated description | answered | yes | 2024-08-07: 'can you update the description with what you mention in ..., regarding #30509 ?' Status from: 2024-08-07: ryanofsky: 'Added following to the description:' |
| sedited (member) | question, interface | whether Data blobs and enums should be strongly typed in capnp for non-C++ clients | answered · author explained tradeoffs of maintenance burden and suggested incremental adoption | answered | yes | 2024-11-08: 'More generally I have been wondering if it would be a good idea to eventually strongly type these `Data` blobs (and similarly the enums)?' Status from: 2024-11-12: ryanofsky: 'tradeoff is just that defining capnproto structs for all the serialized classes adds more code that will need to be main' |
| pseudoramdom (member) | question, approach | how to build the branch on macOS and whether to use MULTIPROCESS=1 or build libmultiprocess from source | answered · author explained build options and rebased branch to resolve compilation errors | answered | yes | 2025-02-10: 'Can I use the branch as is or would I need to rebase on `master`' Status from: 2025-02-10: ryanofsky: 'definitely it should be easiest to just check out this branch and use it as is' |
| zaidmstrr (contributor) | suggestion, correctness | remove flush from chain.capnp to avoid build errors after removal upstream | fixed · fix pushed · author rebased and resolved conflict | changed | yes | 2025-05-24: 'removes this `flush()` from the codebase. I think it should also be removed from `chain.capnp` otherwise it will give build errors.' Status from: 2025-06-03: ryanofsky: 'Rebased... due to silent conflicts with #32438 and #32641' |
| zaidmstrr (contributor) | objection, correctness | build fails because rpcRunLater was removed upstream | fixed · fix pushed · author rebased to resolve conflict | changed | no | 2025-07-21: 'The method `rpcRunLater` was removed in commit, thus there are some build errors.' Status from: 2025-07-24: ryanofsky: 'Rebased... to fix silent merge conflict with #32862' |
| l0rinc (member) | objection, correctness | missing struct fields like best_height in estimateSmartFee and mismatched integer and enum types lead to serialization bugs and compilation issues | fixed · fix pushed · fixed by author in subsequent push | changed | yes | 2025-11-25: 'It seems to me the capnp wrapper needs some updates (missing field, 32/64 bit params, param rename, bool vs uint64, enums as unsigned, optional mapping, unused declarations, comments, etc).' Status from: 2025-12-12: ryanofsky: 'You found a lot of annoying discrepencies and a real bug in the estimateSmartFee best_height return value, which should ' |
| xyzconstant (contributor) | question, interface | whether ChainClient should be reduced to lifecycle and mock-time hooks and the rest moved to WalletLoader | accepted · reviewer accepted author explanation that ChainClient is designed for node-spawned processes | answered | yes | 2026-04-27: 'Would it be worth reducing `ChainClient` to just lifecycle + mock-time hooks and moving the rest to `WalletLoader`?' Status from: 2026-05-07: xyzconstant: 'I get it now, thanks you!' |
| willcl-ark (member) | objection, safety | node could hit undefined behavior when switching on broadcastmethod without safe enum mapping or a default case | answered · author explained C++ callers enforce explicit values, and non-C++ callers must handle omitted values safely | answered | yes | 2026-05-20: 'broadcastmethod is switched on (e.g. here) with no default case, as is our convention. but we don't check incoming value is mapped to the enum safely.' Status from: 2026-05-20: ryanofsky: 'In C++, the compiler enforces at each call site that explicit values need to be passed for all parameters which don't ha' |
| willcl-ark (member) | suggestion, interface | rename descendants parameter to cluster_count post-cluster mempool | fixed · fix pushed · renamed to clusterCount in latest push | answered | yes | 2026-05-20: 'even though these are positional, I think we can rename descendants -> cluster_count post-cluster mempool (and as this is public facing).' Status from: 2026-10-14: ryanofsky: 'Renamed to clusterCount in 2ee7fc45cf3658b954bff017db45024b2a37a0ba.' |
| willcl-ark (member) | objection, safety | node segfaults if a non-C++ client passes null transaction data or unhandled enum values over IPC | answered · author replied explaining that capnp allows null/defaults and discussed trade-offs and future mitigations | answered | yes | 2026-05-20: 'Also what happens here is tx is a null pointer (allowed by capnp spec AFAIU)? I think we will immediately try to deference it in BroadcastTransaction and segfault.' Status from: 2026-05-20: ryanofsky: 'This is a legitimate concern, and it's not specific to this method or to these parameters.' |
| willcl-ark (member) | question, approach | why not omit ChainClient::start from the exposed interface or throw an exception rather than asserting | answered · author explained libmultiprocess generation requirements and agreed an exception could be thrown instead | answered | yes | 2026-05-20: 'Why not omit start from the exposed base interface or make this throw a protocol error instead of asserting.' Status from: 2026-05-20: ryanofsky: 'This could throw an exception instead of assert. It shouldn't matter very much because I don't think there's a way this ' |
| zaidmstrr (contributor) | objection, correctness | node aborts with an assertion failure if an IPC client requests getBlockHash for a height exceeding the active chain tip | answered · author replied that node-side crash is fixed in #36520 by throwing an exception instead of asserting | answered | yes | 2026-09-09: 'when we pass the height, which is greater than the current tip, then the node crashes and exits with the error... Assertion `chainman().ActiveChain()[height]' failed. Aborted (core dumped)' Status from: 2026-10-14: ryanofsky: 'Thanks for reporting the getBlockHash crash. #36520 changes getBlockHash to throw an exception instead of asserting' |
| enirox001 (contributor) | question, maintenance | why FeeCalculation, EstimationResult, and EstimatorBucket are in common.capnp if unreferenced elsewhere, and whether they should be removed | fixed · fix pushed · author removed unused fee estimation structs in latest push | answered | yes | 2026-10-05: 'The data structures added here; `FeeCalculation`, `EstimationResult` and `EstimatorBucket` in common.capnp what is it for? i do not think it is referenced anywhere apart from here, should they be remo' Status from: 2026-10-14: ryanofsky: 'Removed them in 2ee7fc45cf3658b954bff017db45024b2a37a0ba, along with the note in block_policy_estimator.h that referred ' |
| enirox001 (contributor) | suggestion, maintenance | remove stale declaration or include in src/rpc/request.h | answered · author explained the struct mapping note in request.h is still accurate | answered | yes | 2026-10-05: 'I think this is stale' Status from: 2026-10-14: ryanofsky: 'I think this one is still accurate: JSONRPCRequest in chain.capnp has a field for each member of this struct except cont' |
| enirox001 (contributor) | suggestion, interface | allow size and fee fields in getTransactionAncestry to be omitted or marked optional rather than always returning empty values | answered · author explained returning size and fee has minimal overhead and avoids complexity | answered | yes | 2026-10-05: 'perhaps something similar could be done for this? or an optional parameter added to the method here?' Status from: 2026-10-14: ryanofsky: 'I think it's simpler to leave this as is. The size and fee values come from the same mempool lookup as the counts' |
| enirox001 (contributor) | question, interface | why metadata is omitted here as well | answered · author explained function pointers cannot be sent over IPC and will be addressed in #10102 | answered | yes | 2026-10-05: 'Is there a reason why the metadata is omitted here as well?' Status from: 2026-10-14: ryanofsky: 'metadata_fn is a function pointer, so it can't be sent over IPC... I'll look at it as part of #10102.' |
| xyzconstant (contributor) | objection, safety | node aborts due to an unhandled remote exception if an external ChainNotifications client raises an error during notification callbacks | answered · author explained notification exceptions are caught and logged in #36520 | changed | yes | 2026-10-06: 'Basically, the node isn't handling errors raised by external IPC servers it makes calls to.' Status from: 2026-10-14: ryanofsky: '#36520 fixes this by catching ipc::Exception errors from notification handlers in NotificationsProxy and logging them' |
| rustaceanrob (member) | suggestion, interface | define a structured Coin type for undo data and allow clients to optionally receive spent coins in block notifications | answered · author agreed structured undo data would be useful and deferred to a follow-up after #24230 | answered | yes | 2026-10-07: 'Could we consider creating a more detailed type here? I would suggest a "Coin" that contains the script and amount after disk deserialization, then defining a list of coins as the undo data.' Status from: 2026-10-14: ryanofsky: 'Agree this would be useful... I'd prefer to leave both for a follow-up, since this PR is just trying to wrap the existin' |
Support:
- cbergqvist (contributor) (ACK, 2024-02-28): Surface-level ACK of 41831ab. :) [not substantive]
- ariard (contributor) (2024-03-07) [read]: being able to work with any socket (TCP or UNIX) makes a lot of sense. you could have bitcoin-node on some secure computing host. and then one or more bitcoin-walet on over-the-network hosts.
- josibake (member) (Concept ACK, 2024-09-26): Concept ACK [not substantive]
- sedited (member) (ACK, 2024-11-19): lgtm ACK 34d3e2a6eaaab9de5328c3e64739f1392696c7db [not substantive]
- darosior (member) (Tested ACK, 2024-12-09) [tested]: tested ACK 395d5eed0475558de2c2a66914d5a5814ce38f3c. I've tested this with the Rust wallet PoC... exercises getHeight, getBlockHash, hasBlocks, findAncestorByHeight, findAncestorByHash, findCommonAnce
- sedited (member) (ACK, 2024-12-09): Re-ACK 64b833854a34d87cde4e0ca4173d75012c401a7a [not substantive]
- Sjors (member) (2026-05-12) [both]: implemented Rust bindings and an Electrs demo using IPC instead of RPC and P2P for block fetching, tx broadcast, tip updates, and mempool sync
- willcl-ark (member) (Concept ACK, 2026-05-20) [read]: Exposing interfaces::Chain over IPC seems very useful, especially for downstream users. I have been using a version of this branch myself on a node 'dashboard'... Long term, I think IPC should become
- enirox001 (contributor) (2026-10-05) [read]: This is nice to have... in general having downstream clients able to call these methods is good.
- rustaceanrob (member) (2026-10-07) [read]: allows indexes (like silent payments partial secrets or Utreexo proofs) to run on separate processes from bitcoin-node rather than supporting them directly in the node
Participants:
- cbergqvist (contributor, support, 5 statements 2024-02-26 to 2024-02-28): gave surface-level ACK after review suggestions were addressed
- ariard (contributor, support, 2 statements 2024-03-07 to 2024-03-07): supported socket-based IPC for node and wallet separation
- sedited (member, support, 15 statements 2024-07-05 to 2026-04-20): ACKed the PR, tested against Rust BDK wallet PoC, and asked design questions
- josibake (member, support, 2 statements 2024-08-07 to 2024-09-26): Concept ACK and requested PR description clarification
- darosior (member, support, 5 statements 2024-12-05 to 2024-12-16): Tested ACK using custom Rust BDK wallet client
- Big621 (none, neutral, 2 statements 2025-01-15 to 2025-01-15): approved without comment or prior project history
- pseudoramdom (member, question, 1 statements 2025-02-10 to 2025-02-10): asked questions about building with multiprocess on macOS
- zaidmstrr (contributor, objection, 3 statements 2025-05-24 to 2026-09-09): reported node aborts on out-of-bounds getBlockHash, plus earlier rebase build issues
- maflcko (member, neutral, 2 statements 2025-07-21 to 2025-07-24): noted typo in comment and CI lint failure
- l0rinc (member, objection, 17 statements 2025-09-30 to 2025-11-25): requested changes due to missing struct fields and parameter type discrepancies
- Sjors (member, support, 2 statements 2026-04-01 to 2026-05-12): developed Rust bindings, demonstrated Electrs integration, and opened discussion on interface scope
- xyzconstant (contributor, objection, 3 statements 2026-04-27 to 2026-10-06): reported node aborts when an external ChainNotifications server returns an error, and asked about ChainClient
- willcl-ark (member, objection, 6 statements 2026-05-20 to 2026-05-20): Concept ACK, but raised concern that null pointers or unhandled enums cause node segfaults
- enirox001 (contributor, support, 7 statements 2026-10-05 to 2026-10-05): supported exposing interfaces for downstream clients; raised questions and suggestions on capnp structures
- rustaceanrob (member, support, 2 statements 2026-10-07 to 2026-10-07): supported enabling external index processes over IPC and suggested exposing structured undo data
Dependencies
Files
33 lines under test/bench/ci.
- src/ipc/libmultiprocess/src/mp/util.cpp +216/-31
- src/ipc/capnp/chain.cpp +217/-0
- src/ipc/libmultiprocess/test/mp/test/test.cpp +208/-6
- src/ipc/libmultiprocess/test/mp/test/connect_tests.cpp +212/-0
- src/ipc/capnp/chain.capnp +196/-0
- src/ipc/libmultiprocess/src/mp/proxy.cpp +106/-47
- src/ipc/capnp/common-types.h +134/-0
- src/ipc/libmultiprocess/test/mp/test/listen_tests.cpp +48/-77
- src/ipc/libmultiprocess/include/mp/proxy-io.h +79/-42
- src/ipc/capnp/chain-types.h +109/-0
- src/ipc/libmultiprocess/src/mp/gen.cpp +48/-45
- src/ipc/libmultiprocess/test/mp/test/unixlistener.h +84/-0
- src/ipc/libmultiprocess/include/mp/util.h +63/-15
- src/ipc/libmultiprocess/test/mp/test/foo.h +56/-1
- src/ipc/libmultiprocess/include/mp/proxy-types.h +40/-14
- src/ipc/libmultiprocess/include/mp/proxy.h +30/-23
- src/ipc/libmultiprocess/cmake/compat_config.cmake +0/-42
- src/ipc/libmultiprocess/include/mp/type-number.h +24/-15
- src/ipc/libmultiprocess/ci/scripts/ci.sh +34/-1
- src/ipc/libmultiprocess/.github/workflows/ci.yml +19/-14
- src/ipc/libmultiprocess/CMakeLists.txt +24/-9
- src/ipc/libmultiprocess/test/mp/test/spawn_tests.cpp +28/-5
- src/ipc/capnp/common.capnp +32/-0
- test/functional/interface_ipc.py +32/-0
- src/ipc/libmultiprocess/test/mp/test/foo.capnp +28/-0
- src/ipc/libmultiprocess/include/mp/type-chrono.h +25/-2
- src/ipc/libmultiprocess/test/mp/test/common.h +27/-0
- src/ipc/libmultiprocess/include/mp/type-struct.h +12/-12
- src/ipc/libmultiprocess/cmake/iwyu-wrapper.sh +22/-0
- src/ipc/libmultiprocess/include/mp/type-interface.h +10/-10
- src/ipc/libmultiprocess/shell.nix +13/-6
- src/ipc/libmultiprocess/include/mp/type-context.h +15/-3
- src/ipc/capnp/handler.capnp +17/-0
- CMakeLists.txt +14/-0
- src/ipc/libmultiprocess/.clang-tidy +12/-2
- src/ipc/libmultiprocess/.github/workflows/bitcoin-core-ci.yml +8/-5
- src/ipc/libmultiprocess/example/calculator.cpp +4/-9
- src/ipc/libmultiprocess/example/printer.cpp +4/-9
- src/ipc/libmultiprocess/example/example.cpp +7/-5
- src/ipc/libmultiprocess/include/mp/type-char.h +9/-3
- src/ipc/capnp/handler-types.h +10/-0
- src/ipc/libmultiprocess/ci/scripts/run.sh +6/-4
- src/ipc/libmultiprocess/doc/versions.md +8/-2
- src/ipc/libmultiprocess/include/mp/type-map.h +6/-4
- src/ipc/libmultiprocess/ci/configs/netbsd.bash +1/-7
- src/ipc/libmultiprocess/cmake/pthread_checks.cmake +8/-0
- src/ipc/libmultiprocess/include/mp/type-set.h +4/-4
- src/ipc/libmultiprocess/include/mp/type-threadmap.h +4/-4
- src/ipc/libmultiprocess/include/mp/type-unordered-set.h +4/-4
- src/ipc/libmultiprocess/include/mp/type-vector.h +4/-4
- src/ipc/capnp/init.capnp +7/-0
- src/ipc/libmultiprocess/ci/configs/olddeps.bash +5/-2
- src/ipc/libmultiprocess/test/mp/test/foo-types.h +7/-0
- src/interfaces/chain.h +3/-3
- src/ipc/libmultiprocess/ci/configs/newdeps.bash +6/-0
- src/ipc/libmultiprocess/doc/design.md +3/-3
- src/ipc/libmultiprocess/doc/usage.md +6/-0
- src/node/interfaces.cpp +3/-3
- src/ipc/CMakeLists.txt +5/-0
- src/ipc/libmultiprocess/include/mp/type-data.h +2/-3
- src/ipc/libmultiprocess/ci/configs/sanitize.bash +2/-2
- src/ipc/libmultiprocess/ci/scripts/config.sh +2/-2
- src/ipc/libmultiprocess/include/mp/type-string.h +3/-1
- src/ipc/libmultiprocess/ci/configs/default.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/freebsd.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/gnu32.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/llvm.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/macos.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/openbsd.bash +1/-1
- src/ipc/libmultiprocess/cmake/TargetCapnpSources.cmake +1/-1
- src/ipc/libmultiprocess/doc/install.md +1/-1
- src/ipc/libmultiprocess/include/mp/type-function.h +1/-1
- src/ipc/libmultiprocess/include/mp/version.h +1/-1
- src/rpc/request.h +2/-0
- src/ipc/capnp/init-types.h +1/-0
- src/ipc/libmultiprocess/ci/README.md +1/-0
- src/ipc/libmultiprocess/include/mp/config.h.in +1/-0
- src/ipc/libmultiprocess/test/CMakeLists.txt +1/-0
- test/functional/test_framework/ipc_util.py +1/-0
Card
The short text the ranking pass compares across PRs.
Exposes the Chain interface over IPC using Cap'n Proto, splitting it into a version-stable Chain interface with basic block lookup methods and an UnstableChain interface for Bitcoin Core internal consumers. Currently, chain data and notifications are inaccessible over IPC, blocking multiprocess wallet separation (#10102) and external clients connecting via -ipcbind. This is a core prerequisite for running the wallet in an isolated process (#19460) and depends on #36520.
Data
dossier JSON · extract JSON · model ledger, generated 2026-11-01T01:16, confidence high, input hash b956e89027e38084
thread read through 2026-10-15 (143 statements), head f770bb7f73
labels: IPC
thread read through 2026-10-15 (143 statements), head f770bb7f73
labels: IPC
Categories
IPC / multiprocess
P1 · unblocks #10102
Exposes Chain over IPC, unblocking process separation for the wallet (#10102) and enabling external clients like indexes and downstream tools to read chain data. Multiple contributors and maintainers tested and relied on this to build decoupled wallet prototypes, indexes, and electrs bindings.
Membership: Carries IPC label, updates libmultiprocess subtree, and implements Cap'n Proto proxy bindings for interfaces::Chain.
Factors: security/stability 0, bug 0, performance 0, user value 2, leverage 3