Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/interfaces/chain.h
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ class Chain
//! support for writing null values to settings.json.
//! 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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

//! 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;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)


//! Replace a setting in <datadir>/settings.json with a new value.
//! Null can be passed to erase the setting.
Expand Down Expand Up @@ -408,7 +408,8 @@ class ChainClient
//! Load saved state.
virtual bool load() = 0;

//! Start client execution and provide a scheduler.
//! Start client execution and provide a scheduler. (Scheduler is
//! ignored if client is out-of-process).
virtual void start(CScheduler& scheduler) = 0;

//! Shut down client.
Expand Down
5 changes: 5 additions & 0 deletions src/ipc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,18 @@
# file COPYING or https://opensource.org/license/mit/.

add_library(bitcoin_ipc STATIC EXCLUDE_FROM_ALL
capnp/chain.cpp
capnp/mining.cpp
capnp/protocol.cpp
interfaces.cpp
process.cpp
)

target_capnp_sources(bitcoin_ipc ${CMAKE_CURRENT_SOURCE_DIR}
capnp/chain.capnp
capnp/common.capnp
capnp/echo.capnp
capnp/handler.capnp
capnp/init.capnp
capnp/mining.capnp
capnp/rpc.capnp
Expand All @@ -22,6 +25,8 @@ target_link_libraries(bitcoin_ipc
core_interface
univalue
Boost::headers
PUBLIC
bitcoin_common
)

if(BUILD_TESTS)
Expand Down
107 changes: 107 additions & 0 deletions src/ipc/capnp/chain-types.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright (c) The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or https://opensource.org/license/mit.

#ifndef BITCOIN_IPC_CAPNP_CHAIN_TYPES_H
#define BITCOIN_IPC_CAPNP_CHAIN_TYPES_H

#include <coins.h>
#include <ipc/capnp/chain.capnp.proxy.h>
#include <ipc/capnp/common.capnp.proxy-types.h>
#include <ipc/capnp/handler.capnp.proxy-types.h>
#include <interfaces/chain.h>
#include <kernel/types.h>
#include <policy/fees/block_policy_estimator.h>
#include <primitives/block.h>
#include <rpc/server.h>

//! Specialization of handleRpc needed because it takes a CRPCCommand& reference
//! argument, so a manual cleanup callback is needed to free the passed
//! CRPCCommand struct and proxy ActorCallback object.
template <>
struct mp::ProxyServerMethodTraits<ipc::capnp::messages::Chain::HandleRpcParams>
{
using Context = ServerContext<ipc::capnp::messages::Chain,
ipc::capnp::messages::Chain::HandleRpcParams,
ipc::capnp::messages::Chain::HandleRpcResults>;
static ::capnp::Void invoke(Context& context);
};

//! Specialization of start method needed to provide CScheduler& reference
//! argument.
template <>
struct mp::ProxyServerMethodTraits<ipc::capnp::messages::ChainClient::StartParams>
{
using ChainContext = ServerContext<ipc::capnp::messages::ChainClient,
ipc::capnp::messages::ChainClient::StartParams,
ipc::capnp::messages::ChainClient::StartResults>;
static void invoke(ChainContext& context);
};

namespace mp {
//! Overload CustomBuildMessage, CustomPassMessage, and CustomReadMessage to
//! serialize interfaces::FoundBlock parameters. Custom conversion functions are
//! needed because there is not a 1:1 correspondence between members of the C++
//! FoundBlock class and the Cap'n Proto FoundBlockParam and FoundBlockResult
//! structs. Separate param and result structs are needed because Cap'n Proto
//! only has input and output parameters, not in/out parameters like C++.
void CustomBuildMessage(InvokeContext& invoke_context,
const interfaces::FoundBlock& dest,
ipc::capnp::messages::FoundBlockParam::Builder&& builder);
void CustomPassMessage(InvokeContext& invoke_context,
const ipc::capnp::messages::FoundBlockParam::Reader& reader,
ipc::capnp::messages::FoundBlockResult::Builder&& builder,
std::function<void(const interfaces::FoundBlock&)>&& fn);
void CustomReadMessage(InvokeContext& invoke_context,
const ipc::capnp::messages::FoundBlockResult::Reader& reader,
const interfaces::FoundBlock& dest);

//! Overload CustomBuildMessage and CustomPassMessage to serialize
//! interfaces::BlockInfo parameters. Custom conversion functions are needed
//! because BlockInfo structs contain pointer and reference members, and custom
//! code is needed to deal with their lifetimes. Specifics are described in the
//! implementation of these functions.
void CustomBuildMessage(InvokeContext& invoke_context,
const interfaces::BlockInfo& block,
ipc::capnp::messages::BlockInfo::Builder&& builder);
void CustomPassMessage(InvokeContext& invoke_context,
const ipc::capnp::messages::BlockInfo::Reader& reader,
::capnp::Void builder,
std::function<void(const interfaces::BlockInfo&)>&& fn);

//! CScheduler& server-side argument handling. Skips argument so it can
//! be handled by ProxyServerMethodTraits ChainClient::Start code.
template <typename Accessor, typename ServerContext, typename Fn, typename... Args>
void CustomPassField(TypeList<CScheduler&>, ServerContext& server_context, const Fn& fn, Args&&... args)
{
fn.invoke(server_context, std::forward<Args>(args)...);
}

//! CRPCCommand& server-side argument handling. Skips argument so it can
//! be handled by ProxyServerMethodTraits Chain::HandleRpc code.
template <typename Accessor, typename ServerContext, typename Fn, typename... Args>
void CustomPassField(TypeList<const CRPCCommand&>, ServerContext& server_context, const Fn& fn, Args&&... args)
{
fn.invoke(server_context, std::forward<Args>(args)...);
}

//! Override to avoid assert failures that would happen trying to serialize
//! spent coins. Probably it would be best for Coin serialization code not
//! to assert, but avoiding serialization in this case is harmless.
bool CustomHasValue(InvokeContext& invoke_context, const Coin& coin);

//! Overload CustomBuildMessage and CustomReadMessage to serialize HelpResult.
inline void CustomBuildMessage(InvokeContext& invoke_context, const HelpResult& dest, ipc::capnp::messages::HelpResult::Builder&& builder)
{
builder.setMessage(dest.what());
}

template <typename Input>
void ThrowField(TypeList<HelpResult>, InvokeContext& invoke_context, Input&& input)
{
auto data = input.get().getMessage();
throw HelpResult(std::string(CharCast(data.begin()), data.size()));
}
} // namespace mp

#endif // BITCOIN_IPC_CAPNP_CHAIN_TYPES_H
173 changes: 173 additions & 0 deletions src/ipc/capnp/chain.capnp
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
# Copyright (c) The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit.

@0x94f21a4864bd2c65;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a simple explanation comment for these salts?

@ryanofsky ryanofsky Dec 12, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


using Cxx = import "/capnp/c++.capnp";
$Cxx.namespace("ipc::capnp::messages");

using Proxy = import "/mp/proxy.capnp";
$Proxy.include("interfaces/chain.h");
$Proxy.include("rpc/server.h");
$Proxy.includeTypes("ipc/capnp/chain-types.h");

using Common = import "common.capnp";
using Handler = import "handler.capnp";

interface Chain $Proxy.wrap("interfaces::Chain") {
destroy @0 (context :Proxy.Context) -> ();
getHeight @1 (context :Proxy.Context) -> (result :Int32, hasResult :Bool);
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

getBlockHash @2 (context :Proxy.Context, height :Int32) -> (result :Data);
haveBlockOnDisk @3 (context :Proxy.Context, height :Int32) -> (result :Bool);
findLocatorFork @4 (context :Proxy.Context, locator :Data) -> (result :Int32, hasResult :Bool);
hasBlockFilterIndex @5 (context :Proxy.Context, filterType :UInt8) -> (result :Bool);
blockFilterMatchesAny @6 (context :Proxy.Context, filterType :UInt8, blockHash :Data, filterSet :List(Data)) -> (result :Bool, hasResult :Bool);
findBlock @7 (context :Proxy.Context, hash :Data, block :FoundBlockParam) -> (block :FoundBlockResult, result :Bool);
findFirstBlockWithTimeAndHeight @8 (context :Proxy.Context, minTime :Int64, minHeight :Int32, block :FoundBlockParam) -> (block :FoundBlockResult, result :Bool);
findAncestorByHeight @9 (context :Proxy.Context, blockHash :Data, ancestorHeight :Int32, ancestor :FoundBlockParam) -> (ancestor :FoundBlockResult, result :Bool);
findAncestorByHash @10 (context :Proxy.Context, blockHash :Data, ancestorHash :Data, ancestor :FoundBlockParam) -> (ancestor :FoundBlockResult, result :Bool);
findCommonAncestor @11 (context :Proxy.Context, blockHash1 :Data, blockHash2 :Data, ancestor :FoundBlockParam, block1 :FoundBlockParam, block2 :FoundBlockParam) -> (ancestor :FoundBlockResult, block1 :FoundBlockResult, block2 :FoundBlockResult, result :Bool);
findCoins @12 (context :Proxy.Context, coins :List(Common.Pair(Data, Data))) -> (coins :List(Common.Pair(Data, Data)));
guessVerificationProgress @13 (context :Proxy.Context, blockHash :Data) -> (result :Float64);
hasBlocks @14 (context :Proxy.Context, blockHash :Data, minHeight :Int32, maxHeight: Int32, hasMaxHeight :Bool) -> (result :Bool);
isRBFOptIn @15 (context :Proxy.Context, tx :Data) -> (result :Int32);
isInMempool @16 (context :Proxy.Context, txid :Data) -> (result :Bool);
hasDescendantsInMempool @17 (context :Proxy.Context, txid :Data) -> (result :Bool);
broadcastTransaction @18 (context :Proxy.Context, tx: Data, maxTxFee :Int64, broadcastMethod :Int32) -> (error: Text, result :Bool);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 1c9dcf2

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 1c9dcf2

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #29409 (comment)

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.

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.

IIUC, the concern in both of these cases is that non-C++ clients calling this interface (like rust or python clients) could call this method but leave the tx argument unset or leave the broadcastMethod argument unset, and then the implementation of this method could crash or fail if it is not checking for invalid values.

This is a legitimate concern, and it's not specific to this method or to these parameters. In C++, the compiler enforces at each call site that explicit values need to be passed for all parameters which don't have defaults. But in capnproto, all parameters are optional because fields without explicit defaults have implicit defaults of 0 or null.

The Chain interface was designed to be called by C++ clients, and with C++ clients this issue does not occur because clients don't need to interact with cap'nproto directly. They can call C++ methods with all normal language restrictions enforced, and the calls are forwarded over capnproto.

But if non-c++ other clients are used, they either need to be careful when they omit parameters, or the interface needs to be changed to allow parameters to be safely omitted. One way to do this is to assign default values in interfaces like c6638fa. Another way is to make method implementations handle invalid values better, for example by throwing exceptions instead of asserting.

getTransactionAncestry @19 (context :Proxy.Context, txid :Data) -> (ancestors :UInt64, descendants :UInt64, ancestorsize :UInt64, ancestorfees :Int64);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 1c9dcf2

even though these are positional, I think we can rename descendants -> cluster_count post-cluster mempool (and as this is public facing).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #29409 (comment)

even though these are positional, I think we can rename descendants -> cluster_count post-cluster mempool (and as this is public facing).

Good catch. It looks like the parameter was renamed in commit 957ae23 and this PR was not updated. Currently there is checking to make sure capnproto parameter types and c++ types are compatible (compilation will fail otherwise) but there it isn't checked that parameters have the same names so sometimes names can get out of sync.

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 :UInt32, descendants :UInt32);
checkChainLimits @23 (context :Proxy.Context, tx :Data) -> (result :Common.ResultVoid);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 1c9dcf2

I wonder if this tx isn't the same as 1c9dcf2 (this PR) ?

CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::StageAddition is going to try and dereference.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #29409 (comment)

I wonder if this tx isn't the same as 1c9dcf2 (this PR) ?

CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::StageAddition is going to try and dereference.

Yes this is the same issue. checkChainLimits calls CheckPolicyLimits which calls StageAddition which does not check whether tx is null before dereferencing.

And In this case, the issue applies to C++ clients as well as rust and python clients because C++ clients can also call checkChainLimits with a null pointer.

Depending on what the goal is, there are various ways this could be addressed. One way would be to change checkChainLimits to throw an exception if a null pointer is passed.

In general it's not a realistic goal for this PR to ensure that it's safe to call all Chain methods with all possible arguments and ensure that the node will not crash, but it should be good to do where feasible.

estimateSmartFee @24 (context :Proxy.Context, numBlocks :Int32, conservative :Bool, wantCalc :Bool) -> (calc :Common.FeeCalculation, result :Data);
estimateMaxBlocks @25 (context :Proxy.Context) -> (result :UInt32);
mempoolMinFee @26 (context :Proxy.Context) -> (result :Data);
relayMinFee @27 (context :Proxy.Context) -> (result :Data);
relayIncrementalFee @28 (context :Proxy.Context) -> (result :Data);
relayDustFee @29 (context :Proxy.Context) -> (result :Data);
havePruned @30 (context :Proxy.Context) -> (result :Bool);
getPruneHeight @31 (context :Proxy.Context) -> (result: Int32, hasResult: Bool);
isReadyToBroadcast @32 (context :Proxy.Context) -> (result :Bool);
isInitialBlockDownload @33 (context :Proxy.Context) -> (result :Bool);
shutdownRequested @34 (context :Proxy.Context) -> (result :Bool);
initMessage @35 (context :Proxy.Context, message :Text) -> ();
initWarning @36 (context :Proxy.Context, message :Common.BilingualStr) -> ();
initError @37 (context :Proxy.Context, message :Common.BilingualStr) -> ();
showProgress @38 (context :Proxy.Context, title :Text, progress :Int32, resumePossible :Bool) -> ();
handleNotifications @39 (context :Proxy.Context, notifications :ChainNotifications) -> (result :Handler.Handler);
waitForNotificationsIfTipChanged @40 (context :Proxy.Context, oldTip :Data) -> ();
waitForNotifications @41 (context :Proxy.Context) -> ();
handleRpc @42 (context :Proxy.Context, command :RPCCommand) -> (result :Handler.Handler);
rpcEnableDeprecated @43 (context :Proxy.Context, method :Text) -> (result :Bool);
getSetting @44 (context :Proxy.Context, name :Text) -> (result :Text);
getSettingsList @45 (context :Proxy.Context, name :Text) -> (result :List(Text));
getRwSetting @46 (context :Proxy.Context, name :Text) -> (result :Text);
updateRwSetting @47 (context :Proxy.Context, name :Text, update: SettingsUpdateCallback) -> (result :Bool);
overwriteRwSetting @48 (context :Proxy.Context, name :Text, value :Text, action :Int32) -> (result :Bool);
deleteRwSettings @49 (context :Proxy.Context, name :Text, action: Int32) -> (result :Bool);
requestMempoolTransactions @50 (context :Proxy.Context, notifications :ChainNotifications) -> ();
hasAssumedValidChain @51 (context :Proxy.Context) -> (result :Bool);
}

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) -> ();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original has enums for reason

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

