multiprocess: Add capnp wrapper for Chain interface #29409

ryanofsky · 2024-02-08T13:54:40Z

Changes making it possible to call interface::Chain over a socket.

Note: The changes in this PR allow a bitcoin-node process to be controlled over a socket. The socket can either be an UNIX socket created by specifying the -ipcbind option, or it can be a socketpair created by a controlling process and passed to bitcoin-node via the -ipcfd=<n> with a file descriptor number. This PR is a dependency of #10102 which allows the wallet code to run in a separate process and use the Chain interface.

This PR is part of the process separation project.

DrahtBot · 2024-02-08T13:54:43Z

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/29409.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	josibake, darosior
Stale ACK	cbergqvist, sedited

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

ensure that serializing and deserializing objects -> ensure that serializing and deserializing objects use the same parameters [the original line is an incomplete sentence and thus unclear; adding "use the same parameters" completes the thought and restores comprehensibility]

^2026-01-06

DrahtBot · 2024-02-08T15:41:22Z

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

_{Debug: https://github.com/bitcoin/bitcoin/runs/21365328047}

cbergqvist

I'm eager for multiprocess to get in, but unable to do in-depth reviews in tolerable time due to lack of domain-knowledge. Did a surface level review of 2fd1042. Hope you don't mind.

src/ipc/capnp/common-types.h

src/ipc/capnp/chain-types.h

src/ipc/capnp/common-types.h

ryanofsky

Thanks for the review!

Updated 2fd1042 -> 41831ab (pr/ipc-chain.2 -> pr/ipc-chain.3, compare) with suggested changes.

src/ipc/capnp/common-types.h

src/ipc/capnp/chain-types.h

src/ipc/capnp/common-types.h

cbergqvist

Thanks for incorporating some of my suggestions. Surface-level ACK of 41831ab. :)

ariard

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.

ariard · 2024-03-07T02:54:34Z

src/ipc/capnp/common-types.h

