Clean up solana-tokens (#10667)

* Use a trait object in solana-tokens' ThinClient

* Inline arg resolution

Not worth the code complexity

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

tokens/src/arg_parser.rs

@@ -2,8 +2,13 @@ use crate::args::{
     Args, BalancesArgs, Command, DistributeTokensArgs, StakeArgs, TransactionLogArgs,
 };
 use clap::{value_t, value_t_or_exit, App, Arg, ArgMatches, SubCommand};
-use solana_clap_utils::input_validators::{is_valid_pubkey, is_valid_signer};
+use solana_clap_utils::{
+    input_validators::{is_valid_pubkey, is_valid_signer},
+    keypair::{pubkey_from_path, signer_from_path},
+};
 use solana_cli_config::CONFIG_FILE;
+use solana_remote_wallet::remote_wallet::maybe_wallet_manager;
+use std::error::Error;
 use std::ffi::OsString;
 use std::process::exit;
 
@@ -225,36 +230,102 @@ fn create_db_path(campaign_name: Option<String>) -> String {
         .to_string()
 }
 
-fn parse_distribute_tokens_args(matches: &ArgMatches<'_>) -> DistributeTokensArgs<String, String> {
+fn parse_distribute_tokens_args(
-    DistributeTokensArgs {
+    matches: &ArgMatches<'_>,
+) -> Result<DistributeTokensArgs, Box<dyn Error>> {
+    let mut wallet_manager = maybe_wallet_manager()?;
+    let signer_matches = ArgMatches::default(); // No default signer
+
+    let sender_keypair_str = value_t_or_exit!(matches, "sender_keypair", String);
+    let sender_keypair = signer_from_path(
+        &signer_matches,
+        &sender_keypair_str,
+        "sender",
+        &mut wallet_manager,
+    )?;
+
+    let fee_payer_str = value_t_or_exit!(matches, "fee_payer", String);
+    let fee_payer = signer_from_path(
+        &signer_matches,
+        &fee_payer_str,
+        "fee-payer",
+        &mut wallet_manager,
+    )?;
+
+    Ok(DistributeTokensArgs {
         input_csv: value_t_or_exit!(matches, "input_csv", String),
         from_bids: matches.is_present("from_bids"),
         transaction_db: create_db_path(value_t!(matches, "campaign_name", String).ok()),
         dollars_per_sol: value_t!(matches, "dollars_per_sol", f64).ok(),
         dry_run: matches.is_present("dry_run"),
-        sender_keypair: value_t_or_exit!(matches, "sender_keypair", String),
+        sender_keypair,
-        fee_payer: value_t_or_exit!(matches, "fee_payer", String),
+        fee_payer,
         stake_args: None,
-    }
+    })
 }
 
-fn parse_distribute_stake_args(matches: &ArgMatches<'_>) -> DistributeTokensArgs<String, String> {
+fn parse_distribute_stake_args(
+    matches: &ArgMatches<'_>,
+) -> Result<DistributeTokensArgs, Box<dyn Error>> {
+    let mut wallet_manager = maybe_wallet_manager()?;
+    let signer_matches = ArgMatches::default(); // No default signer
+
+    let sender_keypair_str = value_t_or_exit!(matches, "sender_keypair", String);
+    let sender_keypair = signer_from_path(
+        &signer_matches,
+        &sender_keypair_str,
+        "sender",
+        &mut wallet_manager,
+    )?;
+
+    let fee_payer_str = value_t_or_exit!(matches, "fee_payer", String);
+    let fee_payer = signer_from_path(
+        &signer_matches,
+        &fee_payer_str,
+        "fee-payer",
+        &mut wallet_manager,
+    )?;
+
+    let stake_account_address_str = value_t_or_exit!(matches, "stake_account_address", String);
+    let stake_account_address = pubkey_from_path(
+        &signer_matches,
+        &stake_account_address_str,
+        "stake account address",
+        &mut wallet_manager,
+    )?;
+
+    let stake_authority_str = value_t_or_exit!(matches, "stake_authority", String);
+    let stake_authority = signer_from_path(
+        &signer_matches,
+        &stake_authority_str,
+        "stake authority",
+        &mut wallet_manager,
+    )?;
+
+    let withdraw_authority_str = value_t_or_exit!(matches, "withdraw_authority", String);
+    let withdraw_authority = signer_from_path(
+        &signer_matches,
+        &withdraw_authority_str,
+        "withdraw authority",
+        &mut wallet_manager,
+    )?;
+
     let stake_args = StakeArgs {
-        stake_account_address: value_t_or_exit!(matches, "stake_account_address", String),
+        stake_account_address,
         sol_for_fees: value_t_or_exit!(matches, "sol_for_fees", f64),
-        stake_authority: value_t_or_exit!(matches, "stake_authority", String),
+        stake_authority,
-        withdraw_authority: value_t_or_exit!(matches, "withdraw_authority", String),
+        withdraw_authority,
     };
-    DistributeTokensArgs {
+    Ok(DistributeTokensArgs {
         input_csv: value_t_or_exit!(matches, "input_csv", String),
         from_bids: false,
         transaction_db: create_db_path(value_t!(matches, "campaign_name", String).ok()),
         dollars_per_sol: None,
         dry_run: matches.is_present("dry_run"),
-        sender_keypair: value_t_or_exit!(matches, "sender_keypair", String),
+        sender_keypair,
-        fee_payer: value_t_or_exit!(matches, "fee_payer", String),
+        fee_payer,
         stake_args: Some(stake_args),
-    }
+    })
 }
 
 fn parse_balances_args(matches: &ArgMatches<'_>) -> BalancesArgs {
@@ -272,7 +343,7 @@ fn parse_transaction_log_args(matches: &ArgMatches<'_>) -> TransactionLogArgs {
     }
 }
 
-pub fn parse_args<I, T>(args: I) -> Args<String, String>
+pub fn parse_args<I, T>(args: I) -> Result<Args, Box<dyn Error>>
 where
     I: IntoIterator<Item = T>,
     T: Into<OsString> + Clone,
@@ -283,10 +354,10 @@ where
 
     let command = match matches.subcommand() {
         ("distribute-tokens", Some(matches)) => {
-            Command::DistributeTokens(parse_distribute_tokens_args(matches))
+            Command::DistributeTokens(parse_distribute_tokens_args(matches)?)
         }
         ("distribute-stake", Some(matches)) => {
-            Command::DistributeTokens(parse_distribute_stake_args(matches))
+            Command::DistributeTokens(parse_distribute_stake_args(matches)?)
         }
         ("balances", Some(matches)) => Command::Balances(parse_balances_args(matches)),
         ("transaction-log", Some(matches)) => {
@@ -297,9 +368,10 @@ where
             exit(1);
         }
     };
-    Args {
+    let args = Args {
         config_file,
         url,
         command,
-    }
+    };
+    Ok(args)
 }

tokens/src/args.rs

@@ -1,25 +1,21 @@
-use clap::ArgMatches;
-use solana_clap_utils::keypair::{pubkey_from_path, signer_from_path};
-use solana_remote_wallet::remote_wallet::{maybe_wallet_manager, RemoteWalletManager};
 use solana_sdk::{pubkey::Pubkey, signature::Signer};
-use std::{error::Error, sync::Arc};
 
-pub struct DistributeTokensArgs<P, K> {
+pub struct DistributeTokensArgs {
     pub input_csv: String,
     pub from_bids: bool,
     pub transaction_db: String,
     pub dollars_per_sol: Option<f64>,
     pub dry_run: bool,
-    pub sender_keypair: K,
+    pub sender_keypair: Box<dyn Signer>,
-    pub fee_payer: K,
+    pub fee_payer: Box<dyn Signer>,
-    pub stake_args: Option<StakeArgs<P, K>>,
+    pub stake_args: Option<StakeArgs>,
 }
 
-pub struct StakeArgs<P, K> {
+pub struct StakeArgs {
     pub sol_for_fees: f64,
-    pub stake_account_address: P,
+    pub stake_account_address: Pubkey,
-    pub stake_authority: K,
+    pub stake_authority: Box<dyn Signer>,
-    pub withdraw_authority: K,
+    pub withdraw_authority: Box<dyn Signer>,
 }
 
 pub struct BalancesArgs {
@@ -33,85 +29,14 @@ pub struct TransactionLogArgs {
     pub output_path: String,
 }
 
-pub enum Command<P, K> {
+pub enum Command {
-    DistributeTokens(DistributeTokensArgs<P, K>),
+    DistributeTokens(DistributeTokensArgs),
     Balances(BalancesArgs),
     TransactionLog(TransactionLogArgs),
 }
 
-pub struct Args<P, K> {
+pub struct Args {
     pub config_file: String,
     pub url: Option<String>,
-    pub command: Command<P, K>,
+    pub command: Command,
-}
-
-pub fn resolve_stake_args(
-    wallet_manager: &mut Option<Arc<RemoteWalletManager>>,
-    args: StakeArgs<String, String>,
-) -> Result<StakeArgs<Pubkey, Box<dyn Signer>>, Box<dyn Error>> {
-    let matches = ArgMatches::default();
-    let resolved_args = StakeArgs {
-        stake_account_address: pubkey_from_path(
-            &matches,
-            &args.stake_account_address,
-            "stake account address",
-            wallet_manager,
-        )
-        .unwrap(),
-        sol_for_fees: args.sol_for_fees,
-        stake_authority: signer_from_path(
-            &matches,
-            &args.stake_authority,
-            "stake authority",
-            wallet_manager,
-        )
-        .unwrap(),
-        withdraw_authority: signer_from_path(
-            &matches,
-            &args.withdraw_authority,
-            "withdraw authority",
-            wallet_manager,
-        )
-        .unwrap(),
-    };
-    Ok(resolved_args)
-}
-
-pub fn resolve_command(
-    command: Command<String, String>,
-) -> Result<Command<Pubkey, Box<dyn Signer>>, Box<dyn Error>> {
-    match command {
-        Command::DistributeTokens(args) => {
-            let mut wallet_manager = maybe_wallet_manager()?;
-            let matches = ArgMatches::default();
-            let resolved_stake_args = args
-                .stake_args
-                .map(|args| resolve_stake_args(&mut wallet_manager, args));
-            let resolved_args = DistributeTokensArgs {
-                input_csv: args.input_csv,
-                from_bids: args.from_bids,
-                transaction_db: args.transaction_db,
-                dollars_per_sol: args.dollars_per_sol,
-                dry_run: args.dry_run,
-                sender_keypair: signer_from_path(
-                    &matches,
-                    &args.sender_keypair,
-                    "sender",
-                    &mut wallet_manager,
-                )
-                .unwrap(),
-                fee_payer: signer_from_path(
-                    &matches,
-                    &args.fee_payer,
-                    "fee-payer",
-                    &mut wallet_manager,
-                )
-                .unwrap(),
-                stake_args: resolved_stake_args.map_or(Ok(None), |r| r.map(Some))?,
-            };
-            Ok(Command::DistributeTokens(resolved_args))
-        }
-        Command::Balances(args) => Ok(Command::Balances(args)),
-        Command::TransactionLog(args) => Ok(Command::TransactionLog(args)),
-    }
 }

tokens/src/commands.rs

@@ -92,11 +92,11 @@ fn create_allocation(bid: &Bid, dollars_per_sol: f64) -> Allocation {
     }
 }
 
-fn distribute_tokens<T: Client>(
+fn distribute_tokens(
-    client: &ThinClient<T>,
+    client: &ThinClient,
     db: &mut PickleDb,
     allocations: &[Allocation],
-    args: &DistributeTokensArgs<Pubkey, Box<dyn Signer>>,
+    args: &DistributeTokensArgs,
 ) -> Result<(), Error> {
     for allocation in allocations {
         let new_stake_account_keypair = Keypair::new();
@@ -206,9 +206,9 @@ fn new_spinner_progress_bar() -> ProgressBar {
     progress_bar
 }
 
-pub fn process_distribute_tokens<T: Client>(
+pub fn process_distribute_tokens(
-    client: &ThinClient<T>,
+    client: &ThinClient,
-    args: &DistributeTokensArgs<Pubkey, Box<dyn Signer>>,
+    args: &DistributeTokensArgs,
 ) -> Result<Option<usize>, Error> {
     let mut allocations: Vec<Allocation> =
         read_allocations(&args.input_csv, args.from_bids, args.dollars_per_sol);
@@ -290,8 +290,8 @@ pub fn process_distribute_tokens<T: Client>(
     Ok(opt_confirmations)
 }
 
-fn finalize_transactions<T: Client>(
+fn finalize_transactions(
-    client: &ThinClient<T>,
+    client: &ThinClient,
     db: &mut PickleDb,
     dry_run: bool,
 ) -> Result<Option<usize>, Error> {
@@ -322,8 +322,8 @@ fn finalize_transactions<T: Client>(
 
 // Update the finalized bit on any transactions that are now rooted
 // Return the lowest number of confirmations on the unfinalized transactions or None if all are finalized.
-fn update_finalized_transactions<T: Client>(
+fn update_finalized_transactions(
-    client: &ThinClient<T>,
+    client: &ThinClient,
     db: &mut PickleDb,
 ) -> Result<Option<usize>, Error> {
     let transaction_infos = db::read_transaction_infos(db);
@@ -368,10 +368,7 @@ fn update_finalized_transactions<T: Client>(
     Ok(confirmations)
 }
 
-pub fn process_balances<T: Client>(
+pub fn process_balances(client: &ThinClient, args: &BalancesArgs) -> Result<(), csv::Error> {
-    client: &ThinClient<T>,
-    args: &BalancesArgs,
-) -> Result<(), csv::Error> {
     let allocations: Vec<Allocation> =
         read_allocations(&args.input_csv, args.from_bids, args.dollars_per_sol);
     let allocations = merge_allocations(&allocations);
@@ -438,7 +435,7 @@ pub fn test_process_distribute_tokens_with_client<C: Client>(client: C, sender_k
         .unwrap()
         .to_string();
 
-    let args: DistributeTokensArgs<Pubkey, Box<dyn Signer>> = DistributeTokensArgs {
+    let args = DistributeTokensArgs {
         sender_keypair: Box::new(sender_keypair),
         fee_payer: Box::new(fee_payer),
         dry_run: false,
@@ -534,13 +531,13 @@ pub fn test_process_distribute_stake_with_client<C: Client>(client: C, sender_ke
         .unwrap()
         .to_string();
 
-    let stake_args: StakeArgs<Pubkey, Box<dyn Signer>> = StakeArgs {
+    let stake_args = StakeArgs {
         stake_account_address,
         stake_authority: Box::new(stake_authority),
         withdraw_authority: Box::new(withdraw_authority),
         sol_for_fees: 1.0,
     };
-    let args: DistributeTokensArgs<Pubkey, Box<dyn Signer>> = DistributeTokensArgs {
+    let args = DistributeTokensArgs {
         fee_payer: Box::new(fee_payer),
         dry_run: false,
         input_csv,

tokens/src/main.rs

@@ -1,19 +1,14 @@
 use solana_cli_config::Config;
 use solana_cli_config::CONFIG_FILE;
 use solana_client::rpc_client::RpcClient;
-use solana_tokens::{
+use solana_tokens::{arg_parser::parse_args, args::Command, commands, thin_client::ThinClient};
-    arg_parser::parse_args,
-    args::{resolve_command, Command},
-    commands,
-    thin_client::ThinClient,
-};
 use std::env;
 use std::error::Error;
 use std::path::Path;
 use std::process;
 
 fn main() -> Result<(), Box<dyn Error>> {
-    let command_args = parse_args(env::args_os());
+    let command_args = parse_args(env::args_os())?;
     let config = if Path::new(&command_args.config_file).exists() {
         Config::load(&command_args.config_file)?
     } else {
@@ -27,7 +22,7 @@ fn main() -> Result<(), Box<dyn Error>> {
     let json_rpc_url = command_args.url.unwrap_or(config.json_rpc_url);
     let client = RpcClient::new(json_rpc_url);
 
-    match resolve_command(command_args.command)? {
+    match command_args.command {
         Command::DistributeTokens(args) => {
             let thin_client = ThinClient::new(client, args.dry_run);
             commands::process_distribute_tokens(&thin_client, &args)?;

tokens/src/thin_client.rs

@@ -114,14 +114,17 @@ impl Client for BankClient {
     }
 }
 
-pub struct ThinClient<C: Client> {
+pub struct ThinClient<'a> {
-    client: C,
+    client: Box<dyn Client + 'a>,
     dry_run: bool,
 }
 
-impl<C: Client> ThinClient<C> {
+impl<'a> ThinClient<'a> {
-    pub fn new(client: C, dry_run: bool) -> Self {
+    pub fn new<C: Client + 'a>(client: C, dry_run: bool) -> Self {
-        Self { client, dry_run }
+        Self {
+            client: Box::new(client),
+            dry_run,
+        }
     }
 
     pub fn send_transaction(&self, transaction: Transaction) -> Result<Signature> {