Merge #346: refactor(compile): use ValueEnum for script type

79b2b83 test(compile): cover sh and sh-wsh script types (Ikeh Celestine Chukwudi)
36e41a3 refactor(compile): use ValueEnum for script type (Ikeh Celestine Chukwudi)

Pull request description:

  ### Description

  This PR replaces the `String` `script_type` in `CompileCommand` with a `ScriptType` enum deriving `clap::ValueEnum`. The goal is to make the `match` in `execute` exhaustive and keep the list of accepted script types in one place, instead of repeating it in the argument definition and in the match.

  Fixes #345

  ### Notes to the reviewers

  The change is limited to `src/handlers/descriptor.rs` and follows the existing `DatabaseType` enum in `src/persister.rs`.

  * Added the `ScriptType` enum with the variants `Sh`, `Wsh`, `ShWsh` and `Tr`, gated behind the `compiler` feature like the rest of the `compile` command.
  * Changed `script_type` from `String` to `ScriptType`, replacing the hand-written `value_parser` list with `value_enum`.
  * The `match` in `execute` now matches on the enum variants.
  * Removed the `_ =>` arm returning "Invalid script type", which was unreachable because clap already rejects any other value.

  There is no CLI behavior change. clap renders the variants in kebab-case, so `--type` still accepts `sh`, `wsh`, `sh-wsh` and `tr`, the default is still `wsh`, and the `TYPE` env var still works.

  I checked manually that `compile --type sh-wsh` produces the same descriptor before and after the change, and the existing `compile` tests pass.

  ### Changelog notice

  None. This is an internal refactor with no user-facing change.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk-cli/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

ACKs for top commit:
  tvpeter:
    ACK 79b2b83

Tree-SHA512: cb8ab8fef5d109149740f22c20d6bee768079d082f3f84b7cb80541b605b66f2235d386ff080d4e31b24285a8e7935ab87deeae500abf9895f91d766226ba500

Author: tvpeter

src/handlers/descriptor.rs

@@ -13,6 +13,8 @@ use crate::{
 };
 use clap::Parser;
 #[cfg(feature = "compiler")]
+use clap::ValueEnum;
+#[cfg(feature = "compiler")]
 use {
     bdk_wallet::{
         bitcoin::{
@@ -70,9 +72,24 @@ pub struct CompileCommand {
     #[arg(env = "POLICY", required = true, index = 1)]
     policy: String,
     /// Sets the script type used to embed the compiled policy.
-    #[arg(env = "TYPE", short = 't', long = "type", default_value = "wsh", value_parser = ["sh","wsh", "sh-wsh", "tr"]
-        )]
-    script_type: String,
+    #[arg(
+        env = "TYPE",
+        short = 't',
+        long = "type",
+        value_enum,
+        default_value_t = ScriptType::Wsh
+    )]
+    script_type: ScriptType,
+}
+
+/// Script types the compiled policy can be embedded in.
+#[cfg(feature = "compiler")]
+#[derive(Clone, Copy, ValueEnum, Debug, Eq, PartialEq)]
+pub enum ScriptType {
+    Sh,
+    Wsh,
+    ShWsh,
+    Tr,
 }
 
 #[cfg(feature = "compiler")]
@@ -87,11 +104,11 @@ impl AppCommand<AppContext<Init>> for CompileCommand {
 
         // Compile per branch, not once up front: the contexts have different script
         // limits, and the narrowest one would reject policies valid for the requested type.
-        let descriptor = match self.script_type.as_str() {
-            "sh" => Descriptor::new_sh(policy.compile()?),
-            "wsh" => Descriptor::new_wsh(policy.compile()?),
-            "sh-wsh" => Descriptor::new_sh_wsh(policy.compile()?),
-            "tr" => {
+        let descriptor = match self.script_type {
+            ScriptType::Sh => Descriptor::new_sh(policy.compile()?),
+            ScriptType::Wsh => Descriptor::new_wsh(policy.compile()?),
+            ScriptType::ShWsh => Descriptor::new_sh_wsh(policy.compile()?),
+            ScriptType::Tr => {
                 // Use a randomized unspendable internal key (H + rG) instead of a fixed NUMS
                 // point. This improves privacy by preventing observers from determining whether
                 // key-path spending is disabled. `r` is returned so the user can verify the key
@@ -113,11 +130,6 @@ impl AppCommand<AppContext<Init>> for CompileCommand {
                 let tree = TapTree::Leaf(Arc::new(policy.compile()?));
                 Descriptor::new_tr(xonly_internal_key.to_string(), Some(tree))
             }
-            _ => {
-                return Err(Error::Generic(
-                    "Invalid script type. Supported: sh, wsh, sh-wsh, tr".into(),
-                ));
-            }
         }?;
 
         Ok(DescriptorResult {

tests/integration/init.rs

@@ -223,6 +223,26 @@ mod test_compile {
             .stdout(predicate::str::contains("wsh("));
     }
 
+    #[test]
+    fn test_compile_sh_and_sh_wsh() {
+        let temp_dir = TempDir::new().unwrap();
+        let cli = BdkCli::new("testnet", Some(temp_dir.path().to_path_buf()));
+
+        let policy = "pk(02e5b88fdb71c696e1a473f309a47535b7190e21a22bd25e7fc8bd055db3bba12f)";
+
+        // compile sh
+        cli.cmd("compile", &[policy, "--type", "sh"])
+            .assert()
+            .success()
+            .stdout(predicate::str::contains("\"descriptor\": \"sh(pk("));
+
+        // compile sh-wsh
+        cli.cmd("compile", &[policy, "--type", "sh-wsh"])
+            .assert()
+            .success()
+            .stdout(predicate::str::contains("\"descriptor\": \"sh(wsh(pk("));
+    }
+
     #[test]
     fn test_compile_invalid_policy() {
         let temp_dir = TempDir::new().unwrap();