From a541122c857479a5fbc12596846f6978ae0429b0 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Fri, 14 Nov 2025 16:32:27 +0400 Subject: [PATCH 1/6] feat(compile): use random internal key for tr descriptor --- Cargo.lock | 1 + Cargo.toml | 1 + src/error.rs | 7 ++++ src/handlers.rs | 87 +++++++++++++++++++++++++++++++++---------------- 4 files changed, 68 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4ff8e29..06dc09b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -192,6 +192,7 @@ dependencies = [ "dirs", "env_logger", "log", + "serde", "serde_json", "shlex", "thiserror", diff --git a/Cargo.toml b/Cargo.toml index d5767f3..f8a860a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ clap = { version = "4.5", features = ["derive","env"] } dirs = { version = "6.0.0" } env_logger = "0.11.6" log = "0.4" +serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" thiserror = "2.0.11" tokio = { version = "1", features = ["full"] } diff --git a/src/error.rs b/src/error.rs index 1b8b5b4..b70365d 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 6c58a83..104976d 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -38,12 +38,21 @@ 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, - descriptor::{Descriptor, Legacy, Miniscript}, - miniscript::{Tap, descriptor::TapTree, policy::Concrete}, +use { + bdk_wallet::{ + bitcoin::{ + XOnlyPublicKey, + key::{Parity, rand}, + secp256k1::PublicKey, + }, + descriptor::{Descriptor, Legacy, Miniscript}, + miniscript::{Tap, descriptor::TapTree, policy::Concrete}, + }, + serde::Serialize, }; + use cli_table::{Cell, CellStruct, Style, Table, format::Justify}; use serde_json::json; #[cfg(feature = "cbf")] @@ -997,30 +1006,42 @@ 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()?; + + #[derive(Serialize)] + struct CompileSubcommandOutput { + descriptor: String, + #[serde(skip_serializing_if = "Option::is_none")] + r: Option, + } + + 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}")))?; + // Generate random scalar r and compute rG (r times the generator point G) + let secp = Secp256k1::new(); + let (r_secret, r_point) = secp.generate_keypair(&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 internal_key_point = nums_point.combine(&r_point)?; + 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( @@ -1028,19 +1049,29 @@ pub(crate) fn handle_compile_subcommand( )); } }?; + + let compile_subcmd_output = CompileSubcommandOutput { + descriptor: descriptor.to_string(), + r, + }; + 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()))?; + compile_subcmd_output.descriptor.cell(), + ]]; + + if let Some(r_value) = &compile_subcmd_output.r { + rows.push(vec!["r".cell().bold(true), r_value.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()}), - )?) + Ok(serde_json::to_string_pretty(&compile_subcmd_output)?) } } From 6c3e0710abfd3397ddf061776856435e03f142c0 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Fri, 14 Nov 2025 16:48:39 +0400 Subject: [PATCH 2/6] chore(compile): simplify --- Cargo.lock | 1 - Cargo.toml | 1 - src/handlers.rs | 45 ++++++++++++++++----------------------------- 3 files changed, 16 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 06dc09b..4ff8e29 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -192,7 +192,6 @@ dependencies = [ "dirs", "env_logger", "log", - "serde", "serde_json", "shlex", "thiserror", diff --git a/Cargo.toml b/Cargo.toml index f8a860a..d5767f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,6 @@ clap = { version = "4.5", features = ["derive","env"] } dirs = { version = "6.0.0" } env_logger = "0.11.6" log = "0.4" -serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" thiserror = "2.0.11" tokio = { version = "1", features = ["full"] } diff --git a/src/handlers.rs b/src/handlers.rs index 104976d..ac799ba 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -40,17 +40,14 @@ use bdk_wallet::rusqlite::Connection; use bdk_wallet::{KeychainKind, SignOptions, Wallet}; #[cfg(feature = "compiler")] -use { - bdk_wallet::{ - bitcoin::{ - XOnlyPublicKey, - key::{Parity, rand}, - secp256k1::PublicKey, - }, - descriptor::{Descriptor, Legacy, Miniscript}, - miniscript::{Tap, descriptor::TapTree, policy::Concrete}, +use bdk_wallet::{ + bitcoin::{ + XOnlyPublicKey, + key::{Parity, rand}, + secp256k1::PublicKey, }, - serde::Serialize, + descriptor::{Descriptor, Legacy, Miniscript}, + miniscript::{Tap, descriptor::TapTree, policy::Concrete}, }; use cli_table::{Cell, CellStruct, Style, Table, format::Justify}; @@ -1011,13 +1008,6 @@ pub(crate) fn handle_compile_subcommand( let segwit_policy: Miniscript = policy.compile()?; let taproot_policy: Miniscript = policy.compile()?; - #[derive(Serialize)] - struct CompileSubcommandOutput { - descriptor: String, - #[serde(skip_serializing_if = "Option::is_none")] - r: Option, - } - let mut r = None; let descriptor = match script_type.as_str() { @@ -1048,20 +1038,13 @@ pub(crate) fn handle_compile_subcommand( "Invalid script type. Supported types: sh, wsh, sh-wsh, tr".to_string(), )); } - }?; - - let compile_subcmd_output = CompileSubcommandOutput { - descriptor: descriptor.to_string(), - r, - }; + }? + .to_string(); if pretty { - let mut rows = vec![vec![ - "Descriptor".cell().bold(true), - compile_subcmd_output.descriptor.cell(), - ]]; + let mut rows = vec![vec!["Descriptor".cell().bold(true), descriptor.cell()]]; - if let Some(r_value) = &compile_subcmd_output.r { + if let Some(r_value) = &r { rows.push(vec!["r".cell().bold(true), r_value.cell()]); } @@ -1071,7 +1054,11 @@ pub(crate) fn handle_compile_subcommand( .map_err(|e| Error::Generic(e.to_string()))?; Ok(format!("{table}")) } else { - Ok(serde_json::to_string_pretty(&compile_subcmd_output)?) + 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 09a381b7933c1bd8a2d436fa3ed5c3050432a0d0 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Wed, 19 Nov 2025 00:20:35 +0400 Subject: [PATCH 3/6] test(compile): move invalid cases tests for compile cmd to new test file --- src/handlers.rs | 49 +----------------------------------------------- tests/compile.rs | 43 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 48 deletions(-) create mode 100644 tests/compile.rs diff --git a/src/handlers.rs b/src/handlers.rs index ac799ba..ffe4357 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -1052,6 +1052,7 @@ pub(crate) fn handle_compile_subcommand( .table() .display() .map_err(|e| Error::Generic(e.to_string()))?; + Ok(format!("{table}")) } else { let mut output = json!({"descriptor": descriptor}); @@ -1448,52 +1449,4 @@ mod test { let descriptor = json_result.get("descriptor").unwrap().as_str().unwrap(); assert_eq!(descriptor, expected_and_ab); } - - #[cfg(feature = "compiler")] - #[test] - fn test_compile_invalid_cases() { - use super::handle_compile_subcommand; - use bdk_wallet::bitcoin::Network; - - // Test invalid policy syntax - let result = 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( - 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()); - - // Test malformed policy with unmatched parentheses - let result = 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( - Network::Testnet, - "unknown_func(A)".to_string(), - "tr".to_string(), - false, - ); - assert!(result.is_err()); - } } diff --git a/tests/compile.rs b/tests/compile.rs new file mode 100644 index 0000000..46862f1 --- /dev/null +++ b/tests/compile.rs @@ -0,0 +1,43 @@ +// Copyright (c) 2020-2025 Bitcoin Dev Kit Developers +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +//! Compile Command Tests +//! +//! Tests for compile command and subcommands + +use std::process::{Command, Output}; + +fn run_cmd(cmd: &str) -> Output { + Command::new("cargo") + .args(format!("run -- {}", cmd).split_whitespace()) + .output() + .unwrap() +} + +#[test] +fn test_invalid_cases() { + // Test invalid policy syntax + let output = run_cmd("compile invalid_policy"); + assert!(!output.status.success()); + + // Test invalid script type + let output = run_cmd("compile pk(A) -t invalid_type"); + assert!(!output.status.success()); + + // Test empty policy + let output = run_cmd("compile"); + assert!(!output.status.success()); + + // Test malformed policy with unmatched parentheses + let output = run_cmd(r#"compile "pk(A"#); + assert!(!output.status.success()); + + // Test policy with unknown function + let output = run_cmd(r#"compile "unknown_func(A)"#); + assert!(!output.status.success()); +} From d763986614d80238cb9625e1de62dd8fe5795a20 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Wed, 19 Nov 2025 00:31:00 +0400 Subject: [PATCH 4/6] test(compile): delete obsoleted test for compile taproot descriptors --- src/handlers.rs | 40 ---------------------------------------- 1 file changed, 40 deletions(-) diff --git a/src/handlers.rs b/src/handlers.rs index ffe4357..26ec331 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -1409,44 +1409,4 @@ mod test { let full_signed_psbt = Psbt::from_str("cHNidP8BAIkBAAAAASWJHzxzyVORV/C3lAynKHVVL7+Rw7/Jj8U9fuvD24olAAAAAAD+////AiBOAAAAAAAAIgAgLzY9yE4jzTFJnHtTjkc+rFAtJ9NB7ENFQ1xLYoKsI1cfqgKVAAAAACIAIFsbWgDeLGU8EA+RGwBDIbcv4gaGG0tbEIhDvwXXa/E7LwEAAAABALUCAAAAAAEBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD/////BALLAAD/////AgD5ApUAAAAAIgAgWxtaAN4sZTwQD5EbAEMhty/iBoYbS1sQiEO/Bddr8TsAAAAAAAAAACZqJKohqe3i9hw/cdHe/T+pmd+jaVN1XGkGiXmZYrSL69g2l06M+QEgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQErAPkClQAAAAAiACBbG1oA3ixlPBAPkRsAQyG3L+IGhhtLWxCIQ78F12vxOwEFR1IhA/JV2U/0pXW+iP49QcsYilEvkZEd4phmDM8nV8wC+MeDIQLKhV/gEZYmlsQXnsL5/Uqv5Y8O31tmWW1LQqIBkiqzCVKuIgYCyoVf4BGWJpbEF57C+f1Kr+WPDt9bZlltS0KiAZIqswkEboH3lCIGA/JV2U/0pXW+iP49QcsYilEvkZEd4phmDM8nV8wC+MeDBDS6ZSEBBwABCNsEAEgwRQIhAJzT6busDV9h12M/LNquZ17oOHFn7whg90kh9gjSpvshAiBEDu/1EYVD7BqJJzExPhq2CX/Vsap/ULLjfRRo99nEKQFHMEQCIGoFCvJ2zPB7PCpznh4+1jsY03kMie49KPoPDdr7/T9TAiB3jV7wzR9BH11FSbi+8U8gSX95PrBlnp1lOBgTUIUw3QFHUiED8lXZT/Sldb6I/j1ByxiKUS+RkR3imGYMzydXzAL4x4MhAsqFX+ARliaWxBeewvn9Sq/ljw7fW2ZZbUtCogGSKrMJUq4AACICAsqFX+ARliaWxBeewvn9Sq/ljw7fW2ZZbUtCogGSKrMJBG6B95QiAgPyVdlP9KV1voj+PUHLGIpRL5GRHeKYZgzPJ1fMAvjHgwQ0umUhAA==").unwrap(); assert!(is_final(&full_signed_psbt).is_ok()); } - - #[cfg(feature = "compiler")] - #[test] - fn test_compile_taproot() { - use super::{NUMS_UNSPENDABLE_KEY_HEX, 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 - ); - - // Test simple pk policy compilation to taproot - let result = 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); - - // Test more complex policy - let result = 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); - } } From 189c710bb444fd9f0af8c1003c8ca23bca6112f9 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Wed, 19 Nov 2025 01:20:24 +0400 Subject: [PATCH 5/6] test(compile): add basic taproot/sh field validation tests - Add basic tests validating that taproot results contain 'r' field while sh results do not - Fix test execution by enabling 'compiler' feature - Add error message validation for invalid cases Future iterations will include more comprehensive test coverage. --- tests/compile.rs | 59 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/tests/compile.rs b/tests/compile.rs index 46862f1..fe93dbc 100644 --- a/tests/compile.rs +++ b/tests/compile.rs @@ -10,34 +10,61 @@ //! //! Tests for compile command and subcommands -use std::process::{Command, Output}; +use std::process::Command; -fn run_cmd(cmd: &str) -> Output { - Command::new("cargo") - .args(format!("run -- {}", cmd).split_whitespace()) - .output() - .unwrap() +fn run_cmd(cmd: &str) -> Result { + let full_cmd = format!("run --features compiler -- {}", cmd); + let args = shlex::split(&full_cmd).unwrap(); + + let output = Command::new("cargo").args(args).output().unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + + if output.status.success() { + Ok(stdout) + } else { + Err(stderr) + } +} + +#[test] +fn test_compile_taproot() { + let stdout = run_cmd(r#"compile "pk(ABC)" -t tr"#).unwrap(); + let json: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + + assert!(json.get("descriptor").is_some()); + assert!(json.get("r").is_some()); +} + +#[test] +fn test_compile_sh() { + let stdout = run_cmd(r#"compile "pk(ABC)" -t sh"#).unwrap(); + let json: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + + assert!(json.get("descriptor").is_some()); + assert!(json.get("r").is_none()); } #[test] fn test_invalid_cases() { // Test invalid policy syntax - let output = run_cmd("compile invalid_policy"); - assert!(!output.status.success()); + let stderr = run_cmd(r#"compile "invalid_policy""#).unwrap_err(); + assert!(stderr.contains("Miniscript error")); // Test invalid script type - let output = run_cmd("compile pk(A) -t invalid_type"); - assert!(!output.status.success()); + let stderr = run_cmd(r#"compile "pk(A)" -t invalid_type"#).unwrap_err(); + assert!(stderr.contains("error: invalid value 'invalid_type' for '--type '")); // Test empty policy - let output = run_cmd("compile"); - assert!(!output.status.success()); + let stderr = run_cmd("compile").unwrap_err(); + assert!(stderr.contains("error: the following required arguments were not provided")); + assert!(stderr.contains("")); // Test malformed policy with unmatched parentheses - let output = run_cmd(r#"compile "pk(A"#); - assert!(!output.status.success()); + let stderr = run_cmd(r#"compile "pk(A""#).unwrap_err(); + assert!(stderr.contains("Miniscript error: expected )")); // Test policy with unknown function - let output = run_cmd(r#"compile "unknown_func(A)"#); - assert!(!output.status.success()); + let stderr = run_cmd(r#"compile "unknown_func(A)""#).unwrap_err(); + assert!(stderr.contains("Miniscript error: unexpected «unknown_func»")); } From df6ee82bed157912248de5eb6fb14a9f48631403 Mon Sep 17 00:00:00 2001 From: Vadim Anufriev Date: Wed, 19 Nov 2025 16:12:16 +0400 Subject: [PATCH 6/6] ci(dependencies): add `shlex` to dev-dependencies for running test on CI --- Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index d5767f3..0aceedf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,3 +54,6 @@ verify = [] # Extra utility tools # Compile policies compiler = [] + +[dev-dependencies] +shlex = "1.3.0"