From a139efc94aae6272986874c63b65541b4b8e5ee8 Mon Sep 17 00:00:00 2001 From: Olorunshogo Moses BAMTEFA Date: Fri, 9 Oct 2026 14:29:34 +0100 Subject: [PATCH 1/2] fix(compile): validate policy keys and hashes before compiling descriptors --- src/handlers/descriptor.rs | 129 +++++++++++++++++++++++++------------ tests/integration/init.rs | 78 +++++++++++++++++++--- 2 files changed, 158 insertions(+), 49 deletions(-) diff --git a/src/handlers/descriptor.rs b/src/handlers/descriptor.rs index e35fcea3..9d20c4ca 100644 --- a/src/handlers/descriptor.rs +++ b/src/handlers/descriptor.rs @@ -20,7 +20,11 @@ use { key::{Parity, rand}, secp256k1::{PublicKey, Scalar, Secp256k1, SecretKey}, }, - miniscript::{Descriptor, descriptor::TapTree, policy::Concrete}, + miniscript::{ + Descriptor, FromStrKey, MiniscriptKey, + descriptor::{DescriptorPublicKey, TapTree}, + policy::Concrete, + }, }, std::{str::FromStr, sync::Arc}, }; @@ -73,6 +77,83 @@ pub struct CompileCommand { #[arg(env = "TYPE", short = 't', long = "type", default_value = "wsh", value_parser = ["sh","wsh", "sh-wsh", "tr"] )] script_type: String, + /// Skip key and hash validation, accepting placeholders (e.g. pk(A)). Useful for + /// inspecting a policy's shape without real keys. Descriptors compiled with this + /// flag are not usable for spending. + #[arg(long = "allow-placeholders")] + allow_placeholders: bool, +} + +#[cfg(feature = "compiler")] +/// Converts an x-only key into `Self`, so the `tr` branch can build its internal key +/// generically over the policy's key type. `String` can't get a `From` +/// impl here (orphan rules), hence this local trait instead. +trait TrInternalKey: MiniscriptKey { + fn from_xonly(key: XOnlyPublicKey) -> Self; +} + +#[cfg(feature = "compiler")] +impl TrInternalKey for String { + fn from_xonly(key: XOnlyPublicKey) -> Self { + key.to_string() + } +} + +#[cfg(feature = "compiler")] +impl TrInternalKey for DescriptorPublicKey { + fn from_xonly(key: XOnlyPublicKey) -> Self { + DescriptorPublicKey::from(key) + } +} + +#[cfg(feature = "compiler")] +/// Parses `policy_str` as `Pk` and compiles it for `script_type`. Returns the +/// descriptor string and, for `tr`, the random tweak `r` used to derive the +/// unspendable internal key so the caller can verify it. +fn compile_policy( + policy_str: &str, + script_type: &str, +) -> Result<(String, Option), Error> { + let policy: Concrete = Concrete::from_str(policy_str) + .map_err(|e| Error::Generic(format!("Invalid policy: {e}")))?; + + let mut r = None; + + // Compile per branch, not once up front: the contexts have different script + // limits, and the narrowest one would reject policies valid for the requested type. + let descriptor = match script_type { + "sh" => Descriptor::new_sh(policy.compile()?), + "wsh" => Descriptor::new_wsh(policy.compile()?), + "sh-wsh" => Descriptor::new_sh_wsh(policy.compile()?), + "tr" => { + // Use a randomized unspendable internal key (H + rG) instead of a fixed NUMS + // point. This improves privacy by preventing observers from determining whether + // key-path spending is disabled. `r` is returned so the user can verify the key + // is derived from the NUMS point. See BIP-341. + let secp = Secp256k1::new(); + let r_secret = SecretKey::new(&mut rand::thread_rng()); + r = Some(r_secret.display_secret().to_string()); + + let nums_key = XOnlyPublicKey::from_str(NUMS_UNSPENDABLE_KEY_HEX) + .map_err(|e| Error::Generic(format!("Invalid NUMS key: {e}")))?; + let nums_point = PublicKey::from_x_only_public_key(nums_key, Parity::Even); + + let internal_key_point = nums_point + .add_exp_tweak(&secp, &Scalar::from(r_secret)) + .map_err(|e| Error::Generic(format!("Failed to tweak NUMS key: {e}")))?; + let (xonly_internal_key, _) = internal_key_point.x_only_public_key(); + + let tree = TapTree::Leaf(Arc::new(policy.compile()?)); + Descriptor::new_tr(Pk::from_xonly(xonly_internal_key), Some(tree)) + } + _ => { + return Err(Error::Generic( + "Invalid script type. Supported: sh, wsh, sh-wsh, tr".into(), + )); + } + }?; + + Ok((descriptor.to_string(), r)) } #[cfg(feature = "compiler")] @@ -80,48 +161,14 @@ impl AppCommand> for CompileCommand { type Output = DescriptorResult; fn execute(&self, _ctx: &mut AppContext) -> Result { - let policy: Concrete = Concrete::from_str(&self.policy) - .map_err(|e| Error::Generic(format!("Invalid policy: {e}")))?; - - let mut r = None; - - // Compile per branch, not once up front: the contexts have different script - // limits, and the narrowest one would reject policies valid for the requested type. - let descriptor = match self.script_type.as_str() { - "sh" => Descriptor::new_sh(policy.compile()?), - "wsh" => Descriptor::new_wsh(policy.compile()?), - "sh-wsh" => Descriptor::new_sh_wsh(policy.compile()?), - "tr" => { - // Use a randomized unspendable internal key (H + rG) instead of a fixed NUMS - // point. This improves privacy by preventing observers from determining whether - // key-path spending is disabled. `r` is returned so the user can verify the key - // is derived from the NUMS point. See BIP-341. - let secp = Secp256k1::new(); - let r_secret = SecretKey::new(&mut rand::thread_rng()); - r = Some(r_secret.display_secret().to_string()); - - let nums_key = XOnlyPublicKey::from_str(NUMS_UNSPENDABLE_KEY_HEX) - .map_err(|e| Error::Generic(format!("Invalid NUMS key: {e}")))?; - let nums_point = PublicKey::from_x_only_public_key(nums_key, Parity::Even); - - let internal_key_point = - nums_point - .add_exp_tweak(&secp, &Scalar::from(r_secret)) - .map_err(|e| Error::Generic(format!("Failed to tweak NUMS key: {e}")))?; - let (xonly_internal_key, _) = internal_key_point.x_only_public_key(); - - let tree = TapTree::Leaf(Arc::new(policy.compile()?)); - Descriptor::new_tr(xonly_internal_key.to_string(), Some(tree)) - } - _ => { - return Err(Error::Generic( - "Invalid script type. Supported: sh, wsh, sh-wsh, tr".into(), - )); - } - }?; + let (descriptor, r) = if self.allow_placeholders { + compile_policy::(&self.policy, &self.script_type)? + } else { + compile_policy::(&self.policy, &self.script_type)? + }; Ok(DescriptorResult { - descriptor: Some(descriptor.to_string()), + descriptor: Some(descriptor), mnemonic: None, multipath_descriptor: None, public_descriptors: None, diff --git a/tests/integration/init.rs b/tests/integration/init.rs index 5244d996..51d41045 100644 --- a/tests/integration/init.rs +++ b/tests/integration/init.rs @@ -199,6 +199,10 @@ mod test_compile { /// A policy can be valid for tr or wsh type and still exceed the limits of the /// legacy context, whose 520-byte redeemScript cap does not apply to it. + /// + /// Uses placeholder keys (K01, K02, ...) since only the policy's shape, not its + /// keys, matters for this test - `--allow-placeholders` keeps that shape-only + /// intent even though real key validation is on by default. #[test] fn test_compile_policy_beyond_legacy_limits() { let temp_dir = TempDir::new().unwrap(); @@ -211,16 +215,22 @@ mod test_compile { let policy = format!("thresh(2,{keys})"); // compile tr - cli.cmd("compile", &[&policy, "--type", "tr"]) - .assert() - .success() - .stdout(predicate::str::contains("tr(")); + cli.cmd( + "compile", + &[&policy, "--type", "tr", "--allow-placeholders"], + ) + .assert() + .success() + .stdout(predicate::str::contains("tr(")); // compile wsh - cli.cmd("compile", &[&policy, "--type", "wsh"]) - .assert() - .success() - .stdout(predicate::str::contains("wsh(")); + cli.cmd( + "compile", + &[&policy, "--type", "wsh", "--allow-placeholders"], + ) + .assert() + .success() + .stdout(predicate::str::contains("wsh(")); } #[test] @@ -233,6 +243,58 @@ mod test_compile { .failure() .stderr(predicate::str::contains("Invalid policy")); } + + #[test] + fn test_compile_rejects_placeholder_key_by_default() { + let temp_dir = TempDir::new().unwrap(); + let cli = BdkCli::new("testnet", Some(temp_dir.path().to_path_buf())); + + cli.cmd("compile", &["pk(ABC)", "--type", "wsh"]) + .assert() + .failure() + .stderr(predicate::str::contains("Invalid policy")); + } + + #[test] + fn test_compile_rejects_placeholder_hash_by_default() { + let temp_dir = TempDir::new().unwrap(); + let cli = BdkCli::new("testnet", Some(temp_dir.path().to_path_buf())); + + let policy = + "and(pk(02e5b88fdb71c696e1a473f309a47535b7190e21a22bd25e7fc8bd055db3bba12f),sha256(H))"; + + cli.cmd("compile", &[policy, "--type", "wsh"]) + .assert() + .failure() + .stderr(predicate::str::contains("Invalid policy")); + } + + #[test] + fn test_compile_allows_placeholder_key_with_flag() { + let temp_dir = TempDir::new().unwrap(); + let cli = BdkCli::new("testnet", Some(temp_dir.path().to_path_buf())); + + cli.cmd( + "compile", + &["pk(ABC)", "--type", "wsh", "--allow-placeholders"], + ) + .assert() + .success() + .stdout(predicate::str::contains("wsh(pk(ABC))")); + } + + #[test] + fn test_compile_accepts_xpub_key() { + let temp_dir = TempDir::new().unwrap(); + let cli = BdkCli::new("testnet", Some(temp_dir.path().to_path_buf())); + + let policy = "pk(xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8/0/*)"; + + cli.cmd("compile", &[policy, "--type", "wsh"]) + .assert() + .success() + .stdout(predicate::str::contains("wsh(")); + } } // --- CONFIG COMMAND TESTS --- From 0eca373dbe622907e651cb0b5369573ba88dda45 Mon Sep 17 00:00:00 2001 From: Olorunshogo Moses BAMTEFA Date: Mon, 12 Oct 2026 16:57:51 +0100 Subject: [PATCH 2/2] fix(compile): keep key validation when adopting ScriptType --- src/handlers/descriptor.rs | 42 ++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/src/handlers/descriptor.rs b/src/handlers/descriptor.rs index 9d20c4ca..f6272fa4 100644 --- a/src/handlers/descriptor.rs +++ b/src/handlers/descriptor.rs @@ -13,6 +13,8 @@ use crate::{ }; use clap::Parser; #[cfg(feature = "compiler")] +use clap::ValueEnum; +#[cfg(feature = "compiler")] use { bdk_wallet::{ bitcoin::{ @@ -74,9 +76,14 @@ pub struct CompileCommand { #[arg(env = "POLICY", required = true, index = 1)] policy: String, /// Sets the script type used to embed the compiled policy. - #[arg(env = "TYPE", short = 't', long = "type", default_value = "wsh", value_parser = ["sh","wsh", "sh-wsh", "tr"] - )] - script_type: String, + #[arg( + env = "TYPE", + short = 't', + long = "type", + value_enum, + default_value_t = ScriptType::Wsh + )] + script_type: ScriptType, /// Skip key and hash validation, accepting placeholders (e.g. pk(A)). Useful for /// inspecting a policy's shape without real keys. Descriptors compiled with this /// flag are not usable for spending. @@ -84,6 +91,16 @@ pub struct CompileCommand { allow_placeholders: bool, } +/// Script types the compiled policy can be embedded in. +#[cfg(feature = "compiler")] +#[derive(Clone, Copy, ValueEnum, Debug, Eq, PartialEq)] +pub enum ScriptType { + Sh, + Wsh, + ShWsh, + Tr, +} + #[cfg(feature = "compiler")] /// Converts an x-only key into `Self`, so the `tr` branch can build its internal key /// generically over the policy's key type. `String` can't get a `From` @@ -112,7 +129,7 @@ impl TrInternalKey for DescriptorPublicKey { /// unspendable internal key so the caller can verify it. fn compile_policy( policy_str: &str, - script_type: &str, + script_type: ScriptType, ) -> Result<(String, Option), Error> { let policy: Concrete = Concrete::from_str(policy_str) .map_err(|e| Error::Generic(format!("Invalid policy: {e}")))?; @@ -122,10 +139,10 @@ fn compile_policy( // Compile per branch, not once up front: the contexts have different script // limits, and the narrowest one would reject policies valid for the requested type. let descriptor = match script_type { - "sh" => Descriptor::new_sh(policy.compile()?), - "wsh" => Descriptor::new_wsh(policy.compile()?), - "sh-wsh" => Descriptor::new_sh_wsh(policy.compile()?), - "tr" => { + ScriptType::Sh => Descriptor::new_sh(policy.compile()?), + ScriptType::Wsh => Descriptor::new_wsh(policy.compile()?), + ScriptType::ShWsh => Descriptor::new_sh_wsh(policy.compile()?), + ScriptType::Tr => { // Use a randomized unspendable internal key (H + rG) instead of a fixed NUMS // point. This improves privacy by preventing observers from determining whether // key-path spending is disabled. `r` is returned so the user can verify the key @@ -146,11 +163,6 @@ fn compile_policy( let tree = TapTree::Leaf(Arc::new(policy.compile()?)); Descriptor::new_tr(Pk::from_xonly(xonly_internal_key), Some(tree)) } - _ => { - return Err(Error::Generic( - "Invalid script type. Supported: sh, wsh, sh-wsh, tr".into(), - )); - } }?; Ok((descriptor.to_string(), r)) @@ -162,9 +174,9 @@ impl AppCommand> for CompileCommand { fn execute(&self, _ctx: &mut AppContext) -> Result { let (descriptor, r) = if self.allow_placeholders { - compile_policy::(&self.policy, &self.script_type)? + compile_policy::(&self.policy, self.script_type)? } else { - compile_policy::(&self.policy, &self.script_type)? + compile_policy::(&self.policy, self.script_type)? }; Ok(DescriptorResult {