Skip to content
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.

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

allow disabling the checksum with alternate Display #478

Merged
merged 3 commits into from
Oct 18, 2022

Conversation

apoelstra
Copy link
Member

Fixes #477

This will let us avoid some allocations when formatting descriptors,
and make it simpler to selectively append a checksum.
This allows us to remove the checksum with {:#}, and avoid extra
allocations in all cases. Deprecates the old to_string_no_checksum()
methods, which were inefficient and (in the case of Taproot) not
even exported.

Fixes rust-bitcoin#477
Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK 7f5bbdf. The nit is just a stylistic comment.

impl<'f, 'a> fmt::Write for Formatter<'f, 'a> {
fn write_str(&mut self, s: &str) -> fmt::Result {
self.fmt.write_str(s)?;
if self.eng.input(s).is_ok() {
Copy link
Member

Choose a reason for hiding this comment

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

nit: This can be written as self.engine.input(s).map_err(|| fmt::Error)

@sanket1729 sanket1729 merged commit 72dab64 into rust-bitcoin:master Oct 18, 2022
@apoelstra apoelstra deleted the 2022-10--alt-display branch October 18, 2022 19:00
sanket1729 added a commit to sanket1729/elements-miniscript that referenced this pull request Oct 21, 2022
…022_10

72dab64 (HEAD -> master, upstream/master, origin/master, origin/HEAD) Merge rust-bitcoin/rust-miniscript#478: allow disabling the checksum with alternate `Display`

Also had to fix a bunch of test cases with bare descriptors without el
prefix.
Add a small API to get the nested segwit descriptors without the el
prefix
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.

feature request: # flag should turn off checksum when formatting descriptors
2 participants