From 50da6e91bed9aa234f70f0c900c74941b8e8f9c9 Mon Sep 17 00:00:00 2001 From: Patrick Kuo Date: Thu, 19 May 2022 09:53:03 +0100 Subject: [PATCH] [refactoring][wallet cli] - Wallet command refactoring (#1882) * make `WalletCommand::execute` `self` instead of `&mut self` and removed unneeded cloning * remove unnecessary Arc RWLock --- crates/sui-faucet/src/faucet/simple_faucet.rs | 12 +- sui/src/bin/wallet.rs | 4 +- sui/src/wallet_commands.rs | 131 +++++++----------- 3 files changed, 51 insertions(+), 96 deletions(-) diff --git a/crates/sui-faucet/src/faucet/simple_faucet.rs b/crates/sui-faucet/src/faucet/simple_faucet.rs index dd61605e0982e..93491a8b79e80 100644 --- a/crates/sui-faucet/src/faucet/simple_faucet.rs +++ b/crates/sui-faucet/src/faucet/simple_faucet.rs @@ -120,11 +120,7 @@ impl SimpleFaucet { budget, ) .await?; - let signature = context - .keystore - .read() - .unwrap() - .sign(&signer, &data.to_bytes())?; + let signature = context.keystore.sign(&signer, &data.to_bytes())?; let response = context .gateway .execute_transaction(Transaction::new(data, signature)) @@ -148,11 +144,7 @@ impl SimpleFaucet { .gateway .transfer_coin(signer, coin_id, Some(gas_object_id), budget, recipient) .await?; - let signature = context - .keystore - .read() - .unwrap() - .sign(&signer, &data.to_bytes())?; + let signature = context.keystore.sign(&signer, &data.to_bytes())?; let effects = context .gateway .execute_transaction(Transaction::new(data, signature)) diff --git a/sui/src/bin/wallet.rs b/sui/src/bin/wallet.rs index 8806459f93a1d..0440901f15fec 100644 --- a/sui/src/bin/wallet.rs +++ b/sui/src/bin/wallet.rs @@ -157,7 +157,7 @@ async fn try_main() -> Result<(), anyhow::Error> { ); shell.run_async(&mut out, &mut stderr()).await?; - } else if let Some(mut cmd) = options.cmd { + } else if let Some(cmd) = options.cmd { cmd.execute(&mut context).await?.print(!options.json); } else { ClientOpt::command().print_long_help()? @@ -209,7 +209,7 @@ async fn handle_command( context: &mut WalletContext, completion_cache: CompletionCache, ) -> Result { - let mut wallet_opts = wallet_opts?; + let wallet_opts = wallet_opts?; let result = wallet_opts.command.execute(context).await?; // Update completion cache diff --git a/sui/src/wallet_commands.rs b/sui/src/wallet_commands.rs index 32324569fe90f..93165988a9c05 100644 --- a/sui/src/wallet_commands.rs +++ b/sui/src/wallet_commands.rs @@ -6,7 +6,6 @@ use std::{ collections::BTreeSet, fmt::{Debug, Display, Formatter, Write}, path::Path, - sync::{Arc, RwLock}, time::Instant, }; @@ -243,13 +242,9 @@ pub enum WalletCommands { }, } -pub struct SimpleTransactionSigner { - pub keystore: Arc>>, -} - impl WalletCommands { pub async fn execute( - &mut self, + self, context: &mut WalletContext, ) -> Result { let ret = Ok(match self { @@ -258,19 +253,15 @@ impl WalletCommands { gas, gas_budget, } => { - let sender = context.try_get_object_owner(gas).await?; + let sender = context.try_get_object_owner(&gas).await?; let sender = sender.unwrap_or(context.active_address()?); - let compiled_modules = build_move_package_to_bytes(Path::new(path), false)?; + let compiled_modules = build_move_package_to_bytes(Path::new(&path), false)?; let data = context .gateway - .publish(sender, compiled_modules, *gas, *gas_budget) + .publish(sender, compiled_modules, gas, gas_budget) .await?; - let signature = context - .keystore - .read() - .unwrap() - .sign(&sender, &data.to_bytes())?; + let signature = context.keystore.sign(&sender, &data.to_bytes())?; let response = context .gateway .execute_transaction(Transaction::new(data, signature)) @@ -282,7 +273,7 @@ impl WalletCommands { WalletCommands::Object { id } => { // Fetch the object ref - let object_read = context.gateway.get_object_info(*id).await?; + let object_read = context.gateway.get_object_info(id).await?; WalletCommandResult::Object(object_read) } WalletCommands::Call { @@ -295,7 +286,7 @@ impl WalletCommands { args, } => { let (cert, effects) = call_move( - package, module, function, type_args, gas, gas_budget, args, context, + package, &module, &function, type_args, gas, gas_budget, args, context, ) .await?; WalletCommandResult::Call(cert, effects) @@ -307,18 +298,14 @@ impl WalletCommands { gas, gas_budget, } => { - let from = context.get_object_owner(object_id).await?; + let from = context.get_object_owner(&object_id).await?; let time_start = Instant::now(); let data = context .gateway - .transfer_coin(from, *object_id, *gas, *gas_budget, *to) + .transfer_coin(from, object_id, gas, gas_budget, to) .await?; - let signature = context - .keystore - .read() - .unwrap() - .sign(&from, &data.to_bytes())?; + let signature = context.keystore.sign(&from, &data.to_bytes())?; let response = context .gateway .execute_transaction(Transaction::new(data, signature)) @@ -339,32 +326,23 @@ impl WalletCommands { } WalletCommands::Objects { address } => { - let address = match address { - Some(a) => *a, - None => context.active_address()?, - }; + let address = address.unwrap_or(context.active_address()?); WalletCommandResult::Objects(context.gateway.get_owned_objects(address).await?) } WalletCommands::SyncClientState { address } => { - let address = match address { - Some(a) => *a, - None => context.active_address()?, - }; + let address = address.unwrap_or(context.active_address()?); context.gateway.sync_account_state(address).await?; WalletCommandResult::SyncClientState } WalletCommands::NewAddress => { - let address = context.keystore.write().unwrap().add_random_key()?; + let address = context.keystore.add_random_key()?; context.config.accounts.push(address); context.config.save()?; WalletCommandResult::NewAddress(address) } WalletCommands::Gas { address } => { - let address = match address { - Some(a) => *a, - None => context.active_address()?, - }; + let address = address.unwrap_or(context.active_address()?); let coins = context .gas_objects(address) .await? @@ -380,16 +358,12 @@ impl WalletCommands { gas, gas_budget, } => { - let signer = context.get_object_owner(coin_id).await?; + let signer = context.get_object_owner(&coin_id).await?; let data = context .gateway - .split_coin(signer, *coin_id, amounts.clone(), *gas, *gas_budget) + .split_coin(signer, coin_id, amounts, gas, gas_budget) .await?; - let signature = context - .keystore - .read() - .unwrap() - .sign(&signer, &data.to_bytes())?; + let signature = context.keystore.sign(&signer, &data.to_bytes())?; let response = context .gateway .execute_transaction(Transaction::new(data, signature)) @@ -403,16 +377,12 @@ impl WalletCommands { gas, gas_budget, } => { - let signer = context.get_object_owner(primary_coin).await?; + let signer = context.get_object_owner(&primary_coin).await?; let data = context .gateway - .merge_coins(signer, *primary_coin, *coin_to_merge, *gas, *gas_budget) + .merge_coins(signer, primary_coin, coin_to_merge, gas, gas_budget) .await?; - let signature = context - .keystore - .read() - .unwrap() - .sign(&signer, &data.to_bytes())?; + let signature = context.keystore.sign(&signer, &data.to_bytes())?; let response = context .gateway .execute_transaction(Transaction::new(data, signature)) @@ -423,29 +393,26 @@ impl WalletCommands { } WalletCommands::Switch { address, gateway } => { if let Some(addr) = address { - if !context.config.accounts.contains(addr) { + if !context.config.accounts.contains(&addr) { return Err(anyhow!("Address {} not managed by wallet", addr)); } - context.config.active_address = Some(*addr); + context.config.active_address = Some(addr); context.config.save()?; } - if let Some(gateway) = gateway { + if let Some(gateway) = &gateway { // TODO: handle embedded gateway context.config.gateway = GatewayType::RPC(gateway.clone()); context.config.save()?; } - if Option::is_none(address) && Option::is_none(gateway) { + if Option::is_none(&address) && Option::is_none(&gateway) { return Err(anyhow!( "No address or gateway specified. Please Specify one." )); } - WalletCommandResult::Switch(SwitchResponse { - address: *address, - gateway: gateway.clone(), - }) + WalletCommandResult::Switch(SwitchResponse { address, gateway }) } WalletCommands::ActiveAddress {} => { WalletCommandResult::ActiveAddress(context.active_address().ok()) @@ -458,22 +425,22 @@ impl WalletCommands { gas_budget, } => { let args_json = json!([ - unwrap_or(name, EXAMPLE_NFT_NAME), - unwrap_or(description, EXAMPLE_NFT_DESCRIPTION), - unwrap_or(url, EXAMPLE_NFT_URL) + unwrap_or(&name, EXAMPLE_NFT_NAME), + unwrap_or(&description, EXAMPLE_NFT_DESCRIPTION), + unwrap_or(&url, EXAMPLE_NFT_URL) ]); let mut args = vec![]; for a in args_json.as_array().unwrap() { args.push(SuiJsonValue::new(a.clone()).unwrap()); } let (_, effects) = call_move( - &ObjectID::from(SUI_FRAMEWORK_ADDRESS), + ObjectID::from(SUI_FRAMEWORK_ADDRESS), "DevNetNFT", "mint", - &[], + vec![], gas, - &gas_budget.unwrap_or(3000), - &args, + gas_budget.unwrap_or(3000), + args, context, ) .await?; @@ -493,7 +460,7 @@ impl WalletCommands { pub struct WalletContext { pub config: PersistedConfig, - pub keystore: Arc>>, + pub keystore: Box, pub gateway: GatewayClient, } @@ -506,7 +473,7 @@ impl WalletContext { )) })?; let config = config.persisted(config_path); - let keystore = Arc::new(RwLock::new(config.keystore.init()?)); + let keystore = config.keystore.init()?; let gateway = config.gateway.init()?; let context = Self { config, @@ -689,36 +656,32 @@ impl Display for WalletCommandResult { } async fn call_move( - package: &ObjectID, + package: ObjectID, module: &str, function: &str, - type_args: &[TypeTag], - gas: &Option, - gas_budget: &u64, - args: &[SuiJsonValue], + type_args: Vec, + gas: Option, + gas_budget: u64, + args: Vec, context: &mut WalletContext, ) -> Result<(SuiCertifiedTransaction, SuiTransactionEffects), anyhow::Error> { - let gas_owner = context.try_get_object_owner(gas).await?; + let gas_owner = context.try_get_object_owner(&gas).await?; let sender = gas_owner.unwrap_or(context.active_address()?); let data = context .gateway .move_call( sender, - *package, + package, module.to_string(), function.to_string(), - type_args.to_owned(), - args.to_vec(), - *gas, - *gas_budget, + type_args, + args, + gas, + gas_budget, ) .await?; - let signature = context - .keystore - .read() - .unwrap() - .sign(&sender, &data.to_bytes())?; + let signature = context.keystore.sign(&sender, &data.to_bytes())?; let transaction = Transaction::new(data, signature); let response = context .gateway @@ -734,7 +697,7 @@ async fn call_move( Ok((cert, effects)) } -fn unwrap_or<'a>(val: &'a mut Option, default: &'a str) -> &'a str { +fn unwrap_or<'a>(val: &'a Option, default: &'a str) -> &'a str { match val { Some(v) => v, None => default,