Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 2 additions & 2 deletions Cargo-minimal.lock
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ checksum = "d965446196e3b7decd44aa7ee49e31d630118f90ef12f97900f262eb915c951d"

[[package]]
name = "bitcoin"
version = "0.32.0"
version = "0.32.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7170e7750a20974246f17ece04311b4205a6155f1db564c5b224af817663c3ea"
checksum = "ad8929a18b8e33ea6b3c09297b687baaa71fb1b97353243a3f1029fad5c59c5b"
dependencies = [
"base58ck",
"base64 0.21.7",
Expand Down
4 changes: 2 additions & 2 deletions Cargo-recent.lock
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ checksum = "d965446196e3b7decd44aa7ee49e31d630118f90ef12f97900f262eb915c951d"

[[package]]
name = "bitcoin"
version = "0.32.0"
version = "0.32.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7170e7750a20974246f17ece04311b4205a6155f1db564c5b224af817663c3ea"
checksum = "ad8929a18b8e33ea6b3c09297b687baaa71fb1b97353243a3f1029fad5c59c5b"
dependencies = [
"base58ck",
"base64 0.21.7",
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ base64 = ["bitcoin/base64"]

[dependencies]
bech32 = { version = "0.11.0", default-features = false }
bitcoin = { version = "0.32.0", default-features = false }
bitcoin = { version = "0.32.6", default-features = false }

# Do NOT use this as a feature! Use the `serde` feature instead.
actual-serde = { package = "serde", version = "1.0.103", optional = true }
Expand Down
276 changes: 276 additions & 0 deletions src/descriptor/key_map.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,276 @@
use bitcoin::psbt::{GetKey, GetKeyError, KeyRequest};
use bitcoin::secp256k1::{Secp256k1, Signing};
use bitcoin::PrivateKey;

use crate::descriptor::{DescriptorSecretKey, KeyMap};
use crate::BTreeMap;

/// A wrapper around KeyMap that implements GetKey for PSBT signing.
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct KeyMapWrapper {
map: KeyMap,
}

impl From<KeyMap> for KeyMapWrapper {
fn from(map: KeyMap) -> Self { KeyMapWrapper { map } }
}

impl GetKey for KeyMapWrapper {
type Error = GetKeyError;

fn get_key<C: Signing>(
&self,
key_request: KeyRequest,
secp: &Secp256k1<C>,
) -> Result<Option<bitcoin::PrivateKey>, Self::Error> {
Ok(self
.map
.iter()
.find_map(|(_desc_pk, desc_sk)| -> Option<PrivateKey> {
match desc_sk.get_key(key_request.clone(), secp) {
Ok(Some(pk)) => Some(pk),
Ok(None) | Err(_) => None,
Copy link
Member

Choose a reason for hiding this comment

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

In c7d4171:

Here (and below) you swallow the error from DescriptorSecretKey::get_key and turn it into a "key not found" by returning None.

This is an issue with the code on master. I will open a fix PR and then I'd like you to also backport that.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, now I'm unsure. We did not discuss this at all in #851 but I think your original code was correct. These errors occur if you try to get an x-only key from an xpriv (even this I'm unsure of, now..). But when doing a lookup in a map where we may have a combination of xprivs and bare secret keys, then individual errors aren't actually errors. They're just "key not found".

Copy link
Member

Choose a reason for hiding this comment

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

Lemme PR some unit tests and I'll just ask you to backport those :P.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've cherry-picked the commits and updated the required types for KeyMapWrapper.

}
}))
}
}

impl GetKey for DescriptorSecretKey {
type Error = GetKeyError;

fn get_key<C: Signing>(
&self,
key_request: KeyRequest,
secp: &Secp256k1<C>,
) -> Result<Option<PrivateKey>, Self::Error> {
match (self, key_request) {
(DescriptorSecretKey::Single(single_priv), key_request) => {
let sk = single_priv.key;
let pk = sk.public_key(secp);
let pubkey_map = BTreeMap::from([(pk, sk)]);
pubkey_map.get_key(key_request, secp)
}
(DescriptorSecretKey::XPrv(descriptor_xkey), KeyRequest::Pubkey(public_key)) => {
let xpriv = descriptor_xkey
.xkey
.derive_priv(secp, &descriptor_xkey.derivation_path)
.map_err(GetKeyError::Bip32)?;
let pk = xpriv.private_key.public_key(secp);

if public_key.inner.eq(&pk) {
Ok(Some(xpriv.to_priv()))
} else {
Ok(None)
}
}
(
DescriptorSecretKey::XPrv(descriptor_xkey),
ref key_request @ KeyRequest::Bip32(ref key_source),
) => {
if let Some(key) = descriptor_xkey.xkey.get_key(key_request.clone(), secp)? {
return Ok(Some(key));
}

if descriptor_xkey.matches(key_source, secp).is_some() {
let (_, derivation_path) = key_source;
return Ok(Some(
descriptor_xkey
.xkey
.derive_priv(secp, &derivation_path)
.map_err(GetKeyError::Bip32)?
.to_priv(),
));
}

Ok(None)
}
(DescriptorSecretKey::XPrv(_), KeyRequest::XOnlyPubkey(_)) => {
Err(GetKeyError::NotSupported)
}
(
desc_multi_sk @ DescriptorSecretKey::MultiXPrv(_descriptor_multi_xkey),
key_request,
) => Ok(desc_multi_sk.clone().into_single_keys().iter().find_map(
|desc_sk| match desc_sk.get_key(key_request.clone(), secp) {
Ok(Some(pk)) => Some(pk),
Ok(None) | Err(_) => None,
},
)),
_ => Ok(None),
}
}
}

#[cfg(test)]
mod tests {
use core::str::FromStr;

use bitcoin::bip32::{ChildNumber, DerivationPath, IntoDerivationPath, Xpriv};

use super::*;
use crate::Descriptor;

#[test]
fn get_key_single_key() {
let secp = Secp256k1::new();

let descriptor_sk_s =
"[90b6a706/44'/0'/0'/0/0]cMk8gWmj1KpjdYnAWwsEDekodMYhbyYBhG8gMtCCxucJ98JzcNij";

let single = match descriptor_sk_s.parse::<DescriptorSecretKey>().unwrap() {
DescriptorSecretKey::Single(single) => single,
_ => panic!("unexpected DescriptorSecretKey variant"),
};

let want_sk = single.key;
let descriptor_s = format!("wpkh({})", descriptor_sk_s);
let (_, keymap) = Descriptor::parse_descriptor(&secp, &descriptor_s).unwrap();
let keymap_wrapper = KeyMapWrapper::from(keymap);

let pk = want_sk.public_key(&secp);
let request = KeyRequest::Pubkey(pk);
let got_sk = keymap_wrapper
.get_key(request, &secp)
.expect("get_key call errored")
.expect("failed to find the key");
assert_eq!(got_sk, want_sk)
}

#[test]
fn get_key_xpriv_single_key_xpriv() {
let secp = Secp256k1::new();

let s = "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi";

let xpriv = s.parse::<Xpriv>().unwrap();
let xpriv_fingerprint = xpriv.fingerprint(&secp);

// Sanity check.
{
let descriptor_sk_s = format!("[{}]{}", xpriv_fingerprint, xpriv);
let descriptor_sk = descriptor_sk_s.parse::<DescriptorSecretKey>().unwrap();
let got = match descriptor_sk {
DescriptorSecretKey::XPrv(x) => x.xkey,
_ => panic!("unexpected DescriptorSecretKey variant"),
};
assert_eq!(got, xpriv);
}

let want_sk = xpriv.to_priv();
let descriptor_s = format!("wpkh([{}]{})", xpriv_fingerprint, xpriv);
let (_, keymap) = Descriptor::parse_descriptor(&secp, &descriptor_s).unwrap();
let keymap_wrapper = KeyMapWrapper::from(keymap);

let pk = want_sk.public_key(&secp);
let request = KeyRequest::Pubkey(pk);
let got_sk = keymap_wrapper
.get_key(request, &secp)
.expect("get_key call errored")
.expect("failed to find the key");
assert_eq!(got_sk, want_sk)
}

#[test]
fn get_key_xpriv_child_depth_one() {
let secp = Secp256k1::new();

let s = "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi";
let master = s.parse::<Xpriv>().unwrap();
let master_fingerprint = master.fingerprint(&secp);

let child_number = ChildNumber::from_hardened_idx(44).unwrap();
let child = master.derive_priv(&secp, &[child_number]).unwrap();

// Sanity check.
{
let descriptor_sk_s = format!("[{}/44']{}", master_fingerprint, child);
let descriptor_sk = descriptor_sk_s.parse::<DescriptorSecretKey>().unwrap();
let got = match descriptor_sk {
DescriptorSecretKey::XPrv(ref x) => x.xkey,
_ => panic!("unexpected DescriptorSecretKey variant"),
};
assert_eq!(got, child);
}

let want_sk = child.to_priv();
let descriptor_s = format!("wpkh({}/44')", s);
let (_, keymap) = Descriptor::parse_descriptor(&secp, &descriptor_s).unwrap();
let keymap_wrapper = KeyMapWrapper::from(keymap);

let pk = want_sk.public_key(&secp);
let request = KeyRequest::Pubkey(pk);
let got_sk = keymap_wrapper
.get_key(request, &secp)
.expect("get_key call errored")
.expect("failed to find the key");
assert_eq!(got_sk, want_sk)
}

#[test]
fn get_key_xpriv_with_path() {
let secp = Secp256k1::new();

let s = "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi";
let master = s.parse::<Xpriv>().unwrap();
let master_fingerprint = master.fingerprint(&secp);

let first_external_child = "44'/0'/0'/0/0";
let derivation_path = first_external_child.into_derivation_path().unwrap();

let child = master.derive_priv(&secp, &derivation_path).unwrap();

// Sanity check.
{
let descriptor_sk_s =
format!("[{}/{}]{}", master_fingerprint, first_external_child, child);
let descriptor_sk = descriptor_sk_s.parse::<DescriptorSecretKey>().unwrap();
let got = match descriptor_sk {
DescriptorSecretKey::XPrv(ref x) => x.xkey,
_ => panic!("unexpected DescriptorSecretKey variant"),
};
assert_eq!(got, child);
}

let want_sk = child.to_priv();
let descriptor_s = format!("wpkh({}/44'/0'/0'/0/*)", s);
let (_, keymap) = Descriptor::parse_descriptor(&secp, &descriptor_s).unwrap();
let keymap_wrapper = KeyMapWrapper::from(keymap);

let key_source = (master_fingerprint, derivation_path);
let request = KeyRequest::Bip32(key_source);
let got_sk = keymap_wrapper
.get_key(request, &secp)
.expect("get_key call errored")
.expect("failed to find the key");

assert_eq!(got_sk, want_sk)
}

#[test]
fn get_key_xpriv_with_key_origin() {
let secp = Secp256k1::new();

let descriptor_str = "wpkh([d34db33f/84h/1h/0h]tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)";
let (_descriptor_pk, keymap) = Descriptor::parse_descriptor(&secp, descriptor_str).unwrap();
let keymap_wrapper = KeyMapWrapper::from(keymap);

let descriptor_sk = DescriptorSecretKey::from_str("[d34db33f/84h/1h/0h]tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*").unwrap();
let xpriv = match descriptor_sk {
DescriptorSecretKey::XPrv(descriptor_xkey) => descriptor_xkey,
_ => unreachable!(),
};

let path = DerivationPath::from_str("84'/1'/0'/0").unwrap();
let expected_pk = xpriv.xkey.derive_priv(&secp, &path).unwrap().to_priv();

let (fp, _) = xpriv.origin.unwrap();
let key_request = KeyRequest::Bip32((fp, path));

let pk = keymap_wrapper
.get_key(key_request, &secp)
.expect("get_key should not fail")
.expect("get_key should return a `PrivateKey`");

assert_eq!(pk, expected_pk);
}
}
2 changes: 2 additions & 0 deletions src/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,14 @@ pub use self::tr::{TapTree, Tr};

pub mod checksum;
mod key;
mod key_map;

pub use self::key::{
ConversionError, DefiniteDescriptorKey, DerivPaths, DescriptorKeyParseError,
DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey, DescriptorXKey, InnerXKey,
SinglePriv, SinglePub, SinglePubKey, Wildcard,
};
pub use self::key_map::KeyMapWrapper;

/// Alias type for a map of public key to secret key
///
Expand Down
Loading