Skip to content

Commit

Permalink
refac(pallets/subspace): making code more robust (#38)
Browse files Browse the repository at this point in the history
* improved test loggin

* Refac: fixing commune

* WIP: middle of major refactor, dangerous code

* Refac: making code more readable, cleaning up

* fmt

* Refac: applied PR suggestions

* applying requested changes

* refac: deleting unused variables, improving debug prints

* fix: clippy errors
  • Loading branch information
functor-flow authored Mar 18, 2024
1 parent 5530b2b commit b5e9e81
Show file tree
Hide file tree
Showing 20 changed files with 850 additions and 963 deletions.
18 changes: 15 additions & 3 deletions pallets/subspace/src/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
171 changes: 63 additions & 108 deletions pallets/subspace/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -50,6 +44,8 @@ mod subnet;
mod voting;
mod weights;

// TODO: better error handling in whole file

#[frame_support::pallet]
pub mod pallet {
#![allow(
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -921,7 +926,6 @@ pub mod pallet {
InvalidMinStake,
InvalidMinDelegationFee,

InvalidGlobalParams,
InvalidMaxNameLength,
InvalidMaxAllowedSubnets,
InvalidMaxAllowedModules,
Expand All @@ -934,6 +938,7 @@ pub mod pallet {
InvalidBurnRate,
InvalidMinBurn,
InvalidMaxBurn,
InvalidTargetRegistrationsPerInterval,

// VOTING
ProposalDoesNotExist,
Expand All @@ -943,11 +948,14 @@ pub mod pallet {
VoterIsNotRegistered,
VoterIsRegistered,
InvalidVoteMode,
InvalidImmunityPeriod,
InvalidFounderShare,
InvalidIncentiveRatio,
InvalidMaxWeightAge,
InvalidMaxStake,

AlreadyControlled,
AlreadyController,
// OTHER
ArithmeticError,
}

// ==================
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pallets/subspace/src/math.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
Loading

0 comments on commit b5e9e81

Please sign in to comment.