Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Commit

Permalink
Remove sanity module and some TODOs
Browse files Browse the repository at this point in the history
  • Loading branch information
emostov committed Mar 16, 2022
1 parent e0a09e0 commit bde07d5
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 31 deletions.
11 changes: 2 additions & 9 deletions frame/nomination-pools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,22 +553,17 @@ impl<T: Config> BondedPool<T> {
// We checked for zero above
.div(bonded_balance);

// TODO make sure these checks make sense. Taken from staking design chat with Al

// Pool points can inflate relative to balance, but only if the pool is slashed.
// If we cap the ratio of points:balance so one cannot join a pool that has been slashed
// 90%,

ensure!(points_to_balance_ratio_floor < 10u32.into(), Error::<T>::OverflowRisk);
// while restricting the balance to 1/10th of max total issuance,
// TODO instead of this, where we multiply and it could saturate, watch out for being close
// to the point of saturation.
ensure!(
bonded_balance < BalanceOf::<T>::max_value().div(10u32.into()),
Error::<T>::OverflowRisk
);
// then we can be decently confident the bonding pool points will not overflow
// `BalanceOf<T>`.
// `BalanceOf<T>`. Note that these are just heuristics.

Ok(())
}
Expand Down Expand Up @@ -834,7 +829,7 @@ impl<T: Config> SubPools<T> {
pub struct TotalUnbondingPools<T: Config>(PhantomData<T>);
impl<T: Config> Get<u32> for TotalUnbondingPools<T> {
fn get() -> u32 {
// TODO: This may be too dangerous in the scenario bonding_duration gets decreased because
// NOTE: this may be dangerous in the scenario bonding_duration gets decreased because
// we would no longer be able to decode `SubPoolsWithEra`, which uses `TotalUnbondingPools`
// as the bound
T::StakingInterface::bonding_duration() + T::PostUnbondingPoolsWindow::get()
Expand Down Expand Up @@ -1332,8 +1327,6 @@ pub mod pallet {
// Kill accounts from storage by making their balance go below ED. We assume that
// the accounts have no references that would prevent destruction once we get to
// this point.
// TODO: in correct scenario, these two accounts should be zero when we reach there
// anyway.
debug_assert_eq!(
T::Currency::free_balance(&bonded_pool.reward_account()),
Zero::zero()
Expand Down
19 changes: 0 additions & 19 deletions frame/nomination-pools/src/sanity.rs

This file was deleted.

3 changes: 0 additions & 3 deletions frame/nomination-pools/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,6 @@ mod join {
#[test]
#[should_panic = "Defensive failure has been triggered!"]
fn join_panics_when_reward_pool_not_found() {
// TODO: but we should fail defensively..
ExtBuilder::default().build_and_execute(|| {
StakingMock::set_bonded_balance(Pools::create_bonded_account(123), 100);
BondedPool::<Runtime> {
Expand Down Expand Up @@ -1658,8 +1657,6 @@ mod withdraw_unbonded_other {
assert_ok!(Pools::unbond_other(Origin::signed(10), 10));

let mut sub_pools = SubPoolsStorage::<Runtime>::get(1).unwrap();
// TODO: [now] in the future we could use StakingMock::unbond_era_for(current_era)
// instead of current_era + 3.
let unbond_pool = sub_pools.with_era.get_mut(&(current_era + 3)).unwrap();
// Sanity check
assert_eq!(*unbond_pool, UnbondPool { points: 10, balance: 10 });
Expand Down

0 comments on commit bde07d5

Please sign in to comment.