Only allow mnemonic from stdin (#3023)

src/subcommand/wallet/restore.rs

@@ -1,61 +1,41 @@
 use super::*;
 
 #[derive(Debug, Parser)]
-#[clap(group(
-  ArgGroup::new("source").required(true).args(&["descriptor", "mnemonic"]))
-)]
 pub(crate) struct Restore {
-  #[arg(long, help = "Restore wallet from <DESCRIPTOR> from stdin.")]
-  descriptor: bool,
-  #[arg(long, help = "Restore wallet from <MNEMONIC>.")]
-  mnemonic: Option<Mnemonic>,
-  #[arg(
-    long,
-    requires = "mnemonic",
-    help = "Use <PASSPHRASE> when deriving wallet"
-  )]
+  #[clap(value_enum, long, help = "Restore wallet from <SOURCE> on stdin.")]
+  from: Source,
+  #[arg(long, help = "Use <PASSPHRASE> when deriving wallet")]
   pub(crate) passphrase: Option<String>,
 }
 
+#[derive(clap::ValueEnum, Debug, Clone)]
+enum Source {
+  Descriptor,
+  Mnemonic,
+}
+
 impl Restore {
   pub(crate) fn run(self, wallet: Wallet) -> SubcommandResult {
     ensure!(!wallet.exists()?, "wallet `{}` already exists", wallet.name);
 
-    if self.descriptor {
-      let mut buffer = Vec::new();
-      std::io::stdin().read_to_end(&mut buffer)?;
-
-      let wallet_descriptors: ListDescriptorsResult = serde_json::from_slice(&buffer)?;
-
-      wallet.initialize_from_descriptors(wallet_descriptors.descriptors)?;
-    } else if let Some(mnemonic) = self.mnemonic {
-      wallet.initialize(mnemonic.to_seed(self.passphrase.unwrap_or_default()))?;
-    } else {
-      unreachable!();
+    let mut buffer = String::new();
+    io::stdin().read_to_string(&mut buffer)?;
+
+    match self.from {
+      Source::Descriptor => {
+        ensure!(
+          self.passphrase.is_none(),
+          "descriptor does not take a passphrase"
+        );
+        let wallet_descriptors: ListDescriptorsResult = serde_json::from_str(&buffer)?;
+        wallet.initialize_from_descriptors(wallet_descriptors.descriptors)?;
+      }
+      Source::Mnemonic => {
+        let mnemonic = Mnemonic::from_str(&buffer)?;
+        wallet.initialize(mnemonic.to_seed(self.passphrase.unwrap_or_default()))?;
+      }
     }
 
     Ok(None)
   }
 }
-
-#[cfg(test)]
-mod tests {
-  use super::*;
-
-  #[test]
-  fn descriptor_and_mnemonic_conflict() {
-    assert_regex_match!(
-      Arguments::try_parse_from([
-        "ord",
-        "wallet",
-        "restore",
-        "--descriptor",
-        "--mnemonic",
-        "oil oil oil oil oil oil oil oil oil oil oil oil"
-      ])
-      .unwrap_err()
-      .to_string(),
-      ".*--descriptor.*cannot be used with.*--mnemonic.*"
-    );
-  }
-}

tests/wallet/dump.rs

@@ -33,7 +33,7 @@ fn dumped_descriptors_restore() {
 
   let bitcoin_rpc_server = test_bitcoincore_rpc::spawn();
 
-  CommandBuilder::new("wallet restore --descriptor")
+  CommandBuilder::new("wallet restore --from descriptor")
     .stdin(serde_json::to_string(&output).unwrap().as_bytes().to_vec())
     .bitcoin_rpc_server(&bitcoin_rpc_server)
     .run_and_extract_stdout();
@@ -59,7 +59,7 @@ fn dump_and_restore_descriptors_with_minify() {
 
   let bitcoin_rpc_server = test_bitcoincore_rpc::spawn();
 
-  CommandBuilder::new("wallet restore --descriptor")
+  CommandBuilder::new("wallet restore --from descriptor")
     .stdin(serde_json::to_string(&output).unwrap().as_bytes().to_vec())
     .bitcoin_rpc_server(&bitcoin_rpc_server)
     .run_and_extract_stdout();

tests/wallet/restore.rs

@@ -14,7 +14,8 @@ fn restore_generates_same_descriptors() {
 
   let rpc_server = test_bitcoincore_rpc::spawn();
 
-  CommandBuilder::new(["wallet", "restore", "--mnemonic", &mnemonic.to_string()])
+  CommandBuilder::new(["wallet", "restore", "--from", "mnemonic"])
+    .stdin(mnemonic.to_string().into())
     .bitcoin_rpc_server(&rpc_server)
     .run_and_extract_stdout();
 
@@ -42,9 +43,10 @@ fn restore_generates_same_descriptors_with_passphrase() {
     "restore",
     "--passphrase",
     passphrase,
-    "--mnemonic",
-    &mnemonic.to_string(),
+    "--from",
+    "mnemonic",
   ])
+  .stdin(mnemonic.to_string().into())
   .bitcoin_rpc_server(&rpc_server)
   .run_and_extract_stdout();
 
@@ -65,7 +67,7 @@ fn restore_to_existing_wallet_fails() {
     .stderr_regex(".*")
     .run_and_deserialize_output::<ListDescriptorsResult>();
 
-  CommandBuilder::new("wallet restore --descriptor")
+  CommandBuilder::new("wallet restore --from descriptor")
     .stdin(serde_json::to_string(&output).unwrap().as_bytes().to_vec())
     .bitcoin_rpc_server(&bitcoin_rpc_server)
     .expected_exit_code(1)
@@ -86,7 +88,7 @@ fn restore_to_existing_wallet_fails() {
 fn restore_with_wrong_descriptors_fails() {
   let bitcoin_rpc_server = test_bitcoincore_rpc::spawn();
 
-  CommandBuilder::new("wallet --name foo restore --descriptor")
+  CommandBuilder::new("wallet --name foo restore --from descriptor")
       .stdin(r#"
 {
   "wallet_name": "bar",
@@ -141,9 +143,52 @@ fn restore_with_wrong_descriptors_fails() {
 fn restore_with_compact_works() {
   let bitcoin_rpc_server = test_bitcoincore_rpc::spawn();
 
-  CommandBuilder::new("wallet restore --descriptor")
+  CommandBuilder::new("wallet restore --from descriptor")
     .stdin(r#"{"wallet_name":"foo","descriptors":[{"desc":"rawtr(cVMYXp8uf1yFU9AAY6NJu1twA2uT94mHQBGkfgqCCzp6RqiTWCvP)#tah5crv7","timestamp":1706047934,"active":false,"internal":null,"range":null,"next":null},{"desc":"rawtr(cVdVu6VRwYXsTPMiptqVYLcp7EtQi5sjxLzbPTSNwW6CkCxBbEFs)#5afaht8d","timestamp":1706047934,"active":false,"internal":null,"range":null,"next":null},{"desc":"tr([c0b9536d/86'/1'/0']tprv8fXhtVjj3vb7kgxKuiWXzcUsur44gbLbbtwxL4HKmpzkBNoMrYqbQhMe7MWhrZjLFc9RBpTRYZZkrS8HH1Q3SmD5DkfpjKqtd97q1JWfqzr/0/*)#dweuu0ww","timestamp":1706047839,"active":true,"internal":false,"range":[0,1000],"next":1},{"desc":"tr([c0b9536d/86'/1'/0']tprv8fXhtVjj3vb7kgxKuiWXzcUsur44gbLbbtwxL4HKmpzkBNoMrYqbQhMe7MWhrZjLFc9RBpTRYZZkrS8HH1Q3SmD5DkfpjKqtd97q1JWfqzr/1/*)#u6uap67k","timestamp":1706047839,"active":true,"internal":true,"range":[0,1013],"next":14}]}"#.into())
     .bitcoin_rpc_server(&bitcoin_rpc_server)
     .expected_exit_code(0)
     .run_and_extract_stdout();
 }
+
+#[test]
+fn restore_with_blank_mnemonic_generates_same_descriptors() {
+  let (mnemonic, descriptors) = {
+    let rpc_server = test_bitcoincore_rpc::spawn();
+
+    let create::Output { mnemonic, .. } = CommandBuilder::new("wallet create")
+      .bitcoin_rpc_server(&rpc_server)
+      .run_and_deserialize_output();
+
+    (mnemonic, rpc_server.descriptors())
+  };
+
+  let rpc_server = test_bitcoincore_rpc::spawn();
+
+  CommandBuilder::new(["wallet", "restore", "--from", "mnemonic"])
+    .stdin(mnemonic.to_string().into())
+    .bitcoin_rpc_server(&rpc_server)
+    .run_and_extract_stdout();
+
+  assert_eq!(rpc_server.descriptors(), descriptors);
+}
+
+#[test]
+fn passphrase_conflicts_with_descriptor() {
+  let bitcoin_rpc_server = test_bitcoincore_rpc::spawn();
+  let ord_rpc_server = TestServer::spawn(&bitcoin_rpc_server);
+
+  CommandBuilder::new([
+    "wallet",
+    "restore",
+    "--from",
+    "descriptor",
+    "--passphrase",
+    "supersecurepassword",
+  ])
+  .stdin("".into())
+  .bitcoin_rpc_server(&bitcoin_rpc_server)
+  .ord_rpc_server(&ord_rpc_server)
+  .expected_exit_code(1)
+  .expected_stderr("error: descriptor does not take a passphrase\n")
+  .run_and_extract_stdout();
+}