-
Notifications
You must be signed in to change notification settings - Fork 420
deps(bdk_testenv): bump electrsd to 0.36.1
#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.
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
base: master
Are you sure you want to change the base?
deps(bdk_testenv): bump electrsd to 0.36.1
#1826
Conversation
|
It looks like I'll need to open a PR on corepc-client to add I'll do the same in |
af3988d to
b4d655a
Compare
ef27736 to
f52f04d
Compare
|
@oleonardolima I noticed that the |
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 |
|
Once #1988 lands, we won't be relying on |
f52f04d to
e8fc019
Compare
It appears that other methods are missing in the corepc-client at electrsd |
2ab640d to
366c9d9
Compare
As of the latest v0.35 release, it appears that all the necessary methods are implemented in It requires a more significant refactor and changes for the |
|
Replaying a comment from discord
|
|
We may be able to unblock this by having a way to create a new RPC client using the node credentials of a |
Cool, I'll take a look at your PR this morning. |
366c9d9 to
a2784f5
Compare
Alright, but if I got it right, the idea would to still rely on |
Thanks for the suggestion, I'll cherry-pick this one and update the remaining tests. |
|
Also, it looks like the latest one is v0.36.1, I'll give it a try on that one too. |
The approach I had in mind is to bump |
33be1c7 to
a83a8fe
Compare
|
AFAICT the only blocker now is: rust-bitcoin/corepc#425 |
a83a8fe to
62d9c07
Compare
- 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`.
- add new methods `list_unspent` and `get_raw_transaction_verbose` to `TestEnv`, it's need as they require specific arguments in the call, that are not available in corepc API.
fd8926d to
10baef9
Compare
electrsd to 0.31.0electrsd to 0.36.1
It's not a block anymore, by using the latest |
| let (chain, _) = LocalChain::from_genesis(env.genesis_hash()?); | ||
| let chain_tip = chain.tip(); | ||
|
|
||
| // TODO: (@oleonardolima) We should use the `ClientExt` trait instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this currently possible or do we need the new bdk_bitcoind_rpc client ready?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already possible, lingering TODO.
| env.mine_blocks(1, Some(sender_addr.clone()))?; | ||
| env.mine_blocks(101, None)?; | ||
|
|
||
| // TODO: (@oleonardolima) remove unwraps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lingering TODO here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already solved those, forgot to remove the comment.
| impl ScenarioEnv { | ||
| fn new() -> anyhow::Result<Self> { | ||
| let env = TestEnv::new()?; | ||
| let client = env.rpc_client(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought: why not drop TestEnv::rpc_client altogether and rely only on ClientExt::get_rpc_client(&env)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the standard-way to use testenv's rpc client, the ClientExt is just a workaround to upgrade the bdk_testenv in bdk_bitcoind_rpc while we don't have the new client that relies on the same corepc types.
| // let unconfirmed_txs = mempool_txids | ||
| // .iter() | ||
| // // .map(|txid| -> Result<_, _> { client.get_raw_transaction(*txid)?.into_model() | ||
| // })? .map( | ||
| // |txid| -> Result< | ||
| // GetRawTransaction, | ||
| // bdk_testenv::corepc_client::client_sync::Error, | ||
| // > { client.get_raw_transaction(*txid) }, | ||
| // ) | ||
| // .collect::<Vec<Result<_, _>>>() | ||
| // .map(|get_raw_tx| get_raw_tx.into_model()) | ||
| // .map(|tx| (tx.0, 0)) | ||
| // .collect::<Vec<(Transaction, u64)>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs resolving
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's solved, forgot to remove the comment.
| @@ -1,3 +1,3 @@ | |||
| [package] | |||
| name = "bdk_testenv" | |||
| version = "0.13.1" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to bump bdk_testenv's version as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only bumped in the release PRs.
|
Tests are failing locally for me: |
Indeed, needs debugging. Still unsure why it's now failing. |
|
I think you need to import |
partially addresses #1949
Description
It upgrades the
electrsdversion from0.28.0to0.36.1, migrating from oldbitcoindtocorepc-nodeand using thecorepc-node_28_2feature.Updates the
bitcoindclient type forTestEnv, and its internal methods. Also, updates: bdk_bitcoind_rpc, bdk_esplora, bdk_electrum usage ofbdk_testenvandTestEnv.Notes to the reviewers
As we are upgrading to the latest
electrsd, we can drop for now the pinning forhomein the MSRV step.Also for both
list_unspentandget_raw_transaction_verbosecorepc-client APIs didn't support some required arguments in some of the tests. That said, I implemented our own RPC calls as two new methods inTestEnv.Changelog notice
TBD
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features: