From a47d5b80db4274728b6d833d98aceebc571a2b5d Mon Sep 17 00:00:00 2001
From: samkim-crypto <skim13@cs.stanford.edu>
Date: Tue, 19 Nov 2024 18:14:28 +0900
Subject: [PATCH] clean up transfer hook account validator and parser

---
 token/cli/src/clap_app.rs | 53 +++++++++++++++++++--------------------
 token/cli/src/command.rs  | 12 +++++----
 2 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/token/cli/src/clap_app.rs b/token/cli/src/clap_app.rs
index 1e3eab345bd..c5b6397299e 100644
--- a/token/cli/src/clap_app.rs
+++ b/token/cli/src/clap_app.rs
@@ -186,38 +186,37 @@ impl fmt::Display for AccountMetaRole {
         write!(f, "{:?}", self)
     }
 }
-pub fn parse_transfer_hook_account<T>(string: T) -> Result<AccountMeta, String>
-where
-    T: AsRef<str> + fmt::Display,
-{
-    match string.as_ref().split(':').collect::<Vec<_>>().as_slice() {
-        [address, role] => {
-            let address = Pubkey::from_str(address).map_err(|e| format!("{e}"))?;
-            let meta = match AccountMetaRole::from_str(role).map_err(|e| format!("{e}"))? {
-                AccountMetaRole::Readonly => AccountMeta::new_readonly(address, false),
-                AccountMetaRole::Writable => AccountMeta::new(address, false),
-                AccountMetaRole::ReadonlySigner => AccountMeta::new_readonly(address, true),
-                AccountMetaRole::WritableSigner => AccountMeta::new(address, true),
-            };
-            Ok(meta)
+
+#[derive(Clone, Copy)]
+pub(crate) struct TransferHookAccount {
+    address: Pubkey,
+    role: AccountMetaRole,
+}
+impl FromStr for TransferHookAccount {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s.split(':').collect::<Vec<_>>().as_slice() {
+            [address, role] => {
+                let address = Pubkey::from_str(address).map_err(|e| format!("{e}"))?;
+                let role = AccountMetaRole::from_str(role).map_err(|e| format!("{e}"))?;
+                Ok(Self { address, role })
+            }
+            _ => Err("Transfer hook account must be present as <ADDRESS>:<ROLE>".to_string()),
         }
-        _ => Err("Transfer hook account must be present as <ADDRESS>:<ROLE>".to_string()),
     }
 }
-fn validate_transfer_hook_account<T>(string: T) -> Result<(), String>
-where
-    T: AsRef<str> + fmt::Display,
-{
-    match string.as_ref().split(':').collect::<Vec<_>>().as_slice() {
-        [address, role] => {
-            is_valid_pubkey(address)?;
-            AccountMetaRole::from_str(role)
-                .map(|_| ())
-                .map_err(|e| format!("{e}"))
+impl TransferHookAccount {
+    pub(crate) fn create_account_meta(&self) -> AccountMeta {
+        match self.role {
+            AccountMetaRole::Readonly => AccountMeta::new_readonly(self.address, false),
+            AccountMetaRole::Writable => AccountMeta::new(self.address, false),
+            AccountMetaRole::ReadonlySigner => AccountMeta::new_readonly(self.address, true),
+            AccountMetaRole::WritableSigner => AccountMeta::new(self.address, true),
         }
-        _ => Err("Transfer hook account must be present as <ADDRESS>:<ROLE>".to_string()),
     }
 }
+
 #[derive(Debug, Clone, PartialEq, EnumIter, EnumString, IntoStaticStr)]
 #[strum(serialize_all = "kebab-case")]
 pub enum CliAuthorityType {
@@ -1427,7 +1426,7 @@ pub fn app<'a>(
                 .arg(
                     Arg::with_name("transfer_hook_account")
                         .long("transfer-hook-account")
-                        .validator(|s| validate_transfer_hook_account(s))
+                        .value_parser(clap::value_parser!(TransferHookAccount))
                         .value_name("PUBKEY:ROLE")
                         .takes_value(true)
                         .multiple(true)
diff --git a/token/cli/src/command.rs b/token/cli/src/command.rs
index d5b18af7225..712ecec7687 100644
--- a/token/cli/src/command.rs
+++ b/token/cli/src/command.rs
@@ -3923,11 +3923,13 @@ pub async fn process_command<'a>(
             let use_unchecked_instruction = arg_matches.is_present("use_unchecked_instruction");
             let expected_fee = arg_matches.get_one::<Amount>("expected_fee").copied();
             let memo = value_t!(arg_matches, "memo", String).ok();
-            let transfer_hook_accounts = arg_matches.values_of("transfer_hook_account").map(|v| {
-                v.into_iter()
-                    .map(|s| parse_transfer_hook_account(s).unwrap())
-                    .collect::<Vec<_>>()
-            });
+            let transfer_hook_accounts = arg_matches
+                .get_many::<TransferHookAccount>("transfer_hook_account")
+                .map(|v| {
+                    v.into_iter()
+                        .map(|account| account.create_account_meta())
+                        .collect::<Vec<_>>()
+                });
 
             command_transfer(
                 config,