Skip to content
Open
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
31 changes: 24 additions & 7 deletions crates/chain/src/indexer/keychain_txout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
spk_txout::SpkTxOutIndex,
DescriptorExt, DescriptorId, Indexed, Indexer, KeychainIndexed, SpkIterator,
};
use alloc::{borrow::ToOwned, vec::Vec};
use alloc::{borrow::ToOwned, string::String, string::ToString, vec::Vec};
use bitcoin::{
key::Secp256k1, Amount, OutPoint, Script, ScriptBuf, SignedAmount, Transaction, TxOut, Txid,
};
Expand Down Expand Up @@ -973,33 +973,50 @@ pub enum InsertDescriptorError<K> {
},
}

impl<K: core::fmt::Debug> core::fmt::Display for InsertDescriptorError<K> {
/// Returns a shortened string representation of a descriptor.
/// Shows the first and last `edge_len` characters, separated by `...`.
fn short_descriptor(desc: &Descriptor<DescriptorPublicKey>, edge_len: usize) -> String {
Copy link
Contributor

Choose a reason for hiding this comment

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

I've tried this with the following bin:

use bdk_chain::bitcoin::key::Secp256k1;
use bdk_chain::indexer::keychain_txout::KeychainTxOutIndex;
use bdk_chain::miniscript::Descriptor;

const DESC: &str = "...";

fn main() {
    let lookahead = 10;
    let use_cache = true;
    let mut index = KeychainTxOutIndex::new(lookahead, use_cache);
    let secp = Secp256k1::new();
    let (desc, _) = Descriptor::parse_descriptor(&secp, DESC).unwrap();
    index.insert_descriptor("some", desc.clone()).unwrap();
    if let Err(e) = index.insert_descriptor("other", desc) {
        eprintln!("{}", e);
    }
}

I think the output is not very clear:

descriptor 'tr([...d45f9rlk' is already in use by another keychain 'some'.

I would keep only a fixed size prefix in the output, including the descriptor type and trying to capture the origin keypath if possible.
@qatkk proposed to increase the suffix to capture the checksum, but without any clear accessors to those components it is a "best guess" effort.
Maybe we should propose the implementation of them at the miniscript level, and add methods to get strings from DescriptorType for example.

let s = ToString::to_string(desc);

// +3 accounts for "..." so the shortened string won't exceed original length.
if s.len() <= edge_len * 2 + 3 {
return s;
}

format!("{}...{}", &s[..edge_len], &s[s.len() - edge_len..])
}

impl<K: core::fmt::Display> core::fmt::Display for InsertDescriptorError<K> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
InsertDescriptorError::DescriptorAlreadyAssigned {
existing_assignment: existing,
existing_assignment,
descriptor,
} => {
let short_desc = short_descriptor(descriptor, 4);
write!(
f,
"attempt to re-assign descriptor {descriptor:?} already assigned to {existing:?}"
"descriptor '{}' is already in use by another keychain '{}'.",
short_desc, existing_assignment
)
}
InsertDescriptorError::KeychainAlreadyAssigned {
existing_assignment: existing,
existing_assignment,
keychain,
} => {
let short_desc = short_descriptor(existing_assignment, 4);
write!(
f,
"attempt to re-assign keychain {keychain:?} already assigned to {existing:?}"
"keychain '{}' is already associated with another descriptor '{}'.",
keychain, short_desc
)
}
}
}
}

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

