diff --git a/frame/nomination-pools/Cargo.toml b/frame/nomination-pools/Cargo.toml index e0158215e797a..29ca30e762bd1 100644 --- a/frame/nomination-pools/Cargo.toml +++ b/frame/nomination-pools/Cargo.toml @@ -35,8 +35,13 @@ pallet-balances = { version = "4.0.0-dev", path = "../balances" } sp-tracing = { version = "10.0.0", path = "../../primitives/tracing" } [features] -default = [ "std" ] -fuzzing = [ "pallet-balances", "sp-tracing" ] +default = ["std"] +# Enable `VersionedRuntimeUpgrade` for the migrations that is currently still experimental. +experimental = [ + "frame-support/experimental" +] +fuzzing = ["pallet-balances", "sp-tracing"] + std = [ "codec/std", "frame-support/std", diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index c4bebc5a1d030..59a6df905270b 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -535,6 +535,35 @@ impl PoolMember { } } + /// Total balance of the member, both active and unbonding. + /// Doesn't mutate state. + #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] + fn total_balance(&mut self) -> BalanceOf { + let pool = match BondedPool::::get(self.pool_id).defensive() { + Some(pool) => pool, + None => return Zero::zero(), + }; + + let active_balance = pool.points_to_balance(self.active_points()); + + let sub_pools = match SubPoolsStorage::::get(self.pool_id) { + Some(sub_pools) => sub_pools, + None => return active_balance, + }; + + let unbonding_balance = self.unbonding_eras.iter().fold( + BalanceOf::::zero(), + |accumulator, (era, unlocked_points)| { + // if the [`SubPools::with_era`] has already been merged into the + // [`SubPools::no_era`] use this pool instead. + let era_pool = sub_pools.with_era.get(era).unwrap_or(&sub_pools.no_era); + accumulator.saturating_add(era_pool.point_to_balance(*unlocked_points)) + }, + ); + + active_balance.saturating_add(unbonding_balance) + } + /// Total points of this member, both active and unbonding. fn total_points(&self) -> BalanceOf { self.active_points().saturating_add(self.unbonding_points()) @@ -963,6 +992,7 @@ impl BondedPool { } /// Issue points to [`Self`] for `new_funds`. + /// Increase the [`TotalValueLocked`] by `new_funds`. fn issue(&mut self, new_funds: BalanceOf) -> BalanceOf { let points_to_issue = self.balance_to_point(new_funds); self.points = self.points.saturating_add(points_to_issue); @@ -1183,9 +1213,8 @@ impl BondedPool { /// 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 is [`BondType::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( @@ -1216,6 +1245,9 @@ impl BondedPool { // found, we exit early. BondType::Later => T::Staking::bond_extra(&bonded_account, amount)?, } + TotalValueLocked::::mutate(|tvl| { + tvl.saturating_accrue(amount); + }); Ok(points_issued) } @@ -1231,6 +1263,19 @@ impl BondedPool { }); }; } + + fn withdraw_from_staking(&self, num_slashing_spans: u32) -> Result { + let bonded_account = self.bonded_account(); + + let prev_total = T::Staking::total_stake(&bonded_account.clone()).unwrap_or_default(); + let outcome = T::Staking::withdraw_unbonded(bonded_account.clone(), num_slashing_spans); + let diff = + prev_total.saturating_sub(T::Staking::total_stake(&bonded_account).unwrap_or_default()); + TotalValueLocked::::mutate(|tvl| { + tvl.saturating_reduce(diff); + }); + outcome + } } /// A reward pool. @@ -1416,8 +1461,8 @@ impl UnbondPool { new_points } - /// Dissolve some points from the unbonding pool, reducing the balance of the pool - /// proportionally. + /// Dissolve some points from the unbonding pool, reducing the balance of the pool and the + /// [`TotalValueLocked`] proportionally. /// /// This is the opposite of `issue`. /// @@ -1504,7 +1549,7 @@ pub mod pallet { use sp_runtime::Perbill; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(5); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(6); #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] @@ -1577,6 +1622,14 @@ pub mod pallet { type MaxUnbonding: Get; } + /// The sum of funds across all pools. + /// + /// This might be higher but never lower than the actual sum of the currently unlocking and + /// bonded funds as this is only decreased if a user withdraws unlocked funds or a slash + /// happened. + #[pallet::storage] + pub type TotalValueLocked = StorageValue<_, BalanceOf, ValueQuery>; + /// Minimum amount to bond to join a pool. #[pallet::storage] pub type MinJoinBond = StorageValue<_, BalanceOf, ValueQuery>; @@ -1795,9 +1848,9 @@ pub mod pallet { CannotWithdrawAny, /// The amount does not meet the minimum bond to either join or create a pool. /// - /// The depositor can never unbond to a value less than - /// `Pallet::depositor_min_bond`. The caller does not have nominating - /// permissions for the pool. Members can never unbond to a value below `MinJoinBond`. + /// The depositor can never unbond to a value less than `Pallet::depositor_min_bond`. The + /// caller does not have nominating permissions for the pool. Members can never unbond to a + /// value below `MinJoinBond`. MinimumBondNotMet, /// The transaction could not be executed due to overflow risk for the pool. OverflowRisk, @@ -2074,7 +2127,7 @@ pub mod pallet { /// Call `withdraw_unbonded` for the pools account. This call can be made by any account. /// - /// This is useful if their are too many unlocking chunks to call `unbond`, and some + /// This is useful if there are too many unlocking chunks to call `unbond`, and some /// can be cleared by withdrawing. In the case there are too many unlocking chunks, the user /// would probably see an error like `NoMoreChunks` emitted from the staking system when /// they attempt to unbond. @@ -2087,10 +2140,12 @@ pub mod pallet { ) -> DispatchResult { let _ = ensure_signed(origin)?; let pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; + // For now we only allow a pool to withdraw unbonded if its not destroying. If the pool // is destroying then `withdraw_unbonded` can be used. ensure!(pool.state != PoolState::Destroying, Error::::NotDestroying); - T::Staking::withdraw_unbonded(pool.bonded_account(), num_slashing_spans)?; + pool.withdraw_from_staking(num_slashing_spans)?; + Ok(()) } @@ -2141,8 +2196,7 @@ pub mod pallet { // Before calculating the `balance_to_unbond`, we call withdraw unbonded to ensure the // `transferrable_balance` is correct. - let stash_killed = - T::Staking::withdraw_unbonded(bonded_pool.bonded_account(), num_slashing_spans)?; + let stash_killed = bonded_pool.withdraw_from_staking(num_slashing_spans)?; // defensive-only: the depositor puts enough funds into the stash so that it will only // be destroyed when they are leaving. @@ -2796,9 +2850,12 @@ impl Pallet { } // Equivalent of (current_balance / current_points) * points - balance(u256(current_balance).saturating_mul(u256(points))) - // We check for zero above - .div(current_points) + balance( + u256(current_balance) + .saturating_mul(u256(points)) + // We check for zero above + .div(u256(current_points)), + ) } /// If the member has some rewards, transfer a payout from the reward pool to the member. @@ -3169,8 +3226,35 @@ impl Pallet { "depositor must always have MinCreateBond stake in the pool, except for when the \ pool is being destroyed and the depositor is the last member", ); + + let expected_tvl: BalanceOf = BondedPools::::iter() + .map(|(id, inner)| { + T::Staking::total_stake( + &BondedPool { id, inner: inner.clone() }.bonded_account(), + ) + .unwrap_or_default() + }) + .reduce(|acc, total_balance| acc + total_balance) + .unwrap_or_default(); + + assert_eq!( + TotalValueLocked::::get(), + expected_tvl, + "TVL deviates from the actual sum of funds of all Pools." + ); + + let total_balance_members: BalanceOf = PoolMembers::::iter() + .map(|(_, mut member)| member.total_balance()) + .reduce(|acc, total_balance| acc + total_balance) + .unwrap_or_default(); + + assert!( + TotalValueLocked::::get() <= total_balance_members, + "TVL must be equal to or less than the total balance of all PoolMembers." + ); Ok(()) })?; + ensure!( MaxPoolMembers::::get().map_or(true, |max| all_members <= max), Error::::MaxPoolMembers @@ -3269,25 +3353,30 @@ impl sp_staking::OnStakingUpdate> for Pall // anything here. slashed_bonded: BalanceOf, slashed_unlocking: &BTreeMap>, + total_slashed: BalanceOf, ) { - if let Some(pool_id) = ReversePoolIdLookup::::get(pool_account) { - let mut sub_pools = match SubPoolsStorage::::get(pool_id).defensive() { - Some(sub_pools) => sub_pools, - None => return, + if let Some(pool_id) = ReversePoolIdLookup::::get(pool_account).defensive() { + match SubPoolsStorage::::get(pool_id) { + Some(mut sub_pools) => { + for (era, slashed_balance) in slashed_unlocking.iter() { + if let Some(pool) = sub_pools.with_era.get_mut(era) { + pool.balance = *slashed_balance; + Self::deposit_event(Event::::UnbondingPoolSlashed { + era: *era, + pool_id, + balance: *slashed_balance, + }); + } + } + SubPoolsStorage::::insert(pool_id, sub_pools); + }, + None => {}, }; - for (era, slashed_balance) in slashed_unlocking.iter() { - if let Some(pool) = sub_pools.with_era.get_mut(era) { - pool.balance = *slashed_balance; - Self::deposit_event(Event::::UnbondingPoolSlashed { - era: *era, - pool_id, - balance: *slashed_balance, - }); - } - } + TotalValueLocked::::mutate(|tvl| { + tvl.saturating_reduce(total_slashed); + }); Self::deposit_event(Event::::PoolSlashed { pool_id, balance: slashed_bonded }); - SubPoolsStorage::::insert(pool_id, sub_pools); } } } diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 2ae4cd1b86857..dea0ee8bcf726 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -724,4 +724,103 @@ pub mod v5 { Ok(()) } } + + /// This migration accumulates and initializes the [`TotalValueLocked`] for all pools. + pub struct VersionUncheckedMigrateV5ToV6(sp_std::marker::PhantomData); + impl OnRuntimeUpgrade for VersionUncheckedMigrateV5ToV6 { + fn on_runtime_upgrade() -> Weight { + let migrated = BondedPools::::count(); + // The TVL should be the sum of all the funds that are actively staked and in the + // unbonding process of the account of each pool. + let tvl: BalanceOf = BondedPools::::iter() + .map(|(id, inner)| { + T::Staking::total_stake( + &BondedPool { id, inner: inner.clone() }.bonded_account(), + ) + .unwrap_or_default() + }) + .reduce(|acc, total_balance| acc + total_balance) + .unwrap_or_default(); + + TotalValueLocked::::set(tvl); + + log!(info, "Upgraded {} pools with a TVL of {:?}", migrated, tvl); + + // reads: migrated * (BondedPools + Staking::total_stake) + count + onchain + // version + // + // writes: current version + TVL + T::DbWeight::get().reads_writes(migrated.saturating_mul(2).saturating_add(2).into(), 2) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, TryRuntimeError> { + ensure!( + PoolMembers::::iter_keys().count() == PoolMembers::::iter_values().count(), + "There are undecodable PoolMembers in storage. This migration will not fix that." + ); + ensure!( + BondedPools::::iter_keys().count() == BondedPools::::iter_values().count(), + "There are undecodable BondedPools in storage. This migration will not fix that." + ); + ensure!( + SubPoolsStorage::::iter_keys().count() == + SubPoolsStorage::::iter_values().count(), + "There are undecodable SubPools in storage. This migration will not fix that." + ); + ensure!( + Metadata::::iter_keys().count() == Metadata::::iter_values().count(), + "There are undecodable Metadata in storage. This migration will not fix that." + ); + + Ok(Vec::new()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_data: Vec) -> Result<(), TryRuntimeError> { + // ensure [`TotalValueLocked`] contains a value greater zero if any instances of + // BondedPools exist. + if !BondedPools::::count().is_zero() { + ensure!(!TotalValueLocked::::get().is_zero(), "tvl written incorrectly"); + } + + ensure!( + Pallet::::on_chain_storage_version() >= 6, + "nomination-pools::migration::v6: wrong storage version" + ); + + // These should not have been touched - just in case. + ensure!( + PoolMembers::::iter_keys().count() == PoolMembers::::iter_values().count(), + "There are undecodable PoolMembers in storage." + ); + ensure!( + BondedPools::::iter_keys().count() == BondedPools::::iter_values().count(), + "There are undecodable BondedPools in storage." + ); + ensure!( + SubPoolsStorage::::iter_keys().count() == + SubPoolsStorage::::iter_values().count(), + "There are undecodable SubPools in storage." + ); + ensure!( + Metadata::::iter_keys().count() == Metadata::::iter_values().count(), + "There are undecodable Metadata in storage." + ); + + Ok(()) + } + } + + /// [`VersionUncheckedMigrateV5ToV6`] wrapped in a + /// [`frame_support::migrations::VersionedRuntimeUpgrade`], ensuring the migration is only + /// performed when on-chain version is 5. + #[cfg(feature = "experimental")] + pub type VersionCheckedMigrateV5ToV6 = frame_support::migrations::VersionedRuntimeUpgrade< + 5, + 6, + VersionUncheckedMigrateV5ToV6, + crate::pallet::Pallet, + ::DbWeight, + >; } diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 7d0d729a40d41..77f63c8c1e472 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -3,7 +3,7 @@ use crate::{self as pools}; use frame_support::{assert_ok, parameter_types, PalletId}; use frame_system::RawOrigin; use sp_runtime::{BuildStorage, FixedU128}; -use sp_staking::Stake; +use sp_staking::{OnStakingUpdate, Stake}; pub type BlockNumber = u64; pub type AccountId = u128; @@ -28,7 +28,8 @@ parameter_types! { pub static CurrentEra: EraIndex = 0; pub static BondingDuration: EraIndex = 3; pub storage BondedBalanceMap: BTreeMap = Default::default(); - pub storage UnbondingBalanceMap: BTreeMap = Default::default(); + // map from user, to a vec of era to amount being unlocked in that era. + pub storage UnbondingBalanceMap: BTreeMap> = Default::default(); #[derive(Clone, PartialEq)] pub static MaxUnbonding: u32 = 8; pub static StakingMinBond: Balance = 10; @@ -42,6 +43,14 @@ impl StakingMock { x.insert(who, bonded); BondedBalanceMap::set(&x) } + + pub fn slash_to(pool_id: PoolId, amount: Balance) { + let acc = Pools::create_bonded_account(pool_id); + let bonded = BondedBalanceMap::get(); + let pre_total = bonded.get(&acc).unwrap(); + Self::set_bonded_balance(acc, pre_total - amount); + Pools::on_slash(&acc, pre_total - amount, &Default::default(), amount); + } } impl sp_staking::StakingInterface for StakingMock { @@ -87,8 +96,11 @@ impl sp_staking::StakingInterface for StakingMock { let mut x = BondedBalanceMap::get(); *x.get_mut(who).unwrap() = x.get_mut(who).unwrap().saturating_sub(amount); BondedBalanceMap::set(&x); + + let era = Self::current_era(); + let unlocking_at = era + Self::bonding_duration(); let mut y = UnbondingBalanceMap::get(); - *y.entry(*who).or_insert(Self::Balance::zero()) += amount; + y.entry(*who).or_insert(Default::default()).push((unlocking_at, amount)); UnbondingBalanceMap::set(&y); Ok(()) } @@ -98,11 +110,13 @@ impl sp_staking::StakingInterface for StakingMock { } fn withdraw_unbonded(who: Self::AccountId, _: u32) -> Result { - // Simulates removing unlocking chunks and only having the bonded balance locked - let mut x = UnbondingBalanceMap::get(); - x.remove(&who); - UnbondingBalanceMap::set(&x); + let mut unbonding_map = UnbondingBalanceMap::get(); + let staker_map = unbonding_map.get_mut(&who).ok_or("not a staker")?; + + let current_era = Self::current_era(); + staker_map.retain(|(unlocking_at, _amount)| *unlocking_at > current_era); + UnbondingBalanceMap::set(&unbonding_map); Ok(UnbondingBalanceMap::get().is_empty() && BondedBalanceMap::get().is_empty()) } @@ -126,14 +140,17 @@ impl sp_staking::StakingInterface for StakingMock { } fn stake(who: &Self::AccountId) -> Result, DispatchError> { - match ( - UnbondingBalanceMap::get().get(who).copied(), - BondedBalanceMap::get().get(who).copied(), - ) { + match (UnbondingBalanceMap::get().get(who), BondedBalanceMap::get().get(who).copied()) { (None, None) => Err(DispatchError::Other("balance not found")), - (Some(v), None) => Ok(Stake { total: v, active: 0 }), + (Some(v), None) => Ok(Stake { + total: v.into_iter().fold(0u128, |acc, &x| acc.saturating_add(x.1)), + active: 0, + }), (None, Some(v)) => Ok(Stake { total: v, active: v }), - (Some(a), Some(b)) => Ok(Stake { total: a + b, active: b }), + (Some(a), Some(b)) => Ok(Stake { + total: a.into_iter().fold(0u128, |acc, &x| acc.saturating_add(x.1)) + b, + active: b, + }), } } diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index d0fe4e40a18b7..9f64c6e5e655b 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -59,6 +59,9 @@ fn test_setup_works() { assert_eq!(StakingMock::bonding_duration(), 3); assert!(Metadata::::contains_key(1)); + // initial member. + assert_eq!(TotalValueLocked::::get(), 10); + let last_pool = LastPoolId::::get(); assert_eq!( BondedPool::::get(last_pool).unwrap(), @@ -218,10 +221,7 @@ mod bonded_pool { // slash half of the pool's balance. expected result of `fn api_points_to_balance` // to be 1/2 of the pool's balance. - StakingMock::set_bonded_balance( - default_bonded_account(), - Pools::depositor_min_bond() / 2, - ); + StakingMock::slash_to(1, Pools::depositor_min_bond() / 2); assert_eq!(Pallet::::api_points_to_balance(1, 10), 5); // if pool does not exist, points to balance ratio is 0. @@ -238,10 +238,7 @@ mod bonded_pool { // slash half of the pool's balance. expect result of `fn api_balance_to_points` // to be 2 * of the balance to add to the pool. - StakingMock::set_bonded_balance( - default_bonded_account(), - Pools::depositor_min_bond() / 2, - ); + StakingMock::slash_to(1, Pools::depositor_min_bond() / 2); assert_eq!(Pallet::::api_balance_to_points(1, 10), 20); // if pool does not exist, balance to points ratio is 0. @@ -499,12 +496,12 @@ mod join { // Given Balances::make_free_balance_be(&11, ExistentialDeposit::get() + 2); assert!(!PoolMembers::::contains_key(11)); + assert_eq!(TotalValueLocked::::get(), 10); // When assert_ok!(Pools::join(RuntimeOrigin::signed(11), 2, 1)); // Then - assert_eq!( pool_events_since_last_call(), vec![ @@ -513,6 +510,7 @@ mod join { Event::Bonded { member: 11, pool_id: 1, bonded: 2, joined: true }, ] ); + assert_eq!(TotalValueLocked::::get(), 12); assert_eq!( PoolMembers::::get(11).unwrap(), @@ -536,6 +534,7 @@ mod join { pool_events_since_last_call(), vec![Event::Bonded { member: 12, pool_id: 1, bonded: 12, joined: true }] ); + assert_eq!(TotalValueLocked::::get(), 24); assert_eq!( PoolMembers::::get(12).unwrap(), @@ -2221,11 +2220,15 @@ mod unbond { .min_join_bond(10) .add_members(vec![(20, 20)]) .build_and_execute(|| { + assert_eq!(TotalValueLocked::::get(), 30); // can unbond to above limit assert_ok!(Pools::unbond(RuntimeOrigin::signed(20), 20, 5)); assert_eq!(PoolMembers::::get(20).unwrap().active_points(), 15); assert_eq!(PoolMembers::::get(20).unwrap().unbonding_points(), 5); + // tvl remains unchanged. + assert_eq!(TotalValueLocked::::get(), 30); + // cannot go to below 10: assert_noop!( Pools::unbond(RuntimeOrigin::signed(20), 20, 10), @@ -2531,8 +2534,8 @@ mod unbond { .add_members(vec![(40, 40), (550, 550)]) .build_and_execute(|| { let ed = Balances::minimum_balance(); - // Given a slash from 600 -> 100 - StakingMock::set_bonded_balance(default_bonded_account(), 100); + // Given a slash from 600 -> 500 + StakingMock::slash_to(1, 500); // and unclaimed rewards of 600. Balances::make_free_balance_be(&default_reward_account(), ed + 600); @@ -2564,8 +2567,9 @@ mod unbond { Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, Event::Bonded { member: 40, pool_id: 1, bonded: 40, joined: true }, Event::Bonded { member: 550, pool_id: 1, bonded: 550, joined: true }, + Event::PoolSlashed { pool_id: 1, balance: 100 }, Event::PaidOut { member: 40, pool_id: 1, payout: 40 }, - Event::Unbonded { member: 40, pool_id: 1, points: 6, balance: 6, era: 3 } + Event::Unbonded { member: 40, pool_id: 1, balance: 6, points: 6, era: 3 } ] ); @@ -2725,6 +2729,7 @@ mod unbond { ); // When the root kicks then its ok + // Account with ID 100 is kicked. assert_ok!(Pools::fully_unbond(RuntimeOrigin::signed(900), 100)); assert_eq!( @@ -2745,6 +2750,7 @@ mod unbond { ); // When the bouncer kicks then its ok + // Account with ID 200 is kicked. assert_ok!(Pools::fully_unbond(RuntimeOrigin::signed(902), 200)); assert_eq!( @@ -2783,7 +2789,7 @@ mod unbond { ); assert_eq!( *UnbondingBalanceMap::get().get(&default_bonded_account()).unwrap(), - 100 + 200 + vec![(3, 100), (3, 200)], ); }); } @@ -2882,7 +2888,10 @@ mod unbond { } ); assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 0); - assert_eq!(*UnbondingBalanceMap::get().get(&default_bonded_account()).unwrap(), 10); + assert_eq!( + *UnbondingBalanceMap::get().get(&default_bonded_account()).unwrap(), + vec![(6, 10)] + ); }); } @@ -3160,7 +3169,7 @@ mod unbond { assert_eq!(PoolMembers::::get(10).unwrap().unbonding_points(), 0); // slash the default pool - StakingMock::set_bonded_balance(Pools::create_bonded_account(1), 5); + StakingMock::slash_to(1, 5); // cannot unbond even 7, because the value of shares is now less. assert_noop!( @@ -3239,21 +3248,23 @@ mod pool_withdraw_unbonded { #[test] fn pool_withdraw_unbonded_works() { - ExtBuilder::default().build_and_execute(|| { + ExtBuilder::default().add_members(vec![(20, 10)]).build_and_execute(|| { // Given 10 unbond'ed directly against the pool account - assert_ok!(StakingMock::unbond(&default_bonded_account(), 5)); - // and the pool account only has 10 balance - assert_eq!(StakingMock::active_stake(&default_bonded_account()), Ok(5)); - assert_eq!(StakingMock::total_stake(&default_bonded_account()), Ok(10)); - assert_eq!(Balances::free_balance(&default_bonded_account()), 10); + + assert_ok!(Pools::unbond(RuntimeOrigin::signed(20), 20, 5)); + + assert_eq!(StakingMock::active_stake(&default_bonded_account()), Ok(15)); + assert_eq!(StakingMock::total_stake(&default_bonded_account()), Ok(20)); + assert_eq!(Balances::free_balance(&default_bonded_account()), 20); // When + CurrentEra::set(StakingMock::current_era() + StakingMock::bonding_duration() + 1); assert_ok!(Pools::pool_withdraw_unbonded(RuntimeOrigin::signed(10), 1, 0)); // Then there unbonding balance is no longer locked - assert_eq!(StakingMock::active_stake(&default_bonded_account()), Ok(5)); - assert_eq!(StakingMock::total_stake(&default_bonded_account()), Ok(5)); - assert_eq!(Balances::free_balance(&default_bonded_account()), 10); + assert_eq!(StakingMock::active_stake(&default_bonded_account()), Ok(15)); + assert_eq!(StakingMock::total_stake(&default_bonded_account()), Ok(15)); + assert_eq!(Balances::free_balance(&default_bonded_account()), 20); }); } } @@ -3290,8 +3301,9 @@ mod withdraw_unbonded { unbond_pool.balance /= 2; // 295 SubPoolsStorage::::insert(1, sub_pools); // Update the equivalent of the unbonding chunks for the `StakingMock` - let mut x = UnbondingBalanceMap::get(); - *x.get_mut(&default_bonded_account()).unwrap() /= 5; + let x = UnbondingBalanceMap::get(); + // TODO: + // *x.get_mut(&default_bonded_account()).unwrap() /= 5; UnbondingBalanceMap::set(&x); Balances::make_free_balance_be( &default_bonded_account(), @@ -3920,6 +3932,7 @@ mod withdraw_unbonded { #[test] fn full_multi_step_withdrawing_non_depositor() { ExtBuilder::default().add_members(vec![(100, 100)]).build_and_execute(|| { + assert_eq!(TotalValueLocked::::get(), 110); // given assert_ok!(Pools::unbond(RuntimeOrigin::signed(100), 100, 75)); assert_eq!( @@ -3927,6 +3940,9 @@ mod withdraw_unbonded { member_unbonding_eras!(3 => 75) ); + // tvl unchanged. + assert_eq!(TotalValueLocked::::get(), 110); + // progress one era and unbond the leftover. CurrentEra::set(1); assert_ok!(Pools::unbond(RuntimeOrigin::signed(100), 100, 25)); @@ -3939,6 +3955,8 @@ mod withdraw_unbonded { Pools::withdraw_unbonded(RuntimeOrigin::signed(100), 100, 0), Error::::CannotWithdrawAny ); + // tvl unchanged. + assert_eq!(TotalValueLocked::::get(), 110); // now the 75 should be free. CurrentEra::set(3); @@ -3958,6 +3976,8 @@ mod withdraw_unbonded { PoolMembers::::get(100).unwrap().unbonding_eras, member_unbonding_eras!(4 => 25) ); + // tvl updated + assert_eq!(TotalValueLocked::::get(), 35); // the 25 should be free now, and the member removed. CurrentEra::set(4); @@ -4266,6 +4286,7 @@ mod create { let next_pool_stash = Pools::create_bonded_account(2); let ed = Balances::minimum_balance(); + assert_eq!(TotalValueLocked::::get(), 10); assert!(!BondedPools::::contains_key(2)); assert!(!RewardPools::::contains_key(2)); assert!(!PoolMembers::::contains_key(11)); @@ -4279,6 +4300,7 @@ mod create { 456, 789 )); + assert_eq!(TotalValueLocked::::get(), 10 + StakingMock::minimum_nominator_bond()); assert_eq!(Balances::free_balance(&11), 0); assert_eq!( @@ -4560,9 +4582,10 @@ mod set_state { // Given unsafe_set_state(1, PoolState::Open); - let mut bonded_pool = BondedPool::::get(1).unwrap(); - bonded_pool.points = 100; - bonded_pool.put(); + // slash the pool to the point that `max_points_to_balance` ratio is + // surpassed. Making this pool destroyable by anyone. + StakingMock::slash_to(1, 10); + // When assert_ok!(Pools::set_state(RuntimeOrigin::signed(11), 1, PoolState::Destroying)); // Then @@ -4588,6 +4611,7 @@ mod set_state { pool_events_since_last_call(), vec![ Event::StateChanged { pool_id: 1, new_state: PoolState::Destroying }, + Event::PoolSlashed { pool_id: 1, balance: 0 }, Event::StateChanged { pool_id: 1, new_state: PoolState::Destroying }, Event::StateChanged { pool_id: 1, new_state: PoolState::Destroying } ] @@ -4788,6 +4812,7 @@ mod bond_extra { assert_eq!(BondedPools::::get(1).unwrap().points, 30); assert_eq!(Balances::free_balance(10), 35); assert_eq!(Balances::free_balance(20), 20); + assert_eq!(TotalValueLocked::::get(), 30); // when assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::Rewards)); @@ -4795,6 +4820,7 @@ mod bond_extra { // then assert_eq!(Balances::free_balance(10), 35); + assert_eq!(TotalValueLocked::::get(), 31); // 10's share of the reward is 1/3, since they gave 10/30 of the total shares. assert_eq!(PoolMembers::::get(10).unwrap().points, 10 + 1); assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 1); @@ -4804,6 +4830,7 @@ mod bond_extra { // then assert_eq!(Balances::free_balance(20), 20); + assert_eq!(TotalValueLocked::::get(), 33); // 20's share of the rewards is the other 2/3 of the rewards, since they have 20/30 of // the shares assert_eq!(PoolMembers::::get(20).unwrap().points, 20 + 2); @@ -5213,7 +5240,7 @@ mod reward_counter_precision { ); // slash this pool by 99% of that. - StakingMock::set_bonded_balance(default_bonded_account(), DOT + pool_bond / 100); + StakingMock::slash_to(1, pool_bond * 99 / 100); // some whale now joins with the other half ot the total issuance. This will trigger an // overflow. This test is actually a bit too lenient because all the reward counters are @@ -6727,3 +6754,74 @@ mod commission { }) } } +mod slash { + + use super::*; + + #[test] + fn slash_no_subpool_is_tracked() { + let bonded = |points, member_counter| BondedPool:: { + id: 1, + inner: BondedPoolInner { + commission: Commission::default(), + member_counter, + points, + roles: DEFAULT_ROLES, + state: PoolState::Open, + }, + }; + ExtBuilder::default().with_check(0).build_and_execute(|| { + // Given + Balances::make_free_balance_be(&11, ExistentialDeposit::get() + 2); + assert!(!PoolMembers::::contains_key(11)); + assert_eq!(TotalValueLocked::::get(), 10); + + // When + assert_ok!(Pools::join(RuntimeOrigin::signed(11), 2, 1)); + + // Then + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Created { depositor: 10, pool_id: 1 }, + Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::Bonded { member: 11, pool_id: 1, bonded: 2, joined: true }, + ] + ); + assert_eq!(TotalValueLocked::::get(), 12); + + assert_eq!( + PoolMembers::::get(11).unwrap(), + PoolMember:: { pool_id: 1, points: 2, ..Default::default() } + ); + assert_eq!(BondedPool::::get(1).unwrap(), bonded(12, 2)); + + // Given + // The bonded balance is slashed in half + StakingMock::slash_to(1, 6); + + // And + Balances::make_free_balance_be(&12, ExistentialDeposit::get() + 12); + assert!(!PoolMembers::::contains_key(12)); + + // When + assert_ok!(Pools::join(RuntimeOrigin::signed(12), 12, 1)); + + // Then + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::PoolSlashed { pool_id: 1, balance: 6 }, + Event::Bonded { member: 12, pool_id: 1, bonded: 12, joined: true } + ] + ); + assert_eq!(TotalValueLocked::::get(), 18); + + assert_eq!( + PoolMembers::::get(12).unwrap(), + PoolMember:: { pool_id: 1, points: 24, ..Default::default() } + ); + assert_eq!(BondedPool::::get(1).unwrap(), bonded(12 + 24, 3)); + }); + } +} diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index e59b2a3324a62..dcf57a4643233 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -671,8 +671,14 @@ impl StakingLedger { // clean unlocking chunks that are set to zero. self.unlocking.retain(|c| !c.value.is_zero()); - T::EventListeners::on_slash(&self.stash, self.active, &slashed_unlocking); - pre_slash_total.saturating_sub(self.total) + let final_slashed_amount = pre_slash_total.saturating_sub(self.total); + T::EventListeners::on_slash( + &self.stash, + self.active, + &slashed_unlocking, + final_slashed_amount, + ); + final_slashed_amount } } diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index cf08f8be1f27d..c41144278f2c1 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -278,6 +278,7 @@ impl OnStakingUpdate for EventListenerMock { _pool_account: &AccountId, slashed_bonded: Balance, slashed_chunks: &BTreeMap, + _total_slashed: Balance, ) { LedgerSlashPerEra::set((slashed_bonded, slashed_chunks.clone())); } diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 1621af164b375..8b5797d7918fb 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -121,10 +121,12 @@ pub trait OnStakingUpdate { /// * `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. + /// * `slashed_total` - The aggregated balance that was lost due to the slash. fn on_slash( _stash: &AccountId, _slashed_active: Balance, _slashed_unlocking: &BTreeMap, + _slashed_total: Balance, ) { } }