From 768045e5a03dc76d87dae112187deb1e62d6cbbf Mon Sep 17 00:00:00 2001 From: patrick Date: Tue, 10 May 2022 16:46:56 +0100 Subject: [PATCH 1/2] make `WalletCommand::execute` `self` instead of `&mut self` and removed unneeded cloning --- sui/src/bin/wallet.rs | 4 +-- sui/src/wallet_commands.rs | 70 ++++++++++++++++---------------------- 2 files changed, 31 insertions(+), 43 deletions(-) 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..168d9811ee719 100644 --- a/sui/src/wallet_commands.rs +++ b/sui/src/wallet_commands.rs @@ -249,7 +249,7 @@ pub struct SimpleTransactionSigner { impl WalletCommands { pub async fn execute( - &mut self, + self, context: &mut WalletContext, ) -> Result { let ret = Ok(match self { @@ -258,13 +258,13 @@ 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 @@ -282,7 +282,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 { @@ -307,12 +307,12 @@ 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 @@ -339,18 +339,12 @@ 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 } @@ -361,10 +355,7 @@ impl WalletCommands { 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,10 +371,10 @@ 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 @@ -403,10 +394,10 @@ 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 @@ -423,29 +414,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,9 +446,9 @@ 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() { @@ -472,8 +460,8 @@ impl WalletCommands { "mint", &[], gas, - &gas_budget.unwrap_or(3000), - &args, + gas_budget.unwrap_or(3000), + args, context, ) .await?; @@ -698,20 +686,20 @@ async fn call_move( args: &[SuiJsonValue], 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, + gas, + gas_budget, ) .await?; let signature = context @@ -734,7 +722,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, From ef2dc087ef6512a2c4b9369a31782bcdc88f8a3a Mon Sep 17 00:00:00 2001 From: patrick Date: Tue, 17 May 2022 10:16:07 +0100 Subject: [PATCH 2/2] remove unnecessary Arc RWLock --- crates/sui-faucet/src/faucet/simple_faucet.rs | 12 +--- sui/src/wallet_commands.rs | 61 ++++++------------- 2 files changed, 20 insertions(+), 53 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/wallet_commands.rs b/sui/src/wallet_commands.rs index 168d9811ee719..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,10 +242,6 @@ pub enum WalletCommands { }, } -pub struct SimpleTransactionSigner { - pub keystore: Arc>>, -} - impl WalletCommands { pub async fn execute( self, @@ -266,11 +261,7 @@ impl WalletCommands { .gateway .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)) @@ -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) @@ -314,11 +305,7 @@ impl WalletCommands { .gateway .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)) @@ -349,7 +336,7 @@ impl WalletCommands { 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) @@ -376,11 +363,7 @@ impl WalletCommands { .gateway .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)) @@ -399,11 +382,7 @@ impl WalletCommands { .gateway .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)) @@ -455,10 +434,10 @@ impl WalletCommands { 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, @@ -481,7 +460,7 @@ impl WalletCommands { pub struct WalletContext { pub config: PersistedConfig, - pub keystore: Arc>>, + pub keystore: Box, pub gateway: GatewayClient, } @@ -494,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, @@ -677,13 +656,13 @@ 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?; @@ -696,17 +675,13 @@ async fn call_move( package, module.to_string(), function.to_string(), - type_args.to_owned(), - args.to_vec(), + 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