Machinery to fix total issuance accounting and keep it fixed

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<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
 

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<Self::AccountId, Balance = u64>
-            + fungible::Mutate<Self::AccountId>;
+            + fungible::Mutate<Self::AccountId>
+            + fungible::Inspect<Self::AccountId>;
 
         /// Senate members with members management functions.
         type SenateMembers: crate::MemberManagement<Self::AccountId>;
@@ -350,6 +351,16 @@ pub mod pallet {
     pub type MinTake<T> = StorageValue<_, u16, ValueQuery, DefaultMinTake<T>>;
     #[pallet::storage] // --- ITEM ( global_block_emission )
     pub type BlockEmission<T> = StorageValue<_, u64, ValueQuery, DefaultBlockEmission<T>>;
+
+    /// 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<T> = StorageValue<_, u64, ValueQuery, DefaultTotalIssuance<T>>;
     #[pallet::storage] // --- ITEM (target_stakes_per_interval)
@@ -731,6 +742,9 @@ pub mod pallet {
     #[pallet::storage] // --- MAP ( netuid ) --> subnet_locked
     pub type SubnetLocked<T: Config> =
         StorageMap<_, Identity, u16, u64, ValueQuery, DefaultSubnetLocked<T>>;
+    /// The total amount of locked tokens in subnets for subnet registration.
+    #[pallet::storage]
+    pub type TotalSubnetLocked<T: Config> = StorageValue<_, u64, ValueQuery>;
 
     /// =================================
     /// ==== Axon / Promo Endpoints =====
@@ -1428,6 +1442,12 @@ pub mod pallet {
 
             weight
         }
+
+        #[cfg(feature = "try-runtime")]
+        fn try_state(_n: BlockNumberFor<T>) -> 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::<T>::iter() {
+                total_staked.saturating_accrue(stake.2);
+            }
+            ensure!(
+                total_staked == TotalStake::<T>::get(),
+                "TotalStake does not match total staked"
+            );
+
+            // Assert [`TotalSubnetLocked`] accounting is correct
+            let mut total_subnet_locked = 0;
+            for (_, locked) in SubnetLocked::<T>::iter() {
+                total_subnet_locked.saturating_accrue(locked);
+            }
+            ensure!(
+                total_subnet_locked == TotalSubnetLocked::<T>::get(),
+                "TotalSubnetLocked does not match total subnet locked"
+            );
+
+            // Assert [`TotalIssuance`] accounting is correct
+            let currency_issuance = T::Currency::total_issuance();
+            ensure!(
+                TotalIssuance::<T>::get()
+                    == currency_issuance
+                        .saturating_add(total_staked)
+                        .saturating_add(total_subnet_locked),
+                "TotalIssuance accounting discrepancy"
+            );
+
+            Ok(())
+        }
     }
 }
 

pallets/subtensor/src/migration.rs

