Skip to content

fix: no Debug on Display implementations #12

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix: no Debug on Display implementations #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
306dacf
feat: implement Display for KeychainKind
luisschwab Mar 9, 2025
2914c07
feat: implement Display for LoadMismatch
luisschwab Mar 9, 2025
f6d34d7
fix: no Debug on LoadError
luisschwab Mar 9, 2025
b37355a
fix: remove Debugs from CreateTxError
luisschwab Mar 13, 2025
0d8d0ec
feat: add error messages for CreateWithPersistError Display implement…
luisschwab Mar 21, 2025
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
10 changes: 10 additions & 0 deletions wallet/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use alloc::boxed::Box;
use chain::{ChainPosition, ConfirmationBlockTime};
use core::convert::AsRef;
use core::fmt;

use bitcoin::transaction::{OutPoint, Sequence, TxOut};
use bitcoin::{psbt, Weight};
Expand All @@ -37,6 +38,15 @@ impl KeychainKind {
}
}

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

impl AsRef<[u8]> for KeychainKind {
fn as_ref(&self) -> &[u8] {
match self {
Expand Down
6 changes: 3 additions & 3 deletions wallet/src/wallet/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl fmt::Display for CreateTxError {
Self::Descriptor(e) => e.fmt(f),
Self::Policy(e) => e.fmt(f),
CreateTxError::SpendingPolicyRequired(keychain_kind) => {
write!(f, "Spending policy required: {:?}", keychain_kind)
write!(f, "Spending policy required: {}", keychain_kind)
}
CreateTxError::Version0 => {
write!(f, "Invalid version `0`")
Expand All @@ -127,12 +127,12 @@ impl fmt::Display for CreateTxError {
requested,
required,
} => {
write!(f, "TxBuilder requested timelock of `{:?}`, but at least `{:?}` is required to spend from this script", required, requested)
write!(f, "TxBuilder requested timelock of `{}`, but at least `{}` is required to spend from this script", requested, required)
}
CreateTxError::RbfSequenceCsv { sequence, csv } => {
write!(
f,
"Cannot enable RBF with nSequence `{:?}` given a required OP_CSV of `{:?}`",
"Cannot enable RBF with nSequence `{}` given a required OP_CSV of `{}`",
sequence, csv
)
}
Expand Down
42 changes: 40 additions & 2 deletions wallet/src/wallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,9 @@ impl fmt::Display for LoadError {
LoadError::MissingNetwork => write!(f, "loaded data is missing network type"),
LoadError::MissingGenesis => write!(f, "loaded data is missing genesis hash"),
LoadError::MissingDescriptor(k) => {
write!(f, "loaded data is missing descriptor for keychain {k:?}")
write!(f, "loaded data is missing descriptor for {k} keychain")
}
LoadError::Mismatch(mismatch) => write!(f, "data mismatch: {mismatch:?}"),
LoadError::Mismatch(e) => write!(f, "{e}"),
}
}
}
Expand Down Expand Up @@ -232,6 +232,44 @@ pub enum LoadMismatch {
},
}

impl fmt::Display for LoadMismatch {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
LoadMismatch::Network { loaded, expected } => {
write!(
f,
"Network mismatch: loaded {}, expected {}",
loaded, expected
)
}
LoadMismatch::Genesis { loaded, expected } => {
write!(
f,
"Genesis hash mismatch: loaded {}, expected {}",
loaded, expected
)
}
LoadMismatch::Descriptor {
keychain,
loaded,
expected,
} => {
write!(
f,
"Descriptor mismatch for {} keychain: loaded {}, expected {}",
keychain,
loaded
.as_ref()
.map_or("None".to_string(), |d| d.to_string()),
expected
.as_ref()
.map_or("None".to_string(), |d| d.to_string())
)
}
}
}
}

