Skip to content

Commit b45ecb9

Browse files
evanlinjinevanlinjin
committed
Merge #2083: fix: no Debug on Display implementations
823e4e9 fix(file_store)!: remove Debug usage and implement Display for StoreError. (Dmenec) 5a85f6e fix(chain)!: remove Debug usage and implement Display for CalculateFeeError (Dmenec) d0889f3 docs(core): add detailed SyncRequest output example (Dmenec) 5d715c2 fix(chain)!: remove Debug usage and implement Display for InsertDescriptorError (Dmenec) Pull request description: ### Description Fixes #1933, remove the usage of `Debug` on `Display` implemetations mentioned in #1881 (comment) ### Changelog notice - Replaced `Debug` usage with `Display` messages for `InsertDescriptorError`, `CalculateFeeError` and `StoreError`. - `InvalidMagicBytes` now displays expected and actual bytes in hexadecimal instead of using `Debug`. - Added a new `short_descriptor` function to shorten descriptors for concise error output. - Added detailed `SyncRequest` `Display` for Esplora and Electrum examples. ### Checklists #### All Submissions: * [x] I have signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `just check`, `just fmt` and `just test` before committing ACKs for top commit: evanlinjin: ACK 823e4e9 Tree-SHA512: fc3d66b72dc3f7ed3dee3b017687ece73c93883d23cbb019c402e55957368a6487e02b400b092169c326a47a86403f7aa865fe7d7238ccc6eb77afef258ec1ca
2 parents edc282b + 823e4e9 commit b45ecb9

7 files changed

Lines changed: 107 additions & 26 deletions

File tree

‎crates/chain/src/indexer/keychain_txout.rs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -973,33 +973,35 @@ pub enum InsertDescriptorError<K> {
973973
},
974974
}
975975

976-
impl<K: core::fmt::Debug> core::fmt::Display for InsertDescriptorError<K> {
976+
impl<K: core::fmt::Display> core::fmt::Display for InsertDescriptorError<K> {
977977
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
978978
match self {
979979
InsertDescriptorError::DescriptorAlreadyAssigned {
980-
existing_assignment: existing,
980+
existing_assignment,
981981
descriptor,
982982
} => {
983983
write!(
984984
f,
985-
"attempt to re-assign descriptor {descriptor:?} already assigned to {existing:?}"
985+
"descriptor '{}' is already in use by another keychain '{}'",
986+
descriptor, existing_assignment
986987
)
987988
}
988989
InsertDescriptorError::KeychainAlreadyAssigned {
989-
existing_assignment: existing,
990+
existing_assignment,
990991
keychain,
991992
} => {
992993
write!(
993994
f,
994-
"attempt to re-assign keychain {keychain:?} already assigned to {existing:?}"
995+
"keychain '{}' is already associated with another descriptor '{}'",
996+
keychain, existing_assignment
995997
)
996998
}
997999
}
9981000
}
9991001
}
10001002

10011003
#[cfg(feature = "std")]
1002-
impl<K: core::fmt::Debug> std::error::Error for InsertDescriptorError<K> {}
1004+
impl<K: core::fmt::Display + core::fmt::Debug> std::error::Error for InsertDescriptorError<K> {}
10031005

10041006
/// `ChangeSet` represents persistent updates to a [`KeychainTxOutIndex`].
10051007
///

‎crates/chain/src/tx_graph.rs‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -256,15 +256,33 @@ pub enum CalculateFeeError {
256256
impl fmt::Display for CalculateFeeError {
257257
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
258258
match self {
259-
CalculateFeeError::MissingTxOut(outpoints) => write!(
260-
f,
261-
"missing `TxOut` for one or more of the inputs of the tx: {outpoints:?}",
262-
),
263-
CalculateFeeError::NegativeFee(fee) => write!(
264-
f,
265-
"transaction is invalid according to the graph and has negative fee: {}",
266-
fee.display_dynamic()
267-
),
259+
CalculateFeeError::MissingTxOut(outpoints) => {
260+
let max_show = 3;
261+
let shown: Vec<_> = outpoints.iter().take(max_show).collect();
262+
let remaining = outpoints.len().saturating_sub(max_show);
263+
264+
write!(f, "cannot calculate fee, missing previous output(s): ")?;
265+
if outpoints.is_empty() {
266+
write!(f, "<none>")
267+
} else {
268+
write!(f, "{}", shown[0])?;
269+
for op in &shown[1..] {
270+
write!(f, ", {}", op)?;
271+
}
272+
if remaining > 0 {
273+
write!(f, " (+{} more)", remaining)?;
274+
}
275+
Ok(())
276+
}
277+
}
278+
CalculateFeeError::NegativeFee(fee) => {
279+
write!(
280+
f,
281+
"invalid transaction: negative fee {}",
282+
fee.display_dynamic()
283+
)?;
284+
Ok(())
285+
}
268286
}
269287
}
270288
}

‎crates/chain/tests/test_keychain_txout_index.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@ enum TestKeychain {
1818
Internal,
1919
}
2020