@@ -690,3 +690,53 @@ pub fn migrate_populate_staking_hotkeys<T: Config>() -> 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<T: Config>(PhantomData<T>);
+
+    impl<T: Config> OnRuntimeUpgrade for Migration<T> {
+        fn on_runtime_upgrade() -> Weight {
+            // First, we need to initialize the TotalSubnetLocked
+            let subnets_len = crate::SubnetLocked::<T>::iter().count() as u64;
+            let total_subnet_locked: u64 =
+                crate::SubnetLocked::<T>::iter().fold(0, |acc, (_, v)| acc.saturating_add(v));
+            crate::TotalSubnetLocked::<T>::put(total_subnet_locked);
+
+            // Now, we can rejig the total issuance
+            let total_account_balances = <<T as crate::Config>::Currency as fungible::Inspect<
+                <T as frame_system::Config>::AccountId,
+            >>::total_issuance();
+            let total_stake = crate::TotalStake::<T>::get();
+            let total_subnet_locked = crate::TotalSubnetLocked::<T>::get();
+
+            let prev_total_issuance = crate::TotalIssuance::<T>::get();
+            let new_total_issuance = total_account_balances
+                .saturating_add(total_stake)
+                .saturating_add(total_subnet_locked);
+            crate::TotalIssuance::<T>::put(new_total_issuance);
+
+            log::info!(
+                "Subtensor Pallet TI Rejigged: previously: {:?}, new: {:?}",
+                prev_total_issuance,
+                new_total_issuance
+            );
+
+            <T as frame_system::Config>::DbWeight::get()
+                .reads_writes(subnets_len.saturating_add(5), 1)
+        }
+
+        #[cfg(feature = "try-runtime")]
+        fn post_upgrade(_state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
+            // These are usually checked anyway by try-runtime-cli, but just in case check them again
+            // explicitly here.
+            crate::Pallet::<T>::check_accounting_invariants()?;
+            Ok(())
+        }
+    }
+}

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<T: Config> Pallet<T> {
         // 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<T: Config> Pallet<T> {
         TotalHotkeyStake::<T>::mutate(hotkey, |stake| *stake = stake.saturating_sub(current_stake));
         Stake::<T>::remove(hotkey, coldkey);
         TotalStake::<T>::mutate(|stake| *stake = stake.saturating_sub(current_stake));
-        TotalIssuance::<T>::mutate(|issuance| *issuance = issuance.saturating_sub(current_stake));
 
         // Update StakingHotkeys map
         let mut staking_hotkeys = StakingHotkeys::<T>::get(coldkey);
         staking_hotkeys.retain(|h| h != hotkey);
         StakingHotkeys::<T>::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<T: Config> Pallet<T> {
         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<T: Config> Pallet<T> {
         coldkey: &T::AccountId,
         amount: <<T as Config>::Currency as fungible::Inspect<<T as system::Config>::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(

pallets/subtensor/src/utils.rs

@@ -329,6 +329,13 @@ impl<T: Config> Pallet<T> {
     }
 
     pub fn set_subnet_locked_balance(netuid: u16, amount: u64) {
+        let prev_total = TotalSubnetLocked::<T>::get();
+        let prev = SubnetLocked::<T>::get(netuid);
+
+        // Deduct the previous amount and add the new amount to the total
+        TotalSubnetLocked::<T>::put(prev_total.saturating_sub(prev).saturating_add(amount));
+
+        // Finally, set the new amount
         SubnetLocked::<T>::insert(netuid, amount);
     }
 

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::<Test>::insert(1, 100);
+        pallet_subtensor::SubnetLocked::<Test>::insert(2, 5);
+        pallet_balances::TotalIssuance::<Test>::put(1000);
+        pallet_subtensor::TotalStake::<Test>::put(25);
+
+        // Ensure values are NOT initialized prior to running migration
+        assert!(pallet_subtensor::TotalIssuance::<Test>::get() == 0);
+        assert!(pallet_subtensor::TotalSubnetLocked::<Test>::get() == 0);
+
+        pallet_subtensor::migration::initialise_total_issuance::Migration::<Test>::on_runtime_upgrade();
+
+        // Ensure values were initialized correctly
+        assert!(pallet_subtensor::TotalSubnetLocked::<Test>::get() == 105);
+        assert!(
+            pallet_subtensor::TotalIssuance::<Test>::get()
+                == 105u64.saturating_add(1000).saturating_add(25)
+        );
+    });
+}

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::<Test>::get(), 0u64);
+
+        // Locking on one subnet replaces its lock
+        SubtensorModule::set_subnet_locked_balance(0u16, 100u64);
+        assert_eq!(pallet_subtensor::TotalSubnetLocked::<Test>::get(), 100u64);
+        SubtensorModule::set_subnet_locked_balance(0u16, 1500u64);
+        assert_eq!(pallet_subtensor::TotalSubnetLocked::<Test>::get(), 1500u64);
+        SubtensorModule::set_subnet_locked_balance(0u16, 1000u64);
+        assert_eq!(pallet_subtensor::TotalSubnetLocked::<Test>::get(), 1000u64);
+
+        // Locking on an additional subnet is additive
+        SubtensorModule::set_subnet_locked_balance(1u16, 100u64);
+        assert_eq!(pallet_subtensor::TotalSubnetLocked::<Test>::get(), 1100u64);
+        SubtensorModule::set_subnet_locked_balance(1u16, 1000u64);
+        assert_eq!(pallet_subtensor::TotalSubnetLocked::<Test>::get(), 2000u64);
+
+        // We can go all the way back to zero
+        SubtensorModule::set_subnet_locked_balance(0u16, 0u64);
+        assert_eq!(pallet_subtensor::TotalSubnetLocked::<Test>::get(), 1000u64);
+        SubtensorModule::set_subnet_locked_balance(1u16, 0u64);
+        assert_eq!(pallet_subtensor::TotalSubnetLocked::<Test>::get(), 0u64);
+    });
+}

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<NegativeImbalance<Runtime>> for TransactionFeeHandler {
+    fn on_nonzero_unbalanced(credit: NegativeImbalance<Runtime>) {
+        let ti_before = pallet_subtensor::TotalIssuance::<Runtime>::get();
+        pallet_subtensor::TotalIssuance::<Runtime>::put(ti_before.saturating_sub(credit.peek()));
+        drop(credit);
+    }
+}
+
 impl pallet_transaction_payment::Config for Runtime {
     type RuntimeEvent = RuntimeEvent;
 
-    type OnChargeTransaction = CurrencyAdapter<Balances, ()>;
+    type OnChargeTransaction = CurrencyAdapter<Balances, TransactionFeeHandler>;
     //type TransactionByteFee = TransactionByteFee;
 
     // Convert dispatch weight to a chargeable fee.
@@ -1284,7 +1297,7 @@ pub type SignedExtra = (
     pallet_commitments::CommitmentsSignedExtension<Runtime>,
 );
 
-type Migrations = pallet_grandpa::migrations::MigrateV4ToV5<Runtime>;
+type Migrations = pallet_subtensor::migration::initialise_total_issuance::Migration<Runtime>;
 
 // Unchecked extrinsic type as expected by this runtime.
 pub type UncheckedExtrinsic =

runtime/src/migrations/mod.rs

@@ -1 +1,2 @@
 //! Export migrations from here.
+