From 37e04659ecb8b242fa165d8b5ad89a7a13e1f8c9 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Tue, 24 Mar 2026 21:43:08 +0400 Subject: [PATCH 1/6] 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 92e7d215..0b67817f 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/6] 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 3690d4f0..423174b2 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 a98b172d..525db243 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/6] 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 4fac7da6..3bbc7914 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 0b67817f..dcf39bac 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 525db243..68dc133e 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/6] 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 3bbc7914..4226b9d9 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 dcf39bac..1d1070ee 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 68dc133e..3ccca51e 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)) } From b54f5548ce71d22d96cb4c6d848ca5619bc87350 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Sun, 29 Mar 2026 18:52:52 +0400 Subject: [PATCH 5/6] docs(contributing): update CONTRIBUTING.md - Remove MSRV mention as the project no longer enforces it - Add Conventional Commits and GPG signing links to align with other repos --- CONTRIBUTING.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a766fe5a..af1015a5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,10 +46,8 @@ Every new feature should be covered by functional tests where possible. When refactoring, structure your PR to make it easy to review and don't hesitate to split it into multiple small, focused PRs. -The Minimal Supported Rust Version is 1.45 (enforced by our CI). - Commits should cover both the issue fixed and the solution's rationale. -These [guidelines](https://chris.beams.io/posts/git-commit/) should be kept in mind. +These [guidelines](https://chris.beams.io/posts/git-commit/) should be kept in mind. Commit messages follow the ["Conventional Commits 1.0.0"](https://www.conventionalcommits.org/en/v1.0.0/) to make commit histories easier to read by humans and automated tools. All commits must be [GPG signed](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits). To facilitate communication with other contributors, the project is making use of GitHub's "assignee" field. First check that no one is assigned and then From 1ed41d21ec0c9e898e231b958a787f96dcf9fdba Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Mon, 30 Mar 2026 00:43:03 +0400 Subject: [PATCH 6/6] chore(deps): pin `bdk_wallet` to exact version 2.1.0 Prevent cargo from resolving to 2.3.0+ where SignOptions is deprecated and `just pre-push` fails because of that. --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5ae27f65..72363b64 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -311,9 +311,9 @@ dependencies = [ [[package]] name = "bdk_wallet" -version = "2.3.0" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b03f1e31ccc562f600981f747d2262b84428cbff52c9c9cdf14d15fb15bd2286" +checksum = "d30b5dba770184863b5d966ccbc6a11d12c145450be3b6a4435308297e6a12dc" dependencies = [ "bdk_chain", "bip39", diff --git a/Cargo.toml b/Cargo.toml index b989db47..2df44af6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ readme = "README.md" license = "MIT" [dependencies] -bdk_wallet = { version = "2.1.0", features = ["rusqlite", "keys-bip39", "compiler", "std"] } +bdk_wallet = { version = "=2.1.0", features = ["rusqlite", "keys-bip39", "compiler", "std"] } clap = { version = "4.6", features = ["derive","env"] } clap_complete = "4.6" dirs = { version = "6.0.0" }