Skip to content

Conversation

@luisschwab
Copy link
Member

@luisschwab luisschwab commented Nov 20, 2025

This PR implements the missing methods needed to be feature complete with the Esplora API specification.

Also adds Bitcoin Dev Kit Developers to authors on Cargo.toml, pins tracing to 0.1.41, and log to 0.4.28 for the 1.63.0 MSRV.

Changelog

  • Add OutputSpendStatus struct
  • Add ScriptHashTxsSummary and ScriptHashStats structs
  • Add BlockInformation struct
  • Add MempoolStats struct
  • Add MempoolRecentTxs struct
  • Add InvalidStartIndexValue error variant
  • Add get_tx_outspends method (GET /tx/:txid/outspends)
  • Add get_scripthash_stats method (GET /scripthash/:hash)
  • Add get_mempool_address_txs method (GET /address/:address/txs/mempool)
  • Add get_mempool_scripthash_txs method (GET /scripthash/:hash/txs/mempool)
  • Add get_scripthash_utxos method (GET /scripthash/:hash/utxo)
  • Add get_block_info method (GET /block/:hash)
  • Add get_block_txids method (GET /block/:hash/txids)
  • Add get_block_txs method (GET /block/:hash/txs[/:start_index])
  • Add get_mempool_stats method (GET /mempool)
  • Add get_mempool_txids method (GET /mempool/txids)
  • Add get_mempool_recent_txids method (GET /mempool/recent)

@luisschwab luisschwab self-assigned this Nov 20, 2025
@luisschwab luisschwab added the enhancement New feature or request label Nov 20, 2025
@luisschwab luisschwab force-pushed the feat/impl-missing-methods branch 3 times, most recently from cd95f3f to a391f24 Compare November 20, 2025 03:16
@coveralls
Copy link

coveralls commented Nov 20, 2025

Pull Request Test Coverage Report for Build 19939369053

Details

  • 344 of 346 (99.42%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+3.3%) to 90.654%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/async.rs 51 52 98.08%
src/blocking.rs 50 51 98.04%
Totals Coverage Status
Change from base Build 19620365463: 3.3%
Covered Lines: 1387
Relevant Lines: 1530

💛 - Coveralls

@luisschwab luisschwab force-pushed the feat/impl-missing-methods branch from a391f24 to 30e3b95 Compare November 20, 2025 03:21
@luisschwab luisschwab marked this pull request as ready for review November 20, 2025 03:23
@luisschwab luisschwab force-pushed the feat/impl-missing-methods branch from 30e3b95 to f010183 Compare November 20, 2025 03:49
@oleonardolima oleonardolima moved this to In Progress in BDK Chain Nov 20, 2025
@oleonardolima
Copy link
Collaborator

As it has a couple of commits, I'd check first if it is in the final state, ready for review ?

@oleonardolima
Copy link
Collaborator

FWIW, supersedes the methods from #127.

@oleonardolima oleonardolima moved this from In Progress to Needs Review in BDK Chain Nov 20, 2025
@luisschwab luisschwab force-pushed the feat/impl-missing-methods branch 3 times, most recently from 8379e8a to 6dc0ba7 Compare November 20, 2025 17:49
@luisschwab
Copy link
Member Author

As it has a couple of commits, I'd check first if it is in the final state, ready for review ?

Yes, it's ready.

Copy link
Contributor

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

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

I'm about half way done reviewing this, will follow up soon with the rest.

@luisschwab luisschwab force-pushed the feat/impl-missing-methods branch from 9812292 to ecfdece Compare November 24, 2025 01:19
Copy link
Contributor

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

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

  • I verified that the endpoints exist and the types match the responses from esplora. Verified the code is the same for both async and blocking.
  • I tested the new APIs on a fork of esplora-cli.
  • I ran the tests locally

Additional thoughts:

  • There's some cargo doc warnings due to unresolved rustdoc links
  • We could eventually change or deprecate get_blocks which currently returns Vec<BlockSummary> to return a list of BlockInformation which is the more complete block summary.
  • The tests feel a bit heavy. As a client library we should only care that the request succeeds and we can parse the response - not necessarily the values returned from the server. Also, since all of the tests share the same BITCOIND instance, some tests seem to rely on the output of other tests making it hard to run a single test in isolation (e.g. test_get_block_txs). This can make ongoing maintenance harder in the future.
  • You could consider combining the mempool-related tests into one test and avoid having the "sleep 5 seconds" logic in multiple places.

@luisschwab
Copy link
Member Author

luisschwab commented Nov 24, 2025

There's some cargo doc warnings due to unresolved rustdoc links

I'll open a follow up PR that fixes these, adds all the missing documentation, adds #![warn(missing_docs)] to lib.rs and add cargo doc to CI.

You could consider combining the mempool-related tests into one test and avoid having the "sleep 5 seconds" logic in multiple places.

Indeed, this is a better approach

We could eventually change or deprecate get_blocks which currently returns Vec to return a list of BlockInformation which is the more complete block summary.

Yes, but we have to do it after the next release cc @oleonardolima

The tests feel a bit heavy. As a client library we should only care that the request succeeds and we can parse the response - not necessarily the values returned from the server. Also, since all of the tests share the same BITCOIND instance, some tests seem to rely on the output of other tests making it hard to run a single test in isolation (e.g. test_get_block_txs). This can make ongoing maintenance harder in the future.

I was trying to check if the start_index offset behavior was correct, but I agree this shouldn't be the client's concern. I'll cut back on that

@luisschwab luisschwab force-pushed the feat/impl-missing-methods branch from ecfdece to 7e142a7 Compare November 25, 2025 02:55
@luisschwab luisschwab force-pushed the feat/impl-missing-methods branch 2 times, most recently from 6868e62 to ed0b4a8 Compare November 26, 2025 18:10
Copy link
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.

Overall, it looks good, but I haven't tested it locally yet.

I left a few nits and suggestions.

@luisschwab luisschwab force-pushed the feat/impl-missing-methods branch from ed0b4a8 to eed3bdb Compare December 3, 2025 15:01
Copy link
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.

utACK eed3bdb

It looks good, you'll just need to fix the MSRV CI step, you can do it in this PR or another, I'm fine either way.

@luisschwab
Copy link
Member Author

All green now

Copy link
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.

utACK 5d8bcf8

Copy link
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.

ACK 5d8bcf8

@luisschwab
Copy link
Member Author

Friendly ping @ValuedMammal

Signed-off-by: valued mammal <valuedmammal@protonmail.com>
Signed-off-by: valued mammal <valuedmammal@protonmail.com>
Copy link
Contributor

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

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

tACK 5d8bcf8

Copy link
Contributor

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

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

ACK cb0f920

@ValuedMammal ValuedMammal merged commit 9305e41 into bitcoindevkit:master Dec 10, 2025
2 of 25 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Chain Dec 10, 2025
@luisschwab luisschwab deleted the feat/impl-missing-methods branch December 10, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants