From dd125049ca17f40aae28dead441660b2c201b495 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 8f76de0..b7d36e3 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 } @@ -34,7 +35,6 @@ bdk_esplora = { version = "0.22.1", features = ["async-https", "tokio"], optiona bdk_kyoto = { version = "0.15.4", optional = true } bdk_redb = { version = "0.1.1", optional = true } bdk_sp = { version = "0.1.0", optional = true, git = "https://github.com/bitcoindevkit/bdk-sp", tag = "v0.1.0" } -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 } @@ -44,7 +44,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 a3ca537661dbb80620355b6dae296a836701f93c 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`. Tests verify descriptor structure and `r` field uniqueness across randomized compilations. --- Cargo.lock | 7 ++ Cargo.toml | 3 + src/error.rs | 7 ++ src/handlers.rs | 185 +++++++++++++++++++++++++++++++++--------------- 4 files changed, 144 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1d127fe..f40db91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -204,6 +204,7 @@ dependencies = [ "bdk_redb", "bdk_sp", "bdk_wallet", + "claims", "clap", "clap_complete", "cli-table", @@ -720,6 +721,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" diff --git a/Cargo.toml b/Cargo.toml index b7d36e3..3b4ffc6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,3 +69,6 @@ compiler = [] # Experimental silent payment sending capabilities silent-payments = ["dep:bdk_sp"] + +[dev-dependencies] +claims = "0.8.0" diff --git a/src/error.rs b/src/error.rs index 1d5fd82..a3e0462 100644 --- a/src/error.rs +++ b/src/error.rs @@ -49,6 +49,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), @@ -82,6 +85,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 15a4d5a..6c5647b 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::{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; @@ -1279,30 +1285,38 @@ 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 xonly_public_key = XOnlyPublicKey::from_str(NUMS_UNSPENDABLE_KEY_HEX) - .map_err(|e| Error::Generic(format!("Invalid NUMS key: {e}")))?; + 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 = bdk_wallet::bitcoin::secp256k1::PublicKey::from_x_only_public_key( + nums_key, + Parity::Even, + ); + + 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( @@ -1310,19 +1324,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)?) } } @@ -1994,41 +2018,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!(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_eq!(descriptor, expected_and_ab); + 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")] @@ -2036,46 +2105,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 29ef2e2912a6503086682c33b7ffda21f4c0eed7 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Wed, 25 Mar 2026 00:12:30 +0400 Subject: [PATCH 3/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 f40db91..d984ac4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -216,6 +216,7 @@ dependencies = [ "serde", "serde_json", "shlex", + "tap", "thiserror 2.0.18", "tokio", "toml 1.1.0+spec-1.1.0", @@ -2635,6 +2636,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 3b4ffc6..4603662 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 6c5647b..8d893b7 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -48,13 +48,15 @@ use bdk_wallet::{ key::{Parity, rand}, secp256k1::{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 = "silent-payments")] use { bdk_sp::{ @@ -1286,16 +1288,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. @@ -1314,7 +1312,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 6e113b1325e264cc4547bd6c3bad890ea6006722 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Sun, 29 Mar 2026 18:52:52 +0400 Subject: [PATCH 4/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 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 be6bb4b4049ced27ac6d937dd3e47dc5e0182303 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Fri, 3 Apr 2026 00:53:27 +0400 Subject: [PATCH 5/6] 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 2073a67..624e6ba 100644 --- a/README.md +++ b/README.md @@ -186,6 +186,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 03f6b98d4b0af77fc8a60104a4e1f5cdae3b0c1c Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Fri, 8 May 2026 00:53:32 +0400 Subject: [PATCH 6/6] 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 8d893b7..a8ae778 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -1983,12 +1983,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(