We seem to be using fmt::Write::write_str with the implied suggestion that we avoid allocation of a string but I think that fmt::Argumenst::as_str() gets called and only avoids allocation for static strings (no uses of placeholders {}), since we always use {} I'm reasonably confident that this is allocating.
There are some optimisation possibilities by using iterator adapters instead of strings, ie take an iterarator of chars and yield the chars + # + the checksum.
The public API is a bit messy:
checksum::Formatter should not be public because it is not really a general purpose formatter, it is specifically for writing a descriptor to a formatter and overriding the use of alternate to control the addition of the checksum.
descriptor::checksum::desc_checksum could be better named as could descriptor::checksum::verify_checksum to use Rust naming convention (ie., users should be using a single path on functions so checksum::verify seems better, not sure about the other one.
Error handling could be improved if/when we start to do errors elsewhere
After working on https://github.com/rust-bitcoin/rust-miniscript/pull/608 I noticed a few things:
fmt::Write::write_str
with the implied suggestion that we avoid allocation of a string but I think thatfmt::Argumenst::as_str()
gets called and only avoids allocation for static strings (no uses of placeholders{}
), since we always use{}
I'm reasonably confident that this is allocating.checksum::Formatter
should not be public because it is not really a general purpose formatter, it is specifically for writing a descriptor to a formatter and overriding the use ofalternate
to control the addition of the checksum.descriptor::checksum::desc_checksum
could be better named as coulddescriptor::checksum::verify_checksum
to use Rust naming convention (ie., users should be using a single path on functions sochecksum::verify
seems better, not sure about the other one.Related:
323
554