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.

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

Jump to bottom

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

Closed
apoelstra opened this issue Oct 17, 2022 · 2 comments · Fixed by #478
Closed

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

apoelstra opened this issue Oct 17, 2022 · 2 comments · Fixed by #478

Comments

@apoelstra
Copy link
Member

This would avoid the need for hacks like to_string_no_chksum in elements-miniscript, where we wrap descriptors in other stuff and add a new global checksum.
This was referenced Oct 17, 2022
Open
Merged
@sanket1729
Copy link
Member

concept ACK. Will get to this one this week

@apoelstra
Copy link
Member Author

I beat you to it :) I have an implementation that I will push in the next 10 minutes, once my tests all pass on 1.41.

@apoelstra apoelstra mentioned this issue Oct 18, 2022
Merged
sanket1729 added a commit that referenced this issue Oct 18, 2022
@sanket1729
Merge #478: allow disabling the checksum with alternate Display
72dab64 
7f5bbdf fixes for 1.41.0 (Andrew Poelstra)
7577e8c checksum: use wrapper around fmt::Formatter for all descriptor types (Andrew Poelstra)
84f0292 checksum: pull computation apart into an engine (Andrew Poelstra)

Pull request description:

  Fixes #477

ACKs for top commit:
  sanket1729:
    ACK 7f5bbdf. The nit is just a stylistic comment.

Tree-SHA512: f4a5bd67bf15d6e59258ec42f0b046b8321103f1baa0e2f893d4e3379623c636f2092596c0c79a0d3b949a436307f052efca72ad2784e42dd23ad861e3036e85
joemphilips pushed a commit to joemphilips/rust-miniscript that referenced this issue Dec 9, 2022
@apoelstra @joemphilips
checksum: use wrapper around fmt::Formatter for all descriptor types
e664ad4 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

allow disabling the checksum with alternate Display apoelstra/rust-miniscript
2 participants
@apoelstra @sanket1729