diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index b9b5c29f82694..30197ec718337 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -731,6 +731,7 @@ impl pallet_nomination_pools::Config for Runtime { type StakingInterface = pallet_staking::Pallet; type PostUnbondingPoolsWindow = PostUnbondPoolsWindow; type MaxMetadataLen = ConstU32<256>; + type MaxUnbonding = ConstU32<8>; type PalletId = NominationPoolsPalletId; } diff --git a/frame/nomination-pools/Cargo.toml b/frame/nomination-pools/Cargo.toml index 6565e695901e2..5e485e62f21d9 100644 --- a/frame/nomination-pools/Cargo.toml +++ b/frame/nomination-pools/Cargo.toml @@ -30,6 +30,7 @@ pallet-balances = { version = "4.0.0-dev", path = "../balances" } sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } [features] +runtime-benchmarks = [] default = ["std"] std = [ "codec/std", diff --git a/frame/nomination-pools/benchmarking/Cargo.toml b/frame/nomination-pools/benchmarking/Cargo.toml index 700e626e79ae0..8416216d12e94 100644 --- a/frame/nomination-pools/benchmarking/Cargo.toml +++ b/frame/nomination-pools/benchmarking/Cargo.toml @@ -24,7 +24,7 @@ frame-support = { version = "4.0.0-dev", default-features = false, path = "../.. frame-system = { version = "4.0.0-dev", default-features = false, path = "../../system" } pallet-bags-list = { version = "4.0.0-dev", default-features = false, features = ["runtime-benchmarks"], path = "../../bags-list" } pallet-staking = { version = "4.0.0-dev", default-features = false, features = ["runtime-benchmarks"], path = "../../staking" } -pallet-nomination-pools = { version = "1.0.0", default-features = false, path = "../" } +pallet-nomination-pools = { version = "1.0.0", default-features = false, path = "../", features = ["runtime-benchmarks"] } # Substrate Primitives sp-runtime = { version = "6.0.0", default-features = false, path = "../../../primitives/runtime" } diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index c2d118113b60b..50dd97d2cd1a5 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -289,13 +289,13 @@ frame_benchmarking::benchmarks! { .map_err(|_| "balance expected to be a u128") .unwrap(); let scenario = ListScenario::::new(origin_weight, false)?; - let amount = origin_weight - scenario.dest_weight.clone(); let scenario = scenario.add_joiner(amount); let delegator_id = scenario.origin1_delegator.unwrap().clone(); + let all_points = Delegators::::get(&delegator_id).unwrap().points; whitelist_account!(delegator_id); - }: _(Origin::Signed(delegator_id.clone()), delegator_id.clone()) + }: _(Origin::Signed(delegator_id.clone()), delegator_id.clone(), all_points) verify { let bonded_after = T::StakingInterface::active_stake(&scenario.origin1).unwrap(); // We at least went down to the destination bag @@ -304,7 +304,14 @@ frame_benchmarking::benchmarks! { &delegator_id ) .unwrap(); - assert_eq!(delegator.unbonding_era, Some(0 + T::StakingInterface::bonding_duration())); + assert_eq!( + delegator.unbonding_eras.keys().cloned().collect::>(), + vec![0 + T::StakingInterface::bonding_duration()] + ); + assert_eq!( + delegator.unbonding_eras.values().cloned().collect::>(), + vec![all_points] + ); } pool_withdraw_unbonded { @@ -329,7 +336,7 @@ frame_benchmarking::benchmarks! { assert_eq!(CurrencyOf::::free_balance(&joiner), min_join_bond); // Unbond the new delegator - Pools::::unbond(Origin::Signed(joiner.clone()).into(), joiner.clone()).unwrap(); + Pools::::fully_unbond(Origin::Signed(joiner.clone()).into(), joiner.clone()).unwrap(); // Sanity check that unbond worked assert_eq!( @@ -374,7 +381,7 @@ frame_benchmarking::benchmarks! { // Unbond the new delegator pallet_staking::CurrentEra::::put(0); - Pools::::unbond(Origin::Signed(joiner.clone()).into(), joiner.clone()).unwrap(); + Pools::::fully_unbond(Origin::Signed(joiner.clone()).into(), joiner.clone()).unwrap(); // Sanity check that unbond worked assert_eq!( @@ -423,7 +430,7 @@ frame_benchmarking::benchmarks! { // up when unbonding. let reward_account = Pools::::create_reward_account(1); assert!(frame_system::Account::::contains_key(&reward_account)); - Pools::::unbond(Origin::Signed(depositor.clone()).into(), depositor.clone()).unwrap(); + Pools::::fully_unbond(Origin::Signed(depositor.clone()).into(), depositor.clone()).unwrap(); // Sanity check that unbond worked assert_eq!( diff --git a/frame/nomination-pools/benchmarking/src/mock.rs b/frame/nomination-pools/benchmarking/src/mock.rs index b6a43471b8355..94b7f300099a4 100644 --- a/frame/nomination-pools/benchmarking/src/mock.rs +++ b/frame/nomination-pools/benchmarking/src/mock.rs @@ -155,6 +155,7 @@ impl pallet_nomination_pools::Config for Runtime { type StakingInterface = Staking; type PostUnbondingPoolsWindow = PostUnbondingPoolsWindow; type MaxMetadataLen = ConstU32<256>; + type MaxUnbonding = ConstU32<8>; type PalletId = PoolsPalletId; } diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 5b2c206d89031..47655454c5fde 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -309,18 +309,18 @@ use codec::Codec; use frame_support::{ - ensure, + defensive, ensure, pallet_prelude::{MaxEncodedLen, *}, storage::bounded_btree_map::BoundedBTreeMap, traits::{ Currency, Defensive, DefensiveOption, DefensiveResult, DefensiveSaturating, ExistenceRequirement, Get, }, - DefaultNoBound, RuntimeDebugNoBound, + CloneNoBound, DefaultNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; use scale_info::TypeInfo; use sp_core::U256; -use sp_runtime::traits::{AccountIdConversion, Bounded, Convert, Saturating, Zero}; +use sp_runtime::traits::{AccountIdConversion, Bounded, CheckedSub, Convert, Saturating, Zero}; use sp_staking::{EraIndex, OnStakerSlash, StakingInterface}; use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, ops::Div, vec::Vec}; @@ -382,8 +382,8 @@ enum AccountType { } /// A delegator in a pool. -#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebugNoBound)] -#[cfg_attr(feature = "std", derive(Clone, PartialEq))] +#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebugNoBound, CloneNoBound)] +#[cfg_attr(feature = "std", derive(PartialEqNoBound, DefaultNoBound))] #[codec(mel_bound(T: Config))] #[scale_info(skip_type_params(T))] pub struct Delegator { @@ -397,8 +397,96 @@ pub struct Delegator { /// This value lines up with the [`RewardPool::total_earnings`] after a delegator claims a /// payout. pub reward_pool_total_earnings: BalanceOf, - /// The era this delegator started unbonding at. - pub unbonding_era: Option, + /// The eras in which this delegator is unbonding, mapped from era index to the number of + /// points scheduled to unbond in the given era. + pub unbonding_eras: BoundedBTreeMap, T::MaxUnbonding>, +} + +impl Delegator { + #[cfg(any(test, debug_assertions))] + fn total_points(&self) -> BalanceOf { + self.active_points().saturating_add(self.unbonding_points()) + } + + /// Active balance of the delegator. + /// + /// This is derived from the ratio of points in the pool to which the delegator belongs to. + /// Might return different values based on the pool state for the same delegator and points. + pub(crate) fn active_balance(&self) -> BalanceOf { + if let Some(pool) = BondedPool::::get(self.pool_id).defensive() { + pool.points_to_balance(self.points) + } else { + Zero::zero() + } + } + + /// Active points of the delegator. + pub(crate) fn active_points(&self) -> BalanceOf { + self.points + } + + /// Inactive points of the delegator, waiting to be withdrawn. + pub(crate) fn unbonding_points(&self) -> BalanceOf { + self.unbonding_eras + .as_ref() + .iter() + .fold(BalanceOf::::zero(), |acc, (_, v)| acc.saturating_add(*v)) + } + + /// Try and unbond `points` from self, with the given target unbonding era. + /// + /// Returns `Ok(())` and updates `unbonding_eras` and `points` if success, `Err(_)` otherwise. + fn try_unbond( + &mut self, + points: BalanceOf, + unbonding_era: EraIndex, + ) -> Result<(), Error> { + if let Some(new_points) = self.points.checked_sub(&points) { + match self.unbonding_eras.get_mut(&unbonding_era) { + Some(already_unbonding_points) => + *already_unbonding_points = already_unbonding_points.saturating_add(points), + None => self + .unbonding_eras + .try_insert(unbonding_era, points) + .map(|old| { + if old.is_some() { + defensive!("value checked to not exist in the map; qed"); + } + }) + .map_err(|_| Error::::MaxUnbondingLimit)?, + } + self.points = new_points; + Ok(()) + } else { + Err(Error::::NotEnoughPointsToUnbond) + } + } + + /// Withdraw any funds in [`Self::unbonding_eras`] who's deadline in reached and is fully + /// unlocked. + /// + /// Returns a a subset of [`Self::unbonding_eras`] that got withdrawn. + /// + /// Infallible, noop if no unbonding eras exist. + fn withdraw_unlocked( + &mut self, + current_era: EraIndex, + ) -> BoundedBTreeMap, T::MaxUnbonding> { + // NOTE: if only drain-filter was stable.. + let mut removed_points = + BoundedBTreeMap::, T::MaxUnbonding>::default(); + self.unbonding_eras.retain(|e, p| { + if *e > current_era { + true + } else { + removed_points + .try_insert(*e, p.clone()) + .expect("source map is bounded, this is a subset, will be bounded; qed"); + false + } + }); + removed_points + } } /// A pool's possible states. @@ -510,28 +598,45 @@ impl BondedPool { BondedPools::::remove(self.id); } - /// Get the amount of points to issue for some new funds that will be bonded in the pool. - fn points_to_issue(&self, new_funds: BalanceOf) -> BalanceOf { + /// Convert the given amount of balance to points given the current pool state. + /// + /// This is often used for bonding and issuing new funds into the pool. + fn balance_to_point(&self, new_funds: BalanceOf) -> BalanceOf { let bonded_balance = T::StakingInterface::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); - Pallet::::points_to_issue(bonded_balance, self.points, new_funds) + Pallet::::balance_to_point(bonded_balance, self.points, new_funds) } - /// Get the amount of balance to unbond from the pool based on a delegator's points of the pool. - fn balance_to_unbond(&self, delegator_points: BalanceOf) -> BalanceOf { + /// Convert the given number of points to balance given the current pool state. + /// + /// This is often used for unbonding. + fn points_to_balance(&self, points: BalanceOf) -> BalanceOf { let bonded_balance = T::StakingInterface::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); - Pallet::::balance_to_unbond(bonded_balance, self.points, delegator_points) + Pallet::::point_to_balance(bonded_balance, self.points, points) } /// Issue points to [`Self`] for `new_funds`. fn issue(&mut self, new_funds: BalanceOf) -> BalanceOf { - let points_to_issue = self.points_to_issue(new_funds); + let points_to_issue = self.balance_to_point(new_funds); self.points = self.points.saturating_add(points_to_issue); - points_to_issue } + /// Dissolve some points from the pool i.e. unbond the given amount of points from this pool. + /// This is the opposite of issuing some funds into the pool. + /// + /// Mutates self in place, but does not write anything to storage. + /// + /// Returns the equivalent balance amount that actually needs to get unbonded. + fn dissolve(&mut self, points: BalanceOf) -> BalanceOf { + // NOTE: do not optimize by removing `balance`. it must be computed before mutating + // `self.point`. + let balance = self.points_to_balance(points); + self.points = self.points.saturating_sub(points); + balance + } + /// Increment the delegator counter. Ensures that the pool and system delegator limits are /// respected. fn try_inc_delegators(&mut self) -> Result<(), DispatchError> { @@ -582,6 +687,12 @@ impl BondedPool { matches!(self.state, PoolState::Destroying) } + fn is_destroying_and_only_depositor(&self, alleged_depositor_points: BalanceOf) -> bool { + // NOTE: if we add `&& self.delegator_counter == 1`, then this becomes even more strict and + // ensures that there are no unbonding delegators hanging around either. + self.is_destroying() && self.points == alleged_depositor_points + } + /// Whether or not the pool is ok to be in `PoolSate::Open`. If this returns an `Err`, then the /// pool is unrecoverable and should be in the destroying state. fn ok_to_be_open(&self, new_funds: BalanceOf) -> Result<(), DispatchError> { @@ -624,6 +735,7 @@ impl BondedPool { caller: &T::AccountId, target_account: &T::AccountId, target_delegator: &Delegator, + unbonding_points: BalanceOf, ) -> Result<(), DispatchError> { let is_permissioned = caller == target_account; let is_depositor = *target_account == self.roles.depositor; @@ -638,13 +750,26 @@ impl BondedPool { }, // Any delegator who is not the depositor can always unbond themselves (true, false) => (), - // The depositor can only start unbonding if the pool is already being destroyed and - // they are the delegator in the pool. Note that an invariant is once the pool is - // destroying it cannot switch states, so by being in destroying we are guaranteed no - // other delegators can possibly join. (_, true) => { - ensure!(target_delegator.points == self.points, Error::::NotOnlyDelegator); - ensure!(self.is_destroying(), Error::::NotDestroying); + if self.is_destroying_and_only_depositor(target_delegator.active_points()) { + // if the pool is about to be destroyed, anyone can unbond the depositor, and + // they can fully unbond. + } else { + // only the depositor can partially unbond, and they can only unbond up to the + // threshold. + ensure!(is_permissioned, Error::::DoesNotHavePermission); + let balance_after_unbond = { + let new_depositor_points = + target_delegator.active_points().saturating_sub(unbonding_points); + let mut depositor_after_unbond = (*target_delegator).clone(); + depositor_after_unbond.points = new_depositor_points; + depositor_after_unbond.active_balance() + }; + ensure!( + balance_after_unbond >= MinCreateBond::::get(), + Error::::NotOnlyDelegator + ); + } }, }; Ok(()) @@ -652,44 +777,21 @@ impl BondedPool { /// # Returns /// - /// * Ok(true) if [`Call::withdraw_unbonded`] can be called and the target account is the - /// depositor. - /// * Ok(false) if [`Call::withdraw_unbonded`] can be called and target account is *not* the - /// depositor. - /// * Err(DispatchError) if [`Call::withdraw_unbonded`] *cannot* be called. + /// * Ok(()) if [`Call::withdraw_unbonded`] can be called, `Err(DispatchError)` otherwise. fn ok_to_withdraw_unbonded_with( &self, caller: &T::AccountId, target_account: &T::AccountId, target_delegator: &Delegator, sub_pools: &SubPools, - ) -> Result { + ) -> Result<(), DispatchError> { if *target_account == self.roles.depositor { - // This is a depositor - if !sub_pools.no_era.points.is_zero() { - // Unbonded pool has some points, so if they are the last delegator they must be - // here. Since the depositor is the last to unbond, this should never be possible. - ensure!(sub_pools.with_era.len().is_zero(), Error::::NotOnlyDelegator); - ensure!( - sub_pools.no_era.points == target_delegator.points, - Error::::NotOnlyDelegator - ); - } else { - // No points in the `no_era` pool, so they must be in a `with_era` pool - // If there are no other delegators, this can be the only `with_era` pool since the - // depositor was the last to withdraw. This assumes with_era sub pools are destroyed - // whenever their points go to zero. - ensure!(sub_pools.with_era.len() == 1, Error::::NotOnlyDelegator); - sub_pools - .with_era - .values() - .next() - .filter(|only_unbonding_pool| { - only_unbonding_pool.points == target_delegator.points - }) - .ok_or(Error::::NotOnlyDelegator)?; - } - Ok(true) + ensure!( + sub_pools.sum_unbonding_points() == target_delegator.unbonding_points(), + Error::::NotOnlyDelegator + ); + debug_assert_eq!(self.delegator_counter, 1, "only delegator must exist at this point"); + Ok(()) } else { // This isn't a depositor let is_permissioned = caller == target_account; @@ -697,7 +799,7 @@ impl BondedPool { is_permissioned || self.can_kick(caller) || self.is_destroying(), Error::::NotKickerOrDestroying ); - Ok(false) + Ok(()) } } @@ -814,29 +916,46 @@ impl RewardPool { } } +/// An unbonding pool. This is always mapped with an era. #[derive(Encode, Decode, MaxEncodedLen, TypeInfo, DefaultNoBound, RuntimeDebugNoBound)] -#[cfg_attr(feature = "std", derive(Clone, PartialEq))] +#[cfg_attr(feature = "std", derive(Clone, PartialEq, Eq))] #[codec(mel_bound(T: Config))] #[scale_info(skip_type_params(T))] pub struct UnbondPool { + /// The points in this pool. points: BalanceOf, + /// The funds in the pool. balance: BalanceOf, } impl UnbondPool { - fn points_to_issue(&self, new_funds: BalanceOf) -> BalanceOf { - Pallet::::points_to_issue(self.balance, self.points, new_funds) + fn balance_to_point(&self, new_funds: BalanceOf) -> BalanceOf { + Pallet::::balance_to_point(self.balance, self.points, new_funds) } - fn balance_to_unbond(&self, delegator_points: BalanceOf) -> BalanceOf { - Pallet::::balance_to_unbond(self.balance, self.points, delegator_points) + fn point_to_balance(&self, points: BalanceOf) -> BalanceOf { + Pallet::::point_to_balance(self.balance, self.points, points) } /// Issue points and update the balance given `new_balance`. fn issue(&mut self, new_funds: BalanceOf) { - self.points = self.points.saturating_add(self.points_to_issue(new_funds)); + self.points = self.points.saturating_add(self.balance_to_point(new_funds)); self.balance = self.balance.saturating_add(new_funds); } + + /// Dissolve some points from the unbonding pool, reducing the balance of the pool + /// proportionally. + /// + /// This is the opposite of `issue`. + /// + /// Returns the actual amount of `Balance` that was removed from the pool. + fn dissolve(&mut self, points: BalanceOf) -> BalanceOf { + let balance_to_unbond = self.point_to_balance(points); + self.points = self.points.saturating_sub(points); + self.balance = self.balance.saturating_sub(balance_to_unbond); + + balance_to_unbond + } } #[derive(Encode, Decode, MaxEncodedLen, TypeInfo, DefaultNoBound, RuntimeDebugNoBound)] @@ -878,6 +997,25 @@ impl SubPools { self } + + /// The sum of all unbonding points, regardless of whether they are actually unlocked or not. + fn sum_unbonding_points(&self) -> BalanceOf { + self.no_era.points.saturating_add( + self.with_era + .values() + .fold(BalanceOf::::zero(), |acc, pool| acc.saturating_add(pool.points)), + ) + } + + /// The sum of all unbonding balance, regardless of whether they are actually unlocked or not. + #[cfg(any(test, debug_assertions))] + fn sum_unbonding_balance(&self) -> BalanceOf { + self.no_era.balance.saturating_add( + self.with_era + .values() + .fold(BalanceOf::::zero(), |acc, pool| acc.saturating_add(pool.balance)), + ) + } } /// The maximum amount of eras an unbonding pool can exist prior to being merged with the @@ -939,6 +1077,9 @@ pub mod pallet { /// The maximum length, in bytes, that a pools metadata maybe. type MaxMetadataLen: Get; + + /// The maximum number of simultaneous unbonding chunks that can exist per delegator. + type MaxUnbonding: Get; } /// Minimum amount to bond to join a pool. @@ -946,6 +1087,9 @@ pub mod pallet { pub type MinJoinBond = StorageValue<_, BalanceOf, ValueQuery>; /// Minimum bond required to create a pool. + /// + /// This is the amount that the depositor must put as their initial stake in the pool, as an + /// indication of "skin in the game". #[pallet::storage] pub type MinCreateBond = StorageValue<_, BalanceOf, ValueQuery>; @@ -1071,12 +1215,15 @@ pub mod pallet { AccountBelongsToOtherPool, /// The pool has insufficient balance to bond as a nominator. InsufficientBond, - /// The delegator is already unbonding. + /// The delegator is already unbonding in this era. AlreadyUnbonding, - /// The delegator is not unbonding and thus cannot withdraw funds. - NotUnbonding, - /// Unbonded funds cannot be withdrawn yet because the bonding duration has not passed. - NotUnbondedYet, + /// The delegator is fully unbonded (and thus cannot access the bonded and reward pool + /// anymore to, for example, collect rewards). + FullyUnbonding, + /// The delegator cannot unbond further chunks due to reaching the limit. + MaxUnbondingLimit, + /// None of the funds can be withdrawn yet because the bonding duration has not passed. + CannotWithdrawAny, /// The amount does not meet the minimum bond to either join or create a pool. MinimumBondNotMet, /// The transaction could not be executed due to overflow risk for the pool. @@ -1106,6 +1253,8 @@ pub mod pallet { /// Some error occurred that should never happen. This should be reported to the /// maintainers. DefensiveError, + /// Not enough points. Ty unbonding less. + NotEnoughPointsToUnbond, } #[pallet::call] @@ -1156,16 +1305,17 @@ pub mod pallet { // next 2 eras because their vote weight will not be counted until the // snapshot in active era + 1. reward_pool_total_earnings: reward_pool.total_earnings, - unbonding_era: None, + unbonding_eras: Default::default(), }, ); - bonded_pool.put(); + Self::deposit_event(Event::::Bonded { delegator: who, pool_id, bonded: amount, joined: true, }); + bonded_pool.put(); Ok(()) } @@ -1234,12 +1384,14 @@ pub mod pallet { Ok(()) } - /// Unbond _all_ of the `delegator_account`'s funds from the pool. + /// Unbond up to `unbonding_points` of the `delegator_account`'s funds from the pool. It + /// implicitly collects the rewards one last time, since not doing so would mean some + /// rewards would go forfeited. /// /// Under certain conditions, this call can be dispatched permissionlessly (i.e. by any /// account). /// - /// # Conditions for a permissionless dispatch + /// # Conditions for a permissionless dispatch. /// /// * The pool is blocked and the caller is either the root or state-toggler. This is /// refereed to as a kick. @@ -1247,7 +1399,7 @@ pub mod pallet { /// * The pool is destroying, the delegator is the depositor and no other delegators are in /// the pool. /// - /// # Conditions for permissioned dispatch (i.e. the caller is also the + /// ## Conditions for permissioned dispatch (i.e. the caller is also the /// `delegator_account`): /// /// * The caller is not the depositor. @@ -1262,12 +1414,21 @@ pub mod pallet { /// `NoMoreChunks` error from the staking system. #[pallet::weight(T::WeightInfo::unbond())] #[transactional] - pub fn unbond(origin: OriginFor, delegator_account: T::AccountId) -> DispatchResult { + pub fn unbond( + origin: OriginFor, + delegator_account: T::AccountId, + #[pallet::compact] unbonding_points: BalanceOf, + ) -> DispatchResult { let caller = ensure_signed(origin)?; let (mut delegator, mut bonded_pool, mut reward_pool) = Self::get_delegator_with_pools(&delegator_account)?; - bonded_pool.ok_to_unbond_with(&caller, &delegator_account, &delegator)?; + bonded_pool.ok_to_unbond_with( + &caller, + &delegator_account, + &delegator, + unbonding_points, + )?; // Claim the the payout prior to unbonding. Once the user is unbonding their points // no longer exist in the bonded pool and thus they can no longer claim their payouts. @@ -1279,17 +1440,16 @@ pub mod pallet { &mut reward_pool, )?; - let balance_to_unbond = bonded_pool.balance_to_unbond(delegator.points); + let current_era = T::StakingInterface::current_era(); + let unbond_era = T::StakingInterface::bonding_duration().saturating_add(current_era); - // Update the bonded pool. Note that we must do this *after* calculating the balance - // to unbond so we have the correct points for the balance:share ratio. - bonded_pool.points = bonded_pool.points.saturating_sub(delegator.points); + // Try and unbond in the delegator map. + delegator.try_unbond(unbonding_points, unbond_era)?; - // Unbond in the actual underlying pool - T::StakingInterface::unbond(bonded_pool.bonded_account(), balance_to_unbond)?; + // Unbond in the actual underlying nominator. + let unbonding_balance = bonded_pool.dissolve(unbonding_points); + T::StakingInterface::unbond(bonded_pool.bonded_account(), unbonding_balance)?; - let current_era = T::StakingInterface::current_era(); - let unbond_era = T::StakingInterface::bonding_duration().saturating_add(current_era); // Note that we lazily create the unbonding pools here if they don't already exist let mut sub_pools = SubPoolsStorage::::get(delegator.pool_id) .unwrap_or_default() @@ -1305,19 +1465,18 @@ pub mod pallet { // always enough space to insert. .defensive_map_err(|_| Error::::DefensiveError)?; } + sub_pools .with_era .get_mut(&unbond_era) // The above check ensures the pool exists. .defensive_ok_or_else(|| Error::::DefensiveError)? - .issue(balance_to_unbond); - - delegator.unbonding_era = Some(unbond_era); + .issue(unbonding_balance); Self::deposit_event(Event::::Unbonded { delegator: delegator_account.clone(), pool_id: delegator.pool_id, - amount: balance_to_unbond, + amount: unbonding_balance, }); // Now that we know everything has worked write the items to storage. @@ -1349,8 +1508,11 @@ pub mod pallet { Ok(()) } - /// Withdraw unbonded funds for the `target` delegator. Under certain conditions, - /// this call can be dispatched permissionlessly (i.e. by any account). + /// Withdraw unbonded funds from `delegator_account`. If no bonded funds can be unbonded, an + /// error is returned. + /// + /// Under certain conditions, this call can be dispatched permissionlessly (i.e. by any + /// account). /// /// # Conditions for a permissionless dispatch /// @@ -1375,59 +1537,55 @@ pub mod pallet { num_slashing_spans: u32, ) -> DispatchResultWithPostInfo { let caller = ensure_signed(origin)?; - let delegator = + let mut delegator = Delegators::::get(&delegator_account).ok_or(Error::::DelegatorNotFound)?; - let unbonding_era = delegator.unbonding_era.ok_or(Error::::NotUnbonding)?; let current_era = T::StakingInterface::current_era(); - ensure!(current_era >= unbonding_era, Error::::NotUnbondedYet); - let mut sub_pools = SubPoolsStorage::::get(delegator.pool_id) - .defensive_ok_or_else(|| Error::::SubPoolsNotFound)?; let bonded_pool = BondedPool::::get(delegator.pool_id) .defensive_ok_or_else(|| Error::::PoolNotFound)?; - let should_remove_pool = bonded_pool.ok_to_withdraw_unbonded_with( + let mut sub_pools = SubPoolsStorage::::get(delegator.pool_id) + .defensive_ok_or_else(|| Error::::SubPoolsNotFound)?; + + bonded_pool.ok_to_withdraw_unbonded_with( &caller, &delegator_account, &delegator, &sub_pools, )?; + // NOTE: must do this after we have done the `ok_to_withdraw_unbonded_other_with` check. + let withdrawn_points = delegator.withdraw_unlocked(current_era); + ensure!(!withdrawn_points.is_empty(), Error::::CannotWithdrawAny); + // Before calculate the `balance_to_unbond`, with call withdraw unbonded to ensure the - // `non_locked_balance` is correct. + // `transferrable_balance` is correct. T::StakingInterface::withdraw_unbonded( bonded_pool.bonded_account(), num_slashing_spans, )?; - let balance_to_unbond = - if let Some(pool) = sub_pools.with_era.get_mut(&unbonding_era) { - let balance_to_unbond = pool.balance_to_unbond(delegator.points); - pool.points = pool.points.saturating_sub(delegator.points); - pool.balance = pool.balance.saturating_sub(balance_to_unbond); - if pool.points.is_zero() { - // Clean up pool that is no longer used - sub_pools.with_era.remove(&unbonding_era); + let balance_to_unbond = withdrawn_points + .iter() + .fold(BalanceOf::::zero(), |accumulator, (era, unlocked_points)| { + if let Some(era_pool) = sub_pools.with_era.get_mut(&era) { + let balance_to_unbond = era_pool.dissolve(*unlocked_points); + if era_pool.points.is_zero() { + sub_pools.with_era.remove(&era); + } + accumulator.saturating_add(balance_to_unbond) + } else { + // A pool does not belong to this era, so it must have been merged to the + // era-less pool. + accumulator.saturating_add(sub_pools.no_era.dissolve(*unlocked_points)) } - - balance_to_unbond - } else { - // A pool does not belong to this era, so it must have been merged to the - // era-less pool. - let balance_to_unbond = sub_pools.no_era.balance_to_unbond(delegator.points); - sub_pools.no_era.points = - sub_pools.no_era.points.saturating_sub(delegator.points); - sub_pools.no_era.balance = - sub_pools.no_era.balance.saturating_sub(balance_to_unbond); - - balance_to_unbond - } + }) // A call to this function may cause the pool's stash to get dusted. If this happens // before the last delegator has withdrawn, then all subsequent withdraws will be 0. // However the unbond pools do no get updated to reflect this. In the aforementioned // scenario, this check ensures we don't try to withdraw funds that don't exist. // This check is also defensive in cases where the unbond pool does not update its - // balance (e.g. a bug in the slashing hook.) We gracefully proceed in - // order to ensure delegators can leave the pool and it can be destroyed. + // balance (e.g. a bug in the slashing hook.) We gracefully proceed in order to + // ensure delegators can leave the pool and it can be destroyed. .min(bonded_pool.transferrable_balance()); T::Currency::transfer( @@ -1436,44 +1594,32 @@ pub mod pallet { balance_to_unbond, ExistenceRequirement::AllowDeath, ) - .defensive_map_err(|e| e)?; + .defensive()?; + Self::deposit_event(Event::::Withdrawn { delegator: delegator_account.clone(), pool_id: delegator.pool_id, amount: balance_to_unbond, }); - let post_info_weight = if should_remove_pool { - let reward_account = bonded_pool.reward_account(); - ReversePoolIdLookup::::remove(bonded_pool.bonded_account()); - RewardPools::::remove(delegator.pool_id); - Self::deposit_event(Event::::Destroyed { pool_id: delegator.pool_id }); - SubPoolsStorage::::remove(delegator.pool_id); - // 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. - debug_assert_eq!( - T::StakingInterface::total_stake(&bonded_pool.bonded_account()) - .unwrap_or_default(), - Zero::zero() - ); - let reward_pool_remaining = T::Currency::free_balance(&reward_account); - // This shouldn't fail, but if it does we don't really care - let _ = T::Currency::transfer( - &reward_account, - &delegator_account, - reward_pool_remaining, - ExistenceRequirement::AllowDeath, - ); - T::Currency::make_free_balance_be(&bonded_pool.bonded_account(), Zero::zero()); - bonded_pool.remove(); - None + let post_info_weight = if delegator.active_points().is_zero() { + // delegator being reaped. + Delegators::::remove(&delegator_account); + + if delegator_account == bonded_pool.roles.depositor { + Pallet::::dissolve_pool(bonded_pool); + None + } else { + bonded_pool.dec_delegators().put(); + SubPoolsStorage::::insert(&delegator.pool_id, sub_pools); + Some(T::WeightInfo::withdraw_unbonded_update(num_slashing_spans)) + } } else { - bonded_pool.dec_delegators().put(); + // we certainly don't need to delete any pools, because no one is being removed. SubPoolsStorage::::insert(&delegator.pool_id, sub_pools); + Delegators::::insert(&delegator_account, delegator); Some(T::WeightInfo::withdraw_unbonded_update(num_slashing_spans)) }; - Delegators::::remove(&delegator_account); Ok(post_info_weight.into()) } @@ -1545,7 +1691,7 @@ pub mod pallet { pool_id, points, reward_pool_total_earnings: Zero::zero(), - unbonding_era: None, + unbonding_eras: Default::default(), }, ); RewardPools::::insert( @@ -1693,6 +1839,49 @@ pub mod pallet { } impl Pallet { + /// Remove everything related to the given bonded pool. + /// + /// All sub-pools are also deleted. All accounts are dusted and the leftover of the reward + /// account is returned to the depositor. + pub fn dissolve_pool(bonded_pool: BondedPool) { + let reward_account = bonded_pool.reward_account(); + let bonded_account = bonded_pool.bonded_account(); + + ReversePoolIdLookup::::remove(&bonded_account); + RewardPools::::remove(bonded_pool.id); + SubPoolsStorage::::remove(bonded_pool.id); + + // 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. We + // don't work with the system pallet directly, but + // 1. we drain the reward account and kill it. This account should never have any extra + // consumers anyway. + // 2. the bonded account should become a 'killed stash' in the staking system, and all of + // its consumers removed. + debug_assert_eq!(frame_system::Pallet::::consumers(&reward_account), 0); + debug_assert_eq!(frame_system::Pallet::::consumers(&bonded_account), 0); + debug_assert_eq!( + T::StakingInterface::total_stake(&bonded_account).unwrap_or_default(), + Zero::zero() + ); + + // This shouldn't fail, but if it does we don't really care + let reward_pool_remaining = T::Currency::free_balance(&reward_account); + let _ = T::Currency::transfer( + &reward_account, + &bonded_pool.roles.depositor, + reward_pool_remaining, + ExistenceRequirement::AllowDeath, + ); + + // TODO: this is purely defensive. + T::Currency::make_free_balance_be(&reward_account, Zero::zero()); + T::Currency::make_free_balance_be(&bonded_pool.bonded_account(), Zero::zero()); + + Self::deposit_event(Event::::Destroyed { pool_id: bonded_pool.id }); + bonded_pool.remove(); + } + /// Create the main, bonded account of a pool with the given id. pub fn create_bonded_account(id: PoolId) -> T::AccountId { T::PalletId::get().into_sub_account((AccountType::Bonded, id)) @@ -1730,9 +1919,9 @@ impl Pallet { Delegators::::insert(delegator_account, delegator); } - /// Calculate the number of points to issue from a pool as `(current_points / current_balance) * - /// new_funds` except for some zero edge cases; see logic and tests for details. - fn points_to_issue( + /// Calculate the equivalent point of `new_funds` in a pool with `current_balance` and + /// `current_points`. + fn balance_to_point( current_balance: BalanceOf, current_points: BalanceOf, new_funds: BalanceOf, @@ -1758,22 +1947,22 @@ impl Pallet { } } - // Calculate the balance of a pool to unbond as `(current_balance / current_points) * - // delegator_points`. Returns zero if any of the inputs are zero. - fn balance_to_unbond( + /// Calculate the equivalent balance of `points` in a pool with `current_balance` and + /// `current_points`. + fn point_to_balance( current_balance: BalanceOf, current_points: BalanceOf, - delegator_points: BalanceOf, + points: BalanceOf, ) -> BalanceOf { let u256 = |x| T::BalanceToU256::convert(x); let balance = |x| T::U256ToBalance::convert(x); - if current_balance.is_zero() || current_points.is_zero() || delegator_points.is_zero() { + if current_balance.is_zero() || current_points.is_zero() || points.is_zero() { // There is nothing to unbond return Zero::zero() } - // Equivalent of (current_balance / current_points) * delegator_points - balance(u256(current_balance).saturating_mul(u256(delegator_points))) + // Equivalent of (current_balance / current_points) * points + balance(u256(current_balance).saturating_mul(u256(points))) // We check for zero above .div(current_points) } @@ -1782,15 +1971,12 @@ impl Pallet { /// /// Returns the payout amount. fn calculate_delegator_payout( + delegator: &mut Delegator, bonded_pool: &mut BondedPool, reward_pool: &mut RewardPool, - delegator: &mut Delegator, ) -> Result, DispatchError> { let u256 = |x| T::BalanceToU256::convert(x); let balance = |x| T::U256ToBalance::convert(x); - // If the delegator is unbonding they cannot claim rewards. Note that when the delegator - // goes to unbond, the unbond function should claim rewards for the final time. - ensure!(delegator.unbonding_era.is_none(), Error::::AlreadyUnbonding); let last_total_earnings = reward_pool.total_earnings; reward_pool.update_total_earnings_and_balance(bonded_pool.id); @@ -1816,7 +2002,7 @@ impl Pallet { // The points of the reward pool that belong to the delegator. let delegator_virtual_points = // The delegators portion of the reward pool - u256(delegator.points) + u256(delegator.active_points()) // times the amount the pool has earned since the delegator last claimed. .saturating_mul(u256(new_earnings_since_last_claim)); @@ -1859,10 +2045,12 @@ impl Pallet { reward_pool: &mut RewardPool, ) -> Result, DispatchError> { debug_assert_eq!(delegator.pool_id, bonded_pool.id); + // a delegator who has no skin in the game anymore cannot claim any rewards. + ensure!(!delegator.active_points().is_zero(), Error::::FullyUnbonding); let was_destroying = bonded_pool.is_destroying(); let delegator_payout = - Self::calculate_delegator_payout(bonded_pool, reward_pool, delegator)?; + Self::calculate_delegator_payout(delegator, bonded_pool, reward_pool)?; // Transfer payout to the delegator. T::Currency::transfer( @@ -1948,17 +2136,27 @@ impl Pallet { let mut all_delegators = 0u32; Delegators::::iter().for_each(|(_, d)| { assert!(BondedPools::::contains_key(d.pool_id)); + assert!(!d.total_points().is_zero(), "no delegator should have zero points: {:?}", d); *pools_delegators.entry(d.pool_id).or_default() += 1; all_delegators += 1; }); - BondedPools::::iter().for_each(|(id, bonded_pool)| { + BondedPools::::iter().for_each(|(id, inner)| { + let bonded_pool = BondedPool { id, inner }; assert_eq!( pools_delegators.get(&id).map(|x| *x).unwrap_or_default(), bonded_pool.delegator_counter ); assert!(MaxDelegatorsPerPool::::get() .map_or(true, |max| bonded_pool.delegator_counter <= max)); + + let depositor = Delegators::::get(&bonded_pool.roles.depositor).unwrap(); + assert!( + bonded_pool.is_destroying_and_only_depositor(depositor.active_points()) || + depositor.active_points() >= MinCreateBond::::get(), + "depositor must always have MinCreateBond stake in the pool, except for when the \ + pool is being destroyed and the depositor is the last member", + ); }); assert!(MaxDelegators::::get().map_or(true, |max| all_delegators <= max)); @@ -1966,24 +2164,20 @@ impl Pallet { return Ok(()) } - for (pool_id, _) in BondedPools::::iter() { + for (pool_id, _pool) in BondedPools::::iter() { let pool_account = Pallet::::create_bonded_account(pool_id); let subs = SubPoolsStorage::::get(pool_id).unwrap_or_default(); - let sum_unbonding_balance = subs - .with_era - .into_iter() - .map(|(_, v)| v) - .chain(sp_std::iter::once(subs.no_era)) - .map(|unbond_pool| unbond_pool.balance) - .fold(Zero::zero(), |a, b| a + b); + let sum_unbonding_balance = subs.sum_unbonding_balance(); let bonded_balance = T::StakingInterface::active_stake(&pool_account).unwrap_or_default(); let total_balance = T::Currency::total_balance(&pool_account); assert!( total_balance >= bonded_balance + sum_unbonding_balance, - "total_balance {:?} >= bonded_balance {:?} + sum_unbonding_balance {:?}", + "faulty pool: {:?} / {:?}, total_balance {:?} >= bonded_balance {:?} + sum_unbonding_balance {:?}", + pool_id, + _pool, total_balance, bonded_balance, sum_unbonding_balance @@ -1992,6 +2186,20 @@ impl Pallet { Ok(()) } + + /// Fully unbond the shares of `delegator`, when executed from `origin`. + /// + /// This is useful for backwards compatibility with the majority of tests that only deal with + /// full unbonding, not partial unbonding. + #[cfg(any(feature = "runtime-benchmarks", test))] + pub fn fully_unbond( + origin: frame_system::pallet_prelude::OriginFor, + delegator: T::AccountId, + ) -> DispatchResult { + let points = + Delegators::::get(&delegator).map(|d| d.active_points()).unwrap_or_default(); + Self::unbond(origin, delegator, points) + } } impl OnStakerSlash> for Pallet { diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 28b00c3488678..a84c45398ff72 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -2,6 +2,7 @@ use super::*; use crate::{self as pools}; use frame_support::{assert_ok, parameter_types, PalletId}; use frame_system::RawOrigin; +use std::collections::HashMap; pub type AccountId = u128; pub type Balance = u128; @@ -18,9 +19,11 @@ pub fn default_reward_account() -> AccountId { parameter_types! { pub static CurrentEra: EraIndex = 0; - static BondedBalanceMap: std::collections::HashMap = Default::default(); - static UnbondingBalanceMap: std::collections::HashMap = Default::default(); pub static BondingDuration: EraIndex = 3; + static BondedBalanceMap: HashMap = Default::default(); + static UnbondingBalanceMap: HashMap = Default::default(); + #[derive(Clone, PartialEq)] + pub static MaxUnbonding: u32 = 8; pub static Nominations: Vec = vec![]; } @@ -170,6 +173,7 @@ impl pools::Config for Runtime { type PostUnbondingPoolsWindow = PostUnbondingPoolsWindow; type PalletId = PoolsPalletId; type MaxMetadataLen = MaxMetadataLen; + type MaxUnbonding = MaxUnbonding; } type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; @@ -198,6 +202,11 @@ impl ExtBuilder { self } + pub(crate) fn ed(self, ed: Balance) -> Self { + ExistentialDeposit::set(ed); + self + } + pub(crate) fn with_check(self, level: u8) -> Self { CheckLevel::set(level); self @@ -269,6 +278,14 @@ pub(crate) fn pool_events_since_last_call() -> Vec> { events.into_iter().skip(already_seen).collect() } +/// Same as `fully_unbond`, in permissioned setting. +pub fn fully_unbond_permissioned(delegator: AccountId) -> DispatchResult { + let points = Delegators::::get(&delegator) + .map(|d| d.active_points()) + .unwrap_or_default(); + Pools::unbond(Origin::signed(delegator), delegator, points) +} + #[cfg(test)] mod test { use super::*; diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 134d344b99bc9..15beb0572b9e1 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -16,9 +16,9 @@ // limitations under the License. use super::*; -use crate::mock::*; +use crate::{mock::*, Event}; use frame_support::{ - assert_noop, assert_ok, assert_storage_noop, + assert_noop, assert_ok, assert_storage_noop, bounded_btree_map, storage::{with_transaction, TransactionOutcome}, }; @@ -30,6 +30,13 @@ macro_rules! unbonding_pools_with_era { }}; } +macro_rules! delegator_unbonding_eras { + ($( $any:tt )*) => {{ + let x: BoundedBTreeMap = bounded_btree_map!($( $any )*); + x + }}; +} + pub const DEFAULT_ROLES: PoolRoles = PoolRoles { depositor: 10, root: 900, nominator: 901, state_toggler: 902 }; @@ -61,12 +68,7 @@ fn test_setup_works() { ); assert_eq!( Delegators::::get(10).unwrap(), - Delegator:: { - pool_id: last_pool, - points: 10, - reward_pool_total_earnings: 0, - unbonding_era: None - } + Delegator:: { pool_id: last_pool, points: 10, ..Default::default() } ) }) } @@ -87,41 +89,41 @@ mod bonded_pool { // 1 points : 1 balance ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 100); - assert_eq!(bonded_pool.points_to_issue(10), 10); - assert_eq!(bonded_pool.points_to_issue(0), 0); + assert_eq!(bonded_pool.balance_to_point(10), 10); + assert_eq!(bonded_pool.balance_to_point(0), 0); // 2 points : 1 balance ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 50); - assert_eq!(bonded_pool.points_to_issue(10), 20); + assert_eq!(bonded_pool.balance_to_point(10), 20); // 1 points : 2 balance ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 100); bonded_pool.points = 50; - assert_eq!(bonded_pool.points_to_issue(10), 5); + assert_eq!(bonded_pool.balance_to_point(10), 5); // 100 points : 0 balance ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 0); bonded_pool.points = 100; - assert_eq!(bonded_pool.points_to_issue(10), 100 * 10); + assert_eq!(bonded_pool.balance_to_point(10), 100 * 10); // 0 points : 100 balance StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 100); bonded_pool.points = 100; - assert_eq!(bonded_pool.points_to_issue(10), 10); + assert_eq!(bonded_pool.balance_to_point(10), 10); // 10 points : 3 balance ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 30); - assert_eq!(bonded_pool.points_to_issue(10), 33); + assert_eq!(bonded_pool.balance_to_point(10), 33); // 2 points : 3 balance ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 300); bonded_pool.points = 200; - assert_eq!(bonded_pool.points_to_issue(10), 6); + assert_eq!(bonded_pool.balance_to_point(10), 6); // 4 points : 9 balance ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 900); bonded_pool.points = 400; - assert_eq!(bonded_pool.points_to_issue(90), 40); + assert_eq!(bonded_pool.balance_to_point(90), 40); } #[test] @@ -138,37 +140,37 @@ mod bonded_pool { }; StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 100); - assert_eq!(bonded_pool.balance_to_unbond(10), 10); - assert_eq!(bonded_pool.balance_to_unbond(0), 0); + assert_eq!(bonded_pool.points_to_balance(10), 10); + assert_eq!(bonded_pool.points_to_balance(0), 0); // 2 balance : 1 points ratio bonded_pool.points = 50; - assert_eq!(bonded_pool.balance_to_unbond(10), 20); + assert_eq!(bonded_pool.points_to_balance(10), 20); // 100 balance : 0 points ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 0); bonded_pool.points = 0; - assert_eq!(bonded_pool.balance_to_unbond(10), 0); + assert_eq!(bonded_pool.points_to_balance(10), 0); // 0 balance : 100 points ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 0); bonded_pool.points = 100; - assert_eq!(bonded_pool.balance_to_unbond(10), 0); + assert_eq!(bonded_pool.points_to_balance(10), 0); // 10 balance : 3 points ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 100); bonded_pool.points = 30; - assert_eq!(bonded_pool.balance_to_unbond(10), 33); + assert_eq!(bonded_pool.points_to_balance(10), 33); // 2 balance : 3 points ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 200); bonded_pool.points = 300; - assert_eq!(bonded_pool.balance_to_unbond(10), 6); + assert_eq!(bonded_pool.points_to_balance(10), 6); // 4 balance : 9 points ratio StakingMock::set_bonded_balance(bonded_pool.bonded_account(), 400); bonded_pool.points = 900; - assert_eq!(bonded_pool.balance_to_unbond(90), 40); + assert_eq!(bonded_pool.points_to_balance(90), 40); } #[test] @@ -242,72 +244,72 @@ mod unbond_pool { fn points_to_issue_works() { // 1 points : 1 balance ratio let unbond_pool = UnbondPool:: { points: 100, balance: 100 }; - assert_eq!(unbond_pool.points_to_issue(10), 10); - assert_eq!(unbond_pool.points_to_issue(0), 0); + assert_eq!(unbond_pool.balance_to_point(10), 10); + assert_eq!(unbond_pool.balance_to_point(0), 0); // 2 points : 1 balance ratio let unbond_pool = UnbondPool:: { points: 100, balance: 50 }; - assert_eq!(unbond_pool.points_to_issue(10), 20); + assert_eq!(unbond_pool.balance_to_point(10), 20); // 1 points : 2 balance ratio let unbond_pool = UnbondPool:: { points: 50, balance: 100 }; - assert_eq!(unbond_pool.points_to_issue(10), 5); + assert_eq!(unbond_pool.balance_to_point(10), 5); // 100 points : 0 balance ratio let unbond_pool = UnbondPool:: { points: 100, balance: 0 }; - assert_eq!(unbond_pool.points_to_issue(10), 100 * 10); + assert_eq!(unbond_pool.balance_to_point(10), 100 * 10); // 0 points : 100 balance let unbond_pool = UnbondPool:: { points: 0, balance: 100 }; - assert_eq!(unbond_pool.points_to_issue(10), 10); + assert_eq!(unbond_pool.balance_to_point(10), 10); // 10 points : 3 balance ratio let unbond_pool = UnbondPool:: { points: 100, balance: 30 }; - assert_eq!(unbond_pool.points_to_issue(10), 33); + assert_eq!(unbond_pool.balance_to_point(10), 33); // 2 points : 3 balance ratio let unbond_pool = UnbondPool:: { points: 200, balance: 300 }; - assert_eq!(unbond_pool.points_to_issue(10), 6); + assert_eq!(unbond_pool.balance_to_point(10), 6); // 4 points : 9 balance ratio let unbond_pool = UnbondPool:: { points: 400, balance: 900 }; - assert_eq!(unbond_pool.points_to_issue(90), 40); + assert_eq!(unbond_pool.balance_to_point(90), 40); } #[test] fn balance_to_unbond_works() { // 1 balance : 1 points ratio let unbond_pool = UnbondPool:: { points: 100, balance: 100 }; - assert_eq!(unbond_pool.balance_to_unbond(10), 10); - assert_eq!(unbond_pool.balance_to_unbond(0), 0); + assert_eq!(unbond_pool.point_to_balance(10), 10); + assert_eq!(unbond_pool.point_to_balance(0), 0); // 1 balance : 2 points ratio let unbond_pool = UnbondPool:: { points: 100, balance: 50 }; - assert_eq!(unbond_pool.balance_to_unbond(10), 5); + assert_eq!(unbond_pool.point_to_balance(10), 5); // 2 balance : 1 points ratio let unbond_pool = UnbondPool:: { points: 50, balance: 100 }; - assert_eq!(unbond_pool.balance_to_unbond(10), 20); + assert_eq!(unbond_pool.point_to_balance(10), 20); // 100 balance : 0 points ratio let unbond_pool = UnbondPool:: { points: 0, balance: 100 }; - assert_eq!(unbond_pool.balance_to_unbond(10), 0); + assert_eq!(unbond_pool.point_to_balance(10), 0); // 0 balance : 100 points ratio let unbond_pool = UnbondPool:: { points: 100, balance: 0 }; - assert_eq!(unbond_pool.balance_to_unbond(10), 0); + assert_eq!(unbond_pool.point_to_balance(10), 0); // 10 balance : 3 points ratio let unbond_pool = UnbondPool:: { points: 30, balance: 100 }; - assert_eq!(unbond_pool.balance_to_unbond(10), 33); + assert_eq!(unbond_pool.point_to_balance(10), 33); // 2 balance : 3 points ratio let unbond_pool = UnbondPool:: { points: 300, balance: 200 }; - assert_eq!(unbond_pool.balance_to_unbond(10), 6); + assert_eq!(unbond_pool.point_to_balance(10), 6); // 4 balance : 9 points ratio let unbond_pool = UnbondPool:: { points: 900, balance: 400 }; - assert_eq!(unbond_pool.balance_to_unbond(90), 40); + assert_eq!(unbond_pool.point_to_balance(90), 40); } } @@ -408,12 +410,7 @@ mod join { // then assert_eq!( Delegators::::get(&11).unwrap(), - Delegator:: { - pool_id: 1, - points: 2, - reward_pool_total_earnings: 0, - unbonding_era: None - } + Delegator:: { pool_id: 1, points: 2, ..Default::default() } ); assert_eq!(BondedPool::::get(1).unwrap(), bonded(12, 2)); @@ -431,12 +428,7 @@ mod join { // Then assert_eq!( Delegators::::get(&12).unwrap(), - Delegator:: { - pool_id: 1, - points: 24, - reward_pool_total_earnings: 0, - unbonding_era: None - } + Delegator:: { pool_id: 1, points: 24, ..Default::default() } ); assert_eq!(BondedPool::::get(1).unwrap(), bonded(12 + 24, 3)); }); @@ -570,7 +562,12 @@ mod claim_payout { use super::*; fn del(points: Balance, reward_pool_total_earnings: Balance) -> Delegator { - Delegator { pool_id: 1, points, reward_pool_total_earnings, unbonding_era: None } + Delegator { + pool_id: 1, + points, + reward_pool_total_earnings, + unbonding_eras: Default::default(), + } } fn rew(balance: Balance, points: u32, total_earnings: Balance) -> RewardPool { @@ -816,20 +813,18 @@ mod claim_payout { } #[test] - fn calculate_delegator_payout_errors_if_a_delegator_is_unbonding() { - ExtBuilder::default().build_and_execute(|| { + fn reward_payout_errors_if_a_delegator_is_fully_unbonding() { + ExtBuilder::default().add_delegators(vec![(11, 11)]).build_and_execute(|| { + // fully unbond the delegator. + assert_ok!(Pools::fully_unbond(Origin::signed(11), 11)); + let mut bonded_pool = BondedPool::::get(1).unwrap(); let mut reward_pool = RewardPools::::get(1).unwrap(); - let mut delegator = Delegators::::get(10).unwrap(); - delegator.unbonding_era = Some(0 + 3); + let mut delegator = Delegators::::get(11).unwrap(); assert_noop!( - Pools::calculate_delegator_payout( - &mut bonded_pool, - &mut reward_pool, - &mut delegator - ), - Error::::AlreadyUnbonding + Pools::do_reward_payout(&11, &mut delegator, &mut bonded_pool, &mut reward_pool,), + Error::::FullyUnbonding ); }); } @@ -847,9 +842,9 @@ mod claim_payout { // Given no rewards have been earned // When let payout = Pools::calculate_delegator_payout( + &mut delegator, &mut bonded_pool, &mut reward_pool, - &mut delegator, ) .unwrap(); @@ -863,9 +858,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut delegator, &mut bonded_pool, &mut reward_pool, - &mut delegator, ) .unwrap(); @@ -879,9 +874,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut delegator, &mut bonded_pool, &mut reward_pool, - &mut delegator, ) .unwrap(); @@ -895,9 +890,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut delegator, &mut bonded_pool, &mut reward_pool, - &mut delegator, ) .unwrap(); @@ -931,9 +926,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_10, &mut bonded_pool, &mut reward_pool, - &mut del_10, ) .unwrap(); @@ -946,9 +941,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_40, &mut bonded_pool, &mut reward_pool, - &mut del_40, ) .unwrap(); @@ -969,9 +964,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_50, &mut bonded_pool, &mut reward_pool, - &mut del_50, ) .unwrap(); @@ -987,9 +982,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_10, &mut bonded_pool, &mut reward_pool, - &mut del_10, ) .unwrap(); @@ -1002,9 +997,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_40, &mut bonded_pool, &mut reward_pool, - &mut del_40, ) .unwrap(); @@ -1020,9 +1015,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_50, &mut bonded_pool, &mut reward_pool, - &mut del_50, ) .unwrap(); @@ -1046,9 +1041,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_10, &mut bonded_pool, &mut reward_pool, - &mut del_10, ) .unwrap(); @@ -1065,9 +1060,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_10, &mut bonded_pool, &mut reward_pool, - &mut del_10, ) .unwrap(); @@ -1095,9 +1090,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_10, &mut bonded_pool, &mut reward_pool, - &mut del_10, ) .unwrap(); @@ -1110,9 +1105,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_40, &mut bonded_pool, &mut reward_pool, - &mut del_40, ) .unwrap(); @@ -1125,9 +1120,9 @@ mod claim_payout { // When let payout = Pools::calculate_delegator_payout( + &mut del_50, &mut bonded_pool, &mut reward_pool, - &mut del_50, ) .unwrap(); @@ -1377,7 +1372,7 @@ mod unbond { fn unbond_of_1_works() { ExtBuilder::default().build_and_execute(|| { unsafe_set_state(1, PoolState::Destroying).unwrap(); - assert_ok!(Pools::unbond(Origin::signed(10), 10)); + assert_ok!(fully_unbond_permissioned(10)); assert_eq!( SubPoolsStorage::::get(1).unwrap().with_era, @@ -1413,7 +1408,7 @@ mod unbond { Balances::make_free_balance_be(&default_reward_account(), ed + 600); // When - assert_ok!(Pools::unbond(Origin::signed(40), 40)); + assert_ok!(fully_unbond_permissioned(40)); // Then assert_eq!( @@ -1434,12 +1429,15 @@ mod unbond { ); assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 94); - assert_eq!(Delegators::::get(40).unwrap().unbonding_era, Some(0 + 3)); + assert_eq!( + Delegators::::get(40).unwrap().unbonding_eras, + delegator_unbonding_eras!(0 + 3 => 40) + ); assert_eq!(Balances::free_balance(&40), 40 + 40); // We claim rewards when unbonding // When unsafe_set_state(1, PoolState::Destroying).unwrap(); - assert_ok!(Pools::unbond(Origin::signed(550), 550)); + assert_ok!(fully_unbond_permissioned(550)); // Then assert_eq!( @@ -1459,11 +1457,14 @@ mod unbond { } ); assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 2); - assert_eq!(Delegators::::get(550).unwrap().unbonding_era, Some(0 + 3)); + assert_eq!( + Delegators::::get(550).unwrap().unbonding_eras, + delegator_unbonding_eras!(0 + 3 => 550) + ); assert_eq!(Balances::free_balance(&550), 550 + 550); // When - assert_ok!(Pools::unbond(Origin::signed(10), 10)); + assert_ok!(fully_unbond_permissioned(10)); // Then assert_eq!( @@ -1483,7 +1484,10 @@ mod unbond { } ); assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 0); - assert_eq!(Delegators::::get(550).unwrap().unbonding_era, Some(0 + 3)); + assert_eq!( + Delegators::::get(550).unwrap().unbonding_eras, + delegator_unbonding_eras!(0 + 3 => 550) + ); assert_eq!(Balances::free_balance(&550), 550 + 550); }); } @@ -1510,7 +1514,7 @@ mod unbond { let current_era = 1 + TotalUnbondingPools::::get(); CurrentEra::set(current_era); - assert_ok!(Pools::unbond(Origin::signed(10), 10)); + assert_ok!(fully_unbond_permissioned(10)); // Then assert_eq!( @@ -1541,15 +1545,15 @@ mod unbond { // When the nominator trys to kick, then its a noop assert_noop!( - Pools::unbond(Origin::signed(901), 100), + Pools::fully_unbond(Origin::signed(901), 100), Error::::NotKickerOrDestroying ); // When the root kicks then its ok - assert_ok!(Pools::unbond(Origin::signed(900), 100)); + assert_ok!(Pools::fully_unbond(Origin::signed(900), 100)); // When the state toggler kicks then its ok - assert_ok!(Pools::unbond(Origin::signed(902), 200)); + assert_ok!(Pools::fully_unbond(Origin::signed(902), 200)); assert_eq!( BondedPool::::get(1).unwrap(), @@ -1582,7 +1586,7 @@ mod unbond { } #[test] - fn unbond_with_non_admins_works() { + fn unbond_permissionless_works() { // Scenarios where non-admin accounts can unbond others ExtBuilder::default().add_delegators(vec![(100, 100)]).build_and_execute(|| { // Given the pool is blocked @@ -1590,33 +1594,36 @@ mod unbond { // A permissionless unbond attempt errors assert_noop!( - Pools::unbond(Origin::signed(420), 100), + Pools::fully_unbond(Origin::signed(420), 100), Error::::NotKickerOrDestroying ); // Given the pool is destroying unsafe_set_state(1, PoolState::Destroying).unwrap(); - // The depositor cannot be unbonded until they are the last delegator + // The depositor cannot be fully unbonded until they are the last delegator assert_noop!( - Pools::unbond(Origin::signed(420), 10), + Pools::fully_unbond(Origin::signed(10), 10), Error::::NotOnlyDelegator ); // Any account can unbond a delegator that is not the depositor - assert_ok!(Pools::unbond(Origin::signed(420), 100)); + assert_ok!(Pools::fully_unbond(Origin::signed(420), 100)); // Given the pool is blocked unsafe_set_state(1, PoolState::Blocked).unwrap(); // The depositor cannot be unbonded - assert_noop!(Pools::unbond(Origin::signed(420), 10), Error::::NotDestroying); + assert_noop!( + Pools::fully_unbond(Origin::signed(420), 10), + Error::::DoesNotHavePermission + ); // Given the pools is destroying unsafe_set_state(1, PoolState::Destroying).unwrap(); - // The depositor can be unbonded - assert_ok!(Pools::unbond(Origin::signed(420), 10)); + // The depositor can be unbonded by anyone. + assert_ok!(Pools::fully_unbond(Origin::signed(420), 10)); assert_eq!(BondedPools::::get(1).unwrap().points, 0); assert_eq!( @@ -1642,20 +1649,15 @@ mod unbond { fn unbond_errors_correctly() { ExtBuilder::default().build_and_execute(|| { assert_noop!( - Pools::unbond(Origin::signed(11), 11), + Pools::fully_unbond(Origin::signed(11), 11), Error::::DelegatorNotFound ); // Add the delegator - let delegator = Delegator { - pool_id: 2, - points: 10, - reward_pool_total_earnings: 0, - unbonding_era: None, - }; + let delegator = Delegator { pool_id: 2, points: 10, ..Default::default() }; Delegators::::insert(11, delegator); - let _ = Pools::unbond(Origin::signed(11), 11); + let _ = Pools::fully_unbond(Origin::signed(11), 11); }); } @@ -1663,12 +1665,7 @@ mod unbond { #[should_panic = "Defensive failure has been triggered!"] fn unbond_panics_when_reward_pool_not_found() { ExtBuilder::default().build_and_execute(|| { - let delegator = Delegator { - pool_id: 2, - points: 10, - reward_pool_total_earnings: 0, - unbonding_era: None, - }; + let delegator = Delegator { pool_id: 2, points: 10, ..Default::default() }; Delegators::::insert(11, delegator); BondedPool:: { id: 1, @@ -1681,9 +1678,203 @@ mod unbond { } .put(); - let _ = Pools::unbond(Origin::signed(11), 11); + let _ = Pools::fully_unbond(Origin::signed(11), 11); + }); + } + + #[test] + fn partial_unbond_era_tracking() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(Delegators::::get(10).unwrap().active_points(), 10); + assert_eq!(Delegators::::get(10).unwrap().unbonding_points(), 0); + assert_eq!(Delegators::::get(10).unwrap().pool_id, 1); + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!() + ); + assert_eq!(BondedPool::::get(1).unwrap().points, 10); + assert!(SubPoolsStorage::::get(1).is_none()); + assert_eq!(CurrentEra::get(), 0); + assert_eq!(BondingDuration::get(), 3); + + // so the depositor can leave, just keeps the test simpler. + unsafe_set_state(1, PoolState::Destroying).unwrap(); + + // when: casual unbond + assert_ok!(Pools::unbond(Origin::signed(10), 10, 1)); + + // then + assert_eq!(Delegators::::get(10).unwrap().active_points(), 9); + assert_eq!(Delegators::::get(10).unwrap().unbonding_points(), 1); + assert_eq!(BondedPool::::get(1).unwrap().points, 9); + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!(3 => 1) + ); + assert_eq!( + SubPoolsStorage::::get(1).unwrap(), + SubPools { + no_era: Default::default(), + with_era: unbonding_pools_with_era! { + 3 => UnbondPool { points: 1, balance: 1 } + } + } + ); + + // when: casual further unbond, same era. + assert_ok!(Pools::unbond(Origin::signed(10), 10, 5)); + + // then + assert_eq!(Delegators::::get(10).unwrap().active_points(), 4); + assert_eq!(Delegators::::get(10).unwrap().unbonding_points(), 6); + assert_eq!(BondedPool::::get(1).unwrap().points, 4); + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!(3 => 6) + ); + assert_eq!( + SubPoolsStorage::::get(1).unwrap(), + SubPools { + no_era: Default::default(), + with_era: unbonding_pools_with_era! { + 3 => UnbondPool { points: 6, balance: 6 } + } + } + ); + + // when: casual further unbond, next era. + CurrentEra::set(1); + assert_ok!(Pools::unbond(Origin::signed(10), 10, 1)); + + // then + assert_eq!(Delegators::::get(10).unwrap().active_points(), 3); + assert_eq!(Delegators::::get(10).unwrap().unbonding_points(), 7); + assert_eq!(BondedPool::::get(1).unwrap().points, 3); + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!(3 => 6, 4 => 1) + ); + assert_eq!( + SubPoolsStorage::::get(1).unwrap(), + SubPools { + no_era: Default::default(), + with_era: unbonding_pools_with_era! { + 3 => UnbondPool { points: 6, balance: 6 }, + 4 => UnbondPool { points: 1, balance: 1 } + } + } + ); + + // when: unbonding more than our active: error + assert_noop!( + Pools::unbond(Origin::signed(10), 10, 5), + Error::::NotEnoughPointsToUnbond + ); + // instead: + assert_ok!(Pools::unbond(Origin::signed(10), 10, 3)); + + // then + assert_eq!(Delegators::::get(10).unwrap().active_points(), 0); + assert_eq!(Delegators::::get(10).unwrap().unbonding_points(), 10); + assert_eq!(BondedPool::::get(1).unwrap().points, 0); + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!(3 => 6, 4 => 4) + ); + assert_eq!( + SubPoolsStorage::::get(1).unwrap(), + SubPools { + no_era: Default::default(), + with_era: unbonding_pools_with_era! { + 3 => UnbondPool { points: 6, balance: 6 }, + 4 => UnbondPool { points: 4, balance: 4 } + } + } + ); }); } + + #[test] + fn partial_unbond_max_chunks() { + ExtBuilder::default().ed(1).build_and_execute(|| { + // so the depositor can leave, just keeps the test simpler. + unsafe_set_state(1, PoolState::Destroying).unwrap(); + MaxUnbonding::set(2); + + // given + assert_ok!(Pools::unbond(Origin::signed(10), 10, 2)); + CurrentEra::set(1); + assert_ok!(Pools::unbond(Origin::signed(10), 10, 3)); + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!(3 => 2, 4 => 3) + ); + + // when + CurrentEra::set(2); + assert_noop!( + Pools::unbond(Origin::signed(10), 10, 4), + Error::::MaxUnbondingLimit + ); + + // when + MaxUnbonding::set(3); + assert_ok!(Pools::unbond(Origin::signed(10), 10, 1)); + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!(3 => 2, 4 => 3, 5 => 1) + ); + }) + } + + // depositor can unbond inly up to `MinCreateBond`. + #[test] + fn depositor_permissioned_partial_unbond() { + ExtBuilder::default() + .ed(1) + .add_delegators(vec![(100, 100)]) + .build_and_execute(|| { + // given + assert_eq!(MinCreateBond::::get(), 2); + assert_eq!(Delegators::::get(10).unwrap().active_points(), 10); + assert_eq!(Delegators::::get(10).unwrap().unbonding_points(), 0); + + // can unbond a bit.. + assert_ok!(Pools::unbond(Origin::signed(10), 10, 3)); + assert_eq!(Delegators::::get(10).unwrap().active_points(), 7); + assert_eq!(Delegators::::get(10).unwrap().unbonding_points(), 3); + + // but not less than 2 + assert_noop!( + Pools::unbond(Origin::signed(10), 10, 6), + Error::::NotOnlyDelegator + ); + }); + } + + // same as above, but the pool is slashed and therefore the depositor cannot partially unbond. + #[test] + fn depositor_permissioned_partial_unbond_slashed() { + ExtBuilder::default() + .ed(1) + .add_delegators(vec![(100, 100)]) + .build_and_execute(|| { + // given + assert_eq!(MinCreateBond::::get(), 2); + assert_eq!(Delegators::::get(10).unwrap().active_points(), 10); + assert_eq!(Delegators::::get(10).unwrap().unbonding_points(), 0); + + // slash the default pool + StakingMock::set_bonded_balance(Pools::create_bonded_account(1), 5); + + // cannot unbond even 7, because the value of shares is now less. + assert_noop!( + Pools::unbond(Origin::signed(10), 10, 7), + Error::::NotOnlyDelegator + ); + }); + } } mod pool_withdraw_unbonded { @@ -1712,6 +1903,7 @@ mod pool_withdraw_unbonded { mod withdraw_unbonded { use super::*; + use frame_support::bounded_btree_map; #[test] fn withdraw_unbonded_works_against_slashed_no_era_sub_pool() { @@ -1720,15 +1912,15 @@ mod withdraw_unbonded { .build_and_execute(|| { // Given assert_eq!(StakingMock::bonding_duration(), 3); - assert_ok!(Pools::unbond(Origin::signed(550), 550)); - assert_ok!(Pools::unbond(Origin::signed(40), 40)); + assert_ok!(Pools::fully_unbond(Origin::signed(550), 550)); + assert_ok!(Pools::fully_unbond(Origin::signed(40), 40)); assert_eq!(Balances::free_balance(&default_bonded_account()), 600); let mut current_era = 1; CurrentEra::set(current_era); // In a new era, unbond the depositor unsafe_set_state(1, PoolState::Destroying).unwrap(); - assert_ok!(Pools::unbond(Origin::signed(10), 10)); + assert_ok!(Pools::fully_unbond(Origin::signed(10), 10)); let mut sub_pools = SubPoolsStorage::::get(1).unwrap(); let unbond_pool = sub_pools.with_era.get_mut(&(current_era + 3)).unwrap(); @@ -1812,10 +2004,10 @@ mod withdraw_unbonded { Balances::make_free_balance_be(&default_bonded_account(), 100); assert_eq!(StakingMock::total_stake(&default_bonded_account()), Some(100)); - assert_ok!(Pools::unbond(Origin::signed(40), 40)); - assert_ok!(Pools::unbond(Origin::signed(550), 550)); + assert_ok!(Pools::fully_unbond(Origin::signed(40), 40)); + assert_ok!(Pools::fully_unbond(Origin::signed(550), 550)); unsafe_set_state(1, PoolState::Destroying).unwrap(); - assert_ok!(Pools::unbond(Origin::signed(10), 10)); + assert_ok!(Pools::fully_unbond(Origin::signed(10), 10)); SubPoolsStorage::::insert( 1, @@ -1873,7 +2065,7 @@ mod withdraw_unbonded { assert_eq!(Balances::free_balance(&10), 5); assert_eq!(Balances::free_balance(&default_bonded_account()), 10); unsafe_set_state(1, PoolState::Destroying).unwrap(); - assert_ok!(Pools::unbond(Origin::signed(10), 10)); + assert_ok!(Pools::fully_unbond(Origin::signed(10), 10)); // Simulate a slash that is not accounted for in the sub pools. Balances::make_free_balance_be(&default_bonded_account(), 5); @@ -1909,28 +2101,17 @@ mod withdraw_unbonded { Error::::DelegatorNotFound ); - let mut delegator = Delegator { - pool_id: 1, - points: 10, - reward_pool_total_earnings: 0, - unbonding_era: None, - }; + let mut delegator = Delegator { pool_id: 1, points: 10, ..Default::default() }; Delegators::::insert(11, delegator.clone()); - // The delegator has not called `unbond` - assert_noop!( - Pools::withdraw_unbonded(Origin::signed(11), 11, 0), - Error::::NotUnbonding - ); - // Simulate calling `unbond` - delegator.unbonding_era = Some(0 + 3); + delegator.unbonding_eras = delegator_unbonding_eras!(3 + 0 => 10); Delegators::::insert(11, delegator.clone()); // We are still in the bonding duration assert_noop!( Pools::withdraw_unbonded(Origin::signed(11), 11, 0), - Error::::NotUnbondedYet + Error::::CannotWithdrawAny ); // If we error the delegator does not get removed @@ -1946,8 +2127,8 @@ mod withdraw_unbonded { .add_delegators(vec![(100, 100), (200, 200)]) .build_and_execute(|| { // Given - assert_ok!(Pools::unbond(Origin::signed(100), 100)); - assert_ok!(Pools::unbond(Origin::signed(200), 200)); + assert_ok!(Pools::fully_unbond(Origin::signed(100), 100)); + assert_ok!(Pools::fully_unbond(Origin::signed(200), 200)); assert_eq!( BondedPool::::get(1).unwrap(), BondedPool { @@ -1995,7 +2176,7 @@ mod withdraw_unbonded { fn withdraw_unbonded_destroying_permissionless() { ExtBuilder::default().add_delegators(vec![(100, 100)]).build_and_execute(|| { // Given - assert_ok!(Pools::unbond(Origin::signed(100), 100)); + assert_ok!(Pools::fully_unbond(Origin::signed(100), 100)); assert_eq!( BondedPool::::get(1).unwrap(), BondedPool { @@ -2013,7 +2194,7 @@ mod withdraw_unbonded { // Cannot permissionlessly withdraw assert_noop!( - Pools::unbond(Origin::signed(420), 100), + Pools::fully_unbond(Origin::signed(420), 100), Error::::NotKickerOrDestroying ); @@ -2035,14 +2216,14 @@ mod withdraw_unbonded { .add_delegators(vec![(100, 100), (200, 200)]) .build_and_execute(|| { // Given - assert_ok!(Pools::unbond(Origin::signed(100), 100)); + assert_ok!(Pools::fully_unbond(Origin::signed(100), 100)); let mut current_era = 1; CurrentEra::set(current_era); - assert_ok!(Pools::unbond(Origin::signed(200), 200)); + assert_ok!(Pools::fully_unbond(Origin::signed(200), 200)); unsafe_set_state(1, PoolState::Destroying).unwrap(); - assert_ok!(Pools::unbond(Origin::signed(10), 10)); + assert_ok!(Pools::fully_unbond(Origin::signed(10), 10)); assert_eq!( SubPoolsStorage::::get(1).unwrap(), @@ -2054,6 +2235,7 @@ mod withdraw_unbonded { } } ); + // Skip ahead eras to where its valid for the delegators to withdraw current_era += StakingMock::bonding_duration(); CurrentEra::set(current_era); @@ -2077,9 +2259,9 @@ mod withdraw_unbonded { } ); - // Cannot withdraw if their is another delegator in the depositors `with_era` pool + // Cannot withdraw the depositor if their is a delegator in another `with_era` pool. assert_noop!( - Pools::unbond(Origin::signed(420), 10), + Pools::withdraw_unbonded(Origin::signed(420), 10, 0), Error::::NotOnlyDelegator ); @@ -2110,9 +2292,9 @@ mod withdraw_unbonded { fn withdraw_unbonded_depositor_no_era_pool() { ExtBuilder::default().add_delegators(vec![(100, 100)]).build_and_execute(|| { // Given - assert_ok!(Pools::unbond(Origin::signed(100), 100)); + assert_ok!(Pools::fully_unbond(Origin::signed(100), 100)); unsafe_set_state(1, PoolState::Destroying).unwrap(); - assert_ok!(Pools::unbond(Origin::signed(10), 10)); + assert_ok!(Pools::fully_unbond(Origin::signed(10), 10)); // Skip ahead to an era where the `with_era` pools can get merged into the `no_era` // pool. let current_era = TotalUnbondingPools::::get(); @@ -2156,6 +2338,186 @@ mod withdraw_unbonded { assert!(!BondedPools::::contains_key(1)); }); } + + #[test] + fn partial_withdraw_unbonded_depositor() { + ExtBuilder::default().ed(1).build_and_execute(|| { + // so the depositor can leave, just keeps the test simpler. + unsafe_set_state(1, PoolState::Destroying).unwrap(); + + // given + assert_ok!(Pools::unbond(Origin::signed(10), 10, 6)); + CurrentEra::set(1); + assert_ok!(Pools::unbond(Origin::signed(10), 10, 1)); + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!(3 => 6, 4 => 1) + ); + assert_eq!( + SubPoolsStorage::::get(1).unwrap(), + SubPools { + no_era: Default::default(), + with_era: unbonding_pools_with_era! { + 3 => UnbondPool { points: 6, balance: 6 }, + 4 => UnbondPool { points: 1, balance: 1 } + } + } + ); + assert_eq!(Delegators::::get(10).unwrap().active_points(), 3); + assert_eq!(Delegators::::get(10).unwrap().unbonding_points(), 7); + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Created { depositor: 10, pool_id: 1 }, + Event::Bonded { delegator: 10, pool_id: 1, bonded: 10, joined: true }, + Event::PaidOut { delegator: 10, pool_id: 1, payout: 0 }, + Event::Unbonded { delegator: 10, pool_id: 1, amount: 6 }, + Event::PaidOut { delegator: 10, pool_id: 1, payout: 0 }, + Event::Unbonded { delegator: 10, pool_id: 1, amount: 1 } + ] + ); + + // when + CurrentEra::set(2); + assert_noop!( + Pools::withdraw_unbonded(Origin::signed(10), 10, 0), + Error::::CannotWithdrawAny + ); + + // when + CurrentEra::set(3); + assert_ok!(Pools::withdraw_unbonded(Origin::signed(10), 10, 0)); + + // then + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!(4 => 1) + ); + assert_eq!( + SubPoolsStorage::::get(1).unwrap(), + SubPools { + no_era: Default::default(), + with_era: unbonding_pools_with_era! { + 4 => UnbondPool { points: 1, balance: 1 } + } + } + ); + assert_eq!( + pool_events_since_last_call(), + vec![Event::Withdrawn { delegator: 10, pool_id: 1, amount: 6 }] + ); + + // when + CurrentEra::set(4); + assert_ok!(Pools::withdraw_unbonded(Origin::signed(10), 10, 0)); + + // then + assert_eq!( + Delegators::::get(10).unwrap().unbonding_eras, + delegator_unbonding_eras!() + ); + assert_eq!(SubPoolsStorage::::get(1).unwrap(), Default::default()); + assert_eq!( + pool_events_since_last_call(), + vec![Event::Withdrawn { delegator: 10, pool_id: 1, amount: 1 },] + ); + + // when repeating: + assert_noop!( + Pools::withdraw_unbonded(Origin::signed(10), 10, 0), + Error::::CannotWithdrawAny + ); + }); + } + + #[test] + fn partial_withdraw_unbonded_non_depositor() { + ExtBuilder::default().add_delegators(vec![(11, 10)]).build_and_execute(|| { + // given + assert_ok!(Pools::unbond(Origin::signed(11), 11, 6)); + CurrentEra::set(1); + assert_ok!(Pools::unbond(Origin::signed(11), 11, 1)); + assert_eq!( + Delegators::::get(11).unwrap().unbonding_eras, + delegator_unbonding_eras!(3 => 6, 4 => 1) + ); + assert_eq!( + SubPoolsStorage::::get(1).unwrap(), + SubPools { + no_era: Default::default(), + with_era: unbonding_pools_with_era! { + 3 => UnbondPool { points: 6, balance: 6 }, + 4 => UnbondPool { points: 1, balance: 1 } + } + } + ); + assert_eq!(Delegators::::get(11).unwrap().active_points(), 3); + assert_eq!(Delegators::::get(11).unwrap().unbonding_points(), 7); + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Created { depositor: 10, pool_id: 1 }, + Event::Bonded { delegator: 10, pool_id: 1, bonded: 10, joined: true }, + Event::Bonded { delegator: 11, pool_id: 1, bonded: 10, joined: true }, + Event::PaidOut { delegator: 11, pool_id: 1, payout: 0 }, + Event::Unbonded { delegator: 11, pool_id: 1, amount: 6 }, + Event::PaidOut { delegator: 11, pool_id: 1, payout: 0 }, + Event::Unbonded { delegator: 11, pool_id: 1, amount: 1 } + ] + ); + + // when + CurrentEra::set(2); + assert_noop!( + Pools::withdraw_unbonded(Origin::signed(11), 11, 0), + Error::::CannotWithdrawAny + ); + + // when + CurrentEra::set(3); + assert_ok!(Pools::withdraw_unbonded(Origin::signed(11), 11, 0)); + + // then + assert_eq!( + Delegators::::get(11).unwrap().unbonding_eras, + delegator_unbonding_eras!(4 => 1) + ); + assert_eq!( + SubPoolsStorage::::get(1).unwrap(), + SubPools { + no_era: Default::default(), + with_era: unbonding_pools_with_era! { + 4 => UnbondPool { points: 1, balance: 1 } + } + } + ); + assert_eq!( + pool_events_since_last_call(), + vec![Event::Withdrawn { delegator: 11, pool_id: 1, amount: 6 }] + ); + + // when + CurrentEra::set(4); + assert_ok!(Pools::withdraw_unbonded(Origin::signed(11), 11, 0)); + + // then + assert_eq!( + Delegators::::get(11).unwrap().unbonding_eras, + delegator_unbonding_eras!() + ); + assert_eq!(SubPoolsStorage::::get(1).unwrap(), Default::default()); + assert_eq!( + pool_events_since_last_call(), + vec![Event::Withdrawn { delegator: 11, pool_id: 1, amount: 1 }] + ); + + // when repeating: + assert_noop!( + Pools::withdraw_unbonded(Origin::signed(11), 11, 0), + Error::::CannotWithdrawAny + ); + }); + } } mod create { @@ -2185,12 +2547,7 @@ mod create { assert_eq!(Balances::free_balance(&11), 0); assert_eq!( Delegators::::get(11).unwrap(), - Delegator { - pool_id: 2, - points: StakingMock::minimum_bond(), - reward_pool_total_earnings: Zero::zero(), - unbonding_era: None - } + Delegator { pool_id: 2, points: StakingMock::minimum_bond(), ..Default::default() } ); assert_eq!( BondedPool::::get(2).unwrap(), diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 4484ff6b6b295..2d2944cc9a44f 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -87,6 +87,8 @@ pub use self::{ StorageHasher, Twox128, Twox256, Twox64Concat, }, storage::{ + bounded_btree_map::BoundedBTreeMap, + bounded_btree_set::BoundedBTreeSet, bounded_vec::{BoundedSlice, BoundedVec}, migration, weak_bounded_vec::WeakBoundedVec, @@ -138,6 +140,25 @@ macro_rules! bounded_vec { } } +/// Build a bounded btree-map from the given literals. +/// +/// The type of the outcome must be known. +/// +/// Will not handle any errors and just panic if the given literals cannot fit in the corresponding +/// bounded vec type. Thus, this is only suitable for testing and non-consensus code. +#[macro_export] +#[cfg(feature = "std")] +macro_rules! bounded_btree_map { + ($ ( $key:expr => $value:expr ),* $(,)?) => { + { + $crate::traits::TryCollect::<$crate::BoundedBTreeMap<_, _, _>>::try_collect( + $crate::sp_std::vec![$(($key, $value)),*].into_iter() + ).unwrap() + } + }; + +} + /// Generate a new type alias for [`storage::types::StorageValue`], /// [`storage::types::StorageMap`], [`storage::types::StorageDoubleMap`] /// and [`storage::types::StorageNMap`].