Skip to content

Commit 6273d5c

Browse files
reezreez
committed
chore: address review comments
1 parent 9ea0dd0 commit 6273d5c

File tree

3 files changed

+54
-118
lines changed

3 files changed

+54
-118
lines changed

‎crates/esplora/src/async_ext.rs‎

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,7 @@ use bdk_core::{
1010
use esplora_client::Sleeper;
1111
use futures::{stream::FuturesOrdered, TryStreamExt};
1212

13-
use crate::{insert_anchor_or_seen_at_from_status, insert_prevouts};
14-
15-
/// [`esplora_client::Error`]
16-
type Error = Box<esplora_client::Error>;
13+
use crate::{insert_anchor_or_seen_at_from_status, insert_prevouts, Error};
1714

1815
/// Trait to extend the functionality of [`esplora_client::AsyncClient`].
1916
///
@@ -256,15 +253,13 @@ async fn chain_update<S: Sleeper>(
256253
let mut tip = match point_of_agreement {
257254
Some(tip) => tip,
258255
None => {
259-
return Err(Box::new(esplora_client::Error::HeaderHashNotFound(
260-
local_cp_hash,
261-
)));
256+
return Err(esplora_client::Error::HeaderHashNotFound(local_cp_hash).into());
262257
}
263258
};
264259

265260
tip = tip
266261
.extend(conflicts.into_iter().rev().map(|b| (b.height, b.hash)))
267-
.map_err(|_| Box::new(esplora_client::Error::InvalidResponse))?;
262+
.map_err(Error::Checkpoint)?;
268263

269264
for (anchor, _txid) in anchors {
270265
let height = anchor.block_id.height;
@@ -560,23 +555,17 @@ mod test {
560555
use bdk_testenv::{anyhow, bitcoincore_rpc::RpcApi, TestEnv};
561556
use esplora_client::Builder;
562557

563-
use crate::async_ext::{chain_update, fetch_latest_blocks};
558+
use crate::{
559+
async_ext::{chain_update, fetch_latest_blocks},
560+
Error as EsploraError,
561+
};
564562

565563
macro_rules! h {
566564
($index:literal) => {{
567565
bdk_chain::bitcoin::hashes::Hash::hash($index.as_bytes())
568566
}};
569567
}
570568

571-
#[test]
572-
fn ensure_last_index_none_returns_error() {
573-
let last_index: Option<u32> = None;
574-
let err = last_index
575-
.ok_or_else(|| Box::new(esplora_client::Error::InvalidResponse))
576-
.unwrap_err();
577-
assert!(matches!(*err, esplora_client::Error::InvalidResponse));
578-
}
579-
580569
// Test that `chain_update` fails due to wrong network.
581570
#[tokio::test]
582571
async fn test_chain_update_wrong_network_error() -> anyhow::Result<()> {
@@ -600,9 +589,9 @@ mod test {
600589

601590
let anchors = BTreeSet::new();
602591
let res = chain_update(&client, &latest_blocks, &cp, &anchors).await;
603-
use esplora_client::Error;
592+
use esplora_client::Error as ClientError;
604593
assert!(
605-
matches!(*res.unwrap_err(), Error::HeaderHashNotFound(hash) if hash == genesis_hash),
594+
matches!(res.unwrap_err(), EsploraError::Client(ClientError::HeaderHashNotFound(hash)) if hash == genesis_hash),
606595
"`chain_update` should error if it can't connect to the local CP",
607596
);
608597

‎crates/esplora/src/blocking_ext.rs‎

Lines changed: 10 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -7,54 +7,9 @@ use bdk_core::{
77
BlockId, CheckPoint, ConfirmationBlockTime, Indexed, TxUpdate,
88
};
99
use esplora_client::{OutputStatus, Tx};
10-
use std::any::Any;
11-
use std::fmt;
1210
use std::thread::JoinHandle;
1311

14-
use crate::{insert_anchor_or_seen_at_from_status, insert_prevouts};
15-
16-
#[derive(Debug)]
17-
pub enum Error {
18-
Client(esplora_client::Error),
19-
ThreadPanic(Option<String>),
20-
}
21-
22-
impl Error {
23-
fn from_thread_panic(err: Box<dyn Any + Send>) -> Self {
24-
if let Ok(msg) = err.downcast::<String>() {
25-
Self::ThreadPanic(Some(*msg))
26-
} else if let Ok(msg) = err.downcast::<&'static str>() {
27-
Self::ThreadPanic(Some(msg.to_string()))
28-
} else {
29-
Self::ThreadPanic(None)
30-
}
31-
}
32-
}
33-
34-
impl From<esplora_client::Error> for Error {
35-
fn from(err: esplora_client::Error) -> Self {
36-
Self::Client(err)
37-
}
38-
}
39-
40-
impl fmt::Display for Error {
41-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
42-
match self {
43-
Self::Client(err) => write!(f, "{err}"),
44-
Self::ThreadPanic(Some(msg)) => write!(f, "worker thread panicked: {msg}"),
45-
Self::ThreadPanic(None) => write!(f, "worker thread panicked"),
46-
}
47-
}
48-
}
49-
50-
impl std::error::Error for Error {
51-
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
52-
match self {
53-
Self::Client(err) => Some(err),
54-
_ => None,
55-
}
56-
}
57-
}
12+
use crate::{insert_anchor_or_seen_at_from_status, insert_prevouts, Error};
5813

5914
/// Trait to extend the functionality of [`esplora_client::BlockingClient`].
6015
///
@@ -289,7 +244,7 @@ fn chain_update(
289244

290245
tip = tip
291246
.extend(conflicts.into_iter().rev().map(|b| (b.height, b.hash)))
292-
.map_err(|_| Error::from(esplora_client::Error::InvalidResponse))?;
247+
.map_err(Error::Checkpoint)?;
293248

294249
for (anchor, _) in anchors {
295250
let height = anchor.block_id.height;
@@ -324,7 +279,6 @@ fn fetch_txs_with_keychain_spks<I: Iterator<Item = Indexed<SpkWithExpectedTxids>
324279
let mut update = TxUpdate::<ConfirmationBlockTime>::default();
325280
let mut last_active_index = Option::<u32>::None;
326281
let mut consecutive_unused = 0usize;
327-
let mut processed_any = false;
328282
let gap_limit = stop_gap.max(1);
329283

330284
loop {
@@ -358,18 +312,11 @@ fn fetch_txs_with_keychain_spks<I: Iterator<Item = Indexed<SpkWithExpectedTxids>
358312
.collect::<Vec<JoinHandle<Result<TxsOfSpkIndex, Error>>>>();
359313

360314
if handles.is_empty() {
361-
if !processed_any {
362-
return Err(esplora_client::Error::InvalidResponse.into());
363-
}
364315
break;
365316
}
366317

367318
for handle in handles {
368-
let handle_result = handle
369-
.join()
370-
.map_err(Error::from_thread_panic)?;
371-
let (index, txs, evicted) = handle_result?;
372-
processed_any = true;
319+
let (index, txs, evicted) = handle.join().expect("thread must not panic")?;
373320
if txs.is_empty() {
374321
consecutive_unused = consecutive_unused.saturating_add(1);
375322
} else {
@@ -451,7 +398,7 @@ fn fetch_txs_with_txids<I: IntoIterator<Item = Txid>>(
451398
std::thread::spawn(move || {
452399
client
453400
.get_tx_info(&txid)
454-
.map_err(Error::from)
401+
.map_err(Error::Client)
455402
.map(|t| (txid, t))
456403
})
457404
})
@@ -462,10 +409,7 @@ fn fetch_txs_with_txids<I: IntoIterator<Item = Txid>>(
462409
}
463410

464411
for handle in handles {
465-
let handle_result = handle
466-
.join()
467-
.map_err(Error::from_thread_panic)?;
468-
let (txid, tx_info) = handle_result?;
412+
let (txid, tx_info) = handle.join().expect("thread must not panic")?;
469413
if let Some(tx_info) = tx_info {
470414
if inserted_txs.insert(txid) {
471415
update.txs.push(tx_info.to_tx().into());
@@ -516,7 +460,7 @@ fn fetch_txs_with_outpoints<I: IntoIterator<Item = OutPoint>>(
516460
std::thread::spawn(move || {
517461
client
518462
.get_output_status(&op.txid, op.vout as _)
519-
.map_err(Error::from)
463+
.map_err(Error::Client)
520464
})
521465
})
522466
.collect::<Vec<JoinHandle<Result<Option<OutputStatus>, Error>>>>();
@@ -526,10 +470,7 @@ fn fetch_txs_with_outpoints<I: IntoIterator<Item = OutPoint>>(
526470
}
527471

528472
for handle in handles {
529-
let handle_result = handle
530-
.join()
531-
.map_err(Error::from_thread_panic)?;
532-
if let Some(op_status) = handle_result? {
473+
if let Some(op_status) = handle.join().expect("thread must not panic")? {
533474
let spend_txid = match op_status.txid {
534475
Some(txid) => txid,
535476
None => continue,
@@ -562,7 +503,8 @@ fn fetch_txs_with_outpoints<I: IntoIterator<Item = OutPoint>>(
562503
#[cfg(test)]
563504
#[cfg_attr(coverage_nightly, coverage(off))]
564505
mod test {
565-
use crate::blocking_ext::{chain_update, fetch_latest_blocks, Error};
506+
use crate::blocking_ext::{chain_update, fetch_latest_blocks};
507+
use crate::Error as EsploraError;
566508
use bdk_chain::bitcoin;
567509
use bdk_chain::bitcoin::hashes::Hash;
568510
use bdk_chain::bitcoin::Txid;
@@ -580,34 +522,6 @@ mod test {
580522
}};
581523
}
582524

583-
#[test]
584-
fn thread_join_panic_maps_to_error() {
585-
let handle = std::thread::spawn(|| -> Result<(), Error> {
586-
panic!("expected panic for test coverage");
587-
});
588-
589-
let res = (|| -> Result<(), Error> {
590-
let handle_result = handle
591-
.join()
592-
.map_err(Error::from_thread_panic)?;
593-
handle_result
594-
})();
595-
596-
assert!(matches!(
597-
res.unwrap_err(),
598-
Error::ThreadPanic(_)
599-
));
600-
}
601-
602-
#[test]
603-
fn ensure_last_index_none_returns_error() {
604-
let last_index: Option<u32> = None;
605-
let err = last_index
606-
.ok_or_else(|| Error::from(esplora_client::Error::InvalidResponse))
607-
.unwrap_err();
608-
assert!(matches!(err, Error::Client(esplora_client::Error::InvalidResponse)));
609-
}
610-
611525
macro_rules! local_chain {
612526
[ $(($height:expr, $block_hash:expr)), * ] => {{
613527
#[allow(unused_mut)]
@@ -641,7 +555,7 @@ mod test {
641555
let res = chain_update(&client, &latest_blocks, &cp, &anchors);
642556
use esplora_client::Error;
643557
assert!(
644-
matches!(*res.unwrap_err(), Error::HeaderHashNotFound(hash) if hash == genesis_hash),
558+
matches!(res.unwrap_err(), EsploraError::Client(Error::HeaderHashNotFound(hash)) if hash == genesis_hash),
645559
"`chain_update` should error if it can't connect to the local CP",
646560
);
647561

‎crates/esplora/src/lib.rs‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@
2121
//! [`esplora_client::AsyncClient`].
2222
#![cfg_attr(coverage_nightly, feature(coverage_attribute))]
2323

24-
use bdk_core::bitcoin::{Amount, OutPoint, TxOut, Txid};
25-
use bdk_core::{BlockId, ConfirmationBlockTime, TxUpdate};
24+
use bdk_core::bitcoin::{Amount, BlockHash, OutPoint, TxOut, Txid};
25+
use bdk_core::{BlockId, CheckPoint, ConfirmationBlockTime, TxUpdate};
2626
use esplora_client::TxStatus;
27+
use std::fmt;
2728

2829
pub use esplora_client;
2930

@@ -37,6 +38,38 @@ mod async_ext;
3738
#[cfg(feature = "async")]
3839
pub use async_ext::*;
3940

41+
#[derive(Debug)]
42+
pub enum Error {
43+
Client(esplora_client::Error),
44+
Checkpoint(CheckPoint<BlockHash>),
45+
}
46+
47+
impl From<esplora_client::Error> for Error {
48+
fn from(err: esplora_client::Error) -> Self {
49+
Self::Client(err)
50+
}
51+
}
52+
53+
impl fmt::Display for Error {
54+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
55+
match self {
56+
Self::Client(err) => write!(f, "{err}"),
57+
Self::Checkpoint(cp) => {
58+
write!(f, "checkpoint ordering error at height {}", cp.height())
59+
}
60+
}
61+
}
62+
}
63+
64+
impl std::error::Error for Error {
65+
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
66+
match self {
67+
Self::Client(err) => Some(err),
68+
Self::Checkpoint(_) => None,
69+
}
70+
}
71+
}
72+
4073
#[allow(dead_code)]
4174
fn insert_anchor_or_seen_at_from_status(
4275
update: &mut TxUpdate<ConfirmationBlockTime>,

0 commit comments

Comments
 (0)