Add Blockchain RPC methods #5

tvpeter · 2025-11-18T17:12:00Z

Description

This PR adds the following methods for versions 28, 29, and 30 of bitcoind:

getblock
getblockverbose
getblockcount
getblockhash
getblockfilter
getblockheader
getrawmempool
getrawtransaction
also updates corepc-types to v0.11.0

Fixes #4

Depends on #3

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing

New Features:

I've added tests for the new feature
I've added docs for the new feature
[] I've updated CHANGELOG.md

ValuedMammal

So far I agree with the way you've defined the client methods. I mainly focused on reviewing the implementation of the client code, and haven't spent as much time looking at the test code yet.

By convention, the commits should appear as type(module): Summary, for example

3a16fa4 test: Add integration tests
6f95a7f feat(client): Add blockchain methods

Other notes:

Going forward try to have more descriptive API docs
We should look into testing the client using corepc_node to minimize the amount of manual testing needed and so we can run the integration tests in CI.

ValuedMammal · 2025-11-27T01:10:41Z

src/client.rs

+        let bytes: Vec<u8> = Vec::<u8>::from_hex(&hex_string).map_err(Error::HexToBytes)?;
+
+        let block: Block = deserialize(&bytes)
+            .map_err(|e| Error::InvalidResponse(format!("failed to deserialize block: {e}")))?;
+
+        Ok(block)


For things that can be consensus decoded directly from a hex string, like Block, Header, and Transaction, I think it will be easier to use deserialize_hex and have a new Error::DecodeHex error that wraps the FromHexError.

Suggested change

let bytes: Vec<u8> = Vec::<u8>::from_hex(&hex_string).map_err(Error::HexToBytes)?;

let block: Block = deserialize(&bytes)

.map_err(|e| Error::InvalidResponse(format!("failed to deserialize block: {e}")))?;

Ok(block)

bitcoin::consensus::encode::deserialize_hex(&hex_string).map_err(Error::DecodeHex)

For consistency I would apply the same suggestion to get_block_header and get_raw_transaction.

ValuedMammal · 2025-11-27T01:16:12Z

src/client.rs

+    pub fn get_block_verbose(&self, block_hash: &BlockHash) -> Result<GetBlockVerboseOne, Error> {
+        let res: GetBlockVerboseOne = self.call("getblock", &[json!(block_hash), json!(1)])?;
+        Ok(res)
+    }


I assume we first want to deserialize the response as a type from corepc_types (e.g. v29::GetBlockVerboseOne), and then call into_model on it. Also we'll need a test for it.

yes, but the getblock RPC with verbosity 1 returns a JSON object that deserializes directly into GetBlockVerboseOne here

This method should return the model type corepc_types::model::GetBlockVerboseOne by calling into_model.

src/client.rs

- getblockcount - getblockhash - getblockfilter - getblockheader - getrawmempool - getrawtransaction

- add integration tests for blockchain methods

ValuedMammal · 2025-12-12T19:02:18Z

src/client.rs

