From 37e04659ecb8b242fa165d8b5ad89a7a13e1f8c9 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Tue, 24 Mar 2026 21:43:08 +0400 Subject: [PATCH 1/4] chore(deps): make shlex a non-optional dependency Although not directly related to this PR's changes, during review we agreed to make shlex non-optional since it's used by the default `repl` feature and the package is under 20 KiB. See: https://github.com/bitcoindevkit/bdk-cli/pull/225#discussion_r2671741961 --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 92e7d21..0b67817 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ tracing = "0.1.41" tracing-subscriber = "0.3.20" toml = "0.8.23" serde= {version = "1.0", features = ["derive"]} +shlex = "1.3.0" # Optional dependencies bdk_bitcoind_rpc = { version = "0.21.0", features = ["std"], optional = true } @@ -33,7 +34,6 @@ bdk_electrum = { version = "0.23.0", optional = true } bdk_esplora = { version = "0.22.1", features = ["async-https", "tokio"], optional = true } bdk_kyoto = { version = "0.15.1", optional = true } bdk_redb = { version = "0.1.0", optional = true } -shlex = { version = "1.3.0", optional = true } payjoin = { version = "=1.0.0-rc.1", features = ["v1", "v2", "io", "_test-utils"], optional = true} reqwest = { version = "0.12.23", default-features = false, optional = true } url = { version = "2.5.4", optional = true } @@ -42,7 +42,7 @@ url = { version = "2.5.4", optional = true } default = ["repl", "sqlite"] # To use the app in a REPL mode -repl = ["shlex"] +repl = [] # Available database options sqlite = ["bdk_wallet/rusqlite"] From 75b217ce73de50aa89961764cb8159e281b0a4a6 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Tue, 24 Mar 2026 21:59:39 +0400 Subject: [PATCH 2/4] feat(compile): randomize unspendable internal key for taproot descriptor Instead of using a fixed NUMS key as the internal key for taproot descriptors, generate a randomized unspendable key (H + rG) for each compilation. This improves privacy by preventing observers from determining whether key path spending is disabled. The randomness factor `r` is included in the output so the user can verify that the internal key is derived from the NUMS point. Also applies `shorten()` globally in pretty mode and uses `?` operator via dedicated error variants instead of `map_err`. --- src/error.rs | 7 +++++ src/handlers.rs | 71 ++++++++++++++++++++++++++++++++----------------- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/src/error.rs b/src/error.rs index 3690d4f..423174b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -45,6 +45,9 @@ pub enum BDKCliError { #[error("Miniscript error: {0}")] MiniscriptError(#[from] bdk_wallet::miniscript::Error), + #[error("Miniscript compiler error: {0}")] + MiniscriptCompilerError(#[from] bdk_wallet::miniscript::policy::compiler::CompilerError), + #[error("ParseError: {0}")] ParseError(#[from] bdk_wallet::bitcoin::address::ParseError), @@ -78,6 +81,10 @@ pub enum BDKCliError { #[error("Signer error: {0}")] SignerError(#[from] bdk_wallet::signer::SignerError), + #[cfg(feature = "compiler")] + #[error("Secp256k1 error: {0}")] + Secp256k1Error(#[from] bdk_wallet::bitcoin::secp256k1::Error), + #[cfg(feature = "electrum")] #[error("Electrum error: {0}")] Electrum(#[from] bdk_electrum::electrum_client::Error), diff --git a/src/handlers.rs b/src/handlers.rs index a98b172..525db24 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -40,12 +40,18 @@ use bdk_wallet::miniscript::miniscript; #[cfg(feature = "sqlite")] use bdk_wallet::rusqlite::Connection; use bdk_wallet::{KeychainKind, SignOptions, Wallet}; + #[cfg(feature = "compiler")] use bdk_wallet::{ - bitcoin::XOnlyPublicKey, + bitcoin::{ + XOnlyPublicKey, + key::{Parity, rand}, + secp256k1::{PublicKey, Scalar, SecretKey}, + }, descriptor::{Descriptor, Legacy, Miniscript}, miniscript::{Tap, descriptor::TapTree, policy::Concrete}, }; + use clap::CommandFactory; use cli_table::{Cell, CellStruct, Style, Table, format::Justify}; use serde_json::json; @@ -1014,30 +1020,35 @@ pub(crate) fn handle_compile_subcommand( pretty: bool, ) -> Result { let policy = Concrete::::from_str(policy.as_str())?; - let legacy_policy: Miniscript = policy - .compile() - .map_err(|e| Error::Generic(e.to_string()))?; - let segwit_policy: Miniscript = policy - .compile() - .map_err(|e| Error::Generic(e.to_string()))?; - let taproot_policy: Miniscript = policy - .compile() - .map_err(|e| Error::Generic(e.to_string()))?; + + let legacy_policy: Miniscript = policy.compile()?; + let segwit_policy: Miniscript = policy.compile()?; + let taproot_policy: Miniscript = policy.compile()?; + + let mut r = None; let descriptor = match script_type.as_str() { "sh" => Descriptor::new_sh(legacy_policy), "wsh" => Descriptor::new_wsh(segwit_policy), "sh-wsh" => Descriptor::new_sh_wsh(segwit_policy), "tr" => { - // For tr descriptors, we use a well-known unspendable key (NUMS point). - // This ensures the key path is effectively disabled and only script path can be used. - // See https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs + // For tr descriptors, we use a randomized unspendable key (H + rG). + // This improves privacy by preventing observers from determining if key path spending is disabled. + // See BIP-341: https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs + + 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)?; + let nums_point = PublicKey::from_x_only_public_key(nums_key, Parity::Even); - let xonly_public_key = XOnlyPublicKey::from_str(NUMS_UNSPENDABLE_KEY_HEX) - .map_err(|e| Error::Generic(format!("Invalid NUMS key: {e}")))?; + let internal_key_point = nums_point.add_exp_tweak(&secp, &Scalar::from(r_secret))?; + let (xonly_internal_key, _) = internal_key_point.x_only_public_key(); let tree = TapTree::Leaf(Arc::new(taproot_policy)); - Descriptor::new_tr(xonly_public_key.to_string(), Some(tree)) + + Descriptor::new_tr(xonly_internal_key.to_string(), Some(tree)) } _ => { return Err(Error::Generic( @@ -1045,19 +1056,29 @@ pub(crate) fn handle_compile_subcommand( )); } }?; + if pretty { - let table = vec![vec![ + let mut rows = vec![vec![ "Descriptor".cell().bold(true), - descriptor.to_string().cell(), - ]] - .table() - .display() - .map_err(|e| Error::Generic(e.to_string()))?; + shorten(&descriptor, 32, 29).cell(), + ]]; + + if let Some(r_value) = &r { + rows.push(vec!["r".cell().bold(true), shorten(r_value, 4, 4).cell()]); + } + + let table = rows + .table() + .display() + .map_err(|e| Error::Generic(e.to_string()))?; + Ok(format!("{table}")) } else { - Ok(serde_json::to_string_pretty( - &json!({"descriptor": descriptor.to_string()}), - )?) + let mut output = json!({"descriptor": descriptor}); + if let Some(r_value) = r { + output["r"] = json!(r_value); + } + Ok(serde_json::to_string_pretty(&output)?) } } From 741e672b7e4d5c333efc3a887f90eed4037966ab Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Tue, 24 Mar 2026 22:05:54 +0400 Subject: [PATCH 3/4] test(compile): add tests for randomized taproot internal key Add `claims` dev-dependency for ergonomic `assert_ok!`/`assert_err!` macros. Adapt compile tests to verify randomized key behavior: - descriptor structure instead of exact match (key is now random) - presence of `r` field for taproot, absence for other types - uniqueness of `r` across compilations --- Cargo.lock | 7 +++ Cargo.toml | 3 ++ src/handlers.rs | 111 ++++++++++++++++++++++++++++++++++-------------- 3 files changed, 88 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4fac7da..3bbc791 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -202,6 +202,7 @@ dependencies = [ "bdk_kyoto", "bdk_redb", "bdk_wallet", + "claims", "clap", "clap_complete", "cli-table", @@ -700,6 +701,12 @@ dependencies = [ "zeroize", ] +[[package]] +name = "claims" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bba18ee93d577a8428902687bcc2b6b45a56b1981a1f6d779731c86cc4c5db18" + [[package]] name = "clap" version = "4.5.54" diff --git a/Cargo.toml b/Cargo.toml index 0b67817..dcf39ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,3 +63,6 @@ verify = [] # Extra utility tools # Compile policies compiler = [] + +[dev-dependencies] +claims = "0.8.0" diff --git a/src/handlers.rs b/src/handlers.rs index 525db24..68dc133 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -1744,41 +1744,86 @@ mod test { #[cfg(feature = "compiler")] #[test] fn test_compile_taproot() { - use super::{NUMS_UNSPENDABLE_KEY_HEX, handle_compile_subcommand}; + use super::handle_compile_subcommand; use bdk_wallet::bitcoin::Network; - - // Expected taproot descriptors with checksums (using NUMS key from constant) - let expected_pk_a = format!("tr({},pk(A))#a2mlskt0", NUMS_UNSPENDABLE_KEY_HEX); - let expected_and_ab = format!( - "tr({},and_v(v:pk(A),pk(B)))#sfplm6kv", - NUMS_UNSPENDABLE_KEY_HEX - ); + use claims::assert_ok; // Test simple pk policy compilation to taproot - let result = handle_compile_subcommand( + let json_string = assert_ok!(handle_compile_subcommand( Network::Testnet, "pk(A)".to_string(), "tr".to_string(), false, - ); - assert!(result.is_ok()); - let json_string = result.unwrap(); + )); let json_result: serde_json::Value = serde_json::from_str(&json_string).unwrap(); + let descriptor = json_result.get("descriptor").unwrap().as_str().unwrap(); - assert_eq!(descriptor, expected_pk_a); + assert!(descriptor.starts_with("tr(")); + assert!(descriptor.contains(",pk(A))#")); + assert!(json_result.get("r").is_some()); // Test more complex policy - let result = handle_compile_subcommand( + let json_string = assert_ok!(handle_compile_subcommand( Network::Testnet, "and(pk(A),pk(B))".to_string(), "tr".to_string(), false, - ); - assert!(result.is_ok()); - let json_string = result.unwrap(); + )); let json_result: serde_json::Value = serde_json::from_str(&json_string).unwrap(); + let descriptor = json_result.get("descriptor").unwrap().as_str().unwrap(); - assert_eq!(descriptor, expected_and_ab); + assert!(descriptor.starts_with("tr(")); + assert!(descriptor.contains(",and_v(v:pk(A),pk(B)))#")); + assert!(json_result.get("r").is_some()); + } + + #[cfg(feature = "compiler")] + #[test] + fn test_compile_non_taproot_has_no_r() { + use super::handle_compile_subcommand; + use bdk_wallet::bitcoin::Network; + use claims::assert_ok; + + let json_string = assert_ok!(handle_compile_subcommand( + Network::Testnet, + "pk(A)".to_string(), + "wsh".to_string(), + false, + )); + let json_result: serde_json::Value = serde_json::from_str(&json_string).unwrap(); + + let descriptor = json_result.get("descriptor").unwrap().as_str().unwrap(); + assert!(descriptor.starts_with("wsh(pk(A))#")); + assert!(json_result.get("r").is_none()); + } + + #[cfg(feature = "compiler")] + #[test] + fn test_compile_taproot_randomness() { + use super::handle_compile_subcommand; + use bdk_wallet::bitcoin::Network; + use claims::assert_ok; + + // Two compilations of the same policy should produce different internal keys + let result1 = assert_ok!(handle_compile_subcommand( + Network::Testnet, + "pk(A)".to_string(), + "tr".to_string(), + false, + )); + let result2 = assert_ok!(handle_compile_subcommand( + Network::Testnet, + "pk(A)".to_string(), + "tr".to_string(), + false, + )); + + let json1: serde_json::Value = serde_json::from_str(&result1).unwrap(); + let json2: serde_json::Value = serde_json::from_str(&result2).unwrap(); + + let r1 = json1.get("r").unwrap().as_str().unwrap(); + let r2 = json2.get("r").unwrap().as_str().unwrap(); + assert_ne!(r1, r2, "Each compilation should produce a unique r value"); } #[cfg(feature = "compiler")] @@ -1786,46 +1831,46 @@ mod test { fn test_compile_invalid_cases() { use super::handle_compile_subcommand; use bdk_wallet::bitcoin::Network; + use claims::assert_err; // Test invalid policy syntax - let result = handle_compile_subcommand( + assert_err!(handle_compile_subcommand( Network::Testnet, "invalid_policy".to_string(), "tr".to_string(), false, - ); - assert!(result.is_err()); + )); // Test invalid script type - let result = handle_compile_subcommand( + assert_err!(handle_compile_subcommand( Network::Testnet, "pk(A)".to_string(), "invalid_type".to_string(), false, - ); - assert!(result.is_err()); + )); // Test empty policy - let result = - handle_compile_subcommand(Network::Testnet, "".to_string(), "tr".to_string(), false); - assert!(result.is_err()); + assert_err!(handle_compile_subcommand( + Network::Testnet, + "".to_string(), + "tr".to_string(), + false, + )); // Test malformed policy with unmatched parentheses - let result = handle_compile_subcommand( + assert_err!(handle_compile_subcommand( Network::Testnet, "pk(A".to_string(), "tr".to_string(), false, - ); - assert!(result.is_err()); + )); // Test policy with unknown function - let result = handle_compile_subcommand( + assert_err!(handle_compile_subcommand( Network::Testnet, "unknown_func(A)".to_string(), "tr".to_string(), false, - ); - assert!(result.is_err()); + )); } } From 937461dc9e820b957d332478fc4368f4d4f3df95 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Wed, 25 Mar 2026 00:12:30 +0400 Subject: [PATCH 4/4] refactor(compile): lazy compilation and use pipe for readability Move miniscript compilation inside match arms to avoid compiling for all script contexts when only one is needed. Use `tap::Pipe` for concise method chaining in sh/wsh/sh-wsh branches. --- Cargo.lock | 7 +++++++ Cargo.toml | 1 + src/handlers.rs | 16 +++++++--------- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3bbc791..4226b9d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -214,6 +214,7 @@ dependencies = [ "serde", "serde_json", "shlex", + "tap", "thiserror 2.0.17", "tokio", "toml 0.8.23", @@ -2586,6 +2587,12 @@ dependencies = [ "syn", ] +[[package]] +name = "tap" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" + [[package]] name = "tempfile" version = "3.24.0" diff --git a/Cargo.toml b/Cargo.toml index dcf39ba..1d1070e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ tracing-subscriber = "0.3.20" toml = "0.8.23" serde= {version = "1.0", features = ["derive"]} shlex = "1.3.0" +tap = "1.0.1" # Optional dependencies bdk_bitcoind_rpc = { version = "0.21.0", features = ["std"], optional = true } diff --git a/src/handlers.rs b/src/handlers.rs index 68dc133..3ccca51 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -48,13 +48,15 @@ use bdk_wallet::{ key::{Parity, rand}, secp256k1::{PublicKey, Scalar, SecretKey}, }, - descriptor::{Descriptor, Legacy, Miniscript}, + descriptor::{Descriptor, Legacy}, miniscript::{Tap, descriptor::TapTree, policy::Concrete}, }; use clap::CommandFactory; use cli_table::{Cell, CellStruct, Style, Table, format::Justify}; use serde_json::json; +#[cfg(feature = "compiler")] +use tap::Pipe; #[cfg(feature = "electrum")] use crate::utils::BlockchainClient::Electrum; @@ -1021,16 +1023,12 @@ pub(crate) fn handle_compile_subcommand( ) -> Result { let policy = Concrete::::from_str(policy.as_str())?; - let legacy_policy: Miniscript = policy.compile()?; - let segwit_policy: Miniscript = policy.compile()?; - let taproot_policy: Miniscript = policy.compile()?; - let mut r = None; let descriptor = match script_type.as_str() { - "sh" => Descriptor::new_sh(legacy_policy), - "wsh" => Descriptor::new_wsh(segwit_policy), - "sh-wsh" => Descriptor::new_sh_wsh(segwit_policy), + "sh" => policy.compile::()?.pipe(Descriptor::new_sh), + "wsh" => policy.compile::()?.pipe(Descriptor::new_wsh), + "sh-wsh" => policy.compile::()?.pipe(Descriptor::new_sh_wsh), "tr" => { // For tr descriptors, we use a randomized unspendable key (H + rG). // This improves privacy by preventing observers from determining if key path spending is disabled. @@ -1046,7 +1044,7 @@ pub(crate) fn handle_compile_subcommand( let internal_key_point = nums_point.add_exp_tweak(&secp, &Scalar::from(r_secret))?; let (xonly_internal_key, _) = internal_key_point.x_only_public_key(); - let tree = TapTree::Leaf(Arc::new(taproot_policy)); + let tree = TapTree::Leaf(Arc::new(policy.compile::()?)); Descriptor::new_tr(xonly_internal_key.to_string(), Some(tree)) }