Skip to content

Allow BdkElectrumClient to take client by reference #1825

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions crates/electrum/src/bdk_electrum_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use bdk_core::{
};
use electrum_client::{ElectrumApi, Error, HeaderNotification};
use std::{
borrow::Borrow,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you clarify why Borrow would be preferable to Deref in this case, especially since it requires a bunch of additional boilerplate, etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because Borrow has a blanket implementation so that any T impls Borrow<T>.

We still want to keep the ability to take a client by ownership imo.

collections::HashSet,
marker::PhantomData,
sync::{Arc, Mutex},
};

Expand All @@ -16,22 +18,36 @@ const CHAIN_SUFFIX_LENGTH: u32 = 8;
/// Wrapper around an [`electrum_client::ElectrumApi`] which includes an internal in-memory
/// transaction cache to avoid re-fetching already downloaded transactions.
#[derive(Debug)]
pub struct BdkElectrumClient<E> {
pub struct BdkElectrumClient<E, C = electrum_client::Client> {
/// The internal [`electrum_client::ElectrumApi`]
pub inner: E,
/// The transaction cache
tx_cache: Mutex<HashMap<Txid, Arc<Transaction>>>,
/// The header cache
block_header_cache: Mutex<HashMap<u32, Header>>,
_marker: PhantomData<C>,
}

impl<E: ElectrumApi> BdkElectrumClient<E> {
/// Creates a new bdk client from a [`electrum_client::ElectrumApi`]
impl<E: Borrow<electrum_client::Client>> BdkElectrumClient<E, electrum_client::Client> {
/// Creates a new bdk client from any type that can be borrowed as [`electrum_client::Client`].
pub fn new(client: E) -> Self {
Self {
inner: client,
tx_cache: Default::default(),
block_header_cache: Default::default(),
_marker: PhantomData,
}
}
}

impl<E: Borrow<C>, C: ElectrumApi> BdkElectrumClient<E, C> {
/// Creates a new bdk client from any type that implements [`electrum_client::ElectrumApi`].
pub fn with_custom_client(client: E) -> Self {
Self {
inner: client,
tx_cache: Default::default(),
block_header_cache: Default::default(),
_marker: PhantomData,
}
}

Expand All @@ -58,7 +74,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {

drop(tx_cache);

let tx = Arc::new(self.inner.transaction_get(&txid)?);
let tx = Arc::new(self.inner.borrow().transaction_get(&txid)?);

self.tx_cache.lock().unwrap().insert(txid, Arc::clone(&tx));

Expand All @@ -82,7 +98,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {

/// Update a block header at given `height`. Returns the updated header.
fn update_header(&self, height: u32) -> Result<Header, Error> {
let header = self.inner.block_header(height as usize)?;
let header = self.inner.borrow().block_header(height as usize)?;

self.block_header_cache
.lock()
Expand All @@ -96,7 +112,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
///
/// This is a re-export of [`ElectrumApi::transaction_broadcast`].
pub fn transaction_broadcast(&self, tx: &Transaction) -> Result<Txid, Error> {
self.inner.transaction_broadcast(tx)
self.inner.borrow().transaction_broadcast(tx)
}

/// Full scan the keychain scripts specified with the blockchain (via an Electrum client) and
Expand Down Expand Up @@ -130,7 +146,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
let mut request: FullScanRequest<K> = request.into();

let tip_and_latest_blocks = match request.chain_tip() {
Some(chain_tip) => Some(fetch_tip_and_latest_blocks(&self.inner, chain_tip)?),
Some(chain_tip) => Some(fetch_tip_and_latest_blocks(self.inner.borrow(), chain_tip)?),
None => None,
};

Expand Down Expand Up @@ -198,7 +214,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
let mut request: SyncRequest<I> = request.into();

let tip_and_latest_blocks = match request.chain_tip() {
Some(chain_tip) => Some(fetch_tip_and_latest_blocks(&self.inner, chain_tip)?),
Some(chain_tip) => Some(fetch_tip_and_latest_blocks(self.inner.borrow(), chain_tip)?),
None => None,
};

Expand Down Expand Up @@ -260,6 +276,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {

let spk_histories = self
.inner
.borrow()
.batch_script_get_history(spks.iter().map(|(_, s)| s.as_script()))?;

for ((spk_index, _spk), spk_history) in spks.into_iter().zip(spk_histories) {
Expand Down Expand Up @@ -304,7 +321,11 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
// add to our sparsechain `update`:
let mut has_residing = false; // tx in which the outpoint resides
let mut has_spending = false; // tx that spends the outpoint
for res in self.inner.script_get_history(&op_txout.script_pubkey)? {
for res in self
.inner
.borrow()
.script_get_history(&op_txout.script_pubkey)?
{
if has_residing && has_spending {
break;
}
Expand Down Expand Up @@ -356,6 +377,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
// call to get confirmation status of our transaction
if let Some(r) = self
.inner
.borrow()
.script_get_history(spk)?
.into_iter()
.find(|r| r.tx_hash == txid)
Expand All @@ -378,6 +400,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
) -> Result<(), Error> {
if let Ok(merkle_res) = self
.inner
.borrow()
.transaction_get_merkle(&txid, confirmation_height as usize)
{
let mut header = self.fetch_header(merkle_res.block_height as u32)?;
Expand Down
44 changes: 43 additions & 1 deletion crates/electrum/tests/test_electrum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ use bdk_chain::{
use bdk_electrum::BdkElectrumClient;
use bdk_testenv::{anyhow, bitcoincore_rpc::RpcApi, TestEnv};
use core::time::Duration;
use std::collections::{BTreeSet, HashSet};
use std::str::FromStr;
use std::{
collections::{BTreeSet, HashSet},
sync::Arc,
};

// Batch size for `sync_with_electrum`.
const BATCH_SIZE: usize = 5;
Expand Down Expand Up @@ -676,3 +679,42 @@ fn test_check_fee_calculation() -> anyhow::Result<()> {
}
Ok(())
}

#[test]
fn borrowed() -> anyhow::Result<()> {
let env = TestEnv::new()?;

let electrum_client = electrum_client::Client::new(env.electrsd.electrum_url.as_str())?;
let _client = BdkElectrumClient::new(electrum_client);
drop(_client);

let electrum_client = electrum_client::Client::new(env.electrsd.electrum_url.as_str())?;
let _client = BdkElectrumClient::new(&electrum_client);
drop(_client);

let electrum_client = electrum_client::Client::new(env.electrsd.electrum_url.as_str())?;
let _client = BdkElectrumClient::new(Arc::new(electrum_client));
drop(_client);

let electrum_client = electrum_client::Client::new(env.electrsd.electrum_url.as_str())?;
let _client = BdkElectrumClient::new(Box::new(electrum_client));
drop(_client);

let electrum_client =
electrum_client::raw_client::RawClient::new(&env.electrsd.electrum_url, None)?;
let _client =
BdkElectrumClient::<_, electrum_client::raw_client::RawClient<_>>::with_custom_client(
electrum_client,
);
drop(_client);

let electrum_client =
electrum_client::raw_client::RawClient::new(&env.electrsd.electrum_url, None)?;
let _client =
BdkElectrumClient::<_, electrum_client::raw_client::RawClient<_>>::with_custom_client(
Box::new(electrum_client),
);
drop(_client);

Ok(())
}
Loading