+    ///
+    /// # Returns
+    /// The block count as a `u64`
+    pub fn get_block_count(&self) -> Result<u64, Error> {


I think it should return a u32, as it is typically how height is represented.

nit: Can you add a newline after the doc headings?

/// # Returns /// /// The block count as a `u64`.

ValuedMammal · 2025-12-12T19:23:19Z

I can work on testing Core v28.

Edit: tvpeter#1.

tvpeter · 2025-12-12T19:25:39Z

I can work on testing Core v28.

Alright, that's fine👍. Thank you

- update `corepc-types` to v0.11.0 to support Bitcoind v30

ValuedMammal · 2025-12-17T15:48:38Z

To consider this ready to merge I would want to check Client compatibility with the 3 latest versions of Bitcoin Core, I think we should tackle #19 as well, and it would be great to have a draft PR on bdk_bitcoind_rpc using the new Client struct.

oleonardolima

cACK 6a38a71

I did a first round of review in this one, I reviewed commit by commit, so some comments might not apply as they were addressed in other commits. However, I didn't tested it yet.

Overall, I'd like to mention my comment about the verbose options, I don't those are necessary w.r.t to bdk_bitcoind_rpc usage as we don't use those AFAICT.

I think most of the methods could just be a one-line fn call, it's just decoding/deserializing it either by accessing a specific field or calling .into_model(), but that's my personal taste.

About the method's documentation, you could update it to link it to the types (e.g [Header]).

About the tests, do you plan to integrate the usage of corepc_node for it in this PR, or in a follow-up one ?

oleonardolima · 2025-12-19T20:03:16Z

src/client.rs

+    /// Get block verboseone
+    pub fn get_block_verbose(&self, block_hash: &BlockHash) -> Result<GetBlockVerboseOne, Error> {
+        let res: GetBlockVerboseOne = self.call("getblock", &[json!(block_hash), json!(1)])?;
+        Ok(res)
+    }


Do we want to have support for getblock with the verbose flag? I don't see it being used by bdk_bitcoind_rpc.

I think this is the same as get_block_info that is used by the Emitter.

oleonardolima · 2025-12-19T20:04:19Z

src/client.rs

+    /// Get best block hash
    pub fn get_best_block_hash(&self) -> Result<BlockHash, Error> {
        let res: String = self.call("getbestblockhash", &[])?;
        Ok(res.parse()?)
    }


Same here, I don't think we need to support it.

oleonardolima · 2025-12-19T20:08:55Z

src/client.rs

+        let bytes = Vec::<u8>::from_hex(&hex_string).map_err(Error::HexToBytes)?;
+
+        let header = deserialize(&bytes).map_err(|e| {
+            Error::InvalidResponse(format!("failed to deserialize block header: {e}"))
+        })?;


Do you actually need to convert it to bytes and then deserialize it? Can't we use the consensus encode/decode from rust-bitcoin directly to the hex response?

oleonardolima · 2025-12-19T20:09:35Z

src/client.rs

+        let bytes = Vec::<u8>::from_hex(&hex_string).map_err(Error::HexToBytes)?;
+
+        let transaction = deserialize(&bytes).map_err(|e| {
+            Error::InvalidResponse(format!("transaction deserialization failed: {e}"))
+        })?;


Same here, can't the same be accomplished directly through rust-bitcoin consensus encode/decode?

oleonardolima · 2025-12-19T20:19:31Z

src/client.rs

+        let block_string: String = self.call("getblock", &[json!(block_hash), json!(0)])?;
+        let block = deserialize_hex(&block_string)?;


The returned type still a hex, having it named as block_hex would be better IMHO.

oleonardolima · 2025-12-19T20:22:06Z

src/client.rs

-        let res: String = self.call("getbestblockhash", &[])?;
-        Ok(res.parse()?)
+        let best_block_hash: String = self.call("getbestblockhash", &[])?;
+        Ok(best_block_hash.parse()?)


Does the .parse() uses the rust-bitcoin consensus encode/decode in the background ? 🤔

No, it doesn't. Rust-bitcoin implements the FromStr trait for BlockHash making .parse() method available (as it is available on any type that implements the trait). It is preferred for handling endianness representations

oleonardolima · 2025-12-19T20:22:31Z

src/client.rs

+        let block: corepc_types::v30::GetBlockVerboseOne =
            self.call("getblock", &[json!(block_hash), json!(1)])?;
-        Ok(block_verbose_one)
+        let block_model = block.into_model()?;
+
+        Ok(block_model)


nit: I think you can do it all in a one-liner.

oleonardolima · 2025-12-19T20:24:24Z

src/client.rs

+        let block_count: GetBlockCount = self.call("getblockcount", &[])?;
+        let block_count_u64 = block_count.0;
+        let block_count_u32 = block_count_u64.try_into()?;
+        Ok(block_count_u32)


nit: Same here, you can probably do it in a one-liner. ~~Also, are we properly mapping the conversion error to a different error variant?~~, Now I saw the new error variant.

oleonardolima · 2025-12-19T20:25:32Z

src/client.rs

+        let txids: GetRawMempool = self.call("getrawmempool", &[])?;
+        Ok(txids.0)


Do we have an .into_model() for this one that would return a Vec already ? Also, can be a one-liner.

oleonardolima · 2025-12-19T20:28:23Z

src/client.rs

+#[cfg(not(feature = "29_0"))]
+use corepc_types::v30::GetBlockFilter;


nit: we can import the type directly on the method scope, as it's being done for the other ones.

tvpeter · 2025-12-19T21:13:29Z

About the tests, do you plan to integrate the usage of corepc_node for it in this PR, or in a follow-up one ?

yes, the corepc_node will be part of this PR.

Those comments that are not already addressed will be addressed in the next push.
Thank you.

ValuedMammal · 2025-12-20T20:30:34Z

src/client/v29.rs

I don't think we need this v29 module. The GetBlockFilter type is the same since v19.

You were right. They returned the same thing when I tested against v29 and v30.

ValuedMammal · 2025-12-20T20:32:53Z

src/client.rs

+    ///
+    /// The `GetBlockFilter` structure containing the filter data
+    #[cfg(not(feature = "29_0"))]
+    pub fn get_block_filter(&self, block_hash: &BlockHash) -> Result<v30::GetBlockFilter, Error> {


IMO this should return the model type.

ValuedMammal · 2025-12-20T20:38:58Z

tests/test_rpc_client.rs

+        Err(_) => {
+            println!("Block filters not enabled (requires -blockfilterindex=1)");
+        }


Because you're dropping the error, we don't know if it's caused by a misconfigured bitcoind or a faulty client implementation. If it's an error here the test should probably just fail.

This will clarify whether there is a need for the v29 module, as it did not return any filter even after enabling it in the config when I tested on the v29 node until I specifically imported v29::GetBlockFilter

ValuedMammal · 2025-12-20T20:43:49Z

src/client/v28.rs

+    /// The verbose header as a `GetBlockHeaderVerbose` struct.
+    pub fn get_block_header_verbose(
+        &self,
+        hash: &BlockHash,


Suggested change

hash: &BlockHash,

block_hash: &BlockHash,

To stay consistent with the arguments in the docs. Same suggestion for all *_verbose methods. This was originally my mistake, sorry about that.

- refactor methods

oleonardolima self-requested a review November 19, 2025 17:38

oleonardolima assigned tvpeter Nov 19, 2025

oleonardolima added the enhancement New feature or request label Nov 19, 2025

oleonardolima added this to BDK Chain Nov 19, 2025

tvpeter force-pushed the feat/blockchain-rpc-methods branch 2 times, most recently from 6543635 to 3a16fa4 Compare November 25, 2025 05:22

ValuedMammal reviewed Nov 27, 2025

View reviewed changes

tvpeter force-pushed the feat/blockchain-rpc-methods branch 2 times, most recently from 3632601 to 685e258 Compare December 10, 2025 17:16

tvpeter mentioned this pull request Dec 11, 2025

Use corepc_node to minimize manual testing and integration tests in CI #19

Open

tvpeter force-pushed the feat/blockchain-rpc-methods branch from c5f1106 to 61599ad Compare December 11, 2025 14:06

tvpeter requested a review from ValuedMammal December 11, 2025 14:10

tvpeter added 2 commits December 12, 2025 19:41

feat(add-blockchain-mthds): add blockchain methods

b7c85fd

- getblockcount - getblockhash - getblockfilter - getblockheader - getrawmempool - getrawtransaction

feat: Add integration tests

291f42a

- add integration tests for blockchain methods

ValuedMammal reviewed Dec 12, 2025

View reviewed changes

tvpeter force-pushed the feat/blockchain-rpc-methods branch from 61599ad to bb3f1b1 Compare December 13, 2025 09:47

feat: Update corepc-types to v0.11.0

b0731c1

- update `corepc-types` to v0.11.0 to support Bitcoind v30

tvpeter force-pushed the feat/blockchain-rpc-methods branch from bb3f1b1 to b0731c1 Compare December 13, 2025 09:59

tvpeter force-pushed the feat/blockchain-rpc-methods branch from cbbedbe to 0778704 Compare December 18, 2025 07:34

oleonardolima moved this to Needs Review in BDK Chain Dec 19, 2025

oleonardolima reviewed Dec 19, 2025

View reviewed changes

ValuedMammal reviewed Dec 20, 2025

View reviewed changes

ValuedMammal and others added 2 commits December 21, 2025 05:30

feat(client): Add v28 module

df393f8

test: Use corepc-node for integration tests

17266ca

- refactor methods

tvpeter force-pushed the feat/blockchain-rpc-methods branch from c036a8d to 17266ca Compare December 21, 2025 04:32

		let block_string: String = self.call("getblock", &[json!(block_hash), json!(0)])?;
		let block = deserialize_hex(&block_string)?;

		let txids: GetRawMempool = self.call("getrawmempool", &[])?;
		Ok(txids.0)

		#[cfg(not(feature = "29_0"))]
		use corepc_types::v30::GetBlockFilter;

Add Blockchain RPC methods #5

Are you sure you want to change the base?

Add Blockchain RPC methods #5

Uh oh!

Conversation

tvpeter commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklists

All Submissions:

New Features:

Uh oh!

ValuedMammal 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!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

tvpeter commented Dec 12, 2025

Uh oh!

ValuedMammal commented Dec 17, 2025

Uh oh!

oleonardolima 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!

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

Uh oh!

tvpeter commented Dec 19, 2025

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!

Reviewers

tvpeter commented Nov 18, 2025 •

edited

Loading

ValuedMammal commented Dec 12, 2025 •

edited

Loading