Create an async corepc-client by jamillambert · Pull Request #505 · rust-bitcoin/corepc

jamillambert · 2026-02-12T18:26:34Z

This PR adds an async corepc-client with the RPCs used by BDK.

The patches are structured to make it easier to see what was changed from sync to async. First the sync version is copied and renamed, then in a separate patch it is changed to async.

tcharding · 2026-02-16T03:03:11Z

@apoelstra whats your take man? Would you be willing to have jsonrpc take changes to be able to use bitreq in an async manner? It obviously smashes the dep tree if --all-features are used.

tcharding · 2026-02-20T02:20:05Z

Holla at me if/when you would like some review mate.

jamillambert · 2026-02-20T15:22:25Z

Rewrote the async client:

Removed all the version folders where the RPC macros were defined.
Removed macroization from client_async module.
Created bdk_client module that implements the the required RPC methods in it using the shared client methods in mod.rs.
- Supports v25 to v30 by checking the server version and then using the required vtype version before converting into the model type.
- The bdk_client RPC methods all return the modelled types. This made the error handling difficult since all of the error types for converting into the modelled type are version specific types. I am not sure how to handle this ergonomically. Currently they all return the same Error type defined in the async client.
- There are a few RPCs that return a single field, e.g. getblock returns a Block and getblockcount a u32. Would it be better to return e.g. Result<u32> instead of Result<GetBlockCount> ?
Rewrote the tests to use the new bdk_client rpc calls.

@tcharding thoughts? in particular the error handling of converting to the modelled type. Returning a modelled type vs the inner field e.g. Block or u32. And having a bdk_client module that only implements the RPCs, the idea was to be able to add another one for e.g. LDK with their required RPCs and arguments but sharing the new, call etc. methods.

tcharding · 2026-02-20T20:51:51Z

Rewrote the async client:

Removed all the version folders where the RPC macros were defined.

Removed macroization from client_async module.

Created bdk_client module that implements the the required RPC methods in it using the shared client methods in mod.rs.

After writing a massive post I realised this solution is not going to work (see bottom for explanation). I think we should inline the bdk module and just document that its designed explicitly with that project in mind.

How about throwing this at the top:

//! Async JSON-RPC client designed explicitly to support BDK.
//!
//! ## Project decisions
//!
//! * Support Core versions 25 to 30.

Supports v25 to v30 by checking the server version and then using the required vtype version before converting into the model type.

Troublesome, commented already on the code.

The bdk_client RPC methods all return the modelled types. This made the error handling difficult since all of the error types for converting into the modelled type are version specific types. I am not sure how to handle this ergonomically. Currently they all return the same Error type defined in the async client.

Another solution is to have an error type for each function. Then it can have a variant (assuming its an enum) for the exact error returned by into_model.

There are a few RPCs that return a single field, e.g. getblock returns a Block and getblockcount a u32. Would it be better to return e.g. Result<u32> instead of Result<GetBlockCount> ?

Agreed that a wrapper type is not useful to returen (eg GetBlockCount). At first blush I think you are correct and returning the inner type is better (its a u64 in this case). After upgrade to bitcoin v0.33 we may have better concrete types to return if needed but your argument stands.

Rewrote the tests to use the new bdk_client rpc calls.

Nice.

@tcharding thoughts? in particular the error handling of converting to the modelled type. Returning a modelled type vs the inner field e.g. Block or u32. And having a bdk_client module that only implements the RPCs, the idea was to be able to add another one for e.g. LDK with their required RPCs and arguments but sharing the new, call etc. methods.

I don't think separation by project module is not going to work because the modules will conflict with each other. And if we feature gate the features will not be additive. We could try some macro stuff but I think it defeats the purpose which is to write a simple clean client that others can fork if they need to.

apoelstra · 2026-02-25T18:53:17Z

@apoelstra whats your take man? Would you be willing to have jsonrpc take changes to be able to use bitreq in an async manner? It obviously smashes the dep tree if --all-features are used.

Would this then tie all users of the crate to tokio? Why can't it be executor-agnostic?

jamillambert · 2026-02-25T19:22:21Z

@apoelstra whats your take man? Would you be willing to have jsonrpc take changes to be able to use bitreq in an async manner? It obviously smashes the dep tree if --all-features are used.

Would this then tie all users of the crate to tokio? Why can't it be executor-agnostic?

The changes to jsonrpc to make it support async is executor-agnostic. It is only the use of bitreq and async that requires tokio in the current state of this PR.

There is a lint error: this expression creates a reference which is immediately dereferenced by the compiler. Fix it as suggested by the compiler.

In preparation for adding bitreq_http_async feature to jsonrpc move the sync version to the client-sync feature so it is not always on with jsonrpc.

