[clap-v3-utils] Remove deprecated functions (pyth-network#313)

* add `deprecated` feature to produce warnings on use of deprecated functions

* replace `multiple_occurrences` with arg actions

* replace `possible_values` with `PossibleValueParser`

* deprecated `value_of` and `values_of`

* deprecate `unix_timestamp_from_rfc3339_datetime`

* deprecate `cluster_type_of`

* deprecate `commitment_of`

* deprecate `keypair_of`, `keypairs_of`, `pubkey_of`, and `pubkeys_of` functions

* replace deprecated functions from `try_keypair_of`, `try_keypairs_of`, `try_pubkey_of`, and `try_pubkeys_of`

* deprecate `pubkeys_sigs_of`

* allow deprecated on tests

* remove `deprecation` feature from clap-v3-utils

* re-export `pubkeys_sigs_of`

* add helper `extract_keypair` to dedupe `try_keypair_of` and `try_keypairs_of`

* remove unwraps and expects

* bump deprecation version

Author: samkim-crypto

clap-v3-utils/src/input_parsers/mod.rs

@@ -21,12 +21,18 @@ pub mod signer;
     since = "1.17.0",
     note = "Please use the functions in `solana_clap_v3_utils::input_parsers::signer` directly instead"
 )]
+#[allow(deprecated)]
 pub use signer::{
     pubkey_of_signer, pubkeys_of_multiple_signers, pubkeys_sigs_of, resolve_signer, signer_of,
     STDOUT_OUTFILE_TOKEN,
 };
 
 // Return parsed values from matches at `name`
+#[deprecated(
+    since = "2.0.0",
+    note = "Please use the functions `ArgMatches::get_many` or `ArgMatches::try_get_many` instead"
+)]
+#[allow(deprecated)]
 pub fn values_of<T>(matches: &ArgMatches, name: &str) -> Option<Vec<T>>
 where
     T: std::str::FromStr,
@@ -38,6 +44,11 @@ where
 }
 
 // Return a parsed value from matches at `name`
+#[deprecated(
+    since = "2.0.0",
+    note = "Please use the functions `ArgMatches::get_one` or `ArgMatches::try_get_one` instead"
+)]
+#[allow(deprecated)]
 pub fn value_of<T>(matches: &ArgMatches, name: &str) -> Option<T>
 where
     T: std::str::FromStr,
@@ -48,6 +59,11 @@ where
         .and_then(|value| value.parse::<T>().ok())
 }
 
+#[deprecated(
+    since = "2.0.0",
+    note = "Please use `ArgMatches::get_one::<UnixTimestamp>(...)` instead"
+)]
+#[allow(deprecated)]
 pub fn unix_timestamp_from_rfc3339_datetime(
     matches: &ArgMatches,
     name: &str,
@@ -63,14 +79,25 @@ pub fn unix_timestamp_from_rfc3339_datetime(
     since = "1.17.0",
     note = "please use `Amount::parse_decimal` and `Amount::sol_to_lamport` instead"
 )]
+#[allow(deprecated)]
 pub fn lamports_of_sol(matches: &ArgMatches, name: &str) -> Option<u64> {
     value_of(matches, name).map(sol_to_lamports)
 }
 
+#[deprecated(
+    since = "2.0.0",
+    note = "Please use `ArgMatches::get_one::<ClusterType>(...)` instead"
+)]
+#[allow(deprecated)]
 pub fn cluster_type_of(matches: &ArgMatches, name: &str) -> Option<ClusterType> {
     value_of(matches, name)
 }
 
