Skip to content

Conversation

@tcharding
Copy link
Member

Improve the code around trait MiniscriptKey by doing

  • Improve docs
  • Add regression tests
  • Refactor the trait

As part of refactoring the trait `MiniscriptKey`; improve the rustdocs
on the trait.
In preparation for refactoring the `MiniscriptKey` implementations add
regression tests.
The `MiniscriptKey` trait has a default implementation for
`is_uncompressed` that returns `false`. No need to re-implement it.
Do minor improvement to the docs for the `bitocin::PublicKey`
implementation of `is_uncompressed`.
All the non-string keys use hash160 hashes, we can simplify the logic
by mirroring the current implementation for `XOnlyPublicKey`.
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 3092830. Thank you again for the clean changes

@sanket1729 sanket1729 merged commit 1a2a49a into rust-bitcoin:master Apr 27, 2022
@tcharding tcharding deleted the refactor-miniscritp-key branch April 27, 2022 22:16
heap-coder added a commit to heap-coder/rust-miniscript that referenced this pull request Sep 27, 2025
309283029bed9a966595d3affb9581029646658e Simplify to_pubkeyhash (Tobin C. Harding)
d495945b9d3f25446a90e1cc6e5608626c9438f4 Improve docs on is_uncompressed (Tobin C. Harding)
47fc2c6790817f76e45eff3e69cbf2005a63a61d Remove re-implementation of default method (Tobin C. Harding)
d1da214e8d944a37cf31fca1f12033d76106f39e Add regression tests for MiniscriptKey impls (Tobin C. Harding)
0e3635f64854ee56fbd1c1058814d5f8cad2cd1a Improve docs on trait MiniscriptKey (Tobin C. Harding)

Pull request description:

  Improve the code around trait `MiniscriptKey` by doing

  - Improve docs
  - Add regression tests
  - Refactor the trait

ACKs for top commit:
  sanket1729:
    ACK 309283029bed9a966595d3affb9581029646658e. Thank you again for the clean changes

Tree-SHA512: a449544f16459b21f368dcd7e1cefaf7e4e2794347c66e486ecb3b73fc4e3b26fc44c86311a9dbc2bc8185fb3fae31e1c82a214b41bfb3d1c5bb4c4a9fea09f9
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.

2 participants