Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ tokio = { version = "1", features = ["full"] }
cli-table = "0.5.0"
tracing = "0.1.41"
tracing-subscriber = "0.3.20"
shlex = { version = "1.3.0" }

# Optional dependencies
bdk_bitcoind_rpc = { version = "0.21.0", features = ["std"], optional = true }
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 }
Expand All @@ -39,7 +39,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"]
Expand All @@ -60,3 +60,4 @@ verify = []
# Extra utility tools
# Compile policies
compiler = []

7 changes: 7 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down Expand Up @@ -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),
Expand Down
166 changes: 52 additions & 114 deletions src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,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 cli_table::{Cell, CellStruct, Style, Table, format::Justify};
use serde_json::json;
#[cfg(feature = "cbf")]
Expand Down Expand Up @@ -893,50 +899,70 @@ pub(crate) fn handle_compile_subcommand(
pretty: bool,
) -> Result<String, Error> {
let policy = Concrete::<String>::from_str(policy.as_str())?;
let legacy_policy: Miniscript<String, Legacy> = policy
.compile()
.map_err(|e| Error::Generic(e.to_string()))?;
let segwit_policy: Miniscript<String, Segwitv0> = policy
.compile()
.map_err(|e| Error::Generic(e.to_string()))?;
let taproot_policy: Miniscript<String, Tap> = policy
.compile()
.map_err(|e| Error::Generic(e.to_string()))?;

let legacy_policy: Miniscript<String, Legacy> = policy.compile()?;
let segwit_policy: Miniscript<String, Segwitv0> = policy.compile()?;
let taproot_policy: Miniscript<String, Tap> = policy.compile()?;

let mut r = None;
let mut shorten_descriptor = 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 = 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))

shorten_descriptor = Some(Descriptor::new_tr(
shorten(xonly_internal_key, 4, 4),
Some(tree.clone()),
)?);

Descriptor::new_tr(xonly_internal_key.to_string(), Some(tree))
}
_ => {
return Err(Error::Generic(
"Invalid script type. Supported types: sh, wsh, sh-wsh, tr".to_string(),
));
}
}?;

