-
Notifications
You must be signed in to change notification settings - Fork 376
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
Have bdk_electrum
take Electrum client by reference
#1820
Conversation
Previously, `BdkElectrumClient::new` would unnecessarily take an owned `electrum_client::Client` value, which rendered reusing the same client (i.e., connection) for other tasks impossible. Here, we switch to have `BdkElectrumClient` take a `Deref` with target `ElectrumApi`, which allows to give the `electrum_client::Client` *by reference* to `BdkElectrumClient`.
9961701
to
119ffe7
Compare
This is not true. The |
Well, that's still very limiting, no? It has a simple |
Previously, `BdkElectrumClient` has to take ownership of the internal electrum client. This is an alternative of bitcoindevkit#1820. bitcoindevkit#1820 removes the ability to have `BdkElectrumClient` take ownership.
I created #1825 as an alternative, less-breaking solution. |
I'm not sure I understand why it's considered less breaking (note the majority of the diff here is simply moving code)? |
It's breaking because Anyhow, I closed #1825 as I think bitcoindevkit/rust-electrum-client#163 is the superior solution. |
Okay. I'm not sure adding that boilerplate is much preferable, but your choice. Going ahead and closing this. |
Description
Previously,
BdkElectrumClient::new
would unnecessarily take an ownedelectrum_client::Client
value, which rendered reusing the same client (i.e., connection) for other tasks impossible.Here, we switch to have
BdkElectrumClient
take aDeref
with targetElectrumApi
, which allows to give theelectrum_client::Client
by reference toBdkElectrumClient
.Checklists
All Submissions:
cargo fmt
andcargo clippy
before committingBugfixes: