From cb1344d9b9a6c776dcd47609ab67f481f56f3174 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 09:54:18 +1100 Subject: [PATCH 1/8] Improve rustdoc on the MiniscriptKey trait We want to support private keys in descriptors, in preparation improve the rustdocs on the `MiniscriptKey` trait by doing: - Use "key" instead of "pukbey". - Fix the links - Improve spacing, use header body format - Don't link to hashes types --- src/lib.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index eac67c5d1..c32380e8a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -147,34 +147,31 @@ pub use crate::primitives::absolute_locktime::{AbsLockTime, AbsLockTimeError}; pub use crate::primitives::relative_locktime::{RelLockTime, RelLockTimeError}; pub use crate::primitives::threshold::{Threshold, ThresholdError}; -/// Public key trait which can be converted to Hash type +/// Trait representing a key which can be converted to a hash type. pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Hash { - /// Returns true if the pubkey is uncompressed. Defaults to `false`. + /// Returns true if the key is serialized uncompressed (defaults to `false`). fn is_uncompressed(&self) -> bool { false } - /// Returns true if the pubkey is an x-only pubkey. Defaults to `false`. + /// Returns true if the key is an x-only pubkey (defaults to `false`). // This is required to know what in DescriptorPublicKey to know whether the inner // key in allowed in descriptor context fn is_x_only_key(&self) -> bool { false } - /// Returns the number of different derivation paths in this key. Only >1 for keys - /// in BIP389 multipath descriptors. + /// Returns the number of different derivation paths in this key (defaults to `0`). + /// + /// Only >1 for keys in BIP389 multipath descriptors. fn num_der_paths(&self) -> usize { 0 } - /// The associated [`bitcoin::hashes::sha256::Hash`] for this [`MiniscriptKey`], used in the - /// sha256 fragment. + /// The associated `sha256::Hash` for this `MiniscriptKey`, used in the sha256 fragment. type Sha256: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; - /// The associated [`miniscript::hash256::Hash`] for this [`MiniscriptKey`], used in the - /// hash256 fragment. + /// The associated `hash256::Hash` for this `MiniscriptKey`, used in the hash256 fragment. type Hash256: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; - /// The associated [`bitcoin::hashes::ripemd160::Hash`] for this [`MiniscriptKey`] type, used - /// in the ripemd160 fragment. + /// The associated `ripemd160::Hash` for this `MiniscriptKey` type, used in the ripemd160 fragment. type Ripemd160: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; - /// The associated [`bitcoin::hashes::hash160::Hash`] for this [`MiniscriptKey`] type, used in - /// the hash160 fragment. + /// The associated `hash160::Hash` for this `MiniscriptKey` type, used in the hash160 fragment. type Hash160: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; } From cc0ce7982a4358032328028c570bdde770a455ee Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 09:56:57 +1100 Subject: [PATCH 2/8] Move trait method Clean up `MiniscriptKey` implementation for `bitcoin::PublicKey`. - Be uniform, put the trait method implementation below the associated types. - Trait methods have documentation on the trait, remove the unnecessary rustdoc on the implementation. --- src/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c32380e8a..e42c4aa74 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -183,13 +183,12 @@ impl MiniscriptKey for bitcoin::secp256k1::PublicKey { } impl MiniscriptKey for bitcoin::PublicKey { - /// Returns the compressed-ness of the underlying secp256k1 key. - fn is_uncompressed(&self) -> bool { !self.compressed } - type Sha256 = sha256::Hash; type Hash256 = hash256::Hash; type Ripemd160 = ripemd160::Hash; type Hash160 = hash160::Hash; + + fn is_uncompressed(&self) -> bool { !self.compressed } } impl MiniscriptKey for bitcoin::secp256k1::XOnlyPublicKey { From a8d593844a0fa1992f331ed992406952ab7e5470 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 09:59:18 +1100 Subject: [PATCH 3/8] Remove "what" comment We can see that the hashes are specified as strings, no need to comment this. --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index e42c4aa74..43d217eab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -201,7 +201,7 @@ impl MiniscriptKey for bitcoin::secp256k1::XOnlyPublicKey { } impl MiniscriptKey for String { - type Sha256 = String; // specify hashes as string + type Sha256 = String; type Hash256 = String; type Ripemd160 = String; type Hash160 = String; From d8fb367cb82624915e41af3e5a7eca39fc4f4370 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 10:17:19 +1100 Subject: [PATCH 4/8] Improve rustdocs on ToPublicKey Clean up the `ToPublicKey` trait docs. Note this leaves the links to the hash types in there since they are the return type. --- src/lib.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 43d217eab..c0207884c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -207,21 +207,21 @@ impl MiniscriptKey for String { type Hash160 = String; } -/// Trait describing public key types which can be converted to bitcoin pubkeys +/// Trait describing key types that can be converted to bitcoin public keys. pub trait ToPublicKey: MiniscriptKey { - /// Converts an object to a public key + /// Converts key to a public key. fn to_public_key(&self) -> bitcoin::PublicKey; - /// Convert an object to x-only pubkey + /// Converts key to an x-only public key. fn to_x_only_pubkey(&self) -> bitcoin::secp256k1::XOnlyPublicKey { let pk = self.to_public_key(); bitcoin::secp256k1::XOnlyPublicKey::from(pk.inner) } - /// Obtain the public key hash for this MiniscriptKey - /// Expects an argument to specify the signature type. - /// This would determine whether to serialize the key as 32 byte x-only pubkey - /// or regular public key when computing the hash160 + /// Obtains the pubkey hash for this key (as a `MiniscriptKey`). + /// + /// Expects an argument to specify the signature type. This determines whether to serialize + /// the key as 32 byte x-only pubkey or regular public key when computing the hash160. fn to_pubkeyhash(&self, sig_type: SigType) -> hash160::Hash { match sig_type { SigType::Ecdsa => hash160::Hash::hash(&self.to_public_key().to_bytes()), @@ -229,16 +229,16 @@ pub trait ToPublicKey: MiniscriptKey { } } - /// Converts the generic associated [`MiniscriptKey::Sha256`] to [`sha256::Hash`] + /// Converts the associated [`MiniscriptKey::Sha256`] type to a [`sha256::Hash`]. fn to_sha256(hash: &::Sha256) -> sha256::Hash; - /// Converts the generic associated [`MiniscriptKey::Hash256`] to [`hash256::Hash`] + /// Converts the associated [`MiniscriptKey::Hash256`] type to a [`hash256::Hash`]. fn to_hash256(hash: &::Hash256) -> hash256::Hash; - /// Converts the generic associated [`MiniscriptKey::Ripemd160`] to [`ripemd160::Hash`] + /// Converts the associated [`MiniscriptKey::Ripemd160`] type to a [`ripemd160::Hash`]. fn to_ripemd160(hash: &::Ripemd160) -> ripemd160::Hash; - /// Converts the generic associated [`MiniscriptKey::Hash160`] to [`hash160::Hash`] + /// Converts the associated [`MiniscriptKey::Hash160`] type to a [`hash160::Hash`]. fn to_hash160(hash: &::Hash160) -> hash160::Hash; } From b295680af9eb8071733c48fcc08c01cd76a6a525 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 10:18:40 +1100 Subject: [PATCH 5/8] Remove code comment on is_x_only_key The `is_x_only_key` trait method is used for more than one thing, the code comment is either stale, not exhaustive, or wrong. Let's just remove it. --- src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c0207884c..4ed121ed8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -153,8 +153,6 @@ pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Ha fn is_uncompressed(&self) -> bool { false } /// Returns true if the key is an x-only pubkey (defaults to `false`). - // This is required to know what in DescriptorPublicKey to know whether the inner - // key in allowed in descriptor context fn is_x_only_key(&self) -> bool { false } /// Returns the number of different derivation paths in this key (defaults to `0`). From b7a559d1793189ef10530d68b60b53f150b9ed31 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 10:25:48 +1100 Subject: [PATCH 6/8] Remove default trait method implementations Implementors of `MiniscriptKey` must reason about the three trait methods, this implies the trait methods are required, not provided. We are using the default impls to remove code duplication, this is an abuse of the default impls. It makes the docs read incorrectly, by implying that implementors _do not_ need to reason about these trait methods. Remove default trait method implementations and manually implement the trait methods for all current implementors. --- fuzz/src/lib.rs | 4 ++++ src/interpreter/mod.rs | 2 ++ src/lib.rs | 18 +++++++++++++++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index f9cae61df..e57e7a6c2 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -40,6 +40,10 @@ impl MiniscriptKey for FuzzPk { type Ripemd160 = u8; type Hash160 = u8; type Hash256 = u8; + + fn is_uncompressed(&self) -> bool { false } + fn is_x_only_key(&self) -> bool { false } + fn num_der_paths(&self) -> usize { 0 } } impl ToPublicKey for FuzzPk { diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 77ad18136..1d6ed9695 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -130,6 +130,8 @@ impl MiniscriptKey for BitcoinKey { BitcoinKey::XOnlyPublicKey(_) => false, } } + fn is_x_only_key(&self) -> bool { false } + fn num_der_paths(&self) -> usize { 0 } } impl<'txin> Interpreter<'txin> { diff --git a/src/lib.rs b/src/lib.rs index 4ed121ed8..2bea8d005 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -150,15 +150,15 @@ pub use crate::primitives::threshold::{Threshold, ThresholdError}; /// Trait representing a key which can be converted to a hash type. pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Hash { /// Returns true if the key is serialized uncompressed (defaults to `false`). - fn is_uncompressed(&self) -> bool { false } + fn is_uncompressed(&self) -> bool; /// Returns true if the key is an x-only pubkey (defaults to `false`). - fn is_x_only_key(&self) -> bool { false } + fn is_x_only_key(&self) -> bool; /// Returns the number of different derivation paths in this key (defaults to `0`). /// /// Only >1 for keys in BIP389 multipath descriptors. - fn num_der_paths(&self) -> usize { 0 } + fn num_der_paths(&self) -> usize; /// The associated `sha256::Hash` for this `MiniscriptKey`, used in the sha256 fragment. type Sha256: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; @@ -178,6 +178,10 @@ impl MiniscriptKey for bitcoin::secp256k1::PublicKey { type Hash256 = hash256::Hash; type Ripemd160 = ripemd160::Hash; type Hash160 = hash160::Hash; + + fn is_uncompressed(&self) -> bool { false } + fn is_x_only_key(&self) -> bool { false } + fn num_der_paths(&self) -> usize { 0 } } impl MiniscriptKey for bitcoin::PublicKey { @@ -187,6 +191,8 @@ impl MiniscriptKey for bitcoin::PublicKey { type Hash160 = hash160::Hash; fn is_uncompressed(&self) -> bool { !self.compressed } + fn is_x_only_key(&self) -> bool { false } + fn num_der_paths(&self) -> usize { 0 } } impl MiniscriptKey for bitcoin::secp256k1::XOnlyPublicKey { @@ -195,7 +201,9 @@ impl MiniscriptKey for bitcoin::secp256k1::XOnlyPublicKey { type Ripemd160 = ripemd160::Hash; type Hash160 = hash160::Hash; + fn is_uncompressed(&self) -> bool { false } fn is_x_only_key(&self) -> bool { true } + fn num_der_paths(&self) -> usize { 0 } } impl MiniscriptKey for String { @@ -203,6 +211,10 @@ impl MiniscriptKey for String { type Hash256 = String; type Ripemd160 = String; type Hash160 = String; + + fn is_uncompressed(&self) -> bool { false } + fn is_x_only_key(&self) -> bool { false } + fn num_der_paths(&self) -> usize { 0 } } /// Trait describing key types that can be converted to bitcoin public keys. From b56f4467c076d1fa3f552c15889f9e264ebf6ad3 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 28 Mar 2024 11:56:45 +1100 Subject: [PATCH 7/8] Move associated types to top of struct There is no obvious reason why the associated types for `MiniscriptKey` are below the trait methods. Move them to the top. Note, the diff shows moving functions not associated types - same thing. Refactor only, no logic changes. --- src/lib.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 2bea8d005..c288bd8ca 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -149,18 +149,6 @@ pub use crate::primitives::threshold::{Threshold, ThresholdError}; /// Trait representing a key which can be converted to a hash type. pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Hash { - /// Returns true if the key is serialized uncompressed (defaults to `false`). - fn is_uncompressed(&self) -> bool; - - /// Returns true if the key is an x-only pubkey (defaults to `false`). - fn is_x_only_key(&self) -> bool; - - /// Returns the number of different derivation paths in this key (defaults to `0`). - /// - /// Only >1 for keys in BIP389 multipath descriptors. - fn num_der_paths(&self) -> usize; - - /// The associated `sha256::Hash` for this `MiniscriptKey`, used in the sha256 fragment. type Sha256: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; /// The associated `hash256::Hash` for this `MiniscriptKey`, used in the hash256 fragment. @@ -171,6 +159,17 @@ pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Ha /// The associated `hash160::Hash` for this `MiniscriptKey` type, used in the hash160 fragment. type Hash160: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; + + /// Returns true if the key is serialized uncompressed (defaults to `false`). + fn is_uncompressed(&self) -> bool; + + /// Returns true if the key is an x-only pubkey (defaults to `false`). + fn is_x_only_key(&self) -> bool; + + /// Returns the number of different derivation paths in this key (defaults to `0`). + /// + /// Only >1 for keys in BIP389 multipath descriptors. + fn num_der_paths(&self) -> usize; } impl MiniscriptKey for bitcoin::secp256k1::PublicKey { From 73c3e72ca3c19f91586c8b5fd80ddecf7c0239c3 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 17 Nov 2025 13:39:03 +1100 Subject: [PATCH 8/8] Remove whitespace Remove the whitespace between trivial trait method impls. This reduces line count without making the code any harder to read, arguably easier. --- src/lib.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c288bd8ca..c40a3150a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -255,11 +255,8 @@ impl ToPublicKey for bitcoin::PublicKey { fn to_public_key(&self) -> bitcoin::PublicKey { *self } fn to_sha256(hash: &sha256::Hash) -> sha256::Hash { *hash } - fn to_hash256(hash: &hash256::Hash) -> hash256::Hash { *hash } - fn to_ripemd160(hash: &ripemd160::Hash) -> ripemd160::Hash { *hash } - fn to_hash160(hash: &hash160::Hash) -> hash160::Hash { *hash } } @@ -267,11 +264,8 @@ impl ToPublicKey for bitcoin::secp256k1::PublicKey { fn to_public_key(&self) -> bitcoin::PublicKey { bitcoin::PublicKey::new(*self) } fn to_sha256(hash: &sha256::Hash) -> sha256::Hash { *hash } - fn to_hash256(hash: &hash256::Hash) -> hash256::Hash { *hash } - fn to_ripemd160(hash: &ripemd160::Hash) -> ripemd160::Hash { *hash } - fn to_hash160(hash: &hash160::Hash) -> hash160::Hash { *hash } } @@ -288,11 +282,8 @@ impl ToPublicKey for bitcoin::secp256k1::XOnlyPublicKey { fn to_x_only_pubkey(&self) -> bitcoin::secp256k1::XOnlyPublicKey { *self } fn to_sha256(hash: &sha256::Hash) -> sha256::Hash { *hash } - fn to_hash256(hash: &hash256::Hash) -> hash256::Hash { *hash } - fn to_ripemd160(hash: &ripemd160::Hash) -> ripemd160::Hash { *hash } - fn to_hash160(hash: &hash160::Hash) -> hash160::Hash { *hash } }