Skip to content

Refactor: update bdk-bitcoind-rpc to use bdk-bitcoind-client#2119

Open
tvpeter wants to merge 3 commits into
bitcoindevkit:masterfrom
tvpeter:refactor/use-bitcoind-client
Open

Refactor: update bdk-bitcoind-rpc to use bdk-bitcoind-client#2119
tvpeter wants to merge 3 commits into
bitcoindevkit:masterfrom
tvpeter:refactor/use-bitcoind-client

Conversation

@tvpeter
Copy link
Copy Markdown
Collaborator

@tvpeter tvpeter commented Feb 9, 2026

Description

This PR replaces bitcoincore-rpc with bdk-bitcoind-client in bdk-bitcoind-rpc library and its tests and examples.

Depends on bdk-bitcoind-client#5

Notes to the reviewers

  • changed the Emitter to a concrete type
  • In the example crate, I replaced the shared client between threads with two different clients (will come back to improve this later)

Changelog notice

  • replace bitcoincore-rpc with bdk-bitcoind-client

Checklists

All Submissions:

@oleonardolima oleonardolima self-requested a review February 11, 2026 16:45
@oleonardolima oleonardolima added module-blockchain dependencies Pull requests that update a dependency file api A breaking API change labels Feb 11, 2026
@oleonardolima oleonardolima moved this to In Progress in BDK Chain Feb 11, 2026
@ValuedMammal
Copy link
Copy Markdown
Collaborator

Concept ACK

Copy link
Copy Markdown
Collaborator

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

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

cACK 44bfbf9

Comment thread crates/bitcoind_rpc/Cargo.toml Outdated
Comment thread crates/bitcoind_rpc/Cargo.toml Outdated
Comment thread crates/bitcoind_rpc/src/lib.rs Outdated
@oleonardolima oleonardolima added this to the Chain 0.24.0 milestone Mar 19, 2026
@tvpeter tvpeter force-pushed the refactor/use-bitcoind-client branch 2 times, most recently from cf12139 to 3d9ee45 Compare April 23, 2026 05:51
@tvpeter tvpeter marked this pull request as ready for review April 23, 2026 05:56
tvpeter added 2 commits April 30, 2026 04:07
- replace bitcoincore-rpc with bdk_bitcoind_client
- update usage in lib.rs and bip158.rs modules
- Enable bitcoind-client v28_0 and set as
default in bitcoind_rpc
- Update example_bitcoind_rpc_polling to use
bitcoind-client
- Update tests to use bitcoind-client
@tvpeter tvpeter force-pushed the refactor/use-bitcoind-client branch from 3d9ee45 to 959484b Compare April 30, 2026 20:23
Comment thread crates/bitcoind_rpc/tests/common/mod.rs Outdated
Comment thread crates/bitcoind_rpc/src/bip158.rs Outdated
Comment thread crates/bitcoind_rpc/src/lib.rs Outdated
/// contains the next block's block hash (which we use to fetch the next block), we set
/// this to `None` whenever there are no more blocks, or the next block is no longer in the
/// best chain. This gives us an opportunity to re-fetch this result.
last_block: Option<corepc_types::model::GetBlockHeaderVerbose>,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the equivalent of GetBlockResult is GetBlockVerboseOne, but this seems to work as well.

Comment thread crates/bitcoind_rpc/src/lib.rs
Comment thread crates/bitcoind_rpc/tests/test_filter_iter.rs Outdated
Comment thread crates/bitcoind_rpc/Cargo.toml Outdated
Comment thread crates/bitcoind_rpc/Cargo.toml Outdated
Comment thread examples/example_bitcoind_rpc_polling/src/main.rs Outdated
Comment thread crates/bitcoind_rpc/Cargo.toml Outdated
Copy link
Copy Markdown
Collaborator

@ValuedMammal ValuedMammal May 5, 2026

Choose a reason for hiding this comment

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

I get this error when running the filter_iter example, don't know if it's something about my OS. I noticed during the transition from bitcoincore_rpc to bdk_bitcoind_client using bitreq. FWIW I don't have the same problem with SimpleHttpTransport because it can create a fresh tcp socket on the fly.

Matched block 205061
Matched block 208441
Error: JSON-RPC error: transport error: bitreq: Can't assign requested address (os error 49)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I have noticed this randomly when running tests. Sometimes it passes, other times it fails.

@evanlinjin
Copy link
Copy Markdown
Member

evanlinjin commented May 6, 2026

Thanks for this PR.

Before I give it a review, can I please get a bit more context to this Discord message?

In this message, it claims that bdk_bitcoind_client is only meant to support the features we need for BDK. Where else are we talking to bitcoind other than via bdk_bitcoind_rpc?

Sounds like it should NOT be a public crate at all.

Additionally, if the goal is to "support features that we need for BDK", one feature that is missing is the ability to support async IO. The jsonrpc crate that is used internally for bdk_bitcoind_rpc does not support this at all. To elegantly support blocking and async we need some sort of sans-io architecture. I've approached this in electrum_streaming_client, however, I'm not sure if that is the best way about it - would appreciate a look and some opinions.

cc. @notmandatory

@tvpeter tvpeter force-pushed the refactor/use-bitcoind-client branch from 959484b to 73ea0e1 Compare May 6, 2026 15:46
@notmandatory
Copy link
Copy Markdown
Member

As a little recap from our discussion yesterday (please correct me if I've missed something), there are two ways to go with this crate:

  1. move it into a private module in the bdk_bitcoind_rpc crate since it's not intended for use outside of with bdk.
  2. keep it as it's own crate for use as a dependency of bdk_bitcoind_rpc but remove bdk from the name to be more inline with other client crates in the bitcoindevkit org. So something like bitcoind-rpc-client or bitcoind-client but then need to make sure that name is available on crates.io and need to keep it clear in the README it's only going to implement RPC functions and other features needed to support bdk.

The advantage I see of option 1 is it avoids any confusion about the purpose of the client and simplifies release/publishing.

Option 2 could be useful if other related projects like LDK want to use the same bitcoind rpc client for L1 and L2 but they'd need to be OK with exactly the same features bdk needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api A breaking API change dependencies Pull requests that update a dependency file module-blockchain

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

5 participants