if pretty {
let table = vec![vec![
"Descriptor".cell().bold(true),
descriptor.to_string().cell(),
]]
.table()
.display()
.map_err(|e| Error::Generic(e.to_string()))?;
let descriptor = shorten_descriptor.unwrap_or(descriptor);

let mut rows = vec![vec!["Descriptor".cell().bold(true), descriptor.cell()]];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the descriptor value is quite long, can you apply the [shorten](https://github.com/bitcoindevkit/bdk-cli/blob/fb7f6c60ca4ac1ad80750391435bbafd7346c714/src/utils.rs#L381) fn so it displays better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I also used shorten() for r.
I extracted shorten description into a variable to shorten only the internal key and leave the rest of the descriptor as is.

without pretty:

-> % cargo run --all-features -- compile "pk(ABC)" -t tr
{
  "descriptor": "tr(1985c2a86e01bbcdb1b5806422531a73c77e568ecfdd6d2863c158a76628ea50,pk(ABC))#6qsjh5q3",
  "r": "e38caa40d0db7cae7037336c982278a7fc6b770e7a80f8bbce0d79976a14d5e1"
}

with pretty:

-> % cargo run --all-features -- compile "pk(ABC)" -t tr --pretty
+------------+----------------------------------+
| Descriptor | tr(ccb4...b1d0,pk(ABC))#qrk923py |
+------------+----------------------------------+
| r          | a129...7a14                      |
+------------+----------------------------------+

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut rows = vec![vec!["Descriptor".cell().bold(true), descriptor.cell()]];
let mut rows = vec![vec!["Descriptor".cell().bold(true), shorten(descriptor, 32, 29).cell()]];

you can remove: 908, 931-934, 946

you can also revert 951; it should size most screens comfortably.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I’ll add these changes when I do the squash.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the changes, and only then remembered why I did it this way — if we apply shorten globally (not just for tr), we'll get an error for other descriptors because we're trying to shorten an already too short string.

Here's an example of how to reproduce this (if you apply your suggested changes locally):

-> % cargo run --all-features -- compile "pk(ABC)" -t tr --pretty
+------------+------------------------------------------------------------------+
| Descriptor | tr(cf07afc46391a3ce22c1024a682ca...fc9486809e6,pk(ABC))#3qlg4fwz |
+------------+------------------------------------------------------------------+
| r          | ee1c3591351b7c879fd415c0fbe3871bf61c752d7d35ed2ed94f0a278de9073d |
+------------+------------------------------------------------------------------+

-> % cargo run --all-features -- compile "pk(ABC)" -t wsh --pretty
thread 'main' (1258654) panicked at src/utils.rs:383:39:
byte index 32 is out of bounds of `wsh(pk(ABC))#8gmtcnaw`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Also, I'm not sure we should successfully compile a script like "pk(ABC)" at all, but we can discuss this in a separate issue.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I noticed this bug and fixed it in #203 here. We may have to wait for the PR to get merged.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm not sure we should successfully compile a script like "pk(ABC)" at all, but we can discuss this in a separate issue.

Well, policy compilation is actually from the upstream source but I have tested policies that failed. I will try to see if I can know what made this one to pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm not sure we should successfully compile a script like "pk(ABC)" at all, but we can discuss this in a separate issue.

Well, policy compilation is actually from the upstream source but I have tested policies that failed. I will try to see if I can know what made this one to pass.

I just noticed that bdk-cli can create a descriptor that bitcoin-cli doesn't accept as valid:

-> % bdk-cli compile "pk(ABC)"
{
  "descriptor": "wsh(pk(ABC))#8gmtcnaw"
}

-> % bitcoin-cli deriveaddresses "wsh(pk(ABC))#8gmtcnaw"
error code: -5
error message:
pk(): key 'ABC' is not valid

This might confuse some users.

As a simple fix, we could explicitly state in the compile command documentation that we don't validate keys at compile time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I noticed this bug and fixed it in #203 here. We may have to wait for the PR to get merged.

Nice! Let's wait for #203 to merge then.
I've added "Depends on" to the PR description.
If we have a label like "blocked" or "do-not-merge", please add it to this PR.


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)?)
}
}

Expand Down Expand Up @@ -1450,92 +1476,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);
}

#[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());
}
}
78 changes: 78 additions & 0 deletions tests/compile.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright (c) 2020-2025 Bitcoin Dev Kit Developers
//
// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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;

fn run_cmd(cmd: &str) -> Result<String, String> {
let full_cmd = format!("run --features compiler -- {}", cmd);
let args = shlex::split(&full_cmd).unwrap();

let output = Command::new("cargo")
.args(args)
.env_remove("NETWORK")
.env_remove("DATADIR")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ig removing DATADIR env variable is not required? Since the compile command does not use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, DATADIR isn't used in compile command.
I'm planning to reuse this command execution code in other tests, so I preemptively cleared all env vars.

.env_remove("POLICY")
.env_remove("TYPE")
.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 stderr = run_cmd(r#"compile "invalid_policy""#).unwrap_err();
assert!(stderr.contains("Miniscript error"));

// Test invalid script type
let stderr = run_cmd(r#"compile "pk(A)" -t invalid_type"#).unwrap_err();
assert!(stderr.contains("error: invalid value 'invalid_type' for '--type <SCRIPT_TYPE>'"));

// Test empty policy
let stderr = run_cmd("compile").unwrap_err();
assert!(stderr.contains("error: the following required arguments were not provided"));
assert!(stderr.contains("<POLICY>"));

// Test malformed policy with unmatched parentheses
let stderr = run_cmd(r#"compile "pk(A""#).unwrap_err();
assert!(stderr.contains("Miniscript error: expected )"));

// Test policy with unknown function
let stderr = run_cmd(r#"compile "unknown_func(A)""#).unwrap_err();
assert!(stderr.contains("Miniscript error: unexpected «unknown_func»"));
}
Loading