fix: no `Debug` on `Display` implementations #2083

Dmenec · 2025-12-11T11:30:15Z

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:

I have signed all my commits
I followed the contribution guidelines
I ran just check, just fmt and just test before committing

qatkk · 2025-12-21T13:06:33Z

Concept ACK — this approach looks good, thanks for the changes. I ran the tests locally and they all passed.

While trying to recreate the initial issue’s scenario (#1933), I found that bdk_wallet master still references the older API names (list_canonical_txs, filter_chain_unspents, filter_chain_txouts, from_genesis_hash), whereas this PR’s bdk_chain exposes the newer canonicalization API (canonical_iter) and LocalChain::from_genesis. As a result, building bdk_wallet against this PR failed. Is there any branch in bdk_wallet that already adopts the new APIs, so I can test against them?

I have a few small suggestions below. Most of these are nits, but since this PR is already addressing the area, I thought it’d be reasonable to mention them here:

It might be nice to add a CI to prevent using Debug for Display in future changes.
In the short_descriptor function, it could be useful to show the last 8 characters of the descriptor so the full checksum is visible.
A small note on InvalidMagicBytes: it might be helpful to also show any difference between the length of expected and received byte.

evanlinjin

Quality work.

Just a couple requested changes:

Commit messages should contain a scope (as mentioned in the conventional commits spec). The scope should be the name of the package affected: core, chain, example, etc.
Commits that break the API should be marked with !.

And a nit:

Error messages should be somewhat consistent. I personally don't think we should suggest actions in the message, just state what the error is.

crates/chain/src/indexer/keychain_txout.rs

evanlinjin · 2026-01-14T11:22:13Z

@Dmenec are you able to make progress on this?

Dmenec · 2026-01-14T16:09:10Z

@evanlinjin Pushed the updates. Let me know if everything checks out :)

evanlinjin

utACK 532e2ec

I'm happy with how this is looking. Unable to test until I have access to a computer again.

crates/chain/src/indexer/keychain_txout.rs

Dmenec · 2026-01-21T19:34:25Z

Having some trouble with the no_std CI check for the esplora crate... Does it actually support no_std, or should be treated as std‑only?

oleonardolima · 2026-01-21T22:51:44Z

Having some trouble with the no_std CI check for the esplora crate... Does it actually support no_std, or should be treated as std‑only?

@Dmenec It's now fixed on master, a rebase should fix it.

crates/chain/src/tx_graph.rs

crates/file_store/src/lib.rs

examples/example_esplora/src/main.rs

crates/chain/src/indexer/keychain_txout.rs

crates/chain/src/tx_graph.rs

crates/file_store/src/lib.rs

examples/example_electrum/src/main.rs

evanlinjin

Sorry for the bike-shedding. We're very close.

crates/chain/src/indexer/keychain_txout.rs

crates/file_store/src/lib.rs

oleonardolima

Overall it looks good, it just need to address Evan's comments.

crates/chain/src/tx_graph.rs

…iptorError Replace Debug-based formatting with user-friendly Display messages.

…eError Limited to 3 max shown items and added a suffix when there are additional entries.

…rror. Format magic bytes as 0x-prefixed hex.

evanlinjin

ACK 823e4e9

I'm going to merge this as is. My nit-pick can be addressed in follow ups.

evanlinjin · 2026-02-07T10:21:50Z

crates/chain/src/tx_graph.rs

