Skip to content

Conversation

@caarloshenriq
Copy link

@tnull tnull linked an issue Oct 28, 2025 that may be closed by this pull request
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

LGTM

Makes sense, IMO.

@tnull tnull requested a review from elsirion October 28, 2025 11:03
src/lib.rs Outdated
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Mnemonic")
.field("lang", &self.language())
.finish_non_exhaustive()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, unfortunately this is only available since rustc 1.53 (see https://doc.rust-lang.org/std/fmt/struct.DebugStruct.html#method.finish_non_exhaustive). Given that we currently still support 1.41, we should probably just use finish for the time being (preferably leaving a TODO comment to switch to finish_non_exhaustive once we drop that MSRV requirement.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review. I change to .finish() and added a TODO to migrate to finish_non_exhaustive() when we bump MSRV

Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

LGTM, please squash commits and provide a proper commit message (briefly) describing what you did, why it was necessary, and which approach you took.

For guidance on how to write good commit messages, see for example https://cbea.ms/git-commit/

Replace the Debug output with a manual impl that prints only the language and
omits mnemonic words/entropy to avoid secret leakage in logs.

Approach: use `debug_struct("Mnemonic").field("lang", &self.language()).finish()`
to keep MSRV compatibility (no `finish_non_exhaustive`).

Refs:rust-bitcoin#93
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider obscuring seed from Debug implementation of Mnemonic

2 participants