From 4a174a9d8c2a63417771da76b1d72162ed7d926a Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 09:54:18 +1100 Subject: [PATCH 1/7] 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 --- src/lib.rs | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index eac67c5d1..c94c1e46b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -147,34 +147,39 @@ 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. + /// + /// [`sha256::Hash`]: bitcoin::hashes::sha256::Hash 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. + /// + /// [`hash256::Hash`]: crate::hash256::Hash 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. + /// + /// [`ripemd160::Hash`]: bitcoin::hashes::ripemd160::Hash 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. + /// + /// [`hash160::Hash`]: bitcoin::hashes::hash160::Hash type Hash160: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; } From b42afae3c7b39545c5f4236b923ddabe6472cd3e Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 09:56:57 +1100 Subject: [PATCH 2/7] 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 c94c1e46b..59bb4c38b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -191,13 +191,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 837f1e93647382f78c4e25316b61d5f81c05c549 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 09:59:18 +1100 Subject: [PATCH 3/7] 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 59bb4c38b..304075652 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -209,7 +209,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 7c133693d1bb13f6236a74bda25bbc33ca400b7f Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 10:17:19 +1100 Subject: [PATCH 4/7] Improve rustdocs on primary traits The `MiniscriptKey` and `ToPublicKey` traits are more-or-less the first point of call for users of this library, lets clean them up. --- src/lib.rs | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 304075652..d131e699f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -162,24 +162,16 @@ pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Ha /// Only >1 for keys in BIP389 multipath descriptors. fn num_der_paths(&self) -> usize { 0 } - /// The associated [`sha256::Hash`] for this `MiniscriptKey`, used in the sha256 fragment. - /// - /// [`sha256::Hash`]: bitcoin::hashes::sha256::Hash + /// The type 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. - /// - /// [`hash256::Hash`]: crate::hash256::Hash + /// The type used in the hash256 fragment. type Hash256: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; - /// The associated [`ripemd160::Hash`] for this `MiniscriptKey` type, used in the ripemd160 fragment. - /// - /// [`ripemd160::Hash`]: bitcoin::hashes::ripemd160::Hash + /// The type used in the ripemd160 fragment. type Ripemd160: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; - /// The associated [`hash160::Hash`] for this `MiniscriptKey` type, used in the hash160 fragment. - /// - /// [`hash160::Hash`]: bitcoin::hashes::hash160::Hash + /// The type used in the hash160 fragment. type Hash160: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; } @@ -215,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()), @@ -237,16 +229,24 @@ pub trait ToPublicKey: MiniscriptKey { } } - /// Converts the generic associated [`MiniscriptKey::Sha256`] to [`sha256::Hash`] + /// Converts the associated [`MiniscriptKey::Sha256`] type to a [`sha256::Hash`]. + /// + /// [`sha256::Hash`]: bitcoin::hashes::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`]. + /// + /// [`hash256::Hash`]: crate::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`]. + /// + /// [`ripemd160::Hash`]: bitcoin::hashes::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`]. + /// + /// [`hash160::Hash`]: bitcoin::hashes::hash160::Hash fn to_hash160(hash: &::Hash160) -> hash160::Hash; } From bd39c6423fe151ad4316380ee27638cb57e7c7e9 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 10:18:40 +1100 Subject: [PATCH 5/7] 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 d131e699f..7c4426e3d 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 9378d28fbe0b469dff349d7d85f08fe64db134bb Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 18 Oct 2023 10:25:48 +1100 Subject: [PATCH 6/7] 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 think 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 7c4426e3d..d43a4da68 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 type 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 5fa08ae27ee1e6bfbd563af5749a995b7a101b2f Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 28 Mar 2024 11:56:45 +1100 Subject: [PATCH 7/7] 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 | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d43a4da68..5732f780a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -149,17 +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 type used in the sha256 fragment. type Sha256: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; @@ -171,6 +160,17 @@ pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Ha /// The 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 {