From 77636ef2219ef4e74e319077fa758b9a76988ff3 Mon Sep 17 00:00:00 2001 From: Shawn Harmsen Date: Mon, 24 Apr 2023 15:39:14 +0900 Subject: [PATCH] refactor to not use .clone() --- ol/types/src/account.rs | 13 ++-- ol/types/src/config.rs | 73 ++++++++------------ ol/types/src/legacy_recovery.rs | 116 +++++++++++++------------------- ol/types/src/pay_instruction.rs | 103 ++++++++++++++-------------- 4 files changed, 135 insertions(+), 170 deletions(-) diff --git a/ol/types/src/account.rs b/ol/types/src/account.rs index e8b2da5bf0..11a6679839 100644 --- a/ol/types/src/account.rs +++ b/ol/types/src/account.rs @@ -6,7 +6,7 @@ use diem_crypto::x25519::PublicKey; use diem_global_constants::{DEFAULT_PUB_PORT, DEFAULT_VAL_PORT, DEFAULT_VFN_PORT}; use diem_types::{ account_address::AccountAddress, - network_address::{NetworkAddress}, + network_address::NetworkAddress, transaction::{SignedTransaction, TransactionPayload}, }; @@ -144,13 +144,13 @@ impl ValConfigs { } } /// Creates the json file needed for onchain account creation - validator - pub fn create_manifest(&self, mut json_path: PathBuf) -> Result<(), anyhow::Error>{ + pub fn create_manifest(&self, mut json_path: PathBuf) -> Result<(), anyhow::Error> { //where file will be saved json_path.push("account.json"); let mut file = File::create(json_path.as_path())?; let buf = serde_json::to_string(&self)?; - // .expect("Config should be export to json"); + // .expect("Config should be export to json"); file.write(&buf.as_bytes())?; @@ -217,16 +217,16 @@ impl ValConfigs { .enumerate() .for_each(|(i, instr)| { println!("{}", instr.text_instruction()); - if !*IS_TEST { + if !*IS_TEST { match Confirm::new().with_prompt("").interact().unwrap() { true => {}, _ => { print!("Autopay configuration aborted. Check batch configuration file or template"); exit(1); } - } + } } - + if let Some(signed) = &self.autopay_signed { let tx = signed.iter().nth(i).unwrap(); let payload = tx.clone().into_raw_transaction().into_payload(); @@ -301,7 +301,6 @@ fn test_parse_account_file() { #[test] fn val_config_ip_address() { - let block = VDFProof { height: 0u64, elapsed_secs: 0u64, diff --git a/ol/types/src/config.rs b/ol/types/src/config.rs index 2516dca68c..4bc094066e 100644 --- a/ol/types/src/config.rs +++ b/ol/types/src/config.rs @@ -28,13 +28,9 @@ const BASE_WAYPOINT: &str = "0:683185844ef67e5c8eeaa158e635de2a4c574ce7bbb7f41f7 pub static IS_PROD: Lazy = Lazy::new(|| { match std::env::var("NODE_ENV") { Ok(val) => { - match val.as_str() { - "prod" => true, - // if anything else is set by user is false - _ => false, - } + matches!(val.as_str(), "prod") } - // default to prod if nothig is set + // default to prod if nothing is set _ => true, } }); @@ -43,7 +39,7 @@ pub static IS_PROD: Lazy = Lazy::new(|| { /// check this is CI environment pub static IS_TEST: Lazy = Lazy::new(|| { // assume default if NODE_ENV=prod and TEST=y. - std::env::var("TEST").unwrap_or("n".to_string()) != "n".to_string() + std::env::var("TEST").unwrap_or_else(|_| "n".to_string()) != *"n" }); /// MinerApp Configuration @@ -123,16 +119,12 @@ impl AppCfg { /// Get where the block/proofs are stored. pub fn get_block_dir(&self) -> PathBuf { - let mut home = self.workspace.node_home.clone(); - home.push(&self.workspace.block_dir); - home + self.workspace.node_home.join(&self.workspace.block_dir) } /// Get where node key_store.json stored. pub fn get_key_store_path(&self) -> PathBuf { - let mut home = self.workspace.node_home.clone(); - home.push("key_store.json"); - home + self.workspace.node_home.join("key_store.json") } /// Get where node key_store.json stored. @@ -169,8 +161,10 @@ impl AppCfg { None => what_vfn_ip().ok(), }; - default_config.workspace.node_home = - config_path.clone().unwrap_or_else(|| what_home(None, None)); + default_config.workspace.node_home = match config_path { + Some(path) => path.to_path_buf(), + None => what_home(None, None).to_path_buf(), + }; if let Some(u) = upstream_peer { default_config.profile.upstream_nodes = vec![u.to_owned()] @@ -184,15 +178,10 @@ impl AppCfg { default_config.chain_info.chain_id = id.to_owned(); }; - if source_path.is_some() { - // let source_path = what_source(); - default_config.workspace.source_path = source_path.clone(); - default_config.workspace.stdlib_bin_path = Some( - source_path - .as_ref() - .unwrap() - .join("language/diem-framework/staged/stdlib.mv"), - ); + if let Some(source) = source_path { + default_config.workspace.source_path = Some(source.to_path_buf()); + default_config.workspace.stdlib_bin_path = + Some(source.join("language/diem-framework/staged/stdlib.mv")); } // override from args @@ -283,7 +272,7 @@ impl AppCfg { /// save the config file to 0L.toml to the workspace home path pub fn save_file(&self) -> Result<(), Error> { let toml = toml::to_string(&self)?; - let home_path = &self.workspace.node_home.clone(); + let home_path = &self.workspace.node_home.to_path_buf(); // create home path if doesn't exist, usually only in dev/ci environments. fs::create_dir_all(&home_path)?; let toml_path = home_path.join(CONFIG_FILE); @@ -394,7 +383,6 @@ pub struct Profile { // /// Node URL and and port to submit transactions. Defaults to localhost:8080 // pub default_node: Option, - /// Other nodes to connect for fallback connections pub upstream_nodes: Vec, @@ -448,25 +436,21 @@ pub struct TxConfigs { /// Miner transactions cost #[serde(default = "default_miner_txs_cost")] pub miner_txs_cost: Option, - /// Cheap or test transation costs + /// Cheap or test transaction costs #[serde(default = "default_cheap_txs_cost")] pub cheap_txs_cost: Option, } impl TxConfigs { /// get the user txs cost preferences for given transaction type - pub fn get_cost(&self, tx_type: TxType) -> TxCost { - let ref baseline = self.baseline_cost.clone(); - let cost = match tx_type { - TxType::Critical => self.critical_txs_cost.as_ref().unwrap_or_else(|| baseline), - TxType::Mgmt => self - .management_txs_cost - .as_ref() - .unwrap_or_else(|| baseline), - TxType::Miner => self.miner_txs_cost.as_ref().unwrap_or_else(|| baseline), - TxType::Cheap => self.cheap_txs_cost.as_ref().unwrap_or_else(|| baseline), - }; - cost.to_owned() + pub fn get_cost(&self, tx_type: TxType) -> &TxCost { + let baseline = &self.baseline_cost; + match tx_type { + TxType::Critical => self.critical_txs_cost.as_ref().unwrap_or(baseline), + TxType::Mgmt => self.management_txs_cost.as_ref().unwrap_or(baseline), + TxType::Miner => self.miner_txs_cost.as_ref().unwrap_or(baseline), + TxType::Cheap => self.cheap_txs_cost.as_ref().unwrap_or(baseline), + } } } @@ -581,14 +565,11 @@ pub fn bootstrap_waypoint_from_rpc(url: Url) -> Result { // let json: WaypointRpc = serde_json::from_value(resp.json().unwrap()).unwrap(); let parsed: serde_json::Value = resp.json()?; - match &parsed["result"] { - serde_json::Value::Object(r) => { - if let serde_json::Value::String(waypoint) = &r["waypoint"] { - let w: Waypoint = waypoint.parse()?; - return Ok(w); - } + if let serde_json::Value::Object(r) = &parsed["result"] { + if let serde_json::Value::String(waypoint) = &r["waypoint"] { + let w: Waypoint = waypoint.parse()?; + return Ok(w); } - _ => {} } bail!("could not get waypoint from json-rpc, url: {:?} ", url) } diff --git a/ol/types/src/legacy_recovery.rs b/ol/types/src/legacy_recovery.rs index 56b8cfece5..0b9070df7e 100644 --- a/ol/types/src/legacy_recovery.rs +++ b/ol/types/src/legacy_recovery.rs @@ -1,5 +1,9 @@ //! recovery +use crate::{ + autopay::AutoPayResource, fullnode_counter::FullnodeCounterResource, + wallet::CommunityWalletsResource, +}; use anyhow::{bail, Error}; use diem_types::{ account_address::AccountAddress, @@ -13,10 +17,6 @@ use diem_types::{ validator_config::{ValidatorConfigResource, ValidatorOperatorConfigResource}, }; use move_core_types::{identifier::Identifier, move_resource::MoveResource}; -use crate::{ - autopay::AutoPayResource, fullnode_counter::FullnodeCounterResource, - wallet::CommunityWalletsResource, -}; use serde::{Deserialize, Serialize}; use std::{convert::TryFrom, fs, io::Write, path::PathBuf}; // use vm_genesis::{OperRecover, ValStateRecover}; @@ -112,7 +112,6 @@ pub struct RecoverConsensusAccounts { pub opers: Vec, } - impl Default for RecoverConsensusAccounts { fn default() -> Self { RecoverConsensusAccounts { @@ -236,65 +235,48 @@ pub fn recover_validator_configs( for i in recover { let account: AccountAddress = i.account.unwrap(); // get deduplicated validators info - match i.role { - Validator => { - let val_cfg = i - .val_cfg - .as_ref() - .unwrap() - .validator_config - .as_ref() - .unwrap() - .clone(); + if i.role == Validator { + let val_cfg = i + .val_cfg + .as_ref() + .unwrap() + .validator_config + .as_ref() + .unwrap(); + + let operator_delegated_account = i.val_cfg.as_ref().unwrap().delegated_account.unwrap(); + // prevent duplicate accounts + if !set.vals.iter().any(|a| a.val_account == account) { + set.vals.push(ValStateRecover { + val_account: account, + operator_delegated_account, + val_auth_key: i.auth_key.unwrap(), + }); + } + + // find the operator's authkey + let oper_data = recover + .iter() + .find(|&a| a.account == Some(operator_delegated_account) && a.role == Operator); - let operator_delegated_account = - i.val_cfg.as_ref().unwrap().delegated_account.unwrap(); - // prevent duplicate accounts - if set - .vals + if let Some(o) = oper_data { + // get the operator info, preventing duplicates + if !set + .opers .iter() - .find(|&a| a.val_account == account) - .is_none() + .any(|a| a.operator_account == operator_delegated_account) { - set.vals.push(ValStateRecover { - val_account: account, - operator_delegated_account, - val_auth_key: i.auth_key.unwrap(), + set.opers.push(OperRecover { + operator_account: o.account.unwrap(), + operator_auth_key: o.auth_key.unwrap(), + validator_to_represent: account, + // TODO: Check conversion of public key + operator_consensus_pubkey: val_cfg.consensus_public_key.to_bytes().to_vec(), + validator_network_addresses: val_cfg.validator_network_addresses.clone(), + fullnode_network_addresses: val_cfg.fullnode_network_addresses.clone(), }); } - - // find the operator's authkey - let oper_data = recover - .iter() - .find(|&a| a.account == Some(operator_delegated_account) && a.role == Operator); - - match oper_data { - Some(o) => { - // get the operator info, preventing duplicates - if set - .opers - .iter() - .find(|&a| a.operator_account == operator_delegated_account) - .is_none() - { - set.opers.push(OperRecover { - operator_account: o.account.unwrap(), - operator_auth_key: o.auth_key.unwrap(), - validator_to_represent: account, - // TODO: Check conversion of public key - operator_consensus_pubkey: val_cfg - .consensus_public_key - .to_bytes() - .to_vec(), - validator_network_addresses: val_cfg.validator_network_addresses, - fullnode_network_addresses: val_cfg.fullnode_network_addresses, - }); - } - } - None => {} - } } - _ => {} } } Ok(set) @@ -320,19 +302,17 @@ pub fn read_from_recovery_file(path: &PathBuf) -> Vec { fn maybe_migrate_fn_address(resource: &mut ValidatorConfigResource) -> &ValidatorConfigResource { let cfg = resource.validator_config.as_ref().unwrap(); match cfg.fullnode_network_addresses() { - Ok(_) => {} // well formed network address, no-op. + Ok(_) => {} // well-formed network address, no-op. Err(_) => { // parse as a single address instead of a vector of addresses - match bcs::from_bytes::(&cfg.fullnode_network_addresses) { - Ok(net) => { - // fix the problematic address. - let mut new_config = cfg.clone(); - // change into vector of addresses. - new_config.fullnode_network_addresses = bcs::to_bytes(&vec![net]).unwrap(); - // Need to wrap in two Options. - resource.validator_config = Some(new_config.to_owned()); + if let Ok(net) = bcs::from_bytes::(&cfg.fullnode_network_addresses) { + // fix the problematic address. + // change into vector of addresses. + let new_addresses = bcs::to_bytes(&vec![net]).unwrap(); + // Only update the fullnode_network_addresses field. + if let Some(ref mut validator_config) = resource.validator_config { + validator_config.fullnode_network_addresses = new_addresses; } - Err(_) => {} } } }; diff --git a/ol/types/src/pay_instruction.rs b/ol/types/src/pay_instruction.rs index 9c6338ab22..e49325a929 100644 --- a/ol/types/src/pay_instruction.rs +++ b/ol/types/src/pay_instruction.rs @@ -7,7 +7,13 @@ use diem_types::{ }; use serde::{Deserialize, Serialize}; use serde_json::Value; -use std::{fs::{self, File}, io::Write, path::PathBuf, process::exit, u64}; +use std::{ + fs::{self, File}, + io::Write, + path::PathBuf, + process::exit, + u64, +}; #[cfg(test)] use crate::fixtures; @@ -185,55 +191,54 @@ impl PayInstruction { /// provide text information on the instruction pub fn text_instruction(&self) -> String { - let times = match &self.duration_epochs { - Some(d) => format!("{} times", d), - None => "".to_owned() - }; - match self.type_of { - - InstructionType::PercentOfBalance => { - format!( - "Instruction {uid}: {note}\nSends {percent_balance:.2?}% of total balance every day {times} (until epoch {epoch_ending}) to address: {destination}?", - uid = &self.uid.unwrap(), - percent_balance = *&self.value_move.unwrap() as f64 /100f64, - times = times, - note = &self.note.clone().unwrap(), - epoch_ending = &self.end_epoch.unwrap(), - destination = &self.destination, - ) - }, - InstructionType::PercentOfChange => { - format!( - "Instruction {uid}: {note}\nSends {percent_balance:.2?}% of new incoming funds every day {times} (until epoch {epoch_ending}) to address: {destination}?", - uid = &self.uid.unwrap(), - percent_balance = *&self.value_move.unwrap() as f64 /100f64, - times = times, - note = &self.note.clone().unwrap(), - epoch_ending = &self.end_epoch.unwrap(), - destination = &self.destination, - ) - }, - InstructionType::FixedRecurring => { - format!( - "Instruction {uid}: {note}\nSend {total_val} every day {times} (until epoch {epoch_ending}) to address: {destination}?", - uid = &self.uid.unwrap(), - total_val = *&self.value_move.unwrap() / 1_000_000, // scaling factor - times = times, - note = &self.note.clone().unwrap(), - epoch_ending = &self.end_epoch.unwrap(), - destination = &self.destination, - ) - }, - InstructionType::FixedOnce => { - format!( - "Instruction {uid}: {note}\nSend {total_val} once to address: {destination}?", - uid = &self.uid.unwrap(), - note = &self.note.clone().unwrap(), - total_val = *&self.value_move.unwrap() / 1_000_000, // scaling factor - destination = &self.destination, - ) + let times = match &self.duration_epochs { + Some(d) => format!("{} times", d), + None => "".to_owned(), + }; + match self.type_of { + InstructionType::PercentOfBalance => { + format!( + "Instruction {uid}: {note}\nSends {percent_balance:.2?}% of total balance every day {times} (until epoch {epoch_ending}) to address: {destination}?", + uid = &self.uid.unwrap(), + percent_balance = self.value_move.unwrap() as f64 / 100f64, + times = times, + note = self.note.as_ref().unwrap(), + epoch_ending = &self.end_epoch.unwrap(), + destination = &self.destination, + ) + } + InstructionType::PercentOfChange => { + format!( + "Instruction {uid}: {note}\nSends {percent_balance:.2?}% of new incoming funds every day {times} (until epoch {epoch_ending}) to address: {destination}?", + uid = &self.uid.unwrap(), + percent_balance = self.value_move.unwrap() as f64 / 100f64, + times = times, + note = self.note.as_ref().unwrap(), + epoch_ending = &self.end_epoch.unwrap(), + destination = &self.destination, + ) + } + InstructionType::FixedRecurring => { + format!( + "Instruction {uid}: {note}\nSend {total_val} every day {times} (until epoch {epoch_ending}) to address: {destination}?", + uid = &self.uid.unwrap(), + total_val = self.value_move.unwrap() / 1_000_000, // scaling factor + times = times, + note = self.note.as_ref().unwrap(), + epoch_ending = &self.end_epoch.unwrap(), + destination = &self.destination, + ) + } + InstructionType::FixedOnce => { + format!( + "Instruction {uid}: {note}\nSend {total_val} once to address: {destination}?", + uid = &self.uid.unwrap(), + note = self.note.as_ref().unwrap(), + total_val = self.value_move.unwrap() / 1_000_000, // scaling factor + destination = &self.destination, + ) + } } - } } }