impl From<LoadMismatch> for LoadError {
fn from(mismatch: LoadMismatch) -> Self {
Self::Mismatch(mismatch)
Expand Down
54 changes: 45 additions & 9 deletions wallet/src/wallet/persisted.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,16 @@ use core::{
pin::Pin,
};

use alloc::boxed::Box;
use alloc::{
boxed::Box,
string::{String, ToString},
};
use chain::Merge;

use crate::{descriptor::DescriptorError, ChangeSet, CreateParams, LoadParams, Wallet};
use crate::{
descriptor::{calc_checksum, DescriptorError},
ChangeSet, CreateParams, LoadParams, Wallet,
};

/// Trait that persists [`PersistedWallet`].
///
Expand Down Expand Up @@ -363,16 +369,46 @@ pub enum CreateWithPersistError<E> {
impl<E: fmt::Display> fmt::Display for CreateWithPersistError<E> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Persist(err) => fmt::Display::fmt(err, f),
Self::DataAlreadyExists(changeset) => write!(
f,
"Cannot create wallet in persister which already contains wallet data: {:?}",
changeset
),
Self::Descriptor(err) => fmt::Display::fmt(&err, f),
Self::Persist(err) => write!(f, "{}", err),
Self::DataAlreadyExists(changeset) => {
write!(
f,
"Cannot create wallet in a persister which already contains data: {}",
changeset_info(changeset)
)
}
Self::Descriptor(err) => {
write!(f, "{err}")
}
}
}
}

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

/// Helper function to display basic information about a [`ChangeSet`].
fn changeset_info(changeset: &ChangeSet) -> String {
let network = match &changeset.network {
Some(network) => network.to_string(),
None => "None".to_string(),
};

let mut checksum = String::new();
if let Some(desc) = &changeset.descriptor {
if let Ok(ck) = calc_checksum(&desc.to_string()) {
checksum += ck.as_str();
}
}
if let Some(desc) = &changeset.change_descriptor {
if let Ok(ck) = calc_checksum(&desc.to_string()) {
checksum += ck.as_str();
}
}
// Don't return an empty checksum
if checksum.is_empty() {
checksum = "None".to_string();
}

format!("Network: {}, Descriptor Checksum: {}", network, checksum)
}
Comment on lines +390 to +414
Copy link
Member

Choose a reason for hiding this comment

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

Nit: change signature of this to take in f: &mut fmt::Formatter instead.

Also:

  • Separate out the descriptor checksums. `descriptor_checksum={}, change_descriptor_checksum={}.
  • All more info: tx_count, anchor_count, block_count, etc.

Copy link
Member Author

@luisschwab luisschwab May 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What do you think @evanlinjin? cc @ValuedMammal

/// Helper function to display basic information about a [`ChangeSet`].
fn changeset_info(f: &mut fmt::Formatter<'_>, changeset: &ChangeSet) -> fmt::Result {
    let network = changeset.network
        .as_ref()
        .map_or("None".to_string(), |n| n.to_string());

    let descriptor_checksum = changeset.descriptor
        .as_ref()
        .and_then(|d| calc_checksum(&d.to_string()).ok())
        .unwrap_or_else(|| "None".to_string());

    let change_descriptor_checksum = changeset.change_descriptor
        .as_ref()
        .and_then(|d| calc_checksum(&d.to_string()).ok())
        .unwrap_or_else(|| "None".to_string());

    let tx_count = changeset.tx_graph.txs.len();

    let anchor_count = changeset.tx_graph.anchors.len();

    let block_count = if let Some(&count) = changeset.local_chain.blocks.keys().last() {
        count
    } else {
        0
    };

    writeln!(f, "")?;
    writeln!(f, "Network: {}", network)?;
    writeln!(f, "Descriptor Checksum: {}", descriptor_checksum)?;
    writeln!(f, "Change Descriptor Checksum: {}", change_descriptor_checksum)?;
    writeln!(f, "Transaction Count: {}", tx_count)?;
    writeln!(f, "Anchor Count: {}", anchor_count)?;
    writeln!(f, "Block Count: {}", block_count)?;
    Ok(())
}

Then it gets called by CreateWithPersistError like this:

impl<E: fmt::Display> fmt::Display for CreateWithPersistError<E> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::Persist(err) => write!(f, "{}", err),
            Self::DataAlreadyExists(changeset) => {
                write!(f, "Cannot create wallet in a persister which already contains data: ")?;
                changeset_info(f, changeset)
            }
            Self::Descriptor(err) => {
                write!(f, "{err}")
            }
        }
    }
}

Loading