refac(pallets/subspace): making code more robust

pallets/subspace/src/global.rs

@@ -31,8 +31,7 @@ impl<T: Config> Pallet<T> {
         }
     }
 
-    // TODO: make sure there are checks for all values
-    pub fn check_global_params(params: GlobalParams) -> DispatchResult {
+    pub fn check_global_params(params: &GlobalParams) -> DispatchResult {
         // checks if params are valid
         let old_params = Self::global_params();
 
@@ -95,10 +94,23 @@ impl<T: Config> Pallet<T> {
             Error::<T>::InvalidMaxBurn
         );
 
+        ensure!(
+            params.target_registrations_per_interval > 0,
+            Error::<T>::InvalidTargetRegistrationsPerInterval
+        );
+
+        ensure!(
+            params.max_allowed_weights > 0,
+            Error::<T>::InvalidMaxAllowedWeights
+        );
+
         Ok(())
     }
 
     pub fn set_global_params(params: GlobalParams) {
+        // Check if the params are valid
+        Self::check_global_params(&params).expect("global params are invalid");
+
         Self::set_global_max_name_length(params.max_name_length);
         Self::set_global_max_allowed_subnets(params.max_allowed_subnets);
         Self::set_max_allowed_modules(params.max_allowed_modules);
@@ -165,7 +177,7 @@ impl<T: Config> Pallet<T> {
         VoteModeGlobal::<T>::get()
     }
     pub fn get_burn_rate() -> u16 {
-        BurnRate::<T>::get().min(100)
+        BurnRate::<T>::get()
     }
 
     pub fn get_burn() -> u64 {

pallets/subspace/src/lib.rs

@@ -24,12 +24,6 @@ use sp_runtime::{
 };
 use sp_std::marker::PhantomData;
 
-// ============================
-//	==== Benchmark Imports =====
-// ============================
-#[cfg(feature = "runtime-benchmarks")]
-mod benchmarking;
-
 pub mod autogen_weights;
 pub use autogen_weights::WeightInfo;
 
@@ -50,6 +44,8 @@ mod subnet;
 mod voting;
 mod weights;
 
+// TODO: better error handling in whole file
+
 #[frame_support::pallet]
 pub mod pallet {
     #![allow(
@@ -585,28 +581,6 @@ pub mod pallet {
     pub(super) type Uids<T: Config> =
         StorageDoubleMap<_, Identity, u16, Blake2_128Concat, T::AccountId, u16, OptionQuery>;
 
-    #[pallet::storage] // --- DMAP ( netuid, module_key ) --> uid
-    pub(super) type Key2Controller<T: Config> = StorageDoubleMap<
-        _,
-        Identity,
-        T::AccountId,
-        Blake2_128Concat,
-        T::AccountId,
-        u16,
-        OptionQuery,
-    >;
-
-    #[pallet::storage] // --- DMAP ( netuid, module_key ) --> uid
-    pub(super) type Controller2Keys<T: Config> = StorageDoubleMap<
-        _,
-        Identity,
-        T::AccountId,
-        Blake2_128Concat,
-        Vec<T::AccountId>,
-        u16,
-        OptionQuery,
-    >;
-
     #[pallet::type_value]
     pub fn DefaultKey<T: Config>() -> T::AccountId {
         T::AccountId::decode(&mut sp_runtime::traits::TrailingZeroInput::zeroes()).unwrap()
@@ -712,16 +686,6 @@ pub mod pallet {
     #[pallet::storage] // --- MAP ( netuid ) --> subnet_total_stake
     pub type TotalStake<T> = StorageMap<_, Identity, u16, u64, ValueQuery>;
 
-    // LOAN VARIABLES
-
-    #[pallet::storage] // --- DMAP ( netuid, module_key ) --> Vec<(delegater, stake )> | Returns the list of delegates
-    pub type LoanTo<T: Config> =
-        StorageMap<_, Identity, T::AccountId, Vec<(T::AccountId, u64)>, ValueQuery>;
-
-    #[pallet::storage] // --- DMAP ( netuid, module_key ) --> Vec<(delegater, stake )> | Returns the list of delegates
-    pub type LoanFrom<T: Config> =
-        StorageMap<_, Identity, T::AccountId, Vec<(T::AccountId, u64)>, ValueQuery>;
-
     // PROFIT SHARE VARIABLES
 
     #[pallet::type_value]
@@ -782,6 +746,50 @@ pub mod pallet {
         DefaultWeights<T>,
     >;
 
+    // ========================================================
+    // ==== Voting System to Update Global and Subnet  ====
+    // ========================================================
+    #[derive(Decode, Encode, PartialEq, Eq, Clone, Debug, TypeInfo)]
+    #[scale_info(skip_type_params(T))]
+    pub struct Proposal<T: Config> {
+        // --- parameters
+        pub subnet_params: SubnetParams<T>,
+        pub global_params: GlobalParams,
+        pub netuid: u16, // FOR SUBNET PROPOSAL ONLY
+        pub votes: u64,
+        pub participants: Vec<T::AccountId>,
+        pub accepted: bool,
+        pub data: Vec<u8>, // for custom proposal
+        pub mode: Vec<u8>, // "global", "subnet", "custom"
+    }
+
+    #[pallet::type_value]
+    pub fn DefaultProposal<T: Config>() -> Proposal<T> {
+        Proposal {
+            global_params: DefaultGlobalParams::<T>::get(),
+            subnet_params: DefaultSubnetParams::<T>::get(),
+            votes: 0,
+            netuid: 0,
+            participants: vec![],
+            accepted: false,
+            mode: vec![],
+            data: vec![],
+        }
+    }
+
+    #[pallet::storage] // --- MAP ( global_proposal_id ) --> global_update_proposal
+    pub(super) type Proposals<T: Config> =
+        StorageMap<_, Identity, u64, Proposal<T>, ValueQuery, DefaultProposal<T>>;
+
+    #[pallet::type_value]
+    pub fn DefaultMaxProposals<T: Config>() -> u64 {
+        128
+    }
+
+    #[pallet::storage]
+    pub(super) type MaxProposals<T: Config> =
+        StorageValue<_, u64, ValueQuery, DefaultMaxProposals<T>>;
+
     #[pallet::event]
     #[pallet::generate_deposit(pub(super) fn deposit_event)]
     pub enum Event<T: Config> {
@@ -856,6 +864,8 @@ pub mod pallet {
                         * uids in the weight matrix. */
         InvalidUid, /* ---- Thrown when a caller attempts to set weight to at least one uid
                      * that does not exist in the metagraph. */
+        InvalidUidsLength, /* ---- Thrown when the caller attempts to set weights with a
+                            * different number of uids than allowed. */
         NotSettingEnoughWeights, /* ---- Thrown when the dispatch attempts to set weights on
                                   * chain with fewer elements than are allowed. */
         TooManyRegistrationsPerBlock, /* ---- Thrown when registrations this block exceeds
@@ -891,14 +901,14 @@ pub mod pallet {
         KeyAlreadyRegistered, //
         ModuleNameDoesNotExist, /* --- Thrown when the user tries to remove a module name that
                                * does not exist. */
-        KeyNameMismatch,
+        EmptyKeys,
+        InvalidShares,
+        ProfitSharesNotAdded,
         NotFounder,
         NameAlreadyRegistered,
-        NotEnoughStaketoSetWeights,
+        NotEnoughStakeToSetWeights,
         NotEnoughStakeToStartNetwork,
-        NetworkRegistrationFailed,
-        NetworkAlreadyRegistered,
-        NotEnoughtStakePerWeight,
+        NotEnoughStakePerWeight,
         NoSelfWeight,
         DifferentLengths,
         NotEnoughBalanceToRegister,
@@ -908,11 +918,6 @@ pub mod pallet {
         StillRegistered,
         MaxAllowedModules, /* --- Thrown when the user tries to set max allowed modules to a
                             * value less than the current number of registered modules. */
-        TooMuchUpdateProposals,
-        InvalidProposalId,
-        UpdateProposalAlreadyVoted,
-        UpdateProposalVoteNotAvailable,
-        NotEnoughVotesToAccept,
         NotEnoughBalanceToTransfer,
         NotAuthorityMode,
         InvalidTrustRatio,
@@ -921,7 +926,6 @@ pub mod pallet {
         InvalidMinStake,
         InvalidMinDelegationFee,
 
-        InvalidGlobalParams,
         InvalidMaxNameLength,
         InvalidMaxAllowedSubnets,
         InvalidMaxAllowedModules,
@@ -934,6 +938,7 @@ pub mod pallet {
         InvalidBurnRate,
         InvalidMinBurn,
         InvalidMaxBurn,
+        InvalidTargetRegistrationsPerInterval,
 
         // VOTING
         ProposalDoesNotExist,
@@ -943,11 +948,14 @@ pub mod pallet {
         VoterIsNotRegistered,
         VoterIsRegistered,
         InvalidVoteMode,
+        InvalidImmunityPeriod,
+        InvalidFounderShare,
+        InvalidIncentiveRatio,
         InvalidMaxWeightAge,
         InvalidMaxStake,
 
-        AlreadyControlled,
-        AlreadyController,
+        // OTHER
+        ArithmeticError,
     }
 
     // ==================
@@ -1023,65 +1031,12 @@ pub mod pallet {
         }
     }
 
-    // ========================================================
-    // ==== Voting System to Update Global and Subnet  ====
-    // ========================================================
-    #[derive(Decode, Encode, PartialEq, Eq, Clone, Debug, TypeInfo)]
-    #[scale_info(skip_type_params(T))]
-    pub struct Proposal<T: Config> {
-        // --- parameters
-        pub subnet_params: SubnetParams<T>,
-        pub global_params: GlobalParams,
-        pub netuid: u16, // FOR SUBNET PROPOSAL ONLY
-        pub votes: u64,
-        pub participants: Vec<T::AccountId>,
-        pub accepted: bool,
-        pub data: Vec<u8>, // for custom proposal
-        pub mode: Vec<u8>, // "global", "subnet", "custom"
-    }
-
-    #[pallet::type_value]
-    pub fn DefaultProposal<T: Config>() -> Proposal<T> {
-        Proposal {
-            global_params: DefaultGlobalParams::<T>::get(),
-            subnet_params: DefaultSubnetParams::<T>::get(),
-            votes: 0,
-            netuid: 0,
-            participants: vec![],
-            accepted: false,
-            mode: vec![],
-            data: vec![],
-        }
-    }
-
-    #[pallet::storage] // --- MAP ( global_proposal_id ) --> global_update_proposal
-    pub(super) type Proposals<T: Config> =
-        StorageMap<_, Identity, u64, Proposal<T>, ValueQuery, DefaultProposal<T>>;
-
-    #[pallet::type_value]
-    pub fn DefaultMaxProposals<T: Config>() -> u64 {
-        128
-    }
-    #[pallet::storage]
-    pub(super) type MaxProposals<T: Config> =
-        StorageValue<_, u64, ValueQuery, DefaultMaxProposals<T>>;
-
     // ================
     // ==== Hooks =====
     // ================
 
     #[pallet::hooks]
     impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
-        // ---- Called on the initialization of this pallet. (the order of on_finalize calls is
-        // determined in the runtime)
-        //
-        // # Args:
-        // 	* 'n': (T::BlockNumber):
-        // 		- The number of the block we are initializing.
-        // fn on_runtime_upgrade() -> frame_support::weights::Weight {
-        // 	migration::on_runtime_upgrade::<T>()
-        // }
-
         fn on_initialize(_block_number: BlockNumberFor<T>) -> Weight {
             Self::block_step();
 
@@ -1258,7 +1213,7 @@ pub mod pallet {
             params.target_registrations_interval = target_registrations_interval;
 
             // Check if the parameters are valid
-            Self::check_global_params(params.clone())?;
+            Self::check_global_params(&params)?;
 
             // if so update them
             Self::do_update_global(origin, params)
@@ -1340,7 +1295,7 @@ pub mod pallet {
             params.vote_threshold = vote_threshold;
 
             // Check if subnet parameters are valid
-            Self::check_subnet_params(params.clone())?;
+            Self::check_subnet_params(&params)?;
 
             // if so update them
             Self::do_update_subnet(origin, netuid, params)
@@ -1408,7 +1363,7 @@ pub mod pallet {
         pub fn get_priority_set_weights(key: &T::AccountId, netuid: u16) -> u64 {
             if Uids::<T>::contains_key(netuid, key) {
                 let uid: u16 = Self::get_uid_for_key(netuid, &key.clone());
-                let current_block_number: u64 = Self::get_current_block_as_u64();
+                let current_block_number: u64 = Self::get_current_block_number();
                 return current_block_number - Self::get_last_update_for_uid(netuid, uid);
             }
             0
@@ -1474,7 +1429,7 @@ where
         // Return high priority so that every extrinsic except set_weights function will
         // have a higher priority than the set_weights call
         // get the current block number
-        let current_block_number: u64 = Pallet::<T>::get_current_block_as_u64();
+        let current_block_number: u64 = Pallet::<T>::get_current_block_number();
         let balance = Pallet::<T>::get_balance_u64(who);
 
         // this is the current block number minus the last update block number

pallets/subspace/src/math.rs

@@ -45,7 +45,7 @@ pub fn mask_diag_sparse(sparse_matrix: &[Vec<(u16, I32F32)>]) -> Vec<Vec<(u16, I
 pub fn inplace_row_normalize_sparse(sparse_matrix: &mut [Vec<(u16, I32F32)>]) {
     for sparse_row in sparse_matrix.iter_mut() {
         let row_sum: I32F32 = sparse_row.iter().map(|(_j, value)| *value).sum();
-        if row_sum > I32F32::from_num(0.0) {
+        if row_sum != I32F32::from_num(0) {
             sparse_row.iter_mut().for_each(|(_j, value)| *value /= row_sum);
         }
     }