Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Machinery to fix total issuance accounting and keep it fixed #595

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion pallets/admin-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
open-junius marked this conversation as resolved.
Show resolved Hide resolved
/// 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>;

Expand Down Expand Up @@ -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<T>, amount: u64) -> DispatchResult {
ensure_root(origin)?;

pallet_subtensor::TotalSubnetLocked::<T>::put(amount);

log::info!("Set pallet_subtensor::TotalSubnetLocked to {}", amount);
Ok(())
}
}
}

Expand Down
31 changes: 31 additions & 0 deletions pallets/admin-utils/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,22 @@ 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::<Test>::get();
let new = 1000u64;
assert_ne!(before, new);
assert_ok!(AdminUtils::sudo_set_total_subnet_locked(
RuntimeOrigin::root(),
new
));
let after = pallet_subtensor::TotalSubnetLocked::<Test>::get();
assert_eq!(after, new);
});
}

#[test]
fn test_sudo_set_liquid_alpha_enabled() {
new_test_ext().execute_with(|| {
Expand Down Expand Up @@ -1219,6 +1235,21 @@ 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::<Test>::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(|| {
Expand Down
15 changes: 15 additions & 0 deletions pallets/subtensor/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,21 @@ 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<T::AccountId>,
/// 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
Expand Down
90 changes: 89 additions & 1 deletion pallets/subtensor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 =====
Expand Down Expand Up @@ -1428,6 +1442,44 @@ pub mod pallet {

weight
}

#[cfg(feature = "try-runtime")]
fn try_state(_n: BlockNumberFor<T>) -> 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(())
}
}

/// Dispatchable functions allow users to interact with the pallet and invoke state changes.
Expand Down Expand Up @@ -2269,6 +2321,42 @@ 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<T>) -> DispatchResult {
open-junius marked this conversation as resolved.
Show resolved Hide resolved
let who = ensure_signed_or_root(origin)?;

let total_account_balances =
<T::Currency as fungible::Inspect<T::AccountId>>::total_issuance();
let total_stake = TotalStake::<T>::get();
let total_subnet_locked = TotalSubnetLocked::<T>::get();

let prev_total_issuance = TotalIssuance::<T>::get();
let new_total_issuance = total_account_balances
.saturating_add(total_stake)
.saturating_add(total_subnet_locked);
TotalIssuance::<T>::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.
Expand Down
7 changes: 7 additions & 0 deletions pallets/subtensor/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
80 changes: 80 additions & 0 deletions pallets/subtensor/tests/root.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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::<Test>::put(total_subnet_locked);
pallet_subtensor::TotalStake::<Test>::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::<Test>::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::<Test>::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::<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);
});
}
17 changes: 15 additions & 2 deletions runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
Loading