From f74d800b0781aff597ed4f1edb455467677733cc Mon Sep 17 00:00:00 2001 From: Liam Date: Fri, 28 Jun 2024 11:37:55 +0200 Subject: [PATCH 1/6] machinery to fix total issuance comment tx fee handler Update pallets/subtensor/src/lib.rs Co-authored-by: Cameron Fairchild --- pallets/admin-utils/src/lib.rs | 14 ++++- pallets/admin-utils/tests/tests.rs | 32 +++++++++++ pallets/subtensor/src/events.rs | 15 ++++++ pallets/subtensor/src/lib.rs | 86 +++++++++++++++++++++++++++++- pallets/subtensor/src/utils.rs | 7 +++ pallets/subtensor/tests/root.rs | 80 +++++++++++++++++++++++++++ runtime/src/lib.rs | 17 +++++- 7 files changed, 247 insertions(+), 4 deletions(-) diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 47799f30d..e17aefbe8 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -26,7 +26,7 @@ pub mod pallet { /// Configure the pallet by specifying the parameters and types on which it depends. #[pallet::config] - pub trait Config: frame_system::Config { + pub trait Config: frame_system::Config + pallet_subtensor::Config { /// Because this pallet emits events, it depends on the runtime's definition of an event. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -996,6 +996,18 @@ pub mod pallet { log::info!("ToggleSetWeightsCommitReveal( netuid: {:?} ) ", netuid); Ok(()) } + + /// Sets the [`pallet_subtensor::TotalSubnetLocked`] value. + #[pallet::call_index(50)] + #[pallet::weight(T::WeightInfo::sudo_set_commit_reveal_weights_enabled())] + pub fn sudo_set_total_subnet_locked(origin: OriginFor, amount: u64) -> DispatchResult { + ensure_root(origin)?; + + pallet_subtensor::TotalSubnetLocked::::put(amount); + + log::info!("Set pallet_subtensor::TotalSubnetLocked to {}", amount); + Ok(()) + } } } diff --git a/pallets/admin-utils/tests/tests.rs b/pallets/admin-utils/tests/tests.rs index f87b43e74..8dca4bb34 100644 --- a/pallets/admin-utils/tests/tests.rs +++ b/pallets/admin-utils/tests/tests.rs @@ -1,3 +1,4 @@ +use frame_support::assert_err; use frame_support::assert_ok; use frame_support::sp_runtime::DispatchError; use frame_system::Config; @@ -1178,3 +1179,34 @@ fn test_sudo_set_target_stakes_per_interval() { assert_eq!(SubtensorModule::get_target_stakes_per_interval(), to_be_set); }); } + +#[test] +fn test_set_total_subnet_locked_ok() { + new_test_ext().execute_with(|| { + // Setup + let before = pallet_subtensor::TotalSubnetLocked::::get(); + let new = 1000u64; + assert_ne!(before, new); + assert_ok!(AdminUtils::sudo_set_total_subnet_locked( + RuntimeOrigin::root(), + new + )); + let after = pallet_subtensor::TotalSubnetLocked::::get(); + assert_eq!(after, new); + }); +} + +#[test] +fn test_set_total_subnet_locked_not_sudo() { + new_test_ext().execute_with(|| { + // Setup + let before = pallet_subtensor::TotalSubnetLocked::::get(); + let new = 1000u64; + let who = U256::from(1); + assert_ne!(before, new); + assert_err!( + AdminUtils::sudo_set_total_subnet_locked(RuntimeOrigin::signed(who), new), + DispatchError::BadOrigin + ); + }); +} diff --git a/pallets/subtensor/src/events.rs b/pallets/subtensor/src/events.rs index 47cc9973b..7bafe337e 100644 --- a/pallets/subtensor/src/events.rs +++ b/pallets/subtensor/src/events.rs @@ -132,5 +132,20 @@ mod events { MinDelegateTakeSet(u16), /// the target stakes per interval is set by sudo/admin transaction TargetStakesPerIntervalSet(u64), + /// Total issuance was rejigged + TotalIssuanceRejigged { + /// If Some a signed account, or root + who: Option, + /// The previous pallet total issuance + prev_total_issuance: u64, + /// The new pallet total issuance + new_total_issuance: u64, + /// The total amount of tokens staked + total_stake: u64, + /// The total amount of tokens in accounts + total_account_balances: u64, + /// The total amount of tokens locked in subnets + total_subnet_locked: u64, + }, } } diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index b590781f5..504375b54 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -103,7 +103,8 @@ pub mod pallet { /// Currency type that will be used to place deposits on neurons type Currency: fungible::Balanced - + fungible::Mutate; + + fungible::Mutate + + fungible::Inspect; /// Senate members with members management functions. type SenateMembers: crate::MemberManagement; @@ -322,6 +323,16 @@ pub mod pallet { pub type MinTake = StorageValue<_, u16, ValueQuery, DefaultMinTake>; #[pallet::storage] // --- ITEM ( global_block_emission ) pub type BlockEmission = StorageValue<_, u64, ValueQuery, DefaultBlockEmission>; + + /// The Subtensor [`TotalIssuance`] represents the total issuance of tokens on the Bittensor network. + /// + /// It comprised of three parts: + /// - The total amount of issued tokens, tracked in the TotalIssuance of the Balances pallet + /// - The total amount of tokens staked in the system, tracked in [`TotalStake`] + /// - The total amount of tokens locked up for subnet reg, tracked in [`TotalSubnetLocked`] + /// + /// Eventually, Bittensor should migrate to using Holds afterwhich time we will not require this + /// seperate accounting. #[pallet::storage] // --- ITEM ( total_issuance ) pub type TotalIssuance = StorageValue<_, u64, ValueQuery, DefaultTotalIssuance>; #[pallet::storage] // --- ITEM (target_stakes_per_interval) @@ -655,6 +666,9 @@ pub mod pallet { #[pallet::storage] // --- MAP ( netuid ) --> subnet_locked pub type SubnetLocked = StorageMap<_, Identity, u16, u64, ValueQuery, DefaultSubnetLocked>; + /// The total amount of locked tokens in subnets for subnet registration. + #[pallet::storage] + pub type TotalSubnetLocked = StorageValue<_, u64, ValueQuery>; /// ================================= /// ==== Axon / Promo Endpoints ===== @@ -1287,6 +1301,40 @@ pub mod pallet { weight } + + #[cfg(feature = "try-runtime")] + fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { + use frame_support::traits::fungible::Inspect; + + // Assert [`TotalStake`] accounting is correct + let mut total_staked = 0; + for stake in Stake::::iter() { + total_staked += stake.2; + } + ensure!( + total_staked == TotalStake::::get(), + "TotalStake does not match total staked" + ); + + // Assert [`TotalSubnetLocked`] accounting is correct + let mut total_subnet_locked = 0; + for (_, locked) in SubnetLocked::::iter() { + total_subnet_locked += locked; + } + ensure!( + total_subnet_locked == TotalSubnetLocked::::get(), + "TotalSubnetLocked does not match total subnet locked" + ); + + // Assert [`TotalIssuance`] accounting is correct + let currency_issuance = T::Currency::total_issuance(); + ensure!( + TotalIssuance::::get() == currency_issuance + total_staked + total_subnet_locked, + "TotalIssuance accounting discrepancy" + ); + + Ok(()) + } } /// Dispatchable functions allow users to interact with the pallet and invoke state changes. @@ -2051,6 +2099,42 @@ pub mod pallet { pub fn dissolve_network(origin: OriginFor, netuid: u16) -> DispatchResult { Self::user_remove_network(origin, netuid) } + + /// Set the [`TotalIssuance`] storage value to the total account balances issued + the + /// total amount staked + the total amount locked in subnets. + #[pallet::call_index(63)] + #[pallet::weight(( + Weight::default() + .saturating_add(T::DbWeight::get().reads(3)) + .saturating_add(T::DbWeight::get().writes(1)), + DispatchClass::Normal, + Pays::Yes + ))] + pub fn rejig_total_issuance(origin: OriginFor) -> DispatchResult { + let who = ensure_signed_or_root(origin)?; + + let total_account_balances = + >::total_issuance(); + let total_stake = TotalStake::::get(); + let total_subnet_locked = TotalSubnetLocked::::get(); + + let prev_total_issuance = TotalIssuance::::get(); + let new_total_issuance = total_account_balances + .saturating_add(total_stake) + .saturating_add(total_subnet_locked); + TotalIssuance::::put(new_total_issuance); + + Self::deposit_event(Event::TotalIssuanceRejigged { + who, + prev_total_issuance, + new_total_issuance, + total_stake, + total_account_balances, + total_subnet_locked, + }); + + Ok(()) + } } // ---- Subtensor helper functions. diff --git a/pallets/subtensor/src/utils.rs b/pallets/subtensor/src/utils.rs index 54b7818c9..00648258a 100644 --- a/pallets/subtensor/src/utils.rs +++ b/pallets/subtensor/src/utils.rs @@ -324,6 +324,13 @@ impl Pallet { } pub fn set_subnet_locked_balance(netuid: u16, amount: u64) { + let prev_total = TotalSubnetLocked::::get(); + let prev = SubnetLocked::::get(netuid); + + // Deduct the prev amount and add the new amount to the total + TotalSubnetLocked::::put(prev_total.saturating_sub(prev).saturating_add(amount)); + + // Finally, set the new amount SubnetLocked::::insert(netuid, amount); } diff --git a/pallets/subtensor/tests/root.rs b/pallets/subtensor/tests/root.rs index 7958c9c81..cfedf8038 100644 --- a/pallets/subtensor/tests/root.rs +++ b/pallets/subtensor/tests/root.rs @@ -1,4 +1,5 @@ use crate::mock::*; +use frame_support::traits::fungible::Mutate; use frame_support::{assert_err, assert_ok}; use frame_system::Config; use frame_system::{EventRecord, Phase}; @@ -972,3 +973,82 @@ fn test_dissolve_network_does_not_exist_err() { ); }); } + +#[test] +fn test_rejig_total_issuance_ok() { + new_test_ext(1).execute_with(|| { + // Setup + let who = U256::from(1); + Balances::set_balance(&who, 100); + let balances_total_issuance = Balances::total_issuance(); + assert!(balances_total_issuance > 0); + let total_stake = 100; + let total_subnet_locked = 1000; + pallet_subtensor::TotalSubnetLocked::::put(total_subnet_locked); + pallet_subtensor::TotalStake::::put(total_stake); + + let expected_total_issuance = balances_total_issuance + total_stake + total_subnet_locked; + + // Rejig total issuance + let total_issuance_before = pallet_subtensor::TotalIssuance::::get(); + assert_ne!(total_issuance_before, expected_total_issuance); + assert_ok!(SubtensorModule::rejig_total_issuance( + RuntimeOrigin::signed(who) + )); + let total_issuance_after = pallet_subtensor::TotalIssuance::::get(); + + // Rejigged + assert_eq!(total_issuance_after, expected_total_issuance); + System::assert_last_event(RuntimeEvent::SubtensorModule( + pallet_subtensor::Event::TotalIssuanceRejigged { + who: Some(who), + new_total_issuance: total_issuance_after, + prev_total_issuance: total_issuance_before, + total_account_balances: balances_total_issuance, + total_stake, + total_subnet_locked, + }, + )); + + // Works with root + assert_ok!(SubtensorModule::rejig_total_issuance(RuntimeOrigin::root())); + System::assert_last_event(RuntimeEvent::SubtensorModule( + pallet_subtensor::Event::TotalIssuanceRejigged { + who: None, + new_total_issuance: total_issuance_after, + prev_total_issuance: total_issuance_after, + total_account_balances: balances_total_issuance, + total_stake, + total_subnet_locked, + }, + )); + }); +} + +#[test] +fn test_set_subnet_locked_balance_adjusts_total_subnet_locked_correctly() { + new_test_ext(1).execute_with(|| { + // Asset starting from zero + assert_eq!(pallet_subtensor::TotalSubnetLocked::::get(), 0u64); + + // Locking on one subnet replaces its lock + SubtensorModule::set_subnet_locked_balance(0u16, 100u64); + assert_eq!(pallet_subtensor::TotalSubnetLocked::::get(), 100u64); + SubtensorModule::set_subnet_locked_balance(0u16, 1500u64); + assert_eq!(pallet_subtensor::TotalSubnetLocked::::get(), 1500u64); + SubtensorModule::set_subnet_locked_balance(0u16, 1000u64); + assert_eq!(pallet_subtensor::TotalSubnetLocked::::get(), 1000u64); + + // Locking on an additional subnet is additive + SubtensorModule::set_subnet_locked_balance(1u16, 100u64); + assert_eq!(pallet_subtensor::TotalSubnetLocked::::get(), 1100u64); + SubtensorModule::set_subnet_locked_balance(1u16, 1000u64); + assert_eq!(pallet_subtensor::TotalSubnetLocked::::get(), 2000u64); + + // We can go all the way back to zero + SubtensorModule::set_subnet_locked_balance(0u16, 0u64); + assert_eq!(pallet_subtensor::TotalSubnetLocked::::get(), 1000u64); + SubtensorModule::set_subnet_locked_balance(1u16, 0u64); + assert_eq!(pallet_subtensor::TotalSubnetLocked::::get(), 0u64); + }); +} diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 2364008fd..63d17103b 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -10,13 +10,15 @@ pub mod check_nonce; mod migrations; use codec::{Decode, Encode, MaxEncodedLen}; +use frame_support::traits::Imbalance; use frame_support::{ dispatch::DispatchResultWithPostInfo, genesis_builder_helper::{build_config, create_default_config}, pallet_prelude::{DispatchError, Get}, - traits::{fungible::HoldConsideration, LinearStoragePrice}, + traits::{fungible::HoldConsideration, LinearStoragePrice, OnUnbalanced}, }; use frame_system::{EnsureNever, EnsureRoot, RawOrigin}; +use pallet_balances::NegativeImbalance; use pallet_commitments::CanCommit; use pallet_grandpa::{ fg_primitives, AuthorityId as GrandpaId, AuthorityList as GrandpaAuthorityList, @@ -333,10 +335,21 @@ parameter_types! { pub FeeMultiplier: Multiplier = Multiplier::one(); } +/// Deduct the transaction fee from the Subtensor Pallet TotalIssuance when dropping the transaction +/// fee. +pub struct TransactionFeeHandler; +impl OnUnbalanced> for TransactionFeeHandler { + fn on_nonzero_unbalanced(credit: NegativeImbalance) { + let ti_before = pallet_subtensor::TotalIssuance::::get(); + pallet_subtensor::TotalIssuance::::put(ti_before.saturating_sub(credit.peek())); + drop(credit); + } +} + impl pallet_transaction_payment::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type OnChargeTransaction = CurrencyAdapter; + type OnChargeTransaction = CurrencyAdapter; //type TransactionByteFee = TransactionByteFee; // Convert dispatch weight to a chargeable fee. From fd27e9d510578169dadd7755691def14457db3f0 Mon Sep 17 00:00:00 2001 From: orriin <167025436+orriin@users.noreply.github.com> Date: Mon, 1 Jul 2024 16:21:41 +0200 Subject: [PATCH 2/6] Update pallets/subtensor/src/lib.rs Co-authored-by: Cameron Fairchild --- pallets/subtensor/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 504375b54..aadb17efb 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -326,7 +326,7 @@ pub mod pallet { /// The Subtensor [`TotalIssuance`] represents the total issuance of tokens on the Bittensor network. /// - /// It comprised of three parts: + /// It is comprised of three parts: /// - The total amount of issued tokens, tracked in the TotalIssuance of the Balances pallet /// - The total amount of tokens staked in the system, tracked in [`TotalStake`] /// - The total amount of tokens locked up for subnet reg, tracked in [`TotalSubnetLocked`] From 0fbcc18977f75616019b76ee6f608bdab35b1104 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Tue, 16 Jul 2024 09:33:16 -0400 Subject: [PATCH 3/6] Update pallets/subtensor/src/lib.rs --- pallets/subtensor/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index aadb17efb..4a47be68d 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -332,7 +332,7 @@ pub mod pallet { /// - The total amount of tokens locked up for subnet reg, tracked in [`TotalSubnetLocked`] /// /// Eventually, Bittensor should migrate to using Holds afterwhich time we will not require this - /// seperate accounting. + /// separate accounting. #[pallet::storage] // --- ITEM ( total_issuance ) pub type TotalIssuance = StorageValue<_, u64, ValueQuery, DefaultTotalIssuance>; #[pallet::storage] // --- ITEM (target_stakes_per_interval) From ea2c952cf2323a7ae4f7d255372f7cccfcc39a63 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Tue, 16 Jul 2024 09:33:23 -0400 Subject: [PATCH 4/6] Update pallets/subtensor/src/utils.rs --- pallets/subtensor/src/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/subtensor/src/utils.rs b/pallets/subtensor/src/utils.rs index 00648258a..74ca46ed2 100644 --- a/pallets/subtensor/src/utils.rs +++ b/pallets/subtensor/src/utils.rs @@ -327,7 +327,7 @@ impl Pallet { let prev_total = TotalSubnetLocked::::get(); let prev = SubnetLocked::::get(netuid); - // Deduct the prev amount and add the new amount to the total + // Deduct the previous amount and add the new amount to the total TotalSubnetLocked::::put(prev_total.saturating_sub(prev).saturating_add(amount)); // Finally, set the new amount From b34c052e5a7204b6f6f85d36047a5723e51e68f6 Mon Sep 17 00:00:00 2001 From: Liam Date: Mon, 22 Jul 2024 15:18:17 +0200 Subject: [PATCH 5/6] fix TI handling in empty_stake_on_coldkey_hotkey_account --- pallets/subtensor/src/staking.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/pallets/subtensor/src/staking.rs b/pallets/subtensor/src/staking.rs index 199234a30..72b56b3dd 100644 --- a/pallets/subtensor/src/staking.rs +++ b/pallets/subtensor/src/staking.rs @@ -6,7 +6,7 @@ use frame_support::{ fungible::{Balanced as _, Inspect as _, Mutate as _}, Fortitude, Precision, Preservation, }, - Imbalance, + Defensive, Imbalance, }, }; @@ -704,11 +704,14 @@ impl Pallet { // TODO: Tech debt: Remove StakingHotkeys entry if stake goes to 0 } - /// Empties the stake associated with a given coldkey-hotkey account pairing. + /// Empties the stake associated with a given coldkey-hotkey account pairing, and moves it to + /// the coldkey free balance. + /// /// This function retrieves the current stake for the specified coldkey-hotkey pairing, /// then subtracts this stake amount from both the TotalColdkeyStake and TotalHotkeyStake. - /// It also removes the stake entry for the hotkey-coldkey pairing and adjusts the TotalStake - /// and TotalIssuance by subtracting the removed stake amount. + /// + /// It also removes the stake entry for the hotkey-coldkey pairing and increases the coldkey + /// free balance by the amount of stake removed. /// /// Returns the amount of stake that was removed. /// @@ -725,13 +728,15 @@ impl Pallet { TotalHotkeyStake::::mutate(hotkey, |stake| *stake = stake.saturating_sub(current_stake)); Stake::::remove(hotkey, coldkey); TotalStake::::mutate(|stake| *stake = stake.saturating_sub(current_stake)); - TotalIssuance::::mutate(|issuance| *issuance = issuance.saturating_sub(current_stake)); // Update StakingHotkeys map let mut staking_hotkeys = StakingHotkeys::::get(coldkey); staking_hotkeys.retain(|h| h != hotkey); StakingHotkeys::::insert(coldkey, staking_hotkeys); + // Add the amount to the coldkey free balance + Self::add_balance_to_coldkey_account(coldkey, current_stake); + current_stake } @@ -745,11 +750,9 @@ impl Pallet { if !Self::coldkey_owns_hotkey(coldkey, hotkey) { // If the stake is below the minimum required, it's considered a small nomination and needs to be cleared. if stake < Self::get_nominator_min_required_stake() { - // Remove the stake from the nominator account. (this is a more forceful unstake operation which ) - // Actually deletes the staking account. - let cleared_stake = Self::empty_stake_on_coldkey_hotkey_account(coldkey, hotkey); - // Add the stake to the coldkey account. - Self::add_balance_to_coldkey_account(coldkey, cleared_stake); + // Remove the stake from the nominator account, and moves it to the free balance. + // This is a more forceful unstake operation which actually deletes the staking account. + Self::empty_stake_on_coldkey_hotkey_account(coldkey, hotkey); } } } @@ -769,8 +772,8 @@ impl Pallet { coldkey: &T::AccountId, amount: <::Currency as fungible::Inspect<::AccountId>>::Balance, ) { - // infallible - let _ = T::Currency::deposit(coldkey, amount, Precision::BestEffort); + let _ = T::Currency::deposit(coldkey, amount, Precision::BestEffort) + .defensive_proof("Account balance should never exceed max balance"); } pub fn set_balance_on_coldkey_account( From 64993cce248ff3e00fb39e70e5994421605e37e7 Mon Sep 17 00:00:00 2001 From: Liam Date: Tue, 23 Jul 2024 15:50:29 +0200 Subject: [PATCH 6/6] initialise total issuance with a migration initialise total issuance with a migration --- pallets/admin-utils/src/lib.rs | 12 --- pallets/admin-utils/tests/tests.rs | 31 -------- pallets/subtensor/src/events.rs | 15 ---- pallets/subtensor/src/lib.rs | 110 ++++++++++----------------- pallets/subtensor/src/migration.rs | 50 ++++++++++++ pallets/subtensor/tests/migration.rs | 25 ++++++ pallets/subtensor/tests/root.rs | 52 ------------- runtime/src/lib.rs | 2 +- runtime/src/migrations/mod.rs | 1 + 9 files changed, 118 insertions(+), 180 deletions(-) diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index e39887554..eab1b6c97 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -1035,18 +1035,6 @@ pub mod pallet { T::Subtensor::ensure_subnet_owner_or_root(origin.clone(), netuid)?; T::Subtensor::do_set_alpha_values(origin, netuid, alpha_low, alpha_high) } - - /// Sets the [`pallet_subtensor::TotalSubnetLocked`] value. - #[pallet::call_index(52)] - #[pallet::weight(T::WeightInfo::sudo_set_commit_reveal_weights_enabled())] - pub fn sudo_set_total_subnet_locked(origin: OriginFor, amount: u64) -> DispatchResult { - ensure_root(origin)?; - - pallet_subtensor::TotalSubnetLocked::::put(amount); - - log::info!("Set pallet_subtensor::TotalSubnetLocked to {}", amount); - Ok(()) - } } } diff --git a/pallets/admin-utils/tests/tests.rs b/pallets/admin-utils/tests/tests.rs index 20d79fb28..9df59978f 100644 --- a/pallets/admin-utils/tests/tests.rs +++ b/pallets/admin-utils/tests/tests.rs @@ -1183,22 +1183,6 @@ fn test_sudo_set_target_stakes_per_interval() { }); } -#[test] -fn test_set_total_subnet_locked_ok() { - new_test_ext().execute_with(|| { - // Setup - let before = pallet_subtensor::TotalSubnetLocked::::get(); - let new = 1000u64; - assert_ne!(before, new); - assert_ok!(AdminUtils::sudo_set_total_subnet_locked( - RuntimeOrigin::root(), - new - )); - let after = pallet_subtensor::TotalSubnetLocked::::get(); - assert_eq!(after, new); - }); -} - #[test] fn test_sudo_set_liquid_alpha_enabled() { new_test_ext().execute_with(|| { @@ -1235,21 +1219,6 @@ fn test_set_alpha_values_dispatch_info_ok() { }); } -#[test] -fn test_set_total_subnet_locked_not_sudo() { - new_test_ext().execute_with(|| { - // Setup - let before = pallet_subtensor::TotalSubnetLocked::::get(); - let new = 1000u64; - let who = U256::from(1); - assert_ne!(before, new); - assert_err!( - AdminUtils::sudo_set_total_subnet_locked(RuntimeOrigin::signed(who), new), - DispatchError::BadOrigin - ); - }) -} - #[test] fn test_sudo_get_set_alpha() { new_test_ext().execute_with(|| { diff --git a/pallets/subtensor/src/events.rs b/pallets/subtensor/src/events.rs index 4e6c39a33..a5fb90e3f 100644 --- a/pallets/subtensor/src/events.rs +++ b/pallets/subtensor/src/events.rs @@ -132,21 +132,6 @@ mod events { MinDelegateTakeSet(u16), /// the target stakes per interval is set by sudo/admin transaction TargetStakesPerIntervalSet(u64), - /// Total issuance was rejigged - TotalIssuanceRejigged { - /// If Some a signed account, or root - who: Option, - /// The previous pallet total issuance - prev_total_issuance: u64, - /// The new pallet total issuance - new_total_issuance: u64, - /// The total amount of tokens staked - total_stake: u64, - /// The total amount of tokens in accounts - total_account_balances: u64, - /// The total amount of tokens locked in subnets - total_subnet_locked: u64, - }, /// a member of the senate is adjusted SenateAdjusted { /// the account ID of the old senate member, if any diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index e595dc50a..e59375461 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -1445,39 +1445,7 @@ pub mod pallet { #[cfg(feature = "try-runtime")] fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { - use frame_support::traits::fungible::Inspect; - use sp_runtime::Saturating; - - // Assert [`TotalStake`] accounting is correct - let mut total_staked = 0; - for stake in Stake::::iter() { - total_staked.saturating_accrue(stake.2); - } - ensure!( - total_staked == TotalStake::::get(), - "TotalStake does not match total staked" - ); - - // Assert [`TotalSubnetLocked`] accounting is correct - let mut total_subnet_locked = 0; - for (_, locked) in SubnetLocked::::iter() { - total_subnet_locked.saturating_accrue(locked); - } - ensure!( - total_subnet_locked == TotalSubnetLocked::::get(), - "TotalSubnetLocked does not match total subnet locked" - ); - - // Assert [`TotalIssuance`] accounting is correct - let currency_issuance = T::Currency::total_issuance(); - ensure!( - TotalIssuance::::get() - == currency_issuance - .saturating_add(total_staked) - .saturating_add(total_subnet_locked), - "TotalIssuance accounting discrepancy" - ); - + Self::check_accounting_invariants()?; Ok(()) } } @@ -2321,42 +2289,6 @@ pub mod pallet { ) -> DispatchResult { Ok(()) } - - /// Set the [`TotalIssuance`] storage value to the total account balances issued + the - /// total amount staked + the total amount locked in subnets. - #[pallet::call_index(67)] - #[pallet::weight(( - Weight::default() - .saturating_add(T::DbWeight::get().reads(3)) - .saturating_add(T::DbWeight::get().writes(1)), - DispatchClass::Normal, - Pays::Yes - ))] - pub fn rejig_total_issuance(origin: OriginFor) -> DispatchResult { - let who = ensure_signed_or_root(origin)?; - - let total_account_balances = - >::total_issuance(); - let total_stake = TotalStake::::get(); - let total_subnet_locked = TotalSubnetLocked::::get(); - - let prev_total_issuance = TotalIssuance::::get(); - let new_total_issuance = total_account_balances - .saturating_add(total_stake) - .saturating_add(total_subnet_locked); - TotalIssuance::::put(new_total_issuance); - - Self::deposit_event(Event::TotalIssuanceRejigged { - who, - prev_total_issuance, - new_total_issuance, - total_stake, - total_account_balances, - total_subnet_locked, - }); - - Ok(()) - } } // ---- Subtensor helper functions. @@ -2402,6 +2334,46 @@ pub mod pallet { } true } + + #[cfg(feature = "try-runtime")] + /// Assets [`TotalStake`], [`TotalSubnetLocked`], and [`TotalIssuance`] accounting invariants + /// are correct. + pub fn check_accounting_invariants() -> Result<(), sp_runtime::TryRuntimeError> { + use frame_support::traits::fungible::Inspect; + use sp_runtime::Saturating; + + // Assert [`TotalStake`] accounting is correct + let mut total_staked = 0; + for stake in Stake::::iter() { + total_staked.saturating_accrue(stake.2); + } + ensure!( + total_staked == TotalStake::::get(), + "TotalStake does not match total staked" + ); + + // Assert [`TotalSubnetLocked`] accounting is correct + let mut total_subnet_locked = 0; + for (_, locked) in SubnetLocked::::iter() { + total_subnet_locked.saturating_accrue(locked); + } + ensure!( + total_subnet_locked == TotalSubnetLocked::::get(), + "TotalSubnetLocked does not match total subnet locked" + ); + + // Assert [`TotalIssuance`] accounting is correct + let currency_issuance = T::Currency::total_issuance(); + ensure!( + TotalIssuance::::get() + == currency_issuance + .saturating_add(total_staked) + .saturating_add(total_subnet_locked), + "TotalIssuance accounting discrepancy" + ); + + Ok(()) + } } } diff --git a/pallets/subtensor/src/migration.rs b/pallets/subtensor/src/migration.rs index 866ff08fd..fb5514b12 100644 --- a/pallets/subtensor/src/migration.rs +++ b/pallets/subtensor/src/migration.rs @@ -690,3 +690,53 @@ pub fn migrate_populate_staking_hotkeys() -> Weight { Weight::zero() } } + +pub mod initialise_total_issuance { + use frame_support::pallet_prelude::Weight; + use frame_support::traits::{fungible, OnRuntimeUpgrade}; + use sp_core::Get; + + use crate::*; + + pub struct Migration(PhantomData); + + impl OnRuntimeUpgrade for Migration { + fn on_runtime_upgrade() -> Weight { + // First, we need to initialize the TotalSubnetLocked + let subnets_len = crate::SubnetLocked::::iter().count() as u64; + let total_subnet_locked: u64 = + crate::SubnetLocked::::iter().fold(0, |acc, (_, v)| acc.saturating_add(v)); + crate::TotalSubnetLocked::::put(total_subnet_locked); + + // Now, we can rejig the total issuance + let total_account_balances = <::Currency as fungible::Inspect< + ::AccountId, + >>::total_issuance(); + let total_stake = crate::TotalStake::::get(); + let total_subnet_locked = crate::TotalSubnetLocked::::get(); + + let prev_total_issuance = crate::TotalIssuance::::get(); + let new_total_issuance = total_account_balances + .saturating_add(total_stake) + .saturating_add(total_subnet_locked); + crate::TotalIssuance::::put(new_total_issuance); + + log::info!( + "Subtensor Pallet TI Rejigged: previously: {:?}, new: {:?}", + prev_total_issuance, + new_total_issuance + ); + + ::DbWeight::get() + .reads_writes(subnets_len.saturating_add(5), 1) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + // These are usually checked anyway by try-runtime-cli, but just in case check them again + // explicitly here. + crate::Pallet::::check_accounting_invariants()?; + Ok(()) + } + } +} diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index d47155862..7c6daa003 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -422,3 +422,28 @@ fn run_migration_and_check(migration_name: &'static str) -> frame_support::weigh // Return the weight of the executed migration weight } + +#[test] +fn test_initialise_ti() { + use frame_support::traits::OnRuntimeUpgrade; + + new_test_ext(1).execute_with(|| { + pallet_subtensor::SubnetLocked::::insert(1, 100); + pallet_subtensor::SubnetLocked::::insert(2, 5); + pallet_balances::TotalIssuance::::put(1000); + pallet_subtensor::TotalStake::::put(25); + + // Ensure values are NOT initialized prior to running migration + assert!(pallet_subtensor::TotalIssuance::::get() == 0); + assert!(pallet_subtensor::TotalSubnetLocked::::get() == 0); + + pallet_subtensor::migration::initialise_total_issuance::Migration::::on_runtime_upgrade(); + + // Ensure values were initialized correctly + assert!(pallet_subtensor::TotalSubnetLocked::::get() == 105); + assert!( + pallet_subtensor::TotalIssuance::::get() + == 105u64.saturating_add(1000).saturating_add(25) + ); + }); +} diff --git a/pallets/subtensor/tests/root.rs b/pallets/subtensor/tests/root.rs index 53b40a983..b5cb98d98 100644 --- a/pallets/subtensor/tests/root.rs +++ b/pallets/subtensor/tests/root.rs @@ -1,7 +1,6 @@ #![allow(clippy::indexing_slicing, clippy::unwrap_used)] use crate::mock::*; -use frame_support::traits::fungible::Mutate; use frame_support::{assert_err, assert_ok}; use frame_system::Config; use frame_system::{EventRecord, Phase}; @@ -976,57 +975,6 @@ fn test_dissolve_network_does_not_exist_err() { }); } -#[test] -fn test_rejig_total_issuance_ok() { - new_test_ext(1).execute_with(|| { - // Setup - let who = U256::from(1); - Balances::set_balance(&who, 100); - let balances_total_issuance = Balances::total_issuance(); - assert!(balances_total_issuance > 0); - let total_stake = 100; - let total_subnet_locked = 1000; - pallet_subtensor::TotalSubnetLocked::::put(total_subnet_locked); - pallet_subtensor::TotalStake::::put(total_stake); - - let expected_total_issuance = balances_total_issuance + total_stake + total_subnet_locked; - - // Rejig total issuance - let total_issuance_before = pallet_subtensor::TotalIssuance::::get(); - assert_ne!(total_issuance_before, expected_total_issuance); - assert_ok!(SubtensorModule::rejig_total_issuance( - RuntimeOrigin::signed(who) - )); - let total_issuance_after = pallet_subtensor::TotalIssuance::::get(); - - // Rejigged - assert_eq!(total_issuance_after, expected_total_issuance); - System::assert_last_event(RuntimeEvent::SubtensorModule( - pallet_subtensor::Event::TotalIssuanceRejigged { - who: Some(who), - new_total_issuance: total_issuance_after, - prev_total_issuance: total_issuance_before, - total_account_balances: balances_total_issuance, - total_stake, - total_subnet_locked, - }, - )); - - // Works with root - assert_ok!(SubtensorModule::rejig_total_issuance(RuntimeOrigin::root())); - System::assert_last_event(RuntimeEvent::SubtensorModule( - pallet_subtensor::Event::TotalIssuanceRejigged { - who: None, - new_total_issuance: total_issuance_after, - prev_total_issuance: total_issuance_after, - total_account_balances: balances_total_issuance, - total_stake, - total_subnet_locked, - }, - )); - }); -} - #[test] fn test_set_subnet_locked_balance_adjusts_total_subnet_locked_correctly() { new_test_ext(1).execute_with(|| { diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 67d711e20..91445dc43 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1297,7 +1297,7 @@ pub type SignedExtra = ( pallet_commitments::CommitmentsSignedExtension, ); -type Migrations = pallet_grandpa::migrations::MigrateV4ToV5; +type Migrations = pallet_subtensor::migration::initialise_total_issuance::Migration; // Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = diff --git a/runtime/src/migrations/mod.rs b/runtime/src/migrations/mod.rs index ecc48efcd..dd75833c5 100644 --- a/runtime/src/migrations/mod.rs +++ b/runtime/src/migrations/mod.rs @@ -1 +1,2 @@ //! Export migrations from here. +