These will both be used by the async client. Move them out of sync client in preparation. Edit the logging function so it doesn't panic.

Create a new folder for the upcoming async client and copy in the existing client_sync code. Code copy only to make the next patch easier to review.

tcharding · 2026-07-08T02:55:00Z

Were you planning on doing the corepc-regtest-client rename as part of this or afterwards? At a cursory look everything looks good.

jamillambert · 2026-07-09T08:45:26Z

Were you planning on doing the corepc-regtest-client rename as part of this or afterwards? At a cursory look everything looks good.

I was planning on doing the rename afterwards since it also requires changing the current testing client.

tcharding · 2026-07-09T22:10:42Z

Sweet. Whats left to undraft this then?

jamillambert · 2026-07-13T18:13:10Z

Sweet. Whats left to undraft this then?

I got it to work locally on BDK. It did require breaking changes due to being async, but I think this is the right choice. This testing resulted in the last patch to make the use in BDK easier, and probably other downstream too.

Undrafted.

Edit the copy of the sync client created in the previous commit to be async. Update the readme and cargo.toml files. Replace macros with functions. There is only one async client so the macros are not needed anymore. Create a new module for the bdk client that has the required RPCs in it that return either the rust-bitcoin type or non-version specific model types.

Add RpcMethodErrorExt to the async error layer and implement it for all method-specific error enums generated by define_method_error. This keeps per-method typed errors as the primary API while providing a common inspection surface for downstream crates. Centralize JSON-RPC not-found detection (-5) and transport detection in one place and reuse that logic across trait impls. Also document the intended usage (generic handling via the trait vs. method-specific enum matching) and add unit tests covering not-found, other RPC codes, transport, model defaults, and trait coverage for all public async method error enums.

tcharding · 2026-07-15T01:52:28Z

+    RpcMethodErrorExt, ServerVersionError, UnexpectedServerVersionError,
+};
+pub use crate::client_async::rpcs::RpcApi;
+pub(crate) use crate::{into_json, log_response};


This import is unusual, why the pub(crate)?

tcharding · 2026-07-15T02:02:20Z

+
+#![cfg(feature = "v30_and_below")]
+#![cfg(not(feature = "v24_and_below"))]
+#![allow(non_snake_case)] // Test names intentionally use double underscore.


I think we would be better off without this.

tcharding · 2026-07-15T02:02:45Z

+#![cfg(feature = "v30_and_below")]
+#![cfg(not(feature = "v24_and_below"))]


Perhaps add a comment stating why we only test versions 25-30

tcharding · 2026-07-15T02:11:03Z

+async fn async__get_best_block_hash__modelled() {
+    let node = BitcoinD::with_wallet(Wallet::None, &[]);
+    let client = async_client_for(&node);
+
+    let model: Result<bitcoin::BlockHash, GetBestBlockHashError> =
+        client.get_best_block_hash().await;
+    let model = model.unwrap();
+    let expected = node.client.best_block_hash().expect("best_block_hash");
+    assert_eq!(model, expected);
+}


What if we do this for the first test and for subsequent tests the same but with the comments?