blockConnected @3 (context :Proxy.Context, role: ChainstateRole, block :BlockInfo) -> ();
blockDisconnected @4 (context :Proxy.Context, block :BlockInfo) -> ();
updatedBlockTip @5 (context :Proxy.Context) -> ();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the use for an updatedBlockTip notification with no information about the new tip?

@sedited sedited Dec 9, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this here for context: I guess one of the benefits of the Notifications and NotificationsProxy is avoiding raw pointer types in these interfaces. I went through the exercise of removing the pointer types from the validation interface before. Maybe this could eliminate the need for the logic in the NotificationsProxy in the first place. Not having to handle pointer types is also useful for the kernel API, where conveying the semantics of these pointers in the notifications to the using developer is tricky.

chainStateFlushed @6 (context :Proxy.Context, role: ChainstateRole, locator :Data) -> ();
}

interface ChainClient $Proxy.wrap("interfaces::ChainClient") {
destroy @0 (context :Proxy.Context) -> ();
registerRpcs @1 (context :Proxy.Context) -> ();
verify @2 (context :Proxy.Context) -> (result :Bool);
load @3 (context :Proxy.Context) -> (result :Bool);
start @4 (context :Proxy.Context, scheduler :Void) -> ();
stop @5 (context :Proxy.Context) -> ();
setMockTime @6 (context :Proxy.Context, time :Int64) -> ();
schedulerMockForward @7 (context :Proxy.Context, deltaSeconds :Int64) -> ();
}
Comment on lines +83 to +92

