From 74303bcf0bfeff09f4112b960928b9fb9bea8910 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Tue, 24 Mar 2026 21:43:08 +0400 Subject: [PATCH 1/8] 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 20190f4..f04d7f1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ tracing = "0.1.44" tracing-subscriber = "0.3.20" toml = "1.1.0" 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.2", optional = true } bdk_esplora = { version = "0.22.1", features = ["async-https", "tokio"], optional = true } bdk_kyoto = { version = "0.15.4", optional = true } bdk_redb = { version = "0.1.1", 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.13.2", default-features = false, optional = true } url = { version = "2.5.8", optional = true } @@ -43,7 +43,7 @@ bdk_bip322 = { version = "0.1.0", optional = true } default = ["repl", "sqlite"] # To use the app in a REPL mode -repl = ["shlex"] +repl = [] # Available database options sqlite = ["bdk_wallet/rusqlite"] From 5e9045277fd622bea005db0a6a0a78f08bd5baec Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Tue, 24 Mar 2026 21:59:39 +0400 Subject: [PATCH 2/8] 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 9846866..dd034e7 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 fb1544f..e78a82a 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; @@ -1060,30 +1066,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( @@ -1091,19 +1102,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 ac924630c7307e34a7d91e3d807a03bc0bead917 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Tue, 24 Mar 2026 22:05:54 +0400 Subject: [PATCH 3/8] 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.toml | 3 ++ src/handlers.rs | 111 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 81 insertions(+), 33 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f04d7f1..29315d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,3 +65,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 e78a82a..51a668d 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -1790,41 +1790,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")] @@ -1832,46 +1877,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 87d8863f7b442196c786934cb30d6c74eb5508a7 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Wed, 25 Mar 2026 00:12:30 +0400 Subject: [PATCH 4/8] 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.toml | 1 + src/handlers.rs | 16 +++++++--------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 29315d7..99434cd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ tracing-subscriber = "0.3.20" toml = "1.1.0" 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 51a668d..ae7d392 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; @@ -1067,16 +1069,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. @@ -1092,7 +1090,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 d63326883cd58734421fe8ae2c2a07b0bd869445 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Sun, 29 Mar 2026 18:52:52 +0400 Subject: [PATCH 5/8] 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 a766fe5..af1015a 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 a84473fb718163f8366b092de8c0b38dd4e4738f Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Fri, 3 Apr 2026 00:53:27 +0400 Subject: [PATCH 6/8] docs: add pre-push recipe description to README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index fd01432..907af0c 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,7 @@ The below are some of the commands included: just # list all available recipes just test # test the project just build # build the project +just pre-push # run full CI checks locally (tests, clippy, fmt) before pushing ``` ### Using `Justfile` to run `bitcoind` as a Client From 39d799d7e69739cadaa92474d84ef7971a64b44e Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Fri, 8 May 2026 00:53:32 +0400 Subject: [PATCH 7/8] test(handlers): drop redundant cfg gate on test module --- src/handlers.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/handlers.rs b/src/handlers.rs index ae7d392..7fd8360 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -1755,12 +1755,6 @@ pub fn handle_descriptor_command( format_descriptor_output(&result, pretty) } -#[cfg(any( - feature = "electrum", - feature = "esplora", - feature = "cbf", - feature = "rpc" -))] #[cfg(test)] mod test { #[cfg(any( From 053073f355c66bcead73149dfa504da56d3f66de Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Fri, 8 May 2026 20:06:57 +0400 Subject: [PATCH 8/8] chore: sync Cargo.lock Rebased onto master to drop the merge commit, which left Cargo.lock with uncommitted changes. The cleaner fix would be to squash these into the commits that introduced the dependencies, but I went with the simpler approach. --- Cargo.lock | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index b27c190..fa885ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -203,6 +203,7 @@ dependencies = [ "bdk_kyoto", "bdk_redb", "bdk_wallet", + "claims", "clap", "clap_complete", "cli-table", @@ -214,6 +215,7 @@ dependencies = [ "serde", "serde_json", "shlex", + "tap", "thiserror 2.0.18", "tokio", "toml 1.1.0+spec-1.1.0", @@ -711,6 +713,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.6.0" @@ -2619,6 +2627,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"