async fn get_best_block_hash() { let node = BitcoinD::with_wallet(Wallet::None, &[]); let client = async_client_for(&node); // Tests that the async-client has this function. let got = client.get_best_block_hash().await.unwrap(); // Grabs the block hash using the sync client which we know works. let want = node.client.best_block_hash().expect("best_block_hash"); // Tests tat the async client returned the same block hash as the async client. assert_eq!(got, want); }

The reason is that we are expecting BDK devs to look at this test file. We are just testing the functionality and how usage of the client might look. I'm undecided about the explicit error stuff, leaning towards not testing it here and just making sure we get it right. There are not that many errors anyways. Open to your thoughts though?

tcharding · 2026-07-15T02:12:24Z

Everything else looks good. The error stuff is pretty funky but I don't have a better solution right now. We can change it later if we want to.

tcharding · 2026-07-15T04:10:05Z

Hey mate, I had a bit of a play because I wanted to get rid of the Raw<Box> stuff, in the end I did not get very far but a bunch of other little cleanups fell out: #664

github-actions Bot added C-client C-integration-test C-jsonrpc labels Feb 12, 2026

jamillambert force-pushed the 0212-WIP-client-async branch from d5524e1 to 9a77aa2 Compare February 12, 2026 18:37

jamillambert changed the title ~~WIP: Create an async corepc-client by adding async support to jsonrpc~~ WIP: Create an async corepc-client (adding async support to jsonrpc) Feb 12, 2026

jamillambert mentioned this pull request Feb 12, 2026

WIP: Create an async corepc-client (adding jsonrpc support into the corepc-client crate) #506

Closed

jamillambert force-pushed the 0212-WIP-client-async branch 2 times, most recently from 6981d8f to 6cd8bb2 Compare February 12, 2026 18:53

tnull mentioned this pull request Feb 13, 2026

Clarify stance on corepc-client to be integration-test-only #507

Closed

tcharding reviewed Feb 16, 2026

View reviewed changes

Comment thread jsonrpc/src/client_async.rs Outdated

tcharding reviewed Feb 16, 2026

View reviewed changes

Comment thread jsonrpc/Cargo.toml

tcharding reviewed Feb 16, 2026

View reviewed changes

Comment thread jsonrpc/Cargo.toml

jamillambert force-pushed the 0212-WIP-client-async branch from 6cd8bb2 to 87dfe15 Compare February 18, 2026 16:58

jamillambert force-pushed the 0212-WIP-client-async branch 4 times, most recently from 1c174a6 to 7a79b87 Compare February 20, 2026 15:09

tcharding reviewed Feb 20, 2026

View reviewed changes

Comment thread client/src/client_async/bdk_client.rs Outdated

tcharding reviewed Feb 20, 2026

View reviewed changes

Comment thread client/src/client_async/bdk_client.rs Outdated

tcharding reviewed Feb 20, 2026

View reviewed changes

Comment thread client/src/client_async/bdk_client.rs Outdated

jamillambert force-pushed the 0212-WIP-client-async branch from 7a79b87 to 2f2ca44 Compare February 25, 2026 16:38

jamillambert force-pushed the 0212-WIP-client-async branch 3 times, most recently from fd21983 to 8448aab Compare February 25, 2026 19:10

jamillambert force-pushed the 0212-WIP-client-async branch from 436d488 to 2750000 Compare June 15, 2026 18:55

jamillambert added 4 commits July 7, 2026 14:46

Fix lint error

073c8fd

There is a lint error: this expression creates a reference which is immediately dereferenced by the compiler. Fix it as suggested by the compiler.

Set bitreq_http feature for sync only

fe8c087

In preparation for adding bitreq_http_async feature to jsonrpc move the sync version to the client-sync feature so it is not always on with jsonrpc.

Move into_json and log_response to crate root

e8d30c6

These will both be used by the async client. Move them out of sync client in preparation. Edit the logging function so it doesn't panic.

Copy client_sync to client_async

8f59c84

Create a new folder for the upcoming async client and copy in the existing client_sync code. Code copy only to make the next patch easier to review.

jamillambert force-pushed the 0212-WIP-client-async branch from 2750000 to 95997c2 Compare July 7, 2026 15:30

github-actions Bot removed the C-jsonrpc label Jul 7, 2026

jamillambert force-pushed the 0212-WIP-client-async branch 2 times, most recently from 90308ff to bde3034 Compare July 7, 2026 16:29

jamillambert force-pushed the 0212-WIP-client-async branch 2 times, most recently from 20a03b0 to 9fb1295 Compare July 13, 2026 18:02

jamillambert marked this pull request as ready for review July 13, 2026 18:08

jamillambert requested review from TheBlueMatt, oleonardolima and tnull as code owners July 13, 2026 18:08

jamillambert added 2 commits July 14, 2026 13:02

jamillambert force-pushed the 0212-WIP-client-async branch from 9fb1295 to 9a64ce6 Compare July 14, 2026 12:03

tcharding mentioned this pull request Jul 14, 2026

Fork or create new crate to replace rust-bitcoincore-rpc bitcoindevkit/bdk_wallet#72

Open

tcharding reviewed Jul 15, 2026

View reviewed changes

tcharding mentioned this pull request Jul 15, 2026

NO MERGE: Review suggestions for 505 #664

Open

		#![cfg(feature = "v30_and_below")]
		#![cfg(not(feature = "v24_and_below"))]

Uh oh!

Conversation

jamillambert commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tcharding commented Feb 16, 2026

Uh oh!

tcharding commented Feb 20, 2026

Uh oh!

jamillambert commented Feb 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tcharding commented Feb 20, 2026

Uh oh!

apoelstra commented Feb 25, 2026

Uh oh!

jamillambert commented Feb 25, 2026

Uh oh!

tcharding commented Jul 8, 2026

Uh oh!

jamillambert commented Jul 9, 2026

Uh oh!

tcharding commented Jul 9, 2026

Uh oh!

jamillambert commented Jul 13, 2026

Uh oh!

tcharding Jul 15, 2026 • 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!

tcharding Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

tcharding Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

tcharding Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

tcharding commented Jul 15, 2026

Uh oh!

tcharding commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jamillambert commented Feb 12, 2026 •

edited

Loading

tcharding Jul 15, 2026 •

edited

Loading