@xyzconstant xyzconstant Apr 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope for this PR, just raising this out of curiosity:

I'm working on an IPC extractor for peer-observer, a non-wallet client that consumes bitcoin-node's interfaces for monitoring. While extending it to cover the Chain interface exposed in this PR, I started to think whether such a tool could fit the ChainClient contract. The doc-comment on ChainClient specifically states this:

Interface to let node manage chain clients (wallets, or maybe tools for monitoring and analysis in the future).

Three of those methods feel pretty wallet-shaped: registerRpcs(), verify() and load(). A non-wallet implementer wouldn't necessarily expose RPCs or have saved state to load/verify.

Would it be worth reducing ChainClient to just lifecycle + mock-time hooks and moving the rest to WalletLoader? Genuinely curious whether there's prior context I'm missing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #29409 (comment)

I'm working on an IPC extractor for peer-observer, a non-wallet client that consumes bitcoin-node's interfaces for monitoring. While extending it to cover the Chain interface exposed in this PR, I started to think whether such a tool could fit the ChainClient contract. The doc-comment on ChainClient specifically states this:

Interface to let node manage chain clients (wallets, or maybe tools for monitoring and analysis in the future).

It's a good question. The doc comment is too ambiguous and should be improved. ChainClient declares methods that bitcoin-node calls and wallets and indexes can implement. It's used in #10102 to let bitcoin-node start a bitcoin-wallet process and control it. In the future the node could also start other processes (indexes, etc) and use this interface to control them. ChainClient probably isn't as likely to be useful for Chain users connecting via IPC as opposed to Chain users being spawned by the node.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get it now, thanks you!