-                fee.display_dynamic()
-            ),
+            CalculateFeeError::MissingTxOut(outpoints) => {
+                let max_show = 3;


If I were to nit-pick, I would say get rid of this and just show all prev outputs.

evanlinjin · 2026-02-07T10:30:36Z

crates/chain/src/tx_graph.rs

+
+                write!(f, "cannot calculate fee, missing previous output(s): ")?;
+                if outpoints.is_empty() {
+                    write!(f, "<none>")


If this ever happens, it's a bug in the library - let's debug_assert! it?

github-project-automation bot added this to BDK Chain Dec 11, 2025

ValuedMammal assigned Dmenec Dec 12, 2025

ValuedMammal moved this to In Progress in BDK Chain Dec 12, 2025

ValuedMammal added this to the Chain 0.24.0 milestone Dec 12, 2025

Dmenec force-pushed the fix/no-debug-on-display-impls branch from 492114b to b73016e Compare December 14, 2025 14:57

ValuedMammal mentioned this pull request Dec 28, 2025

Update bdk_chain to latest (b4c8c6a1) bitcoindevkit/bdk_wallet#360

Draft

8 tasks

evanlinjin requested changes Jan 6, 2026

View reviewed changes

crates/chain/src/indexer/keychain_txout.rs Outdated Show resolved Hide resolved

crates/chain/src/indexer/keychain_txout.rs Outdated Show resolved Hide resolved

crates/chain/src/indexer/keychain_txout.rs Show resolved Hide resolved

Dmenec force-pushed the fix/no-debug-on-display-impls branch from b73016e to 532e2ec Compare January 14, 2026 16:04

evanlinjin approved these changes Jan 15, 2026

View reviewed changes

crates/chain/src/indexer/keychain_txout.rs Outdated Show resolved Hide resolved

Dmenec force-pushed the fix/no-debug-on-display-impls branch 3 times, most recently from 789d8a3 to d65eed0 Compare January 21, 2026 11:06

Dmenec force-pushed the fix/no-debug-on-display-impls branch from f0698e1 to d65eed0 Compare January 22, 2026 13:06

nymius reviewed Jan 23, 2026

View reviewed changes

crates/chain/src/tx_graph.rs Show resolved Hide resolved

nymius reviewed Jan 23, 2026

View reviewed changes

crates/file_store/src/lib.rs Outdated Show resolved Hide resolved

nymius reviewed Jan 23, 2026

View reviewed changes

examples/example_esplora/src/main.rs Outdated Show resolved Hide resolved

nymius reviewed Jan 23, 2026

View reviewed changes

crates/chain/src/indexer/keychain_txout.rs Outdated Show resolved Hide resolved

Dmenec force-pushed the fix/no-debug-on-display-impls branch from d65eed0 to ae17df2 Compare January 27, 2026 13:50

nymius reviewed Jan 27, 2026

View reviewed changes

crates/chain/src/tx_graph.rs Outdated Show resolved Hide resolved

nymius reviewed Jan 27, 2026

View reviewed changes

crates/file_store/src/lib.rs Outdated Show resolved Hide resolved

nymius reviewed Jan 27, 2026

View reviewed changes

examples/example_electrum/src/main.rs Outdated Show resolved Hide resolved

Dmenec force-pushed the fix/no-debug-on-display-impls branch 2 times, most recently from 91abd25 to 75d1bd0 Compare January 27, 2026 16:43

evanlinjin requested changes Jan 28, 2026

View reviewed changes

crates/chain/src/indexer/keychain_txout.rs Outdated Show resolved Hide resolved

crates/file_store/src/lib.rs Outdated Show resolved Hide resolved

oleonardolima reviewed Feb 2, 2026

View reviewed changes

crates/chain/src/tx_graph.rs Outdated Show resolved Hide resolved

Dmenec added 2 commits February 5, 2026 12:50

fix(chain)!: remove Debug usage and implement Display for InsertDescr…

5d715c2

…iptorError Replace Debug-based formatting with user-friendly Display messages.

docs(core): add detailed SyncRequest output example

d0889f3

Dmenec force-pushed the fix/no-debug-on-display-impls branch 2 times, most recently from 2763989 to 744a3de Compare February 5, 2026 12:21

Dmenec added 2 commits February 5, 2026 13:30

fix(chain)!: remove Debug usage and implement Display for CalculateFe…

5a85f6e

…eError Limited to 3 max shown items and added a suffix when there are additional entries.

fix(file_store)!: remove Debug usage and implement Display for StoreE…

823e4e9

…rror. Format magic bytes as 0x-prefixed hex.

Dmenec force-pushed the fix/no-debug-on-display-impls branch from 744a3de to 823e4e9 Compare February 5, 2026 12:30

evanlinjin approved these changes Feb 7, 2026

View reviewed changes

evanlinjin merged commit b45ecb9 into bitcoindevkit:master Feb 7, 2026
17 checks passed

github-project-automation bot moved this from In Progress to Done in BDK Chain Feb 7, 2026

evanlinjin reviewed Feb 7, 2026

View reviewed changes

fix: no Debug on Display implementations #2083

fix: no Debug on Display implementations #2083

Uh oh!

Conversation

Dmenec commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog notice

Checklists

All Submissions:

Uh oh!

qatkk commented Dec 21, 2025

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evanlinjin commented Jan 14, 2026

Uh oh!

Dmenec commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Dmenec commented Jan 21, 2026

Uh oh!

oleonardolima commented Jan 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

oleonardolima left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

evanlinjin Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

evanlinjin Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fix: no `Debug` on `Display` implementations #2083

fix: no `Debug` on `Display` implementations #2083

Dmenec commented Dec 11, 2025 •

edited

Loading

Dmenec commented Jan 14, 2026 •

edited

Loading