Skip to content

deps(bdk_testenv): bump electrsd to 0.31.0 #1826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
oleonardolima wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

deps(bdk_testenv): bump electrsd to 0.31.0 #1826

oleonardolima wants to merge 4 commits into from

Conversation

@oleonardolima
Copy link
Contributor

@oleonardolima oleonardolima commented Feb 6, 2025

Description

Notes to the reviewers

Changelog notice

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

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@oleonardolima
Copy link
Contributor Author

oleonardolima commented Feb 7, 2025

It looks like I'll need to open a PR on corepc-client to add get-block-template RPC, then I'll get back to this one :(

I'll do the same in rust-electrum-client for the tests, but I'll do it to 0.30.x instead to prevent this issue from blocking the PR there.

@oleonardolima oleonardolima force-pushed the deps/bump-electrsd-version branch from af3988d to b4d655a Compare February 9, 2025 02:57
@ValuedMammal ValuedMammal mentioned this pull request Feb 11, 2025
Open
8 tasks
@notmandatory notmandatory mentioned this pull request Mar 5, 2025
Open
6 tasks
@oleonardolima oleonardolima force-pushed the deps/bump-electrsd-version branch 4 times, most recently from ef27736 to f52f04d Compare March 22, 2025 18:46
@notmandatory notmandatory added the dependencies Pull requests that update a dependency file label Apr 23, 2025
@notmandatory notmandatory moved this to In Progress in BDK Chain Apr 23, 2025
@notmandatory notmandatory mentioned this pull request Apr 26, 2025
Open
8 tasks
@ValuedMammal
Copy link
Collaborator

ValuedMammal commented Jul 21, 2025

@oleonardolima I noticed that the getblocktemplate RPC is implemented in corepc-client.

@oleonardolima
Copy link
Contributor Author

oleonardolima commented Jul 22, 2025

@oleonardolima I noticed that the getblocktemplate RPC is implemented in corepc-client.

Yes, it was added a few releases after I started this PR, but IIRC some other methods were missing too. I'll check that again.

@ValuedMammal
Copy link
Collaborator

ValuedMammal commented Jul 23, 2025

Yes, it was added a few releases after I started this PR, but IIRC some other methods were missing too. I'll check that again.

Seems to be lacking good support for get_raw_mempool_verbose, at least for versions of Bitcoin Core later than 0.17.

@oleonardolima
Copy link
Contributor Author

oleonardolima commented Jul 24, 2025

Once #1988 lands, we won't be relying on get_raw_mempool_verbose; instead, we will use get_raw_mempool. It should unblock this one.

@oleonardolima oleonardolima force-pushed the deps/bump-electrsd-version branch from f52f04d to e8fc019 Compare August 5, 2025 02:23
@oleonardolima
Copy link
Contributor Author

oleonardolima commented Aug 5, 2025

Once #1988 lands, we won't be relying on get_raw_mempool_verbose; instead, we will use get_raw_mempool. It should unblock this one.

It appears that other methods are missing in the corepc-client at electrsd 0.34. They're released on corepc-node 0.8, which will be included in the next release of electrsd. Will check if there are alternative methods for those.

@oleonardolima oleonardolima force-pushed the deps/bump-electrsd-version branch 4 times, most recently from 2ab640d to 366c9d9 Compare August 8, 2025 20:11
@oleonardolima
Copy link
Contributor Author

oleonardolima commented Aug 8, 2025

It appears that other methods are missing in the corepc-client at electrsd 0.34. They're released on corepc-node 0.8, which will be included in the next release of electrsd. Will check if there are alternative methods for those.

As of the latest v0.35 release, it appears that all the necessary methods are implemented in corepc-{node|client}. I went ahead and updated the bdk-testenv and bdk-electrum/esplora crates.

It requires a more significant refactor and changes for the bdk_bitcoind_rpc, as the Emitter relies on the bitcoincore-rpc ' RpcApi' trait. I can cover it all in a single PR, but would like to know what you guys think as well.

cc @ValuedMammal @LagginTimes

@ValuedMammal
Copy link
Collaborator

ValuedMammal commented Aug 12, 2025

Replaying a comment from discord

I also noticed that updating electrsd means potentially changing the Emitter to use corepc Client because of how it relates to testenv, etc. We have to make sure switching clients doesn't negatively impact behavior, performance and stuff

@oleonardolima oleonardolima mentioned this pull request Sep 29, 2025
Open
@ValuedMammal
Copy link
Collaborator

ValuedMammal commented Nov 22, 2025

We may be able to unblock this by having a way to create a new RPC client using the node credentials of a TestEnv instance but independent of the actual test client that is used by electrsd. oleonardolima#3

@oleonardolima
Copy link
Contributor Author

oleonardolima commented Nov 22, 2025

We may be able to unblock this by having a way to create a new RPC client using the node credentials of a TestEnv instance but independent of the actual test client that is used by electrsd. oleonardolima#3

Cool, I'll take a look at your PR this morning.

@oleonardolima oleonardolima force-pushed the deps/bump-electrsd-version branch from 366c9d9 to a2784f5 Compare November 24, 2025 01:37
@oleonardolima
Copy link
Contributor Author

oleonardolima commented Nov 24, 2025

We may be able to unblock this by having a way to create a new RPC client using the node credentials of a TestEnv instance but independent of the actual test client that is used by electrsd. oleonardolima#3

Alright, but if I got it right, the idea would to still rely on bitcoincore_rpc API for bdk_bitcoind_rpc tests (which I think makes sense), right ? And when we land bdk_bitcoind_client that wouldn't an issue anymore, right

@oleonardolima
Copy link
Contributor Author

oleonardolima commented Nov 24, 2025

re-posting from oleonardolima#3 (comment)

Thanks for the suggestion, I'll cherry-pick this one and update the remaining tests.

@oleonardolima
Copy link
Contributor Author

oleonardolima commented Nov 24, 2025

Also, it looks like the latest one is v0.36.1, I'll give it a try on that one too.

@ValuedMammal
Copy link
Collaborator

ValuedMammal commented Nov 25, 2025

Alright, but if I got it right, the idea would to still rely on bitcoincore_rpc API for bdk_bitcoind_rpc tests (which I think makes sense), right ? And when we land bdk_bitcoind_client that wouldn't an issue anymore, right

The approach I had in mind is to bump electrsd to the latest version in bdk_testenv which might entail updating some methods in order to benefit from all new corepc types. In bdk_bitcoind_rpc we can start using the TestEnv client for tests, and this ClientExt trait or whatever we're calling it can be used to create a client just for the Emitter with no other code changes needed. When bdk_bitcoind_client is available we'll update the Emitter to use that and remove bitcoincore_rpc. Don't know if I've fully answered the question.

@oleonardolima oleonardolima force-pushed the deps/bump-electrsd-version branch 2 times, most recently from 33be1c7 to a83a8fe Compare November 29, 2025 20:24
@oleonardolima oleonardolima mentioned this pull request Nov 29, 2025
Open
@oleonardolima
Copy link
Contributor Author

oleonardolima commented Nov 29, 2025

AFAICT the only blocker now is: rust-bitcoin/corepc#425

oleonardolima and others added 4 commits November 30, 2025 00:11
@oleonardolima
deps(bdk_testenv): bump electrsd to 0.34.0
594db64
@ValuedMammal @oleonardolima
tests(bdk_bitcoind_rpc): add/use new ClientExit
9453d33 
- add `ClientExt` trait to it's own common module.
- update the `bdk_bitcoind_rpc` lib.rs tests to use `ClientExt`.
- update the `bdk_bitcoind_rpc` test_emitter.rs tests to use `ClientExt`.
- update the `bdk_bitcoind_rpc` test_filter_iter.rs tests to use `ClientExt`.
@oleonardolima
fix(tests): on bdk_chain
25e00ad
@oleonardolima
fix(tests): on bdk_esplora, bdk_electrum
a83a8fe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@oleonardolima oleonardolima

Labels

dependencies Pull requests that update a dependency file

Projects

BDK Chain
Status: In Progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@oleonardolima @ValuedMammal @notmandatory