From 886356245667a228db5f3555311f3da97d8c13c7 Mon Sep 17 00:00:00 2001 From: Liam Date: Fri, 28 Jun 2024 11:37:55 +0200 Subject: [PATCH] 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 | 31 +++++++++++ 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, 246 insertions(+), 4 deletions(-) diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 9a8744dc6..e39887554 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>; @@ -1035,6 +1035,18 @@ 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 9df59978f..b10f96ed6 100644 --- a/pallets/admin-utils/tests/tests.rs +++ b/pallets/admin-utils/tests/tests.rs @@ -1219,6 +1219,22 @@ fn test_set_alpha_values_dispatch_info_ok() { }); } +#[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_get_set_alpha() { new_test_ext().execute_with(|| { @@ -1361,3 +1377,18 @@ fn test_sudo_get_set_alpha() { )); }); } + +#[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 86c7acd1f..133b8d366 100644 --- a/pallets/subtensor/src/events.rs +++ b/pallets/subtensor/src/events.rs @@ -139,5 +139,20 @@ mod events { /// the account ID of the new senate member new_member: T::AccountId, }, + /// 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 1b545a514..47efd0831 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -106,7 +106,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; @@ -337,6 +338,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) @@ -678,6 +689,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 ===== @@ -1333,6 +1347,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. @@ -2107,6 +2155,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(64)] + #[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 15d659a51..911ac5504 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 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 7c6622670..53b40a983 100644 --- a/pallets/subtensor/tests/root.rs +++ b/pallets/subtensor/tests/root.rs @@ -1,6 +1,7 @@ #![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}; @@ -974,3 +975,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 f76ae4558..56d2a4893 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, @@ -380,10 +382,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.