Skip to content

DerivedDescriptor unfinished business #448

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

Merged
merged 2 commits into from
Jul 20, 2022
Merged

DerivedDescriptor unfinished business #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6dc6aca
Make DerivedDescriptorKey first-class citizen
LLFourn Jul 14, 2022
2ad6555
Rename "derive" things that are not doing derivation
LLFourn Jul 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions examples/xpub_descriptors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::str::FromStr;

use miniscript::bitcoin::secp256k1::{Secp256k1, Verification};
use miniscript::bitcoin::{Address, Network};
use miniscript::{Descriptor, DescriptorPublicKey};
use miniscript::{DefiniteDescriptorKey, Descriptor, DescriptorPublicKey};

const XPUB_1: &str = "xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB";
const XPUB_2: &str = "xpub69H7F5d8KSRgmmdJg2KhpAK8SR3DjMwAdkxj3ZuxV27CprR9LgpeyGmXUbC6wb7ERfvrnKZjXoUmmDznezpbZb7ap6r1D3tgFxHmwMkQTPH";
Expand All @@ -40,9 +40,9 @@ fn p2wsh<C: Verification>(secp: &Secp256k1<C>) -> Address {
let s = format!("wsh(sortedmulti(1,{},{}))", XPUB_1, XPUB_2);
// let s = format!("wsh(sortedmulti(1,{},{}))", XPUB_2, XPUB_1);

let address = Descriptor::<DescriptorPublicKey>::from_str(&s)
let address = Descriptor::<DefiniteDescriptorKey>::from_str(&s)
.unwrap()
.derived_descriptor(&secp, 0) // dummy index value if it not a wildcard
.derived_descriptor(&secp)
.unwrap()
.address(Network::Bitcoin)
.unwrap();
Expand Down
142 changes: 81 additions & 61 deletions src/descriptor/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,9 @@ pub enum SinglePubKey {
XOnly(XOnlyPublicKey),
}

/// A derived [`DescriptorPublicKey`]
///
/// Derived keys are guaranteed to never contain wildcards
/// A [`DescriptorPublicKey`] without any wildcards.
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
pub struct DerivedDescriptorKey {
key: DescriptorPublicKey,
index: u32,
}
pub struct DefiniteDescriptorKey(DescriptorPublicKey);

impl fmt::Display for DescriptorSecretKey {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
Expand Down Expand Up @@ -357,22 +352,14 @@ impl FromStr for DescriptorPublicKey {
/// Descriptor key conversion error
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)]
pub enum ConversionError {
/// Attempted to convert a key with a wildcard to a bitcoin public key
Wildcard,
/// Attempted to convert a key with hardened derivations to a bitcoin public key
HardenedChild,
/// Attempted to convert a key with a hardened wildcard to a bitcoin public key
HardenedWildcard,
}

impl fmt::Display for ConversionError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(match *self {
ConversionError::Wildcard => "uninstantiated wildcard in bip32 path",
ConversionError::HardenedChild => "hardened child step in bip32 path",
ConversionError::HardenedWildcard => {
"hardened and uninstantiated wildcard in bip32 path"
}
})
}
}
Expand All @@ -383,7 +370,7 @@ impl error::Error for ConversionError {
use self::ConversionError::*;

match self {
Wildcard | HardenedChild | HardenedWildcard => None,
HardenedChild => None,
}
}
}
Expand Down Expand Up @@ -441,40 +428,50 @@ impl DescriptorPublicKey {
}
}

/// Whether or not the key has a wildcards
/// Whether or not the key has a wildcard
#[deprecated(note = "use has_wildcard instead")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apoelstra / @sanket1729 I wonder if we can come up with a way to remind ourselves to add the 'since' field to this (and to these attributes in general) when we do the release tracking issue that bumps the version. these 'deprecated' attributes are impossible to write correctly because the next release number is not known at time of writing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we could file an issue with clippy to lint this :) "deprecated without since" default warn

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we only want it right before doing release not all the time.

pub fn is_deriveable(&self) -> bool {
self.has_wildcard()
}

/// Whether or not the key has a wildcard
pub fn has_wildcard(&self) -> bool {
match *self {
DescriptorPublicKey::Single(..) => false,
DescriptorPublicKey::XPub(ref xpub) => xpub.wildcard != Wildcard::None,
}
}

/// Derives the [`DescriptorPublicKey`] at `index` if this key is an xpub and has a wildcard.
#[deprecated(note = "use at_derivation_index instead")]
/// Deprecated name of [`at_derivation_index`].
Comment on lines +445 to +446
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I quite like this idea of removing the original comment and replacing with "Deprecated name of ..", I'm going to use that myself in future.

nit: Can you put the attribute below the comment please if you rebase for any other reason.