+#[deprecated(
+    since = "2.0.0",
+    note = "Please use `ArgMatches::get_one::<CommitmentConfig>(...)` instead"
+)]
+#[allow(deprecated)]
 pub fn commitment_of(matches: &ArgMatches, name: &str) -> Option<CommitmentConfig> {
     matches
         .value_of(name)
@@ -246,6 +273,11 @@ pub fn parse_derived_address_seed(arg: &str) -> Result<String, String> {
 }
 
 // Return the keypair for an argument with filename `name` or None if not present.
+#[deprecated(
+    since = "2.0.0",
+    note = "Please use `input_parsers::signer::try_keypair_of` instead"
+)]
+#[allow(deprecated)]
 pub fn keypair_of(matches: &ArgMatches, name: &str) -> Option<Keypair> {
     if let Some(value) = matches.value_of(name) {
         if value == ASK_KEYWORD {
@@ -259,6 +291,11 @@ pub fn keypair_of(matches: &ArgMatches, name: &str) -> Option<Keypair> {
     }
 }
 
+#[deprecated(
+    since = "2.0.0",
+    note = "Please use `input_parsers::signer::try_keypairs_of` instead"
+)]
+#[allow(deprecated)]
 pub fn keypairs_of(matches: &ArgMatches, name: &str) -> Option<Vec<Keypair>> {
     matches.values_of(name).map(|values| {
         values
@@ -276,10 +313,20 @@ pub fn keypairs_of(matches: &ArgMatches, name: &str) -> Option<Vec<Keypair>> {
 
 // Return a pubkey for an argument that can itself be parsed into a pubkey,
 // or is a filename that can be read as a keypair
+#[deprecated(
+    since = "2.0.0",
+    note = "Please use `input_parsers::signer::try_pubkey_of` instead"
+)]
+#[allow(deprecated)]
 pub fn pubkey_of(matches: &ArgMatches, name: &str) -> Option<Pubkey> {
     value_of(matches, name).or_else(|| keypair_of(matches, name).map(|keypair| keypair.pubkey()))
 }
 
+#[deprecated(
+    since = "2.0.0",
+    note = "Please use `input_parsers::signer::try_pubkeys_of` instead"
+)]
+#[allow(deprecated)]
 pub fn pubkeys_of(matches: &ArgMatches, name: &str) -> Option<Vec<Pubkey>> {
     matches.values_of(name).map(|values| {
         values
@@ -294,12 +341,13 @@ pub fn pubkeys_of(matches: &ArgMatches, name: &str) -> Option<Vec<Pubkey>> {
     })
 }
 
+#[allow(deprecated)]
 #[cfg(test)]
 mod tests {
     use {
         super::*,
-        clap::{Arg, Command},
-        solana_sdk::{hash::Hash, pubkey::Pubkey},
+        clap::{Arg, ArgAction, Command},
+        solana_sdk::{commitment_config::CommitmentLevel, hash::Hash, pubkey::Pubkey},
     };
 
     fn app<'ab>() -> Command<'ab> {
@@ -308,7 +356,7 @@ mod tests {
                 Arg::new("multiple")
                     .long("multiple")
                     .takes_value(true)
-                    .multiple_occurrences(true)
+                    .action(ArgAction::Append)
                     .multiple_values(true),
             )
             .arg(Arg::new("single").takes_value(true).long("single"))
@@ -545,4 +593,87 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_unix_timestamp_from_rfc3339_datetime() {
+        let command = Command::new("test").arg(
+            Arg::new("timestamp")
+                .long("timestamp")
+                .takes_value(true)
+                .value_parser(clap::value_parser!(UnixTimestamp)),
+        );
+
+        // success case
+        let matches = command
+            .clone()
+            .try_get_matches_from(vec!["test", "--timestamp", "1234"])
+            .unwrap();
+        assert_eq!(
+            *matches.get_one::<UnixTimestamp>("timestamp").unwrap(),
+            1234,
+        );
+
+        // validation fails
+        let matches_error = command
+            .clone()
+            .try_get_matches_from(vec!["test", "--timestamp", "this_is_an_invalid_arg"])
+            .unwrap_err();
+        assert_eq!(matches_error.kind, clap::error::ErrorKind::ValueValidation);
+    }
+
+    #[test]
+    fn test_cluster_type() {
+        let command = Command::new("test").arg(
+            Arg::new("cluster")
+                .long("cluster")
+                .takes_value(true)
+                .value_parser(clap::value_parser!(ClusterType)),
+        );
+
+        // success case
+        let matches = command
+            .clone()
+            .try_get_matches_from(vec!["test", "--cluster", "testnet"])
+            .unwrap();
+        assert_eq!(
+            *matches.get_one::<ClusterType>("cluster").unwrap(),
+            ClusterType::Testnet
+        );
+
+        // validation fails
+        let matches_error = command
+            .clone()
+            .try_get_matches_from(vec!["test", "--cluster", "this_is_an_invalid_arg"])
+            .unwrap_err();
+        assert_eq!(matches_error.kind, clap::error::ErrorKind::ValueValidation);
+    }
+
+    #[test]
+    fn test_commitment_config() {
+        let command = Command::new("test").arg(
+            Arg::new("commitment")
+                .long("commitment")
+                .takes_value(true)
+                .value_parser(clap::value_parser!(CommitmentConfig)),
+        );
+
+        // success case
+        let matches = command
+            .clone()
+            .try_get_matches_from(vec!["test", "--commitment", "finalized"])
+            .unwrap();
+        assert_eq!(
+            *matches.get_one::<CommitmentConfig>("commitment").unwrap(),
+            CommitmentConfig {
+                commitment: CommitmentLevel::Finalized
+            },
+        );
+
+        // validation fails
+        let matches_error = command
+            .clone()
+            .try_get_matches_from(vec!["test", "--commitment", "this_is_an_invalid_arg"])
+            .unwrap_err();
+        assert_eq!(matches_error.kind, clap::error::ErrorKind::ValueValidation);
+    }
 }

clap-v3-utils/src/input_parsers/signer.rs

@@ -1,7 +1,7 @@
 use {
-    crate::{
-        input_parsers::{keypair_of, keypairs_of, pubkey_of, pubkeys_of},
-        keypair::{pubkey_from_path, resolve_signer_from_path, signer_from_path, ASK_KEYWORD},
+    crate::keypair::{
+        keypair_from_seed_phrase, pubkey_from_path, resolve_signer_from_path, signer_from_path,
+        ASK_KEYWORD, SKIP_SEED_PHRASE_VALIDATION_ARG,
     },
     clap::{builder::ValueParser, ArgMatches},
     solana_remote_wallet::{
@@ -11,7 +11,7 @@ use {
     solana_sdk::{
         derivation_path::{DerivationPath, DerivationPathError},
         pubkey::Pubkey,
-        signature::{Keypair, Signature, Signer},
+        signature::{read_keypair_file, Keypair, Signature, Signer},
     },
     std::{error, rc::Rc, str::FromStr},
     thiserror::Error,
@@ -236,16 +236,35 @@ pub fn try_keypair_of(
     matches: &ArgMatches,
     name: &str,
 ) -> Result<Option<Keypair>, Box<dyn error::Error>> {
-    matches.try_contains_id(name)?;
-    Ok(keypair_of(matches, name))
+    if let Some(value) = matches.try_get_one::<String>(name)? {
+        extract_keypair(matches, name, value)
+    } else {
+        Ok(None)
+    }
 }
 
 pub fn try_keypairs_of(
     matches: &ArgMatches,
     name: &str,
 ) -> Result<Option<Vec<Keypair>>, Box<dyn error::Error>> {
-    matches.try_contains_id(name)?;
-    Ok(keypairs_of(matches, name))
+    Ok(matches.try_get_many::<String>(name)?.map(|values| {
+        values
+            .filter_map(|value| extract_keypair(matches, name, value).ok().flatten())
+            .collect()
+    }))
+}
+
+fn extract_keypair(
+    matches: &ArgMatches,
+    name: &str,
+    path: &str,
+) -> Result<Option<Keypair>, Box<dyn error::Error>> {
+    if path == ASK_KEYWORD {
+        let skip_validation = matches.try_contains_id(SKIP_SEED_PHRASE_VALIDATION_ARG.name)?;
+        keypair_from_seed_phrase(name, skip_validation, true, None, true).map(Some)
+    } else {
+        read_keypair_file(path).map(Some)
+    }
 }
 
 // Return a `Result` wrapped pubkey for an argument that can itself be parsed into a pubkey,
@@ -254,19 +273,31 @@ pub fn try_pubkey_of(
     matches: &ArgMatches,
     name: &str,
 ) -> Result<Option<Pubkey>, Box<dyn error::Error>> {
-    matches.try_contains_id(name)?;
-    Ok(pubkey_of(matches, name))
+    if let Some(pubkey) = matches.try_get_one::<Pubkey>(name)? {
+        Ok(Some(*pubkey))
+    } else {
+        Ok(try_keypair_of(matches, name)?.map(|keypair| keypair.pubkey()))
+    }
 }
 
 pub fn try_pubkeys_of(
     matches: &ArgMatches,
     name: &str,
 ) -> Result<Option<Vec<Pubkey>>, Box<dyn error::Error>> {
-    matches.try_contains_id(name)?;
-    Ok(pubkeys_of(matches, name))
+    if let Some(pubkey_strings) = matches.try_get_many::<String>(name)? {
+        let mut pubkeys = Vec::with_capacity(pubkey_strings.len());
+        for pubkey_string in pubkey_strings {
+            pubkeys.push(pubkey_string.parse::<Pubkey>()?);
+        }
+        Ok(Some(pubkeys))
+    } else {
+        Ok(None)
+    }
 }
 
 // Return pubkey/signature pairs for a string of the form pubkey=signature
+#[deprecated(since = "2.0.0", note = "Please use `try_pubkeys_sigs_of` instead")]
+#[allow(deprecated)]
 pub fn pubkeys_sigs_of(matches: &ArgMatches, name: &str) -> Option<Vec<(Pubkey, Signature)>> {
     matches.values_of(name).map(|values| {
         values
@@ -286,8 +317,20 @@ pub fn try_pubkeys_sigs_of(
     matches: &ArgMatches,
     name: &str,
 ) -> Result<Option<Vec<(Pubkey, Signature)>>, Box<dyn error::Error>> {
-    matches.try_contains_id(name)?;
-    Ok(pubkeys_sigs_of(matches, name))
+    if let Some(pubkey_signer_strings) = matches.try_get_many::<String>(name)? {
+        let mut pubkey_sig_pairs = Vec::with_capacity(pubkey_signer_strings.len());
+        for pubkey_signer_string in pubkey_signer_strings {
+            let (pubkey_string, sig_string) = pubkey_signer_string
+                .split_once('=')
+                .ok_or("failed to parse `pubkey=signature` pair")?;
+            let pubkey = Pubkey::from_str(pubkey_string)?;
+            let sig = Signature::from_str(sig_string)?;
+            pubkey_sig_pairs.push((pubkey, sig));
+        }
+        Ok(Some(pubkey_sig_pairs))
+    } else {
+        Ok(None)
+    }
 }
 
 // Return a signer from matches at `name`
@@ -376,12 +419,14 @@ impl FromStr for PubkeySignature {
     }
 }
 
+#[allow(deprecated)]
 #[cfg(test)]
 mod tests {
     use {
         super::*,
+        crate::input_parsers::{keypair_of, pubkey_of, pubkeys_of},
         assert_matches::assert_matches,
-        clap::{Arg, Command},
+        clap::{Arg, ArgAction, Command},
         solana_remote_wallet::locator::Manufacturer,
         solana_sdk::signature::write_keypair_file,
         std::fs,
@@ -512,7 +557,7 @@ mod tests {
                 Arg::new("multiple")
                     .long("multiple")
                     .takes_value(true)
-                    .multiple_occurrences(true)
+                    .action(ArgAction::Append)
                     .multiple_values(true),
             )
             .arg(Arg::new("single").takes_value(true).long("single"))