Mark nomination-pool funds as deactivated

[kianenigma]

    As a follow-up, all funds locked in nomination pools also have to be marked as deactivated now. 

The migration will be O(pool_count). Each pool would look at its bonded account ledger.total and mark that as deactivated.

Originally posted by @kianenigma in paritytech/substrate#12813 (comment)

[kianenigma]

I took a stab at this, but given the low TVL of pools, if we manage to make pool funds usable for governance soon, it is not worth it.

diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs
index 3cb8abedda..54891f22ee 100644
--- a/frame/nomination-pools/src/lib.rs
+++ b/frame/nomination-pools/src/lib.rs
@@ -900,9 +900,8 @@ impl<T: Config> BondedPool<T> {
 
 	/// Bond exactly `amount` from `who`'s funds into this pool.
 	///
-	/// If the bond type is `Create`, `Staking::bond` is called, and `who`
-	/// is allowed to be killed. Otherwise, `Staking::bond_extra` is called and `who`
-	/// cannot be killed.
+	/// If the bond type is `Create`, `Staking::bond` is called, and `who` is allowed to be killed.
+	/// Otherwise, `Staking::bond_extra` is called and `who` cannot be killed.
 	///
 	/// Returns `Ok(points_issues)`, `Err` otherwise.
 	fn try_bond_funds(
@@ -934,6 +933,9 @@ impl<T: Config> BondedPool<T> {
 			BondType::Later => T::Staking::bond_extra(&bonded_account, amount)?,
 		}
 
+		// the funds in nomination pools are not usable for governance, ergo marked as deactive
+		T::Currency::deactivate(amount);
+
 		Ok(points_issued)
 	}
 
@@ -1835,6 +1837,9 @@ pub mod pallet {
 			)
 			.defensive()?;
 
+			// re-activate the funds.
+			T::Currency::reactivate(balance_to_unbond);
+
 			Self::deposit_event(Event::<T>::Withdrawn {
 				member: member_account.clone(),
 				pool_id: member.pool_id,
@@ -2586,6 +2591,7 @@ impl<T: Config> OnStakerSlash<T::AccountId, BalanceOf<T>> for Pallet<T> {
 		// anything here.
 		slashed_bonded: BalanceOf<T>,
 		slashed_unlocking: &BTreeMap<EraIndex, BalanceOf<T>>,
+		total_slashed: BalanceOf<T>,
 	) {
 		if let Some(pool_id) = ReversePoolIdLookup::<T>::get(pool_account) {
 			let mut sub_pools = match SubPoolsStorage::<T>::get(pool_id).defensive() {
@@ -2606,5 +2612,9 @@ impl<T: Config> OnStakerSlash<T::AccountId, BalanceOf<T>> for Pallet<T> {
 			Self::deposit_event(Event::<T>::PoolSlashed { pool_id, balance: slashed_bonded });
 			SubPoolsStorage::<T>::insert(pool_id, sub_pools);
 		}
+
+		// this will simply subtract the amount that was slashed from `ActiveIssuance`, so that when
+		// we calculate `total_issuance - active_issuance`, it remains accurate.
+		T::Currency::reactivate(total_slashed);
 	}
 }
diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs
index 7d5d418bbf..a347451396 100644
--- a/frame/nomination-pools/src/tests.rs
+++ b/frame/nomination-pools/src/tests.rs
@@ -48,6 +48,10 @@ fn test_setup_works() {
 		assert_eq!(PoolMembers::<Runtime>::count(), 1);
 		assert_eq!(StakingMock::bonding_duration(), 3);
 		assert!(Metadata::<T>::contains_key(1));
+		// all initial balances.
+		assert_eq!(Balances::total_issuance(), 50);
+		// the 10 units in pool.
+		assert_eq!(Balances::inactive_issuance(), 10);
 
 		let last_pool = LastPoolId::<Runtime>::get();
 		assert_eq!(
diff --git a/frame/root-offences/src/mock.rs b/frame/root-offences/src/mock.rs
index 273fbf6141..e0b2eb7480 100644
--- a/frame/root-offences/src/mock.rs
+++ b/frame/root-offences/src/mock.rs
@@ -145,17 +145,6 @@ impl onchain::Config for OnChainSeqPhragmen {
 	type TargetsBound = ConstU32<{ u32::MAX }>;
 }
 
-pub struct OnStakerSlashMock<T: Config>(core::marker::PhantomData<T>);
-impl<T: Config> sp_staking::OnStakerSlash<AccountId, Balance> for OnStakerSlashMock<T> {
-	fn on_slash(
-		_pool_account: &AccountId,
-		slashed_bonded: Balance,
-		slashed_chunks: &BTreeMap<EraIndex, Balance>,
-	) {
-		LedgerSlashPerEra::set((slashed_bonded, slashed_chunks.clone()));
-	}
-}
-
 parameter_types! {
 	pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE;
 	pub static Offset: BlockNumber = 0;
@@ -163,7 +152,6 @@ parameter_types! {
 	pub static SessionsPerEra: SessionIndex = 3;
 	pub static SlashDeferDuration: EraIndex = 0;
 	pub const BondingDuration: EraIndex = 3;
-	pub static LedgerSlashPerEra: (BalanceOf<Test>, BTreeMap<EraIndex, BalanceOf<Test>>) = (Zero::zero(), BTreeMap::new());
 	pub const OffendingValidatorsThreshold: Perbill = Perbill::from_percent(75);
 }
 
@@ -192,7 +180,7 @@ impl pallet_staking::Config for Test {
 	type MaxUnlockingChunks = ConstU32<32>;
 	type HistoryDepth = ConstU32<84>;
 	type VoterList = pallet_staking::UseNominatorsAndValidatorsMap<Self>;
-	type OnStakerSlash = OnStakerSlashMock<Test>;
+	type OnStakerSlash = ();
 	type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig;
 	type WeightInfo = ();
 }
diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs
index 3672056534..dfc97ad4a3 100644
--- a/frame/staking/src/lib.rs
+++ b/frame/staking/src/lib.rs
@@ -678,8 +678,9 @@ impl<T: Config> StakingLedger<T> {
 		// clean unlocking chunks that are set to zero.
 		self.unlocking.retain(|c| !c.value.is_zero());
 
-		T::OnStakerSlash::on_slash(&self.stash, self.active, &slashed_unlocking);
-		pre_slash_total.saturating_sub(self.total)
+		let total_slashed = pre_slash_total.saturating_sub(self.total);
+		T::OnStakerSlash::on_slash(&self.stash, self.active, &slashed_unlocking, total_slashed);
+		total_slashed
 	}
 }
 
diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs
index 9eb4a4890c..ba36cf3cd7 100644
--- a/primitives/staking/src/lib.rs
+++ b/primitives/staking/src/lib.rs
@@ -42,15 +42,17 @@ pub trait OnStakerSlash<AccountId, Balance> {
 	/// * `slashed_active` - The new bonded balance of the staker after the slash was applied.
 	/// * `slashed_unlocking` - A map of slashed eras, and the balance of that unlocking chunk after
 	///   the slash is applied. Any era not present in the map is not affected at all.
+	/// * `total_slashed` - The total amount that was slashed.
 	fn on_slash(
 		stash: &AccountId,
 		slashed_active: Balance,
 		slashed_unlocking: &BTreeMap<EraIndex, Balance>,
+		total_slashed: Balance,
 	);
 }
 
 impl<AccountId, Balance> OnStakerSlash<AccountId, Balance> for () {
-	fn on_slash(_: &AccountId, _: Balance, _: &BTreeMap<EraIndex, Balance>) {
+	fn on_slash(_: &AccountId, _: Balance, _: &BTreeMap<EraIndex, Balance>, _: Balance) {
 		// Nothing to do here
 	}
 }

[kianenigma]

I want to still pursue this as a PR to track TVL of Pools.

[kianenigma]

We now have the nomination pool funds tracked in a storage item (#1322). All that is left to do is to make sure this amount of funds is marked as deactivated upon each update.

Marking @gpestana as the mentor.

[xlc]

why the funds in nomination pool are inactive?

[Doordashcon]

I am guessing pool funds can be reactivated when required to be used in governance?

This accurately sums the active balance in the eco.

[xlc]

what exactly does active mean? tokens that can be used for governance?

[Doordashcon]

what exactly does active mean? tokens that can be used for governance?

from this PR, that seems to be the case.

[xlc]

I am not sure if it says anything explicitly about how this applies to Polkadot so I need a proper confirmation.

[kianenigma]

I am not sure if it says anything explicitly about how this applies to Polkadot so I need a proper confirmation.

The explanation provided is correct. The funds are not usable in governance and are therefore inactive. I am going to close this actually, as it is not really needed soon. #2680 is not that far anymore.