pub fn derive(self, index: u32) -> DefiniteDescriptorKey {
self.at_derivation_index(index)
}

/// Replaces any wildcard (i.e. `/*`) in the key with a particular derivation index, turning it into a
/// *definite* key (i.e. one where all the derivation paths are set).
///
/// # Returns
///
/// - If this key is not an xpub, returns `self`.
/// - If this key is an xpub but does not have a wildcard, returns `self`.
/// - Otherwise, returns the derived xpub at `index` (removing the wildcard).
///
/// Since it's guaranteed that extended keys won't have wildcards, the key is returned as
/// [`DerivedDescriptorKey`].
/// - Otherwise, returns the xpub at derivation `index` (removing the wildcard).
///
/// # Panics
///
/// If `index` ≥ 2^31
pub fn derive(self, index: u32) -> DerivedDescriptorKey {
let derived = match self {
pub fn at_derivation_index(self, index: u32) -> DefiniteDescriptorKey {
let definite = match self {
DescriptorPublicKey::Single(_) => self,
DescriptorPublicKey::XPub(xpub) => {
let derivation_path = match xpub.wildcard {
Wildcard::None => xpub.derivation_path,
Wildcard::Unhardened => xpub
.derivation_path
.into_child(bip32::ChildNumber::from_normal_idx(index).unwrap()),
Wildcard::Hardened => xpub
.derivation_path
.into_child(bip32::ChildNumber::from_hardened_idx(index).unwrap()),
Wildcard::Unhardened => xpub.derivation_path.into_child(
bip32::ChildNumber::from_normal_idx(index).expect("index must < 2^31"),
),
Wildcard::Hardened => xpub.derivation_path.into_child(
bip32::ChildNumber::from_hardened_idx(index).expect("index must < 2^31"),
),
};
DescriptorPublicKey::XPub(DescriptorXKey {
origin: xpub.origin,
Expand All @@ -485,7 +482,7 @@ impl DescriptorPublicKey {
}
};

DerivedDescriptorKey::new(derived, index)
DefiniteDescriptorKey::new(definite)
.expect("The key should not contain any wildcards at this point")
}

Expand All @@ -494,13 +491,10 @@ impl DescriptorPublicKey {
/// and returns the obtained full [`bitcoin::PublicKey`]. All BIP32 derivations
/// always return a compressed key
///
/// Will return an error if the descriptor key has any hardened
/// derivation steps in its path, or if the key has any wildcards.
/// Will return an error if the descriptor key has any hardened derivation steps in its path. To
/// avoid this error you should replace any such public keys first with [`translate_pk`].
///
/// To ensure there are no wildcards, call `.derive(0)` or similar;
/// to avoid hardened derivation steps, start from a `DescriptorSecretKey`
/// and call `to_public`, or call `TranslatePk2::translate_pk2` with
/// some function which has access to secret key data.
/// [`translate_pk`]: crate::TranslatePk::translate_pk
pub fn derive_public_key<C: Verification>(
&self,
secp: &Secp256k1<C>,
Expand All @@ -511,8 +505,9 @@ impl DescriptorPublicKey {
SinglePubKey::XOnly(xpk) => Ok(xpk.to_public_key()),
},
DescriptorPublicKey::XPub(ref xpk) => match xpk.wildcard {
Wildcard::Unhardened => Err(ConversionError::Wildcard),
Wildcard::Hardened => Err(ConversionError::HardenedWildcard),
Wildcard::Unhardened | Wildcard::Hardened => {
unreachable!("we've excluded this error case")
}
Wildcard::None => match xpk.xkey.derive_pub(secp, &xpk.derivation_path.as_ref()) {
Ok(xpub) => Ok(bitcoin::PublicKey::new(xpub.public_key)),
Err(bip32::Error::CannotDeriveFromHardenedKey) => {
Expand Down Expand Up @@ -767,7 +762,7 @@ impl MiniscriptKey for DescriptorPublicKey {
}
}

impl DerivedDescriptorKey {
impl DefiniteDescriptorKey {
/// Computes the raw [`bitcoin::PublicKey`] for this descriptor key.
///
/// Will return an error if the key has any hardened derivation steps
Expand All @@ -778,32 +773,51 @@ impl DerivedDescriptorKey {
&self,
secp: &Secp256k1<C>,
) -> Result<bitcoin::PublicKey, ConversionError> {
self.key.derive_public_key(secp)
}

/// Return the derivation index of this key
pub fn index(&self) -> u32 {
self.index
self.0.derive_public_key(secp)
}

/// Construct an instance from a descriptor key and a derivation index
///
/// Returns `None` if the key contains a wildcard
fn new(key: DescriptorPublicKey, index: u32) -> Option<Self> {
match key {
DescriptorPublicKey::XPub(ref xpk) if xpk.wildcard != Wildcard::None => None,
k => Some(DerivedDescriptorKey { key: k, index }),
fn new(key: DescriptorPublicKey) -> Option<Self> {
if key.has_wildcard() {
None
} else {
Some(Self(key))
}
}

/// The fingerprint of the master key associated with this key, `0x00000000` if none.
pub fn master_fingerprint(&self) -> bip32::Fingerprint {
self.0.master_fingerprint()
}

/// Full path, from the master key
pub fn full_derivation_path(&self) -> bip32::DerivationPath {
self.0.full_derivation_path()
}
}

impl FromStr for DefiniteDescriptorKey {
type Err = DescriptorKeyParseError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let inner = DescriptorPublicKey::from_str(s)?;
Ok(
DefiniteDescriptorKey::new(inner).ok_or(DescriptorKeyParseError(
"cannot parse key with a wilcard as a DerivedDescriptorKey",
))?,
)
}
}

impl fmt::Display for DerivedDescriptorKey {
impl fmt::Display for DefiniteDescriptorKey {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
self.key.fmt(f)
self.0.fmt(f)
}
}

impl MiniscriptKey for DerivedDescriptorKey {
impl MiniscriptKey for DefiniteDescriptorKey {
// This allows us to be able to derive public keys even for PkH s
type RawPkHash = Self;
type Sha256 = sha256::Hash;
Expand All @@ -812,22 +826,22 @@ impl MiniscriptKey for DerivedDescriptorKey {
type Hash160 = hash160::Hash;

fn is_uncompressed(&self) -> bool {
self.key.is_uncompressed()
self.0.is_uncompressed()
}

fn is_x_only_key(&self) -> bool {
self.key.is_x_only_key()
self.0.is_x_only_key()
}

fn to_pubkeyhash(&self) -> Self {
self.clone()
}
}

impl ToPublicKey for DerivedDescriptorKey {
impl ToPublicKey for DefiniteDescriptorKey {
fn to_public_key(&self) -> bitcoin::PublicKey {
let secp = Secp256k1::verification_only();
self.key.derive_public_key(&secp).unwrap()
self.0.derive_public_key(&secp).unwrap()
}

fn hash_to_hash160(hash: &Self) -> hash160::Hash {
Expand All @@ -851,6 +865,12 @@ impl ToPublicKey for DerivedDescriptorKey {
}
}

impl From<DefiniteDescriptorKey> for DescriptorPublicKey {
fn from(d: DefiniteDescriptorKey) -> Self {
d.0
}
}

#[cfg(test)]
mod test {
use core::str::FromStr;
Expand Down Expand Up @@ -957,17 +977,17 @@ mod test {
let public_key = DescriptorPublicKey::from_str("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2").unwrap();
assert_eq!(public_key.master_fingerprint().to_string(), "abcdef00");
assert_eq!(public_key.full_derivation_path().to_string(), "m/0'/1'/2");
assert_eq!(public_key.is_deriveable(), false);
assert_eq!(public_key.has_wildcard(), false);

let public_key = DescriptorPublicKey::from_str("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/*").unwrap();
assert_eq!(public_key.master_fingerprint().to_string(), "abcdef00");
assert_eq!(public_key.full_derivation_path().to_string(), "m/0'/1'");
assert_eq!(public_key.is_deriveable(), true);
assert_eq!(public_key.has_wildcard(), true);

let public_key = DescriptorPublicKey::from_str("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/*h").unwrap();
assert_eq!(public_key.master_fingerprint().to_string(), "abcdef00");
assert_eq!(public_key.full_derivation_path().to_string(), "m/0'/1'");
assert_eq!(public_key.is_deriveable(), true);
assert_eq!(public_key.has_wildcard(), true);
}

#[test]
Expand All @@ -979,7 +999,7 @@ mod test {
assert_eq!(public_key.to_string(), "[2cbe2a6d/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2");
assert_eq!(public_key.master_fingerprint().to_string(), "2cbe2a6d");
assert_eq!(public_key.full_derivation_path().to_string(), "m/0'/1'/2");
assert_eq!(public_key.is_deriveable(), false);
assert_eq!(public_key.has_wildcard(), false);

let secret_key = DescriptorSecretKey::from_str("tprv8ZgxMBicQKsPcwcD4gSnMti126ZiETsuX7qwrtMypr6FBwAP65puFn4v6c3jrN9VwtMRMph6nyT63NrfUL4C3nBzPcduzVSuHD7zbX2JKVc/0'/1'/2'").unwrap();
let public_key = secret_key.to_public(&secp).unwrap();
Expand Down
Loading