21+
impl core::fmt::Display for TestKeychain {
22+
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
23+
match self {
24+
TestKeychain::External => write!(f, "External"),
25+
TestKeychain::Internal => write!(f, "Internal"),
26+
}
27+
}
28+
}
29+
2130
fn parse_descriptor(descriptor: &str) -> Descriptor<DescriptorPublicKey> {
2231
let secp = bdk_chain::bitcoin::secp256k1::Secp256k1::signing_only();
2332
Descriptor::<DescriptorPublicKey>::parse_descriptor(&secp, descriptor)

‎crates/core/src/spk_client.rs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ impl<I, D> SyncRequestBuilder<I, D> {
225225
/// [`chain_tip`](SyncRequestBuilder::chain_tip) (if provided).
226226
///
227227
/// ```rust
228+
/// # use std::io::{self, Write};
228229
/// # use bdk_chain::{bitcoin::{hashes::Hash, ScriptBuf}, local_chain::LocalChain};
229230
/// # use bdk_chain::spk_client::SyncRequest;
230231
/// # let (local_chain, _) = LocalChain::from_genesis(Hash::all_zeros());
@@ -236,7 +237,22 @@ impl<I, D> SyncRequestBuilder<I, D> {
236237
/// // Provide list of scripts to scan for transactions against.
237238
/// .spks(scripts)
238239
/// // This is called for every synced item.
239-
/// .inspect(|item, progress| println!("{} (remaining: {})", item, progress.remaining()))
240+
/// .inspect(|item, progress| {
241+
/// let pc = (100.0 * progress.consumed() as f32) / progress.total() as f32;
242+
/// match item {
243+
/// // In this example I = (), so the first field of Spk is unit.
244+
/// bdk_chain::spk_client::SyncItem::Spk((), spk) => {
245+
/// eprintln!("[ SCANNING {pc:03.0}% ] script {}", spk);
246+
/// }
247+
/// bdk_chain::spk_client::SyncItem::Txid(txid) => {
248+
/// eprintln!("[ SCANNING {pc:03.0}% ] txid {}", txid);
249+
/// }
250+
/// bdk_chain::spk_client::SyncItem::OutPoint(op) => {
251+
/// eprintln!("[ SCANNING {pc:03.0}% ] outpoint {}", op);
252+
/// }
253+
/// }
254+
/// let _ = io::stderr().flush();
255+
/// })
240256
/// // Finish constructing the sync request.
241257
/// .build();
242258
/// ```

‎crates/file_store/src/lib.rs‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,24 @@ pub enum StoreError {
2525

2626
impl core::fmt::Display for StoreError {
2727
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
28+
fn fmt_hex_bytes(f: &mut core::fmt::Formatter<'_>, bytes: &[u8]) -> core::fmt::Result {
29+
for &b in bytes {
30+
write!(f, "{:02x}", b)?;
31+
}
32+
Ok(())
33+
}
34+
2835
match self {
29-
Self::Io(e) => write!(f, "io error trying to read file: {e}"),
30-
Self::InvalidMagicBytes { got, expected } => write!(
31-
f,
32-
"file has invalid magic bytes: expected={expected:?} got={got:?}",
33-
),
34-
Self::Bincode(e) => write!(f, "bincode error while reading entry {e}"),
36+
Self::Io(e) => write!(f, "io error while reading store file: {}", e),
37+
Self::Bincode(e) => write!(f, "bincode error while decoding entry {}", e),
38+
Self::InvalidMagicBytes { got, expected } => {
39+
write!(f, "invalid magic bytes: ")?;
40+
write!(f, "expected 0x")?;
41+
fmt_hex_bytes(f, expected)?;
42+
write!(f, ", got 0x")?;
43+
fmt_hex_bytes(f, got)?;
44+
Ok(())
45+
}
3546
}
3647
}
3748
}

‎examples/example_electrum/src/main.rs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,22 @@ fn main() -> anyhow::Result<()> {
210210
.chain_tip(chain_tip.clone())
211211
.inspect(|item, progress| {
212212
let pc = (100 * progress.consumed()) as f32 / progress.total() as f32;
213-
eprintln!("[ SCANNING {pc:03.0}% ] {item}");
213+
match item {
214+
bdk_chain::spk_client::SyncItem::Spk((keychain, index), spk) => {
215+
eprintln!(
216+
"[ SCANNING {pc:3.0}% ] script {} {} {}",
217+
keychain, index, spk
218+
);
219+
}
220+
bdk_chain::spk_client::SyncItem::Txid(txid) => {
221+
eprintln!("[ SCANNING {pc:3.0}% ] txid {}", txid);
222+
}
223+
bdk_chain::spk_client::SyncItem::OutPoint(op) => {
224+
eprintln!("[ SCANNING {pc:3.0}% ] outpoint {}", op);
225+
}
226+
}
227+
let _ = io::stderr().flush();
214228
});
215-
216229
let canonical_view = graph.canonical_view(
217230
&*chain,
218231
chain_tip.block_id(),

‎examples/example_esplora/src/main.rs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,20 @@ fn main() -> anyhow::Result<()> {
215215
.chain_tip(local_tip.clone())
216216
.inspect(|item, progress| {
217217
let pc = (100 * progress.consumed()) as f32 / progress.total() as f32;
218-
eprintln!("[ SCANNING {pc:03.0}% ] {item}");
219-
// Flush early to ensure we print at every iteration.
218+
match item {
219+
bdk_chain::spk_client::SyncItem::Spk((keychain, index), spk) => {
220+
eprintln!(
221+
"[ SCANNING {pc:3.0}% ] script {} {} {}",
222+
keychain, index, spk
223+
);
224+
}
225+
bdk_chain::spk_client::SyncItem::Txid(txid) => {
226+
eprintln!("[ SCANNING {pc:3.0}% ] txid {}", txid);
227+
}
228+
bdk_chain::spk_client::SyncItem::OutPoint(op) => {
229+
eprintln!("[ SCANNING {pc:3.0}% ] outpoint {}", op);
230+
}
231+
}
220232
let _ = io::stderr().flush();
221233
});
222234

0 commit comments

Comments
 (0)