/// `ChangeSet` represents persistent updates to a [`KeychainTxOutIndex`].
///
Expand Down
26 changes: 22 additions & 4 deletions crates/chain/src/tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,28 @@ pub enum CalculateFeeError {
impl fmt::Display for CalculateFeeError {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
CalculateFeeError::MissingTxOut(outpoints) => write!(
f,
"missing `TxOut` for one or more of the inputs of the tx: {outpoints:?}",
),
CalculateFeeError::MissingTxOut(outpoints) => {
let max_show = 3;
let shown = outpoints.iter().take(max_show);
let remaining = outpoints.len().saturating_sub(max_show);

write!(f, "missing `TxOut` for input(s)")?;
if outpoints.is_empty() {
write!(f, ": <none>")
} else {
write!(f, ": ")?;
for (i, op) in shown.enumerate() {
if i > 0 {
write!(f, ", ")?;
}
write!(f, "{}", op)?;
}
if remaining > 0 {
write!(f, " (+{} more)", remaining)?;
}
Comment on lines +259 to +277
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:

diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs
index 457a478a..d170eccf 100644
--- a/crates/chain/src/tx_graph.rs
+++ b/crates/chain/src/tx_graph.rs
@@ -258,19 +258,16 @@ impl fmt::Display for CalculateFeeError {
         match self {
             CalculateFeeError::MissingTxOut(outpoints) => {
                 let max_show = 3;
-                let shown = outpoints.iter().take(max_show);
+                let shown: Vec<_> = outpoints.iter().take(max_show).collect();
                 let remaining = outpoints.len().saturating_sub(max_show);
 
                 write!(f, "missing `TxOut` for input(s)")?;
                 if outpoints.is_empty() {
                     write!(f, ": <none>")
                 } else {
-                    write!(f, ": ")?;
-                    for (i, op) in shown.enumerate() {
-                        if i > 0 {
-                            write!(f, ", ")?;
-                        }
-                        write!(f, "{}", op)?;
+                    write!(f, ": {}", shown[0])?;
+                    for op in &shown[1..] {
+                        write!(f, ", {}", op)?;
                     }
                     if remaining > 0 {
                         write!(f, " (+{} more)", remaining)?;

Ok(())
}
}
CalculateFeeError::NegativeFee(fee) => write!(
f,
"transaction is invalid according to the graph and has negative fee: {}",
Expand Down
9 changes: 9 additions & 0 deletions crates/chain/tests/test_keychain_txout_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ enum TestKeychain {
Internal,
}

impl core::fmt::Display for TestKeychain {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
TestKeychain::External => write!(f, "External"),
TestKeychain::Internal => write!(f, "Internal"),
}
}
}

fn parse_descriptor(descriptor: &str) -> Descriptor<DescriptorPublicKey> {
let secp = bdk_chain::bitcoin::secp256k1::Secp256k1::signing_only();
Descriptor::<DescriptorPublicKey>::parse_descriptor(&secp, descriptor)
Expand Down
18 changes: 17 additions & 1 deletion crates/core/src/spk_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ impl<I, D> SyncRequestBuilder<I, D> {
/// [`chain_tip`](SyncRequestBuilder::chain_tip) (if provided).
///
/// ```rust
/// # use std::io::{self, Write};
/// # use bdk_chain::{bitcoin::{hashes::Hash, ScriptBuf}, local_chain::LocalChain};
/// # use bdk_chain::spk_client::SyncRequest;
/// # let (local_chain, _) = LocalChain::from_genesis(Hash::all_zeros());
Expand All @@ -236,7 +237,22 @@ impl<I, D> SyncRequestBuilder<I, D> {
/// // Provide list of scripts to scan for transactions against.
/// .spks(scripts)
/// // This is called for every synced item.
/// .inspect(|item, progress| println!("{} (remaining: {})", item, progress.remaining()))
/// .inspect(|item, progress| {
/// let pc = (100.0 * progress.consumed() as f32) / progress.total() as f32;
/// match item {
/// // In this example I = (), so the first field of Spk is unit.
/// bdk_chain::spk_client::SyncItem::Spk((), spk) => {
/// eprintln!("[ SCANNING {pc:03.0}% ] script {}", spk);
/// }
/// bdk_chain::spk_client::SyncItem::Txid(txid) => {
/// eprintln!("[ SCANNING {pc:03.0}% ] txid {}", txid);
/// }
/// bdk_chain::spk_client::SyncItem::OutPoint(op) => {
/// eprintln!("[ SCANNING {pc:03.0}% ] outpoint {}", op);
/// }
/// }
/// let _ = io::stderr().flush();
/// })
/// // Finish constructing the sync request.
/// .build();
/// ```
Expand Down
17 changes: 12 additions & 5 deletions crates/file_store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,18 @@ pub enum StoreError {
impl core::fmt::Display for StoreError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
Self::Io(e) => write!(f, "io error trying to read file: {e}"),
Self::InvalidMagicBytes { got, expected } => write!(
f,
"file has invalid magic bytes: expected={expected:?} got={got:?}",
),
Self::Io(e) => write!(f, "io error trying to read file: {}", e),
Self::InvalidMagicBytes { got, expected } => {
write!(f, "file has invalid magic bytes: expected=0x")?;
Copy link
Contributor

@nymius nymius Jan 23, 2026

Choose a reason for hiding this comment

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

Error: could not open file store

Caused by:
    file has invalid magic bytes: expected=0x62646b5f6578616d706c655f656c65637472756d got=0x62646b5f6578616d706c5f656c65637472756d01

Given the format of the error output I would use invalid magic bytes without the file has.

This is a good opportunity to rewrite the others in the same spirit.

Also, stacking expected and got improves the feedback to the reader:

Error: could not open file store

Caused by:
    invalid magic bytes
expected: 0x62646b5f6578616d706c655f656c65637472756d
got:      0x62646b5f6578616d706c5f656c65637472756d01

for b in expected {
write!(f, "{:02x}", b)?;
}
write!(f, " got=0x")?;
for b in got {
write!(f, "{:02x}", b)?;
}
Ok(())
}
Self::Bincode(e) => write!(f, "bincode error while reading entry {e}"),
}
}
Expand Down
17 changes: 15 additions & 2 deletions examples/example_electrum/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,22 @@ fn main() -> anyhow::Result<()> {
.chain_tip(chain_tip.clone())
.inspect(|item, progress| {
let pc = (100 * progress.consumed()) as f32 / progress.total() as f32;
eprintln!("[ SCANNING {pc:03.0}% ] {item}");
match item {
bdk_chain::spk_client::SyncItem::Spk((keychain, index), spk) => {
eprintln!(
"[ SCANNING {pc:03.0}% ] script {} {} {}",
keychain, index, spk
);
}
bdk_chain::spk_client::SyncItem::Txid(txid) => {
eprintln!("[ SCANNING {pc:03.0}% ] txid {}", txid);
}
bdk_chain::spk_client::SyncItem::OutPoint(op) => {
eprintln!("[ SCANNING {pc:03.0}% ] outpoint {}", op);
}
}
let _ = io::stderr().flush();
});

let canonical_view = graph.canonical_view(
&*chain,
chain_tip.block_id(),
Expand Down
16 changes: 14 additions & 2 deletions examples/example_esplora/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,20 @@ fn main() -> anyhow::Result<()> {
.chain_tip(local_tip.clone())
.inspect(|item, progress| {
let pc = (100 * progress.consumed()) as f32 / progress.total() as f32;
eprintln!("[ SCANNING {pc:03.0}% ] {item}");
// Flush early to ensure we print at every iteration.
match item {
bdk_chain::spk_client::SyncItem::Spk((keychain, index), spk) => {
eprintln!(
"[ SCANNING {pc:03.0}% ] script {} {} {}",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: "[ SCANNING {pc:3.0}% ] script {} {} {}", instead of "[ SCANNING {pc:03.0}% ] script {} {} {}",

I prefer

[ SCANNING   6% ] outpoint 623cf110e23b15074e807a551a751856eb0a95f23a4a60f125d943d1d6e7cb6a:0

to

[ SCANNING 006% ] outpoint 623cf110e23b15074e807a551a751856eb0a95f23a4a60f125d943d1d6e7cb6a:0

keychain, index, spk
);
}
bdk_chain::spk_client::SyncItem::Txid(txid) => {
eprintln!("[ SCANNING {pc:03.0}% ] txid {}", txid);
}
bdk_chain::spk_client::SyncItem::OutPoint(op) => {
eprintln!("[ SCANNING {pc:03.0}% ] outpoint {}", op);
}
}
let _ = io::stderr().flush();
});

Expand Down
Loading