diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 9a8744dc6..eab1b6c97 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>; diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index f7824e2a3..e59375461 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -110,7 +110,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; @@ -350,6 +351,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 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`] + /// + /// Eventually, Bittensor should migrate to using Holds afterwhich time we will not require this + /// separate accounting. #[pallet::storage] // --- ITEM ( total_issuance ) pub type TotalIssuance = StorageValue<_, u64, ValueQuery, DefaultTotalIssuance>; #[pallet::storage] // --- ITEM (target_stakes_per_interval) @@ -731,6 +742,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 ===== @@ -1428,6 +1442,12 @@ pub mod pallet { weight } + + #[cfg(feature = "try-runtime")] + fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { + Self::check_accounting_invariants()?; + Ok(()) + } } /// Dispatchable functions allow users to interact with the pallet and invoke state changes. @@ -2314,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/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( diff --git a/pallets/subtensor/src/utils.rs b/pallets/subtensor/src/utils.rs index c61133e94..6dd9b22dd 100644 --- a/pallets/subtensor/src/utils.rs +++ b/pallets/subtensor/src/utils.rs @@ -329,6 +329,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 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 SubnetLocked::::insert(netuid, amount); } 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 7c6622670..b5cb98d98 100644 --- a/pallets/subtensor/tests/root.rs +++ b/pallets/subtensor/tests/root.rs @@ -974,3 +974,31 @@ fn test_dissolve_network_does_not_exist_err() { ); }); } + +#[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 a4abd124f..91445dc43 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -12,13 +12,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, Contains, LinearStoragePrice}, + traits::{fungible::HoldConsideration, Contains, LinearStoragePrice, OnUnbalanced}, }; use frame_system::{EnsureNever, EnsureRoot, EnsureRootWithSuccess, RawOrigin}; +use pallet_balances::NegativeImbalance; use pallet_commitments::CanCommit; use pallet_grandpa::{ fg_primitives, AuthorityId as GrandpaId, AuthorityList as GrandpaAuthorityList, @@ -388,10 +390,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. @@ -1284,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. +