From 1faf0ed0a0879f19384c9fc0772b8a78d21f15bd Mon Sep 17 00:00:00 2001 From: Alekos Filini Date: Tue, 29 Sep 2020 18:18:50 +0200 Subject: [PATCH] Fix the recovery of a descriptor given a PSBT This commit upgrades `rust-miniscript` with a fix to only return the prefix that matches a `hd_keypath` instead of the full derivation path, and then adapts the signer code accordingly. This commit closes #108 and #109. --- Cargo.toml | 2 +- src/descriptor/mod.rs | 3 +-- src/wallet/mod.rs | 15 +++++++++++++++ src/wallet/signer.rs | 22 +++++++++++++++++----- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3c633af5..ef1a0d2a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ tiny-bip39 = { version = "^0.7", optional = true } [patch.crates-io] bitcoin = { git = "https://github.com/rust-bitcoin/rust-bitcoin/", rev = "478e091" } -miniscript = { git = "https://github.com/MagicalBitcoin/rust-miniscript", branch = "descriptor-public-key" } +miniscript = { git = "https://github.com/MagicalBitcoin/rust-miniscript", rev = "d0322ac" } # Platform-specific dependencies [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index c9a3f88d..ba4c3410 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -358,13 +358,12 @@ impl DescriptorMeta for Descriptor { derive_path = index .get_key_value(&root_fingerprint) .and_then(|(fingerprint, path)| xpub.matches(*fingerprint, path)) - .map(|prefix_path| prefix_path.into_iter().cloned().collect::>()) .map(|prefix| { index .get(&xpub.root_fingerprint()) .unwrap() .into_iter() - .skip(prefix.len()) + .skip(prefix.into_iter().count()) .cloned() .collect() }); diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 55d77642..9991faba 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2404,6 +2404,21 @@ mod test { assert_eq!(extracted.input[0].witness.len(), 2); } + #[test] + fn test_sign_single_xprv_bip44_path() { + let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/44'/0'/0'/0/*)"); + let addr = wallet.get_new_address().unwrap(); + let (psbt, _) = wallet + .create_tx(TxBuilder::with_recipients(vec![(addr.script_pubkey(), 0)]).send_all()) + .unwrap(); + + let (signed_psbt, finalized) = wallet.sign(psbt, None).unwrap(); + assert_eq!(finalized, true); + + let extracted = signed_psbt.extract_tx(); + assert_eq!(extracted.input[0].witness.len(), 2); + } + #[test] fn test_sign_single_xprv_sh_wpkh() { let (wallet, _, _) = get_funded_wallet("sh(wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*))"); diff --git a/src/wallet/signer.rs b/src/wallet/signer.rs index 12905151..0618d763 100644 --- a/src/wallet/signer.rs +++ b/src/wallet/signer.rs @@ -132,6 +132,8 @@ impl From for SignerId { pub enum SignerError { /// The private key is missing for the required public key MissingKey, + /// The private key in use has the right fingerprint but derives differently than expected + InvalidKey, /// The user canceled the operation UserCanceled, /// The sighash is missing in the PSBT input @@ -199,20 +201,30 @@ impl Signer for DescriptorXKey { return Err(SignerError::InputIndexOutOfRange); } - let deriv_path = match psbt.inputs[input_index] + let (public_key, deriv_path) = match psbt.inputs[input_index] .hd_keypaths .iter() - .filter_map(|(_, &(fingerprint, ref path))| self.matches(fingerprint.clone(), &path)) + .filter_map(|(pk, &(fingerprint, ref path))| { + if self.matches(fingerprint.clone(), &path).is_some() { + Some((pk, path)) + } else { + None + } + }) .next() { - Some(deriv_path) => deriv_path, - None => return Ok(()), // TODO: should report an error maybe? + Some((pk, full_path)) => (pk, full_path.clone()), + None => return Ok(()), }; let ctx = Secp256k1::signing_only(); let derived_key = self.xkey.derive_priv(&ctx, &deriv_path).unwrap(); - derived_key.private_key.sign(psbt, Some(input_index)) + if &derived_key.private_key.public_key(&ctx) != public_key { + Err(SignerError::InvalidKey) + } else { + derived_key.private_key.sign(psbt, Some(input_index)) + } } fn sign_whole_tx(&self) -> bool {