+template<typename S>
+auto Wrap(S& s)
+{
+    return ParamsStream{s, TX_WITH_WITNESS, CAddress::V2_NETWORK};


i think Wrap could be parameterized with any CAddress version.
e.g useful if someone wanna test the native crypto noise framework between bitcoin processes.
or in the future experiment with better crypto-primitives for bip324.

re: #29409 (comment)

i think Wrap could be parameterized with any CAddress version. e.g useful if someone wanna test the native crypto noise framework between bitcoin processes. or in the future experiment with better crypto-primitives for bip324.

For now the Wrap function is basically a workaround for not having a serialization option that says "serialize all the data in this object to a stream, I don't care about the specific serialization format." So the formats here just chosen to preserve all data, and don't really matter otherwise.

But I could imagine use-cases like you are suggesting where we may want to customize which serialization format is used depending on the IPC connection. I think it would not be that hard to do by adding parameters to Wrap, which is not called many places, or by writing different CustomReadField/BuildField overloads which do not call Wrap. For now though the easiest thing is for the wrapped stream to just hardcode parameters that serialize all the data.

sedited · 2024-07-05T10:14:06Z

I'm not sure what a good approach for reviewing this might be. How can I validate the changes without tests? Should I be running this as part of a parent pull request? The first two commits seem fairly mechanical, but the last one less so. The comments in the code are helpful, but some pointers in the PR description for what to look out for, or a broad description of the approaches taken would be helpful too. E.g. why is so much custom functionality introduced for findBlock?

ryanofsky · 2024-07-08T16:38:44Z

re: #29409 (comment)

Thanks for the questions.

I'm not sure what a good approach for reviewing this might be. How can I validate the changes without tests? Should I be running this as part of a parent pull request?

You could test this as part of #10102, and I should probably add some unit test code to ipc_test.cpp to make sure new runtime code in capnp/chain.cpp and capnp/common-types.h functions has test coverage.

Most of the code in the PR is evaluated at compile time though, so for example, if there is a mismatch in any of the interface declarations there will be build errors.

The first two commits seem fairly mechanical, but the last one less so. The comments in the code are helpful, but some pointers in the PR description for what to look out for, or a broad description of the approaches taken would be helpful too.

This is helpful feedback, and I will try to add more comments. I know it needs more comments, but it's hard for me to know where to add them without being asked because I'm too familiar with everything here. This PR is a case where "Don't spend more than a few seconds trying to understand it" review really advice applies, so if something doesn't make sense, you should just ask so I can clarify. If something doesn't make sense to you, it probably doesn't make sense to other people either.

E.g. why is so much custom functionality introduced for findBlock?

Will add a comment, but Chain::findBlock and the handful of other Chain methods taking FoundBlock input/output arguments need custom code because these are converted to FoundBlockParam and FoundBlockResult Cap'n Proto structs to make IPC calls, and these structs are recursive. Libmultiprocess has decent support for converting back and forth between C++ classes and capnp structs generally, but it would be hard for it support recursive structs. It also might have a hard time in this case because there is a not 1:1 mapping between fields in the c++ class and capnp structs. So the conversion for these types is completely custom.

Similar custom code also exists for the C++ BlockInfo struct which is complicated to translate to the Capnp BlockInfo struct, because it has internal pointers and references, so it has custom code because it can't really be converted automatically.

ryanofsky

Updated ab6b795 -> eec0d31 (pr/ipc-chain.5 -> pr/ipc-chain.6, compare), rebasing and dropping no longer needed code after libmultiprocess update, adding comments and tests, and splitting up commits. There is still more to do to splitting up commits and adding tests, however.

This PR now shares a little bit of code with #30510, so will get smaller if #30510 is merged. #30510 is also simpler and should be easier to review, so will mark this as a draft, although feedback is still welcome on this PR especially if there are questions or things that need more explanation.

re: #29409 (review)

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

Yes this PR does not actually expose a socket but #30509 does, and #19460 uses it for the wallet (and #19461 uses it for the gui, and Sjors#48 and #30437 use it for the mining interface

ryanofsky · 2024-07-26T16:29:15Z

src/ipc/capnp/common-types.h

+template<typename S>
+auto Wrap(S& s)
+{
+    return ParamsStream{s, TX_WITH_WITNESS, CAddress::V2_NETWORK};


re: #29409 (comment)

i think Wrap could be parameterized with any CAddress version. e.g useful if someone wanna test the native crypto noise framework between bitcoin processes. or in the future experiment with better crypto-primitives for bip324.

For now the Wrap function is basically a workaround for not having a serialization option that says "serialize all the data in this object to a stream, I don't care about the specific serialization format." So the formats here just chosen to preserve all data, and don't really matter otherwise.

But I could imagine use-cases like you are suggesting where we may want to customize which serialization format is used depending on the IPC connection. I think it would not be that hard to do by adding parameters to Wrap, which is not called many places, or by writing different CustomReadField/BuildField overloads which do not call Wrap. For now though the easiest thing is for the wrapped stream to just hardcode parameters that serialize all the data.

sedited · 2025-09-29T11:20:34Z

Err, right. Updated the description with your suggested trace filtering.

ryanofsky · 2025-09-29T14:17:18Z

Err, right. Updated the description with your suggested trace filtering.

Thanks created bitcoin-core/libmultiprocess#219 to track this (feel free to create new issues directly in the future). Thanks for updating the rust client, too!

ryanofsky · 2025-09-30T13:58:46Z

Updated 7ba7ec9 -> 7a526a1 (pr/ipc-chain.17 -> pr/ipc-chain.18, compare) to fix clang-tidy errors https://github.com/bitcoin/bitcoin/actions/runs/17978986033/job/51139663003?pr=29409

Updated 7a526a1 -> 2ae18d0 (pr/ipc-chain.18 -> pr/ipc-chain.19, compare) to fix silent conflict with #32660

Rebased 2ae18d0 -> 2763976 (pr/ipc-chain.19 -> pr/ipc-chain.20, compare)

Rebased 2763976 -> d9efd1e (pr/ipc-chain.20 -> pr/ipc-chain.21, compare) due to silent conflicts with #33567 and #33218

l0rinc

Rebased locally and did a first pass through the changes.
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).

Unrelated nit: while checking the code I noticed that RemoveCvRef and std::remove_cv_t<std::remove_reference_t can likely be changed to std::remove_cvref_t in a few places (should be done in a different PR, if valid)

l0rinc · 2025-09-30T17:09:25Z

src/ipc/capnp/chain.capnp

+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+@0x94f21a4864bd2c65;


Could we add a simple explanation comment for these salts?

re: #29409 (comment)

Could we add a simple explanation comment for these salts?

Not sure about this. Is there a comment you would suggest? I feel like ids are built into the syntax of Cap'n Proto and ubiquitous. If you aren't familiar with Cap'n Proto, I think a comment here would probably just distract you from more semantically relevant parts of the file and leave you confused. Commenting on IDs in capnproto seems like commenting on #ifndef include guards at the top of c++ header files. It would be hard to know where to begin or what information would be helpful.

l0rinc · 2025-09-30T17:26:41Z

src/ipc/capnp/chain.capnp

+    destroy @0 (context :Proxy.Context) -> ();
+    getHeight @1 (context :Proxy.Context) -> (result :Int32, hasResult :Bool);


I wonder how schema evolution is handled by Cap'n Proto - what happens if a field is deprecated and removed or new values are inserted, do I have to check all existing values to make sure I can find a new valid id?

https://capnproto.org/language.html#evolving-your-protocol claims:

New fields, enumerants, and methods may be added to structs, enums, and interfaces, respectively, as long as each new member’s number is larger than all previous members. Similarly, new fields may be added to existing groups and unions.

New parameters may be added to a method. The new parameters must be added to the end of the parameter list and must have default values.

Members can be re-arranged in the source code, so long as their numbers stay the same.

So I guess if you change any value and deprecate the old one you will have to add the new version with max + 1 - wanted to suggest that we could add 10 opportunities between values like:

destroy @10 ... getHeight @20...

so that when we change destroy to return a bool or add a new method, we can do it as:

construct @0 ... destroy @10 ... # deprecated destruct @11 ... getHeight @20...

but it appears that's not possible.

re: #29409 (comment)

I wonder how schema evolution is handled by Cap'n Proto - what happens if a field is deprecated and removed or new values are inserted, do I have to check all existing values to make sure I can find a new valid id?

Yes you do. You need to pick the next unused ID. I think it actually works pretty well in practice, especially if the id's are mostly in order, but it's also possible to include # Next unused id: 42 comments if they are really out of order and it's hard to find the next one.

we could add 10 opportunities between values [...] but it appears that's not possible

Yes like you point out, gaps are not allowed. Technically I think the compiler could allow gaps for interfaces, but for structs it couldn't work because the order fields are added to the struct defines its binary layout. For interfaces the restriction also seems reasonable to make interfaces consistent with structs, and make it possible to see what order methods were added over time.

Since it's also possible to move and rename deprecated fields, this restriction doesn't get in the way of making capnp files readble. Renaming also lets you break source compatibility and require source code to be updated, while maintaining binary compatibility so old binaries and code using previous definitions continue to work. In general, I think the system is very flexible and functions pretty nicely.

l0rinc · 2025-11-22T16:52:56Z

src/ipc/capnp/chain.capnp

+# Copyright (c) 2024 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.


super-nit: I assume the copyright headers are similar to other code in the repo:

Suggested change

# Copyright (c) 2024 The Bitcoin Core developers

# Distributed under the MIT software license, see the accompanying

# file COPYING or http://www.opensource.org/licenses/mit-license.php.

# Copyright (c) 2024-present The Bitcoin Core developers

# Distributed under the MIT software license, see the accompanying

# file COPYING or https://opensource.org/license/mit.

re: #29409 (comment)

super-nit: I assume the copyright headers are similar to other code in the repo:

Nice catch, fixed this up

l0rinc · 2025-11-25T12:01:01Z

src/ipc/capnp/chain.capnp

+    getTransactionAncestry @19 (context :Proxy.Context, txid :Data) -> (ancestors :UInt64, descendants :UInt64, ancestorsize :UInt64, ancestorfees :Int64);
+    calculateIndividualBumpFees @20 (context :Proxy.Context, outpoints :List(Data), targetFeerate :Data) -> (result: List(Common.PairInt64(Data)));
+    calculateCombinedBumpFee @21 (context :Proxy.Context, outpoints :List(Data), targetFeerate :Data) -> (result :Int64, hasResult :Bool);
+    getPackageLimits @22 (context :Proxy.Context) -> (ancestors :UInt64, descendants :UInt64);


May be deliberate, but the parameters in

bitcoin/src/interfaces/chain.h

Line 267 in 5fe753b

virtual void getPackageLimits(unsigned int& limit_ancestor_count, unsigned int& limit_descendant_count) = 0;

are 32 bit

re: #29409 (comment)

May be deliberate, but the parameters in [...] are 32 bit

Good catch. Updated these. Using a bigger type was harmless but not intentional. (It is a compiler error if you use a smaller type.)

l0rinc · 2025-11-25T12:02:35Z

src/ipc/capnp/chain.capnp

+interface ChainNotifications $Proxy.wrap("interfaces::Chain::Notifications") {
+    destroy @0 (context :Proxy.Context) -> ();
+    transactionAddedToMempool @1 (context :Proxy.Context, tx :Data) -> ();
+    transactionRemovedFromMempool @2 (context :Proxy.Context, tx :Data, reason :Int32) -> ();


Original has enums for reason

bitcoin/src/interfaces/chain.h

Line 323 in 5fe753b

virtual void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {}

should we make the param UInt32 like we did one line below for ChainstateRole

Suggested change

transactionRemovedFromMempool @2 (context :Proxy.Context, tx :Data, reason :Int32) -> ();

transactionRemovedFromMempool @2 (context :Proxy.Context, tx :Data, reason :UInt32) -> ();

Or vice versa, since Int32 seems more common (though UInt32 may make more sense).

But looking at #29409 (comment), you mentioned:

After bitcoin-core/libmultiprocess#120 it should also be a compile error if incompatible types are used

which was merged since - shouldn't it be a compile error now?

re: #29409 (comment)

should we make the param UInt32 like we did one line below for ChainstateRole

Nice find noticing the inconsistency. Both types should be safe because casts between signed and unsigned types are well-defined. I tend to prefer signed types for enums so if the enums use -1, -2, -3 values they will be displayed nicely. But unsigned types could be better for some enums, particularly bitmasks. I updated chainstate role enums variables to be signed for consistency.

l0rinc · 2025-11-25T12:26:38Z

src/ipc/capnp/chain.capnp

+    schedulerMockForward @7 (context :Proxy.Context, time :Int64) -> ();
+}
+
+struct FeeCalculation $Proxy.wrap("FeeCalculation") {


best_height seems to be missing:

bitcoin/src/policy/fees/block_policy_estimator.h

Line 98 in 1a7fb5e

unsigned int best_height{0};

introduced in
1a7fb5e#diff-722dfe5b8bcfb83b1d2f2c9b10800a873325eac8dfc343239f6c851220508c4eR98

re: #29409 (comment)

best_height seems to be missing:

Great catch! This is a real bug. Fixed and added comment to the C++ struct to prevent these getting out of sync in the future.

l0rinc · 2025-11-25T13:04:58Z

src/interfaces/chain.h

    //! Depending on the action returned by the update function, this will either
    //! update the setting in memory or write the updated settings to disk.
-    virtual bool updateRwSetting(const std::string& name, const SettingsUpdate& update_function) = 0;
+    virtual bool updateRwSetting(const std::string& name, SettingsUpdate update_function) = 0;


Do we have to wait for SettingsValue to be exposed for the other consts to be removed as well?
Looks a bit weird that this isn't consted anymore, while other similar ones still are:

bitcoin/src/interfaces/node.h

Lines 111 to 116 in 5336bcd

//! Update a setting in <datadir>/settings.json.

virtual void updateRwSetting(const std::string& name, const common::SettingsValue& value) = 0;

//! Force a setting value to be applied, overriding any other configuration

//! source, but not being persisted.

virtual void forceSetting(const std::string& name, const common::SettingsValue& value) = 0;

re: #29409 (comment)

Do we have to wait for SettingsValue to be exposed for the other consts to be removed as well?

It looks like const references there could be removed, but did you mean to link to a different interface file? The node.h interface is used by the GUI, unlike the chain.h interface used by wallet. But it could be changed in a separate PR and I noted some other ideas for cleaning up the settings interfaces previously in #30697 (comment) and #30828 (review)

l0rinc · 2025-11-25T13:07:19Z

src/rpc/request.h

    std::any context;
    JSONRPCVersion m_json_version = JSONRPCVersion::V1_LEGACY;
+    // Note: If you add fields to this struct, you should also update the
+    // JSONRPCRequest struct in ipc/capnp/chain.capnp.


can't we auto-detect that via reflection maybe?

re: #29409 (comment)

can't we auto-detect that via reflection maybe?

Yes there are different ways that could be detected automatically, not even requiring reflection necessarily. Like you could declare a duplicate struct and assert the sizes of the structs are the same. I think it would actually be possible for the code generator to do this automatically.

I do think the comment here makes sense even with additional enforcement, though since it should be helpful to anyone updating this code or reviewing changes to it.

l0rinc · 2025-11-25T13:26:51Z

src/ipc/capnp/common-types.h

+    return T({array.begin(), array.begin() + array.size()});
+}
+
+//! Convert base_blob to kj::ArrayPtr.


while ToArray seems to be used mostly for uint256, there are lots of DataStream calls to it which is not base_blob

Suggested change

//! Convert base_blob to kj::ArrayPtr.

//! Convert contiguous byte data (e.g. base_blob, DataStream) to kj::ArrayPtr.

re: #29409 (comment)

while ToArray seems to be used mostly for uint256, there are lots of DataStream calls to it which is not base_blob

Thanks took your suggestion and also updated the ToBlob comment

l0rinc · 2025-11-25T13:33:00Z

src/ipc/capnp/chain.cpp

+{
+    // This method is never called because ChainClient::Start is overridden by
+    // WalletLoader::Start. The custom implementation is needed just because
+    // the CScheduler& argument this is supposed to pass is not serializable.


is this still accurate?

re: #29409 (comment)

is this still accurate?

Yes the ChainClient::start method does still take a scheduler parameter (so the node and wallet can share a scheduler.

bitcoin/src/interfaces/chain.h

Line 417 in 938d7aa

virtual void start(CScheduler& scheduler) = 0;

This could probably be changed, but the PR is trying to avoid changing the interface or changing behavior at all.

ryanofsky

Thanks for the close review, and sorry for the delay responding and updating this. You found a lot of annoying discrepencies and a real bug in the estimateSmartFee best_height return value, which should all be fixed now.

Rebased d9efd1e -> 89cf624 (pr/ipc-chain.21 -> pr/ipc-chain.22, compare) fixing conflict with #33774 and making suggested updates

Rebased 89cf624 -> b2cbdd3 (pr/ipc-chain.22 -> pr/ipc-chain.23, compare)

ryanofsky · 2025-12-12T13:09:04Z

src/ipc/capnp/chain.capnp

+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+@0x94f21a4864bd2c65;


re: #29409 (comment)

Could we add a simple explanation comment for these salts?

Not sure about this. Is there a comment you would suggest? I feel like ids are built into the syntax of Cap'n Proto and ubiquitous. If you aren't familiar with Cap'n Proto, I think a comment here would probably just distract you from more semantically relevant parts of the file and leave you confused. Commenting on IDs in capnproto seems like commenting on #ifndef include guards at the top of c++ header files. It would be hard to know where to begin or what information would be helpful.

ryanofsky · 2025-12-12T13:26:21Z

src/ipc/capnp/chain.capnp

+    destroy @0 (context :Proxy.Context) -> ();
+    getHeight @1 (context :Proxy.Context) -> (result :Int32, hasResult :Bool);


re: #29409 (comment)

I wonder how schema evolution is handled by Cap'n Proto - what happens if a field is deprecated and removed or new values are inserted, do I have to check all existing values to make sure I can find a new valid id?

Yes you do. You need to pick the next unused ID. I think it actually works pretty well in practice, especially if the id's are mostly in order, but it's also possible to include # Next unused id: 42 comments if they are really out of order and it's hard to find the next one.

we could add 10 opportunities between values [...] but it appears that's not possible

Yes like you point out, gaps are not allowed. Technically I think the compiler could allow gaps for interfaces, but for structs it couldn't work because the order fields are added to the struct defines its binary layout. For interfaces the restriction also seems reasonable to make interfaces consistent with structs, and make it possible to see what order methods were added over time.

Since it's also possible to move and rename deprecated fields, this restriction doesn't get in the way of making capnp files readble. Renaming also lets you break source compatibility and require source code to be updated, while maintaining binary compatibility so old binaries and code using previous definitions continue to work. In general, I think the system is very flexible and functions pretty nicely.

ryanofsky · 2025-12-12T13:31:15Z

src/ipc/capnp/chain.capnp

+# Copyright (c) 2024 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.


re: #29409 (comment)

super-nit: I assume the copyright headers are similar to other code in the repo:

Nice catch, fixed this up

ryanofsky · 2025-12-12T13:32:23Z

src/ipc/capnp/chain.capnp

+    getTransactionAncestry @19 (context :Proxy.Context, txid :Data) -> (ancestors :UInt64, descendants :UInt64, ancestorsize :UInt64, ancestorfees :Int64);
+    calculateIndividualBumpFees @20 (context :Proxy.Context, outpoints :List(Data), targetFeerate :Data) -> (result: List(Common.PairInt64(Data)));
+    calculateCombinedBumpFee @21 (context :Proxy.Context, outpoints :List(Data), targetFeerate :Data) -> (result :Int64, hasResult :Bool);
+    getPackageLimits @22 (context :Proxy.Context) -> (ancestors :UInt64, descendants :UInt64);


re: #29409 (comment)

May be deliberate, but the parameters in [...] are 32 bit

Good catch. Updated these. Using a bigger type was harmless but not intentional. (It is a compiler error if you use a smaller type.)

ryanofsky · 2025-12-12T13:35:30Z

src/ipc/capnp/chain.capnp

+interface ChainNotifications $Proxy.wrap("interfaces::Chain::Notifications") {
+    destroy @0 (context :Proxy.Context) -> ();
+    transactionAddedToMempool @1 (context :Proxy.Context, tx :Data) -> ();
+    transactionRemovedFromMempool @2 (context :Proxy.Context, tx :Data, reason :Int32) -> ();


re: #29409 (comment)

should we make the param UInt32 like we did one line below for ChainstateRole

Nice find noticing the inconsistency. Both types should be safe because casts between signed and unsigned types are well-defined. I tend to prefer signed types for enums so if the enums use -1, -2, -3 values they will be displayed nicely. But unsigned types could be better for some enums, particularly bitmasks. I updated chainstate role enums variables to be signed for consistency.

ryanofsky · 2025-12-12T13:44:04Z

src/ipc/capnp/chain.capnp

+    schedulerMockForward @7 (context :Proxy.Context, time :Int64) -> ();
+}
+
+struct FeeCalculation $Proxy.wrap("FeeCalculation") {


re: #29409 (comment)

best_height seems to be missing:

Great catch! This is a real bug. Fixed and added comment to the C++ struct to prevent these getting out of sync in the future.

ryanofsky · 2025-12-12T13:47:18Z

src/interfaces/chain.h

    //! Depending on the action returned by the update function, this will either
    //! update the setting in memory or write the updated settings to disk.
-    virtual bool updateRwSetting(const std::string& name, const SettingsUpdate& update_function) = 0;
+    virtual bool updateRwSetting(const std::string& name, SettingsUpdate update_function) = 0;


re: #29409 (comment)

Do we have to wait for SettingsValue to be exposed for the other consts to be removed as well?

It looks like const references there could be removed, but did you mean to link to a different interface file? The node.h interface is used by the GUI, unlike the chain.h interface used by wallet. But it could be changed in a separate PR and I noted some other ideas for cleaning up the settings interfaces previously in #30697 (comment) and #30828 (review)

ryanofsky · 2025-12-12T13:49:31Z

src/rpc/request.h

    std::any context;
    JSONRPCVersion m_json_version = JSONRPCVersion::V1_LEGACY;
+    // Note: If you add fields to this struct, you should also update the
+    // JSONRPCRequest struct in ipc/capnp/chain.capnp.


re: #29409 (comment)

can't we auto-detect that via reflection maybe?

Yes there are different ways that could be detected automatically, not even requiring reflection necessarily. Like you could declare a duplicate struct and assert the sizes of the structs are the same. I think it would actually be possible for the code generator to do this automatically.

I do think the comment here makes sense even with additional enforcement, though since it should be helpful to anyone updating this code or reviewing changes to it.

ryanofsky · 2025-12-12T13:52:57Z

src/ipc/capnp/common-types.h

+    return T({array.begin(), array.begin() + array.size()});
+}
+
+//! Convert base_blob to kj::ArrayPtr.


re: #29409 (comment)

while ToArray seems to be used mostly for uint256, there are lots of DataStream calls to it which is not base_blob

Thanks took your suggestion and also updated the ToBlob comment

ryanofsky · 2025-12-12T13:54:38Z

src/ipc/capnp/chain.cpp

+{
+    // This method is never called because ChainClient::Start is overridden by
+    // WalletLoader::Start. The custom implementation is needed just because
+    // the CScheduler& argument this is supposed to pass is not serializable.


re: #29409 (comment)

is this still accurate?

Yes the ChainClient::start method does still take a scheduler parameter (so the node and wallet can share a scheduler.

bitcoin/src/interfaces/chain.h

Line 417 in 938d7aa

virtual void start(CScheduler& scheduler) = 0;

This could probably be changed, but the PR is trying to avoid changing the interface or changing behavior at all.

- Add capnp ToBlob, ToArray, Wrap, Serialize, and Unserialize helper functions - Add support for std::chrono::seconds capnp serialization - Add support for util::Result capnp serialization

Expose Chain interface to external processes spawning or connecting to bitcoin-node.

ryanofsky · 2026-01-06T20:59:51Z

Rebased b2cbdd3 -> b7b6d6c (pr/ipc-chain.23 -> pr/ipc-chain.24, compare) due to silent conflict with #30214

ryanofsky mentioned this pull request Feb 8, 2024

Multiprocess tracking issue #28722

Open

ryanofsky marked this pull request as ready for review February 8, 2024 14:04

DrahtBot added the CI failed label Feb 8, 2024

ryanofsky force-pushed the pr/ipc-chain branch from 22e0549 to 2fd1042 Compare February 8, 2024 20:45

DrahtBot removed the CI failed label Feb 8, 2024

DrahtBot mentioned this pull request Feb 21, 2024

serialization: Support for multiple parameters #28929

Merged

cbergqvist reviewed Feb 28, 2024

View reviewed changes

src/ipc/capnp/common-types.h Show resolved Hide resolved

src/ipc/capnp/chain-types.h Outdated Show resolved Hide resolved

src/ipc/capnp/common-types.h Outdated Show resolved Hide resolved

ryanofsky force-pushed the pr/ipc-chain branch from 2fd1042 to 41831ab Compare February 28, 2024 16:08

ryanofsky commented Feb 28, 2024

View reviewed changes

src/ipc/capnp/common-types.h Show resolved Hide resolved

src/ipc/capnp/chain-types.h Outdated Show resolved Hide resolved

src/ipc/capnp/common-types.h Outdated Show resolved Hide resolved

cbergqvist approved these changes Feb 28, 2024

View reviewed changes

DrahtBot added the CI failed label Feb 28, 2024

ariard reviewed Mar 7, 2024

View reviewed changes

DrahtBot marked this pull request as draft April 17, 2024 17:34

DrahtBot added the Needs rebase label May 15, 2024

ryanofsky force-pushed the pr/ipc-chain branch from 41831ab to 6608b03 Compare June 11, 2024 01:53

ryanofsky marked this pull request as ready for review June 11, 2024 01:55

DrahtBot removed the Needs rebase label Jun 11, 2024

ryanofsky mentioned this pull request Jun 11, 2024

Multiprocess bitcoin #10102

Draft

ryanofsky force-pushed the pr/ipc-chain branch from 6608b03 to ab6b795 Compare June 11, 2024 19:10

DrahtBot removed the CI failed label Jun 12, 2024

DrahtBot mentioned this pull request Jul 12, 2024

ipc: add bitcoin-mine test program #30437

Open

ryanofsky mentioned this pull request Jul 23, 2024

multiprocess: Add -ipcbind option to bitcoin-node #30509

Merged

DrahtBot mentioned this pull request Jul 23, 2024

multiprocess: Add IPC wrapper for Mining interface #30510

Merged

ryanofsky force-pushed the pr/ipc-chain branch from ab6b795 to eec0d31 Compare July 26, 2024 16:05

ryanofsky commented Jul 26, 2024

View reviewed changes

ryanofsky marked this pull request as draft July 26, 2024 16:35

ryanofsky mentioned this pull request Sep 29, 2025

SIGABRT in ~ProxyClientBase with #29409 and rust client bitcoin-core/libmultiprocess#219

Open

ryanofsky force-pushed the pr/ipc-chain branch from 7ba7ec9 to 7a526a1 Compare September 30, 2025 13:58

ryanofsky force-pushed the pr/ipc-chain branch 2 times, most recently from 2ae18d0 to 2763976 Compare October 22, 2025 09:28

DrahtBot mentioned this pull request Nov 6, 2025

cmake: Move IPC tests to ipc/test #33774

Merged

ryanofsky mentioned this pull request Nov 13, 2025

Mining interface tracking issue #33777

Open

15 tasks

ryanofsky force-pushed the pr/ipc-chain branch from 2763976 to d9efd1e Compare November 20, 2025 18:51

DrahtBot added CI failed and removed CI failed labels Nov 20, 2025

l0rinc suggested changes Nov 25, 2025

View reviewed changes

DrahtBot added the Needs rebase label Dec 4, 2025

ryanofsky mentioned this pull request Dec 8, 2025

consider adding a new interface RawTxFeed on Mining IPC #34030

Closed

ryanofsky force-pushed the pr/ipc-chain branch from d9efd1e to 89cf624 Compare December 12, 2025 12:18

DrahtBot removed the Needs rebase label Dec 12, 2025

ryanofsky commented Dec 12, 2025

View reviewed changes

DrahtBot added CI failed and removed CI failed labels Dec 12, 2025

DrahtBot mentioned this pull request Dec 15, 2025

mining: add getTransactions(ByWitnessID) IPC methods #34020

Draft

1 task

ryanofsky force-pushed the pr/ipc-chain branch from 89cf624 to b2cbdd3 Compare December 16, 2025 02:09

bitcoin deleted a comment from Big621 Dec 21, 2025

ryanofsky and others added 4 commits January 6, 2026 05:45

Add capnp serialization code for bitcoin types

7a247cd

- Add capnp ToBlob, ToArray, Wrap, Serialize, and Unserialize helper functions - Add support for std::chrono::seconds capnp serialization - Add support for util::Result capnp serialization

Add capnp wrapper for Handler interface

7f4cd21

Add capnp wrapper for Chain interface

ea89449

multiprocess: Expose Chain interface

b7b6d6c

Expose Chain interface to external processes spawning or connecting to bitcoin-node.

ryanofsky force-pushed the pr/ipc-chain branch from b2cbdd3 to b7b6d6c Compare January 6, 2026 20:58

ryanofsky mentioned this pull request Jan 12, 2026

crash during IPC cleanup after interruptWait() is called #34250

Open

		destroy @0 (context :Proxy.Context) -> ();
		getHeight @1 (context :Proxy.Context) -> (result :Int32, hasResult :Bool);

	transactionRemovedFromMempool @2 (context :Proxy.Context, tx :Data, reason :Int32) -> ();
	transactionRemovedFromMempool @2 (context :Proxy.Context, tx :Data, reason :UInt32) -> ();

	//! Update a setting in <datadir>/settings.json.
	virtual void updateRwSetting(const std::string& name, const common::SettingsValue& value) = 0;

	//! Force a setting value to be applied, overriding any other configuration
	//! source, but not being persisted.
	virtual void forceSetting(const std::string& name, const common::SettingsValue& value) = 0;

	//! Convert base_blob to kj::ArrayPtr.
	//! Convert contiguous byte data (e.g. base_blob, DataStream) to kj::ArrayPtr.

multiprocess: Add capnp wrapper for Chain interface #29409

Are you sure you want to change the base?

multiprocess: Add capnp wrapper for Chain interface #29409

Conversation

ryanofsky commented Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

DrahtBot commented Feb 8, 2024

Uh oh!

cbergqvist left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cbergqvist left a comment

Choose a reason for hiding this comment

Uh oh!

ariard left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sedited commented Jul 5, 2024

Uh oh!

ryanofsky commented Jul 8, 2024

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sedited commented Sep 29, 2025

Uh oh!

ryanofsky commented Sep 29, 2025

Uh oh!

ryanofsky commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

l0rinc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ryanofsky commented Feb 8, 2024 •

edited

Loading

DrahtBot commented Feb 8, 2024 •

edited

Loading

ryanofsky commented Sep 30, 2025 •

edited

Loading

ryanofsky Dec 12, 2025 •

edited

Loading

ryanofsky left a comment •

edited

Loading

ryanofsky Dec 12, 2025 •

edited

Loading