From 17af07a1751ed6159eac428e80b264231ec12e90 Mon Sep 17 00:00:00 2001 From: Luiz Carvalho Date: Wed, 28 Feb 2024 21:07:11 -0300 Subject: [PATCH] refac: remove string utils (#20) Co-authored-by: My Name --- pallets/subspace/src/benchmarking.rs | 10 +++--- pallets/subspace/src/global.rs | 7 ++-- pallets/subspace/src/lib.rs | 4 +-- pallets/subspace/src/module.rs | 54 +++++++++++++--------------- pallets/subspace/src/profit_share.rs | 9 ++--- pallets/subspace/src/registration.rs | 34 ++++++------------ pallets/subspace/src/subnet.rs | 45 +++++++---------------- pallets/subspace/src/utils.rs | 26 -------------- pallets/subspace/src/voting.rs | 40 +++++++++++---------- 9 files changed, 84 insertions(+), 145 deletions(-) delete mode 100644 pallets/subspace/src/utils.rs diff --git a/pallets/subspace/src/benchmarking.rs b/pallets/subspace/src/benchmarking.rs index 04ff5ed4d..66b8957e4 100644 --- a/pallets/subspace/src/benchmarking.rs +++ b/pallets/subspace/src/benchmarking.rs @@ -41,7 +41,7 @@ fn register_helper( module_key.clone(), ); - let netuid = >::get_netuid_for_name(network.clone()); + let netuid = >::get_netuid_for_name(network.clone()).unwrap_or(u16::MAX); netuid } @@ -118,7 +118,7 @@ mod benchmarks { module_key.clone(), ); - let netuid = >::get_netuid_for_name(network); + let netuid = >::get_netuid_for_name(network).unwrap_or(u16::MAX); assert!(>::is_registered(netuid, &module_key), "Register failed"); @@ -425,7 +425,7 @@ mod benchmarks { MIN_STAKE, ); - let netuid = >::get_netuid_for_name(network.clone()); + let netuid = >::get_netuid_for_name(network.clone()).unwrap_or(u16::MAX); #[extrinsic_call] add_subnet_update(RawOrigin::Root, netuid, name, 1, 1, 1, 1, 1, 1, 1, 1, 1); @@ -448,7 +448,7 @@ mod benchmarks { MIN_STAKE, ); - let netuid = >::get_netuid_for_name(network.clone()); + let netuid = >::get_netuid_for_name(network.clone()).unwrap_or(u16::MAX); >::add_subnet_update( RawOrigin::Root.into(), @@ -486,7 +486,7 @@ mod benchmarks { MIN_STAKE, ); - let netuid = >::get_netuid_for_name(network.clone()); + let netuid = >::get_netuid_for_name(network.clone()).unwrap_or(u16::MAX); >::add_subnet_update( RawOrigin::Root.into(), diff --git a/pallets/subspace/src/global.rs b/pallets/subspace/src/global.rs index c1ee60046..ac9ea61b1 100644 --- a/pallets/subspace/src/global.rs +++ b/pallets/subspace/src/global.rs @@ -1,5 +1,6 @@ +use crate::voting::AUTHORITY_MODE; + use super::*; -use crate::utils::is_vec_str; use frame_support::pallet_prelude::DispatchResult; use system::ensure_root; @@ -135,8 +136,8 @@ impl Pallet { pub fn do_update_global(origin: T::RuntimeOrigin, params: GlobalParams) -> DispatchResult { ensure_root(origin)?; - ensure!(is_vec_str(Self::get_vote_mode_global(), "authority"), Error::::InvalidVoteMode); - Self::set_global_params(params.clone()); + ensure!(Self::get_vote_mode_global() == AUTHORITY_MODE, Error::::InvalidVoteMode); + Self::set_global_params(params); Ok(()) } diff --git a/pallets/subspace/src/lib.rs b/pallets/subspace/src/lib.rs index c43a64579..b48d1f668 100644 --- a/pallets/subspace/src/lib.rs +++ b/pallets/subspace/src/lib.rs @@ -1,6 +1,7 @@ #![allow(deprecated, non_camel_case_types, non_snake_case)] #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "512"] + use frame_system::{self as system, ensure_signed}; pub use pallet::*; @@ -43,7 +44,6 @@ mod registration; mod staking; mod step; mod subnet; -mod utils; mod voting; mod weights; @@ -910,7 +910,7 @@ pub mod pallet { max_weight_age: default_params.max_weight_age, }; - self::Pallet::::add_subnet(params.clone()); + self::Pallet::::add_subnet(params); for (uid_usize, (key, name, address, weights)) in self.modules[subnet_idx].iter().enumerate() diff --git a/pallets/subspace/src/module.rs b/pallets/subspace/src/module.rs index e3d6fe240..3a189f093 100644 --- a/pallets/subspace/src/module.rs +++ b/pallets/subspace/src/module.rs @@ -33,36 +33,32 @@ impl Pallet { // --- 1. We check the callers (key) signature. let key = ensure_signed(origin)?; let uid: u16 = Self::get_uid_for_key(netuid, &key); - Self::check_module_params(netuid, params.clone())?; + Self::check_module_params(netuid, ¶ms)?; Self::set_module_params(netuid, uid, params); // --- 8. Return is successful dispatch. Ok(()) } - pub fn check_module_params(_netuid: u16, params: ModuleParams) -> DispatchResult { - // if len(name) > 0, then we update the name. + /// Checks whether the module params are valid. Name and address must be non-empty and below the + /// max name length allowed. + pub fn check_module_params(_netuid: u16, params: &ModuleParams) -> DispatchResult { + let max_name_length = MaxNameLength::::get() as usize; + assert!(params.name.len() > 0); - ensure!( - params.name.len() <= MaxNameLength::::get() as usize, - Error::::ModuleNameTooLong - ); + ensure!(params.name.len() <= max_name_length, Error::::ModuleNameTooLong); assert!(params.address.len() > 0); - ensure!( - params.address.len() <= MaxNameLength::::get() as usize, - Error::::ModuleAddressTooLong - ); - // delegation fee is a percent + ensure!(params.address.len() <= max_name_length, Error::::ModuleAddressTooLong); + Ok(()) } pub fn module_params(netuid: u16, uid: u16) -> ModuleParams { - let module_params: ModuleParams = ModuleParams { + ModuleParams { name: Self::get_module_name(netuid, uid), address: Self::get_module_address(netuid, uid), delegation_fee: Self::get_module_delegation_fee(netuid, uid), controller: Self::get_key_for_uid(netuid, uid), - }; - module_params + } } pub fn set_module_params(netuid: u16, uid: u16, module_params: ModuleParams) { @@ -172,7 +168,7 @@ impl Pallet { // HANDLE THE DELEGATION FEE DelegationFee::::insert( netuid, - replace_key.clone(), + replace_key, DelegationFee::::get(netuid, uid_key.clone()), ); // Make uid - key association. DelegationFee::::remove(netuid, uid_key.clone()); // Make uid - key association. @@ -241,25 +237,25 @@ impl Pallet { pub fn get_module_stats(netuid: u16, uid: u16) -> ModuleStats { let key = Self::get_key_for_uid(netuid, uid); - let emission = Self::get_emission_for_uid(netuid, uid as u16); - let incentive = Self::get_incentive_for_uid(netuid, uid as u16); - let dividends = Self::get_dividends_for_uid(netuid, uid as u16); - let last_update = Self::get_last_update_for_uid(netuid, uid as u16); - let _registration_block = Self::get_registration_block_for_uid(netuid, uid as u16); - let weights = >::get(netuid, uid) + let emission = Self::get_emission_for_uid(netuid, uid); + let incentive = Self::get_incentive_for_uid(netuid, uid); + let dividends = Self::get_dividends_for_uid(netuid, uid); + let last_update = Self::get_last_update_for_uid(netuid, uid); + + let weights: Vec<(u16, u16)> = Weights::::get(netuid, uid) .iter() .filter_map(|(i, w)| if *w > 0 { Some(((*i).into(), (*w).into())) } else { None }) - .collect::>(); + .collect(); let stake_from: Vec<(T::AccountId, u64)> = StakeFrom::::get(netuid, key); - let registration_block = Self::get_registration_block_for_uid(netuid, uid as u16); + let registration_block = Self::get_registration_block_for_uid(netuid, uid); ModuleStats { stake_from, - emission: emission.into(), - incentive: incentive.into(), - dividends: dividends.into(), - last_update: last_update.into(), - registration_block: registration_block.into(), + emission, + incentive, + dividends, + last_update, + registration_block, weights, } } diff --git a/pallets/subspace/src/profit_share.rs b/pallets/subspace/src/profit_share.rs index e787b6c80..49fad0dda 100644 --- a/pallets/subspace/src/profit_share.rs +++ b/pallets/subspace/src/profit_share.rs @@ -32,9 +32,9 @@ impl Pallet { // make sure the normalized shares add up to the unit // convert the normalized shares to u16 let mut normalize_shares: Vec = - normalized_shares_float.iter().map(|x| x.to_num::()).collect::>(); + normalized_shares_float.iter().map(|x| x.to_num::()).collect(); - let mut total_normalized_shares: u16 = normalize_shares.iter().sum::(); + let mut total_normalized_shares: u16 = normalize_shares.iter().sum(); // ensure the profit shares add up to the unit if total_normalized_shares < u16::MAX { @@ -53,9 +53,6 @@ impl Pallet { u16::MAX ); - // check tssat the normalized shares add up to the unit - let _total_normalized_shares: u16 = normalize_shares.iter().sum::(); - // now send the normalized shares to the profit share pallet let profit_share_tuples: Vec<(T::AccountId, u16)> = keys.iter().zip(normalize_shares.iter()).map(|(x, y)| (x.clone(), *y)).collect(); @@ -79,7 +76,7 @@ impl Pallet { for (share_key, share_ratio) in profit_shares.iter() { let share_emission_float: I96F32 = I96F32::from(emission) * (I96F32::from(*share_ratio) / I96F32::from(u16::MAX)); - let share_emission: u64 = share_emission_float.to_num::(); + let share_emission: u64 = share_emission_float.to_num(); emission_shares.push((share_key.clone(), share_emission)); } diff --git a/pallets/subspace/src/registration.rs b/pallets/subspace/src/registration.rs index 3832c6f92..0e3549f84 100644 --- a/pallets/subspace/src/registration.rs +++ b/pallets/subspace/src/registration.rs @@ -3,8 +3,6 @@ use super::*; use frame_support::pallet_prelude::DispatchResult; use frame_system::ensure_signed; -use sp_core::H256; - use sp_std::vec::Vec; // IterableStorageMap @@ -38,13 +36,14 @@ impl Pallet { ); // --- 4. Resolve the network in case it doesn't exist - if !Self::subnet_name_exists(network.clone()) { - // If the subnet doesn't exist, registration will create it. - Self::add_subnet_from_registration(network.clone(), stake_amount, &key)?; - } + let netuid = if let Some(netuid) = Self::get_netuid_for_name(&network) { + netuid + } else { + // Create subnet if it does not exist. + Self::add_subnet_from_registration(network, stake_amount, &key)? + }; // --- 5. Ensure the caller has enough stake to register. - let netuid: u16 = Self::get_netuid_for_name(network.clone()); let min_stake: u64 = MinStake::::get(netuid); let min_burn: u64 = Self::get_min_burn(); @@ -66,7 +65,7 @@ impl Pallet { let uid: u16 = Self::append_module(netuid, &module_key, name.clone(), address.clone()); // --- 9. Add the stake to the module, now that it is registered on the network. - Self::do_add_stake(origin.clone(), netuid, module_key.clone(), stake_amount)?; + Self::do_add_stake(origin, netuid, module_key.clone(), stake_amount)?; // constant -> min_burn logic if min_burn > 0 { @@ -100,23 +99,16 @@ impl Pallet { Ok(()) } + /// Whether the netuid has enough stake to cover the minimal stake and min burn pub fn enough_stake_to_register( _netuid: u16, min_stake: u64, min_burn: u64, stake_amount: u64, ) -> bool { - // the amount has to cover, the minimal stake as well as burn if it's present stake_amount >= (min_stake + min_burn) } - pub fn vec_to_hash(vec_hash: Vec) -> H256 { - let de_ref_hash = &vec_hash; // b: &Vec - let de_de_ref_hash: &[u8] = &de_ref_hash; // c: &[u8] - let real_hash: H256 = H256::from_slice(de_de_ref_hash); - real_hash - } - // Determine which peer to prune from the network by finding the element with the lowest pruning // score out of immunity period. If all modules are in immunity period, return node with lowest // prunning score. This function will always return an element to prune. @@ -165,10 +157,7 @@ impl Pallet { name: Vec, stake: u64, founder_key: &T::AccountId, - ) -> DispatchResult { - // use default parameters - // - + ) -> Result { let num_subnets: u16 = Self::num_subnets(); let max_subnets: u16 = Self::get_global_max_allowed_subnets(); // if we have not reached the max number of subnets, then we can start a new one @@ -187,11 +176,10 @@ impl Pallet { // if we have reached the max number of subnets, then we can start a new one if the stake is // greater than the least staked network let mut params: SubnetParams = Self::default_subnet_params(); - params.name = name.clone(); + params.name = name; params.founder = founder_key.clone(); - let _netuid = Self::add_subnet(params); - Ok(()) + Ok(Self::add_subnet(params)) } pub fn check_module_limits(netuid: u16) { diff --git a/pallets/subspace/src/subnet.rs b/pallets/subspace/src/subnet.rs index a6cd87a2e..1e69d28f2 100644 --- a/pallets/subspace/src/subnet.rs +++ b/pallets/subspace/src/subnet.rs @@ -1,11 +1,12 @@ +use crate::voting::{AUTHORITY_MODE, STAKE_MODE}; + use super::*; -use crate::utils::is_vec_str; use frame_support::{ pallet_prelude::DispatchResult, storage::IterableStorageMap, IterableStorageDoubleMap, }; -pub use sp_std::vec::Vec; +use sp_std::vec::Vec; use substrate_fixed::types::I64F64; extern crate alloc; @@ -33,10 +34,7 @@ impl Pallet { let key = ensure_signed(origin)?; // only the founder can update the network on authority mode - ensure!( - is_vec_str(Self::get_vote_mode_subnet(netuid), "authority"), - Error::::NotAuthorityMode - ); + ensure!(Self::get_vote_mode_subnet(netuid) == AUTHORITY_MODE, Error::::NotAuthorityMode); ensure!(Self::if_subnet_netuid_exists(netuid), Error::::SubnetNameAlreadyExists); ensure!(Self::is_subnet_founder(netuid, &key), Error::::NotFounder); ensure!(Self::if_subnet_netuid_exists(netuid), Error::::SubnetNameAlreadyExists); @@ -87,8 +85,7 @@ impl Pallet { // ensure the vode_mode is in "authority", "stake" ensure!( - is_vec_str(params.vote_mode.clone(), "authority") || - is_vec_str(params.vote_mode.clone(), "stake"), + params.vote_mode.clone() == AUTHORITY_MODE || params.vote_mode.clone() == STAKE_MODE, Error::::InvalidVoteMode ); Ok(()) @@ -284,16 +281,6 @@ impl Pallet { SelfVote::::get(netuid) } - // pub fn total_balance() -> u64 { - // let mut total_balance: u64 = 0; - // // iterate through all of the accounts with balance (noo stake) - - // for ( key, stated_amount ) in as IterableStorageDoubleMap >::iter(){ total_balance = Self::get_balance_u64( &key ) + total_balance; - // } - // return total_balance; - // } - pub fn market_cap() -> u64 { let total_stake: u64 = Self::total_stake(); total_stake @@ -397,17 +384,8 @@ impl Pallet { SubnetNames::::contains_key(netuid).into() } - pub fn get_netuid_for_name(name: Vec) -> u16 { - for (netuid, netname) in as IterableStorageMap>>::iter() { - if name == netname { - return netuid - } - } - u16::MAX - } - - pub fn get_subnet_name(netuid: u16) -> Vec { - SubnetNames::::get(netuid) + pub fn get_netuid_for_name(name: &[u8]) -> Option { + SubnetNames::::iter().find(|(_, n)| n == name).map(|(id, _)| id) } pub fn is_network_founder(netuid: u16, key: &T::AccountId) -> bool { @@ -417,8 +395,12 @@ impl Pallet { } pub fn remote_subnet_for_name(name: Vec) -> u16 { - let netuid = Self::get_netuid_for_name(name.clone()); - Self::remove_subnet(netuid) + if let Some(netuid) = Self::get_netuid_for_name(&name) { + Self::remove_subnet(netuid); + netuid + } else { + u16::MAX + } } pub fn remove_netuid_stake_strorage(netuid: u16) { @@ -441,7 +423,6 @@ impl Pallet { if !Self::if_subnet_exist(netuid) { return 0 } - let _name: Vec = Self::get_subnet_name(netuid); Self::remove_netuid_stake_strorage(netuid); diff --git a/pallets/subspace/src/utils.rs b/pallets/subspace/src/utils.rs deleted file mode 100644 index 93a521806..000000000 --- a/pallets/subspace/src/utils.rs +++ /dev/null @@ -1,26 +0,0 @@ -use sp_std::vec::Vec; - -#[allow(dead_code)] -fn string2vec(s: &str) -> Vec { - let mut v: Vec = Vec::new(); - for c in s.chars() { - v.push(c as u8); - } - v -} -#[allow(dead_code)] -pub fn is_string_equal(s1: &str, s2: &str) -> bool { - let v1: Vec = string2vec(s1); - let v2: Vec = string2vec(s2); - v1 == v2 -} -#[allow(dead_code)] -pub fn is_string_vec(s1: &str, v2: Vec) -> bool { - let v1: Vec = string2vec(s1); - v1 == v2.clone() -} -#[allow(dead_code)] -pub fn is_vec_str(v1: Vec, s2: &str) -> bool { - let v2: Vec = string2vec(s2); - v1 == v2.clone() -} diff --git a/pallets/subspace/src/voting.rs b/pallets/subspace/src/voting.rs index 0af0b977f..d4a1ee68c 100644 --- a/pallets/subspace/src/voting.rs +++ b/pallets/subspace/src/voting.rs @@ -1,7 +1,11 @@ use super::*; -use crate::utils::is_vec_str; use frame_support::pallet_prelude::DispatchResult; +pub const STAKE_MODE: &[u8] = b"stake"; +pub const GLOBAL_MODE: &[u8] = b"global"; +pub const SUBNET_MODE: &[u8] = b"subnet"; +pub const AUTHORITY_MODE: &[u8] = b"authority"; + impl Pallet { pub fn do_unregister_voter(origin: T::RuntimeOrigin) -> DispatchResult { let key = ensure_signed(origin)?; @@ -182,19 +186,17 @@ impl Pallet { let mode = proposal.mode.clone(); // check if proposal is valid - if is_vec_str(mode.clone(), "global") { - Self::check_global_params(proposal.global_params)?; - } else if is_vec_str(mode.clone(), "subnet") { - Self::check_subnet_params(proposal.subnet_params.clone())?; - // check if vote mode is valid - let subnet_params: SubnetParams = Self::subnet_params(proposal.netuid); - ensure!( - is_vec_str(subnet_params.vote_mode.clone(), "stake"), - Error::::InvalidVoteMode - ); - } else { - ensure!(proposal.data.len() > 0, Error::::InvalidProposalData); + match mode.as_slice() { + GLOBAL_MODE => Self::check_global_params(proposal.global_params)?, + SUBNET_MODE => { + Self::check_subnet_params(proposal.subnet_params.clone())?; + // check if vote mode is valid + let subnet_params: SubnetParams = Self::subnet_params(proposal.netuid); + ensure!(subnet_params.vote_mode == STAKE_MODE, Error::::InvalidVoteMode); + }, + _ => ensure!(proposal.data.len() > 0, Error::::InvalidProposalData), } + // check if proposal is valid ensure!(proposal.data.len() < 256, Error::::ProposalDataTooLarge); // avoid an exploit with large data, cap it at 256 bytes @@ -258,7 +260,7 @@ impl Pallet { } pub fn get_voting_power(key: &T::AccountId, proposal: Proposal) -> u64 { - if is_vec_str(proposal.mode.clone(), "subnet") { + if proposal.mode == SUBNET_MODE { Self::get_total_stake_to(proposal.netuid, key) } else { // get all of the stake for the key @@ -268,7 +270,7 @@ impl Pallet { pub fn get_proposal_vote_threshold(proposal_id: u64) -> u64 { let proposal: Proposal = Proposals::::get(proposal_id); - if is_vec_str(proposal.mode.clone(), "subnet") { + if proposal.mode == SUBNET_MODE { let total_stake = Self::get_total_subnet_stake(proposal.netuid); (total_stake * proposal.subnet_params.vote_threshold as u64) / 100 } else { @@ -291,10 +293,10 @@ impl Pallet { proposal.participants = Vec::new(); }); - if is_vec_str(proposal.mode.clone(), "subnet") { + if proposal.mode == SUBNET_MODE { Self::set_subnet_params(proposal.netuid, proposal.subnet_params); Self::deposit_event(Event::SubnetProposalAccepted(proposal_id, proposal.netuid)); - } else if is_vec_str(proposal.mode.clone(), "global") { + } else if proposal.mode == GLOBAL_MODE { Self::set_global_params(proposal.global_params); Self::deposit_event(Event::GlobalProposalAccepted(proposal_id)); } else { @@ -306,7 +308,7 @@ impl Pallet { pub fn get_subnet_proposals(netuid: u16) -> Vec> { let mut proposals: Vec> = Vec::new(); for proposal in Proposals::::iter_values() { - if is_vec_str(proposal.mode.clone(), "subnet") && proposal.netuid == netuid { + if proposal.mode == SUBNET_MODE && proposal.netuid == netuid { proposals.push(proposal); } } @@ -316,7 +318,7 @@ impl Pallet { pub fn get_global_proposals() -> Vec> { let mut proposals: Vec> = Vec::new(); for proposal in Proposals::::iter_values() { - if is_vec_str(proposal.mode.clone(), "global") { + if proposal.mode == GLOBAL_MODE { proposals.push(proposal); } }