Skip to content

Commit

Permalink
[refactoring][wallet cli] - Wallet command refactoring (MystenLabs#1882)
Browse files Browse the repository at this point in the history 
* make `WalletCommand::execute` `self` instead of `&mut self` and removed unneeded cloning

* remove unnecessary Arc RWLock
  • Loading branch information
@patrickkuo
patrickkuo authored May 19, 2022
1 parent ca9984c commit 50da6e9
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 96 deletions.
12 changes: 2 additions & 10 deletions crates/sui-faucet/src/faucet/simple_faucet.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions sui/src/bin/wallet.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()?
Expand Down Expand Up @@ -209,7 +209,7 @@ async fn handle_command(
context: &mut WalletContext,
completion_cache: CompletionCache,
) -> Result<bool, anyhow::Error> {
let mut wallet_opts = wallet_opts?;
let wallet_opts = wallet_opts?;
let result = wallet_opts.command.execute(context).await?;

// Update completion cache
Expand Down
131 changes: 47 additions & 84 deletions sui/src/wallet_commands.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use std::{
collections::BTreeSet,
fmt::{Debug, Display, Formatter, Write},
path::Path,
sync::{Arc, RwLock},
time::Instant,
};

Expand Down Expand Up @@ -243,13 +242,9 @@ pub enum WalletCommands {
},
}

pub struct SimpleTransactionSigner {
pub keystore: Arc<RwLock<Box<dyn Keystore>>>,
}

impl WalletCommands {
pub async fn execute(
&mut self,
self,
context: &mut WalletContext,
) -> Result<WalletCommandResult, anyhow::Error> {
let ret = Ok(match self {
Expand All @@ -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))
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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))
Expand All @@ -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?
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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())
Expand All @@ -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?;
Expand All @@ -493,7 +460,7 @@ impl WalletCommands {

pub struct WalletContext {
pub config: PersistedConfig<WalletConfig>,
pub keystore: Arc<RwLock<Box<dyn Keystore>>>,
pub keystore: Box<dyn Keystore>,
pub gateway: GatewayClient,
}

Expand All @@ -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,
Expand Down Expand Up @@ -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<ObjectID>,
gas_budget: &u64,
args: &[SuiJsonValue],
type_args: Vec<TypeTag>,
gas: Option<ObjectID>,
gas_budget: u64,
args: Vec<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,
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
Expand All @@ -734,7 +697,7 @@ async fn call_move(
Ok((cert, effects))
}

fn unwrap_or<'a>(val: &'a mut Option<String>, default: &'a str) -> &'a str {
fn unwrap_or<'a>(val: &'a Option<String>, default: &'a str) -> &'a str {
match val {
Some(v) => v,
None => default,
Expand Down

0 comments on commit 50da6e9

Please sign in to comment.