struct RPCCommand $Proxy.wrap("CRPCCommand") {
category @0 :Text;
name @1 :Text;
actor @2 :ActorCallback;
argNames @3 :List(RPCArg);
uniqueId @4 :Int64 $Proxy.name("unique_id");
}

# Representation of std::pair<std::string, bool> in CRPCCommand
struct RPCArg {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this corresponds to

CRPCCommand(std::string category, std::string name, Actor actor, std::vector<std::pair<std::string, bool>> args, intptr_t unique_id)

we might want to ment why this one doesn't have a corresponding proxy:

Suggested change
struct RPCArg {
# transport-only struct corresponding to `std::pair<std::string, bool>` in `CRPCCommand` constructor
struct RPCArg {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #29409 (comment)

we might want to ment why this one doesn't have a corresponding proxy:

Good suggestion, added a version of this comment

name @0 :Text;
namedOnly @1: Bool;
}

interface ActorCallback $Proxy.wrap("ProxyCallback<CRPCCommand::Actor>") {
call @0 (context :Proxy.Context, request :JSONRPCRequest, response :Text, lastCallback :Bool) -> (error :Text $Proxy.exception("std::exception"), rpcError :Text $Proxy.exception("UniValue"), typeError :Text $Proxy.exception("UniValue::type_error"), helpResult :HelpResult $Proxy.exception("HelpResult"), response :Text, result: Bool);
}

struct HelpResult {
message @0 :Text;
}

struct JSONRPCRequest $Proxy.wrap("JSONRPCRequest") {
id @0 :Text;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an accurate mapping of

std::optional<UniValue> id = UniValue::VNULL;

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #29409 (comment)

Is this an accurate mapping of std::optional<UniValue>

Yes, it is. The UniValue fields is serialized as JSON text here. And Cap'n Proto allows text fields to be null so std::nullopt value is just represented by an null text field.

method @1 :Text $Proxy.name("strMethod");
params @2 :Text;
mode @3 :Int32;
uri @4 :Text $Proxy.name("URI");
authUser @5 :Text;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this include the version as well at this point?

@ryanofsky ryanofsky Nov 12, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #29409 (comment)

Should this include the version as well at this point?

Yes, good catch. Added the version field and added a comment to JSONRPCRequest struct to prevent this type of bug in the future. I think this bug did not matter too much in practice because this capnp struct is only used to forward RPC requests from the node to the wallet and wallet shouldn't respond to requests differently based on JSONRPC version, but it is still a bug in the serialization that should be fixed.

peerAddr @6 :Text;
version @7: Int32 $Proxy.name("m_json_version");
}

struct FoundBlockParam {
wantHash @0 :Bool;
wantHeight @1 :Bool;
wantTime @2 :Bool;
wantMaxTime @3 :Bool;
wantMtpTime @4 :Bool;
wantInActiveChain @5 :Bool;
wantLocator @6 :Bool;
nextBlock @7: FoundBlockParam;
wantData @8 :Bool;
}

struct FoundBlockResult {
hash @0 :Data;
height @1 :Int32;
time @2 :Int64;
maxTime @3 :Int64;
mtpTime @4 :Int64;
inActiveChain @5 :Bool;
locator @6 :Data;
nextBlock @7: FoundBlockResult;
data @8 :Data;
found @9 :Bool;
}

struct BlockInfo $Proxy.wrap("interfaces::BlockInfo") {
# Fields skipped below with Proxy.skip are pointer fields manually handled
# by CustomBuildMessage / CustomPassMessage overloads.
hash @0 :Data $Proxy.skip;
prevHash @1 :Data $Proxy.skip;
height @2 :Int32 = -1;
fileNumber @3 :Int32 = -1 $Proxy.name("file_number");
dataPos @4 :UInt32 = 0 $Proxy.name("data_pos");
data @5 :Data $Proxy.skip;
undoData @6 :Data $Proxy.skip;
chainTimeMax @7 :UInt32 = 0 $Proxy.name("chain_time_max");
}

struct ChainstateRole $Proxy.wrap("kernel::ChainstateRole") {
validated @0 :Bool;
historical @1 :Bool;
}

interface SettingsUpdateCallback $Proxy.wrap("ProxyCallback<interfaces::SettingsUpdate>") {
destroy @0 (context :Proxy.Context) -> ();
call @1 (context :Proxy.Context, value :Text) -> (value :Text, result: Int32, hasResult: Bool